Update DRL6Expressions.g4 to ANTLR 4 #47
Conversation
| formatParserLocation() ); | ||
| codeAndMessage.add( message ); | ||
| codeAndMessage.add( "ERR 107" ); | ||
| } else if ( e instanceof MismatchedNotSetException ) { |
There was a problem hiding this comment.
Some of these deleted mismatch branches should perhaps be replaced with the InputMismatchException. There is also the LexerNoViableAltException in ANTLR 4. It would be nice to add those together with some negative tests so that we have a better understanding under which conditions these exceptions occur.
See https://github.com/antlr/antlr4/blob/master/doc/parser-rules.md#catching-exceptions.
There was a problem hiding this comment.
@yurloc Thanks. Please file a JIRA to add tests to cover the use cases. There are drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/compiler/lang/ErrorsParserTest.java, but probably it's not enough.
There was a problem hiding this comment.
| public void setUp() throws Exception { | ||
| new EvaluatorRegistry(); |
There was a problem hiding this comment.
Not sure what this is for. It was also removed from the MiscDRLParserTest so I removed it here as well. In the original RuleParserTest there is a comment that says "initializes pluggable operators".
There was a problem hiding this comment.
I think EvaluatorRegistry is not required in parser phase, it's fine to remove.
yurloc
force-pushed
the
drl-expression-antlr4
branch
from
4d37a03
to
c724590
Compare
|
@yurloc Great work! I think you can also add |
Yes, I'm already working on the dumper test locally and it proves some of the syntactic predicates cannot be removed without a replacement. I'll see if there is an easy way to work around that and whether to include it in this PR or the next one. |
|
Btw, @mariofusco , do you think refactoring the A) Refactor |
I think that option B) is much more pragmatic and will allow us to test and use this work also directly into drools as soon as possible. Refactoring |
Agreed. @yurloc Please go ahead to fix the Then, we can go for the option B). Probably creating a feature branch in |
Overview
This PR includes 3 main parts:
DrlExprParser) and some supporting classes.DRLExprParserTestthat usesDrlExprParserto parse an expression string and then makes assertions on the resulting descriptors fromdrools-drl-ast.Grammar changes
I simply went and fixed all issues one by one until the ANTLR compilation finished without errors. That includes:
==). These are trivial changes, please don't focus on these.ANTLR v4 API changes
Most of the supporting classes from
drools-drl-parsercan be reused. That's why the dependency is being added. But some classes use the ANTLR v3 API. Those classes were copied here and updated to the v4 API.