-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds Parser APIs in Java and allows for parsing multiple statements #1634
Conversation
partiql-cli/src/main/kotlin/org/partiql/cli/shell/ShellHighlighter.kt
Outdated
Show resolved
Hide resolved
partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEvaluatorTest.kt
Show resolved
Hide resolved
partiql-parser/src/main/java/org/partiql/parser/PartiQLParserBuilder.java
Show resolved
Hide resolved
partiql-parser/src/main/kotlin/org/partiql/parser/internal/PFile.kt
Outdated
Show resolved
Hide resolved
Adds type information for PR readability
Adds Javadocs to SourceLocations
Removes use of PFile and PFileV1
Moves parser builder as nested class of parser
65d72cd
to
b94dfc4
Compare
Replaces all internal parse() invocations with parseSingle()
public class PartiQLParserBuilder { | ||
|
||
public fun build(): PartiQLParser { | ||
return PartiQLParserDefault() | ||
/** | ||
* TODO | ||
* @return TODO | ||
*/ | ||
@NotNull | ||
public PartiQLParser build() { | ||
return new PartiQLParserDefault(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is part of the follow-up that @alancai98 will be undertaking for removing the older parser.
/** | ||
* TODO | ||
* @param source TODO | ||
* @param ctx TODO | ||
* @return TODO | ||
* @throws PErrorListenerException TODO | ||
* @see PartiQLParserV1#parse(String, Context) | ||
*/ | ||
@NotNull | ||
default Result parseSingle(@NotNull String source, @NotNull Context ctx) throws PErrorListenerException { | ||
Result result = parse(source, ctx); | ||
if (result.statements.size() != 1) { | ||
SourceLocation location; | ||
if (result.statements.size() > 1) { | ||
location = result.locations.get(result.statements.get(1).tag); | ||
} else { | ||
location = null; | ||
} | ||
Map<String, Object> properties = new HashMap<String, Object>() {{ | ||
put("EXPECTED_TOKENS", new ArrayList<String>() {{ | ||
add("EOF"); | ||
}}); | ||
}}; | ||
PError pError = new PError(PError.UNEXPECTED_TOKEN, Severity.ERROR(), PErrorKind.SYNTAX(), location, properties); | ||
ctx.getErrorListener().report(pError); | ||
} | ||
return result; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parse single doesn't make sense actually with the result. That was a bad suggestion on my part (:
Let me know your thoughts, but removing this seems fine.
Relevant Issues
Description
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.