-
Notifications
You must be signed in to change notification settings - Fork 7
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
Exception handling PoC #45
Conversation
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.
Just two comments, looks good otherwise.
}; | ||
lexer.addErrorListener(errorListener); | ||
parser.addErrorListener(errorListener); | ||
public VTLParser.StartContext compile(Reader reader, Consumer<VTLScriptException> errorConsumer) throws IOException { |
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'm not sure about the compile
word here. In antlr it is used in the context of compiling a grammar file.
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.
right, how about validate?
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.
maybe getRootContext
or getStartContext
. Or maybe split this method as it sets up lexer and parser and also gets the start context.
), ctx | ||
); | ||
} | ||
// checkArgument( |
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 if not needed?
This PR is in preparation for #40. The goal is to improve the exception handling in the java-script project.
I created a new
ContextualRuntimeException
that contains a reference to the ParserRuleContext we get from ANTLR so that we can give the position of an error to the user. I also addedVTLScriptException
which extends the ScriptException, mainly because the ScriptException constructors are too restricting.I adjusted
AssignmentVisitor
,ExpressionVisitor
andNativeFunctionsVisitor
to use the new exceptions. Take a look atVTLErrorsTest
for an example of what the result is.