From a314a396e4c0f01e8d36decdb370e9f17dcce4c0 Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Tue, 21 May 2024 15:51:56 +0900 Subject: [PATCH] =?UTF-8?q?[incubator-kie-drools-5818]=20[new-parser]=20Pa?= =?UTF-8?q?rsing=20fails=20if=20a=20Java=20keyw=E2=80=A6=20(#5958)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [incubator-kie-drools-5818] [new-parser] Parsing fails if a Java keyword appears in a qualified name * - Exclude 'new' from 'drlIdentifier' because of 'primary' ambiguity - Add tests --- .../drl/parser/antlr4/DRLExprParserTest.java | 22 +++++-- .../drl/parser/antlr4/MiscDRLParserTest.java | 19 ++++++ .../drl/parser/antlr4/ParserTestUtils.java | 19 ++++++ .../drl/parser/antlr4/DRL6Expressions.g4 | 59 ++++++++++++++++++- 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java index 2a6d4e96a6a..6489f3fdf80 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java @@ -442,12 +442,13 @@ public void testExtraneousInput() { @Test public void testNoViableAlt() { - String source = "x.int"; + String source = "a~a"; parser.parse(source); // Backward Compatibility Notes: - // Old expr parser (DRL6Expressions) allows this expression because it's too tolerant (fail at runtime anyway). - // Backward compatibility doesn't seem to be required in this case. (But we may align with the old tolerant behavior.) + // Old expr parser (DRL6Expressions) allows this expression and only takes "a" ignoring the invalid part "~a" without emitting an error. + // This is rather a bug in the old parser, and the new parser (ANTLR4) correctly emits an error for this case. + // Backward compatibility doesn't seem to be required in this case. if (DrlParser.ANTLR4_PARSER_ENABLED) { assertThat(parser.hasErrors()).isTrue(); assertThat(parser.getErrors()).hasSize(1); @@ -457,7 +458,7 @@ public void testNoViableAlt() { assertThat(exception.getColumn()).isEqualTo(2); assertThat(exception.getOffset()).isEqualTo(2); assertThat(exception.getMessage()) - .isEqualToIgnoringCase("[ERR 101] Line 1:2 no viable alternative at input '.int'"); + .isEqualToIgnoringCase("[ERR 101] Line 1:2 no viable alternative at input 'a'"); } else { assertThat(parser.hasErrors()).isFalse(); } @@ -564,4 +565,17 @@ void andWithMethodCallWithArg() { assertThat(left.getExpression()).isEqualTo("someMethod(value)"); assertThat(right.getExpression()).isEqualTo("4"); } + + @Test + void newBigDecimal() { + String source = "$bd : new BigDecimal(30)"; + ConstraintConnectiveDescr result = parser.parse(source); + assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); + + assertThat(result.getDescrs().size()).isEqualTo(1); + + BindingDescr bind = (BindingDescr) result.getDescrs().get(0); + assertThat(bind.getVariable()).isEqualTo("$bd"); + assertThat(bind.getExpression()).isEqualTo("new BigDecimal(30)"); + } } diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java index bf553765e26..85c8423ba89 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java @@ -5103,4 +5103,23 @@ void errorMessage_shouldNotContainEmptyString() { assertThat(parser.hasErrors()).isTrue(); assertThat(parser.getErrors()).extracting(DroolsError::getMessage).doesNotContain(""); } + + @ParameterizedTest + @MethodSource("org.drools.drl.parser.antlr4.ParserTestUtils#javaKeywords") + void javaKeywordsInPackage(String keyword) { + String pkgName = "org.drools." + keyword; + String text = "package " + pkgName + "\n" + + "rule R\n" + + "when\n" + + " $p : Person()\n" + + "then\n" + + "end\n"; + + PackageDescr pkg = parseAndGetPackageDescr(text); + + assertThat(pkg.getName()).isEqualTo(pkgName); + assertThat(pkg.getRules()).hasSize(1); + + assertThat(pkg.getRules().get(0).getName()).isEqualTo("R"); + } } diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/ParserTestUtils.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/ParserTestUtils.java index 270b8f8c37c..3419eab65e5 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/ParserTestUtils.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/ParserTestUtils.java @@ -18,10 +18,25 @@ */ package org.drools.drl.parser.antlr4; +import java.util.Arrays; +import java.util.List; + import org.drools.drl.parser.DrlParser; public class ParserTestUtils { + // 'new' is not included because it cannot be included in drlIdentifier. + // See https://github.com/apache/incubator-kie-drools/pull/5958 + public static List javaKeywords = + Arrays.asList( + "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", + "continue", "default", "do", "double", "else", "enum", "extends", "final", "finally", "float", + "for", "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", + "package", "private", "protected", "public", "return", "short", "static", "strictfp", + "super", "switch", "synchronized", "this", "throw", "throws", "transient", "try", "void", "volatile", + "while" + ); + private ParserTestUtils() { // It is a utility class, so it should not be instantiated. } @@ -39,4 +54,8 @@ public static DrlParser getParser() { public static void enableOldParser() { DrlParser.ANTLR4_PARSER_ENABLED = false; } + + public static List javaKeywords() { + return javaKeywords; + } } diff --git a/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4 b/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4 index 27734251f1e..f2a5a79f195 100644 --- a/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4 +++ b/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4 @@ -162,6 +162,58 @@ typeArgument drlIdentifier returns [Token token] : drlKeywords | IDENTIFIER + // java keywords + | ABSTRACT + | ASSERT + | BOOLEAN + | BREAK + | BYTE + | CASE + | CATCH + | CHAR + | CLASS + | CONST + | CONTINUE + | DEFAULT + | DO + | DOUBLE + | ELSE + | ENUM + | EXTENDS + | FINAL + | FINALLY + | FLOAT + | FOR + | IF + | GOTO + | IMPLEMENTS + | IMPORT + | INSTANCEOF + | INT + | INTERFACE + | LONG + | NATIVE +// | NEW // avoid ambiguity with 'new_key creator' and 'drlIdentifier' in 'primary' + | PACKAGE + | PRIVATE + | PROTECTED + | PUBLIC + | RETURN + | SHORT + | STATIC + | STRICTFP + | SUPER + | SWITCH + | SYNCHRONIZED + | THIS + | THROW + | THROWS + | TRANSIENT + | TRY + | VOID + | VOLATILE + | WHILE + // Module related keywords | MODULE | OPEN | REQUIRES @@ -172,12 +224,13 @@ drlIdentifier returns [Token token] | PROVIDES | WITH | TRANSITIVE + // other java keywords + | VAR | YIELD + | RECORD | SEALED | PERMITS - | RECORD - | VAR - | THIS + | NON_SEALED ; // matches any drl keywords