Enhance test/grammar coverage. function, attributes, consequence (apache#12)

* Enhance test/grammar coverage. function, attributes, consequence
- better getText preserving whitespaces
- resolving WARNs because of overlapping keywords

* - No need to declare duplicate keywords

Author: tkobayas

drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLLexer.g4

@@ -18,16 +18,20 @@ import JavaLexer;
 // KEYWORDS
 /////////////////
 
-DRL_PACKAGE : 'package';
+// These keywords are already declared in JavaLexer. They should not be overriden with different names, or else Vocabulary's literalName will be null.
+// So no need to declare by DRLLexer
+// PACKAGE : 'package';
+// IMPORT : 'import';
+// STATIC : 'static';
+// EXTENDS : 'extends';
+// SUPER : 'super';
+
+// DRL keywords
 DRL_UNIT : 'unit';
-DRL_IMPORT : 'import';
 DRL_FUNCTION : 'function';
-DRL_STATIC : 'static';
 DRL_GLOBAL : 'global';
 DRL_RULE : 'rule';
 DRL_QUERY : 'query';
-DRL_EXTENDS : 'extends';
-DRL_SUPER : 'super';
 DRL_WHEN : 'when';
 DRL_THEN : 'then';
 DRL_END : 'end';

drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLParser.g4

@@ -24,20 +24,20 @@ drlStatementdef
     | ruledef
     ;
 
-packagedef : DRL_PACKAGE name=drlQualifiedName SEMI? ;
+packagedef : PACKAGE name=drlQualifiedName SEMI? ;
 
 unitdef : DRL_UNIT name=drlQualifiedName SEMI? ;
 
-importdef : DRL_IMPORT (DRL_FUNCTION|DRL_STATIC)? drlQualifiedName (DOT MUL)? SEMI? ;
+importdef : IMPORT (DRL_FUNCTION|STATIC)? drlQualifiedName (DOT MUL)? SEMI? ;
 
 globaldef : DRL_GLOBAL type drlIdentifier SEMI? ;
 
 // rule := RULE stringId (EXTENDS stringId)? annotation* attributes? lhs? rhs END
 
-ruledef : DRL_RULE name=stringId (DRL_EXTENDS stringId)? drlAnnotation* attributes? DRL_WHEN lhs DRL_THEN rhs DRL_END ;
+ruledef : DRL_RULE name=stringId (EXTENDS stringId)? drlAnnotation* attributes? DRL_WHEN lhs DRL_THEN rhs DRL_END ;
 
 lhs : lhsExpression? ;
-lhsExpression : lhsOr* ;
+lhsExpression : lhsOr+ ;
 lhsOr : LPAREN DRL_OR lhsAnd+ RPAREN | lhsAnd (DRL_OR lhsAnd)* ;
 lhsAnd : LPAREN DRL_AND lhsUnary+ RPAREN | lhsUnary (DRL_AND lhsUnary)* ;
 
@@ -111,16 +111,11 @@ drlIdentifier
     ;
 
 drlKeywords
-    : DRL_PACKAGE
-    | DRL_UNIT
-    | DRL_IMPORT
+    : DRL_UNIT
     | DRL_FUNCTION
-    | DRL_STATIC
     | DRL_GLOBAL
     | DRL_RULE
     | DRL_QUERY
-    | DRL_EXTENDS
-    | DRL_SUPER
     | DRL_WHEN
     | DRL_THEN
     | DRL_END
@@ -154,7 +149,7 @@ drlExpression
     : drlPrimary
     | drlExpression bop=DOT
       (
-         identifier
+         drlIdentifier
        | methodCall
        | THIS
        | NEW nonWildcardTypeArguments? innerCreator
@@ -163,7 +158,7 @@ drlExpression
       )
     | drlExpression LBRACK drlExpression RBRACK
     | methodCall
-    | NEW creator
+    | NEW drlCreator
     | LPAREN annotation* typeType (BITAND typeType)* RPAREN drlExpression
     | drlExpression postfix=(INC | DEC)
     | prefix=(ADD|SUB|INC|DEC) drlExpression
@@ -188,8 +183,8 @@ drlExpression
     | switchExpression // Java17
 
     // Java 8 methodReference
-    | drlExpression COLONCOLON typeArguments? identifier
-    | typeType COLONCOLON (typeArguments? identifier | NEW)
+    | drlExpression COLONCOLON typeArguments? drlIdentifier
+    | typeType COLONCOLON (typeArguments? drlIdentifier | NEW)
     | classType COLONCOLON typeArguments? NEW
     ;
 
@@ -290,11 +285,38 @@ unif : IDENTIFIER UNIFY ;
 
 /* extending JavaParser blockStatement */
 drlRhsBlockStatement
-    : localVariableDeclaration SEMI
+    : drlLocalVariableDeclaration SEMI
     | drlRhsStatement
     | localTypeDeclaration
     ;
 
+/* extending JavaParser localVariableDeclaration */
+drlLocalVariableDeclaration
+    : variableModifier* (typeType drlVariableDeclarators | VAR drlIdentifier ASSIGN drlExpression)
+    ;
+
+/* extending JavaParser variableDeclarators */
+drlVariableDeclarators
+    : drlVariableDeclarator (COMMA drlVariableDeclarator)*
+    ;
+
+/* extending JavaParser variableDeclarator */
+drlVariableDeclarator
+    : drlVariableDeclaratorId (ASSIGN drlVariableInitializer)?
+    ;
+
+/* extending JavaParser variableDeclaratorId */
+drlVariableDeclaratorId
+    : drlIdentifier (LBRACK RBRACK)*
+    ;
+
+/* extending JavaParser variableInitializer */
+drlVariableInitializer
+    : arrayInitializer
+    | drlExpression
+    ;
+
+
 /* extending JavaParser statement */
 drlRhsStatement
     : blockLabel=block
@@ -309,21 +331,21 @@ drlRhsStatement
     | SYNCHRONIZED parExpression block
     | RETURN drlRhsExpression? SEMI
     | THROW drlRhsExpression SEMI
-    | BREAK identifier? SEMI
-    | CONTINUE identifier? SEMI
+    | BREAK drlIdentifier? SEMI
+    | CONTINUE drlIdentifier? SEMI
     | YIELD drlRhsExpression SEMI // Java17
     | SEMI
     | statementExpression=drlRhsExpression SEMI
     | switchExpression SEMI? // Java17
-    | identifierLabel=identifier COLON drlRhsStatement
+    | identifierLabel=drlIdentifier COLON drlRhsStatement
     ;
 
 /* extending JavaParser expression */
 drlRhsExpression
     : drlPrimary
     | drlRhsExpression bop=DOT
       (
-         identifier
+         drlIdentifier
        | methodCall
        | THIS
        | NEW nonWildcardTypeArguments? innerCreator
@@ -357,7 +379,20 @@ drlRhsExpression
     | switchExpression // Java17
 
     // Java 8 methodReference
-    | drlRhsExpression COLONCOLON typeArguments? identifier
-    | typeType COLONCOLON (typeArguments? identifier | NEW)
+    | drlRhsExpression COLONCOLON typeArguments? drlIdentifier
+    | typeType COLONCOLON (typeArguments? drlIdentifier | NEW)
     | classType COLONCOLON typeArguments? NEW
-    ;
+    ;
+
+ drlCreator
+     : nonWildcardTypeArguments createdName classCreatorRest
+     | createdName (drlArrayCreatorRest | classCreatorRest)
+     ;
+
+ drlArrayCreatorRest
+     : LBRACK (RBRACK (LBRACK RBRACK)* drlArrayInitializer | expression RBRACK (LBRACK expression RBRACK)* (LBRACK RBRACK)*)
+     ;
+
+ drlArrayInitializer
+     : LBRACE (drlVariableInitializer (COMMA drlVariableInitializer)* (COMMA)? )? RBRACE
+     ;

drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java

@@ -16,6 +16,7 @@
 import org.drools.drl.ast.descr.ExistsDescr;
 import org.drools.drl.ast.descr.ExprConstraintDescr;
 import org.drools.drl.ast.descr.FromDescr;
+import org.drools.drl.ast.descr.FunctionDescr;
 import org.drools.drl.ast.descr.FunctionImportDescr;
 import org.drools.drl.ast.descr.GlobalDescr;
 import org.drools.drl.ast.descr.ImportDescr;
@@ -68,7 +69,7 @@ public Object visitGlobaldef(DRLParser.GlobaldefContext ctx) {
     @Override
     public Object visitImportdef(DRLParser.ImportdefContext ctx) {
         String target = ctx.drlQualifiedName().getText() + (ctx.MUL() != null ? ".*" : "");
-        if (ctx.DRL_FUNCTION() != null || ctx.DRL_STATIC() != null) {
+        if (ctx.DRL_FUNCTION() != null || ctx.STATIC() != null) {
             FunctionImportDescr functionImportDescr = new FunctionImportDescr();
             functionImportDescr.setTarget(target);
             populateStartEnd(functionImportDescr, ctx);
@@ -82,10 +83,39 @@ public Object visitImportdef(DRLParser.ImportdefContext ctx) {
         return super.visitImportdef(ctx);
     }
 
+    @Override
+    public Object visitFunctiondef(DRLParser.FunctiondefContext ctx) {
+        FunctionDescr functionDescr = new FunctionDescr();
+        functionDescr.setNamespace(packageDescr.getNamespace());
+        AttributeDescr dialect = packageDescr.getAttribute("dialect");
+        if (dialect != null) {
+            functionDescr.setDialect(dialect.getValue());
+        }
+        if (ctx.typeTypeOrVoid() != null) {
+            functionDescr.setReturnType(ctx.typeTypeOrVoid().getText());
+        } else {
+            functionDescr.setReturnType("void");
+        }
+        functionDescr.setName(ctx.IDENTIFIER().getText());
+        DRLParser.FormalParametersContext formalParametersContext = ctx.formalParameters();
+        DRLParser.FormalParameterListContext formalParameterListContext = formalParametersContext.formalParameterList();
+        if (formalParameterListContext != null) {
+            List<DRLParser.FormalParameterContext> formalParameterContexts = formalParameterListContext.formalParameter();
+            formalParameterContexts.stream().forEach(formalParameterContext -> {
+                DRLParser.TypeTypeContext typeTypeContext = formalParameterContext.typeType();
+                DRLParser.VariableDeclaratorIdContext variableDeclaratorIdContext = formalParameterContext.variableDeclaratorId();
+                functionDescr.addParameter(typeTypeContext.getText(), variableDeclaratorIdContext.getText());
+            });
+        }
+        functionDescr.setBody(ParserStringUtils.getTextPreservingWhitespace(ctx.block()));
+        packageDescr.addFunction(functionDescr);
+        return super.visitFunctiondef(ctx);
+    }
+
     @Override
     public Object visitRuledef(DRLParser.RuledefContext ctx) {
         currentRule = new RuleDescr(safeStripStringDelimiters(ctx.name.getText()));
-        currentRule.setConsequence(ctx.rhs().getText());
+        currentRule.setConsequence(ParserStringUtils.getTextPreservingWhitespace(ctx.rhs()));
         packageDescr.addRule(currentRule);
 
         Object result = super.visitRuledef(ctx);

drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/ParserStringUtils.java

@@ -1,5 +1,8 @@
 package org.drools.parser;
 
+import org.antlr.v4.runtime.ParserRuleContext;
+import org.antlr.v4.runtime.misc.Interval;
+
 /**
  * will be merged in drools-util
  */
@@ -17,4 +20,15 @@ public static String safeStripStringDelimiters(String value) {
         }
         return value;
     }
+
+    public static String getTextPreservingWhitespace(ParserRuleContext ctx) {
+        int startIndex = ctx.start.getStartIndex();
+        int stopIndex = ctx.stop.getStopIndex();
+        if (startIndex > stopIndex) {
+            // no text
+            return "";
+        }
+        Interval interval = new Interval(startIndex, stopIndex);
+        return ctx.start.getTokenSource().getInputStream().getText(interval);
+    }
 }

drools-drl/drools-drl10-parser/src/test/java/org/drools/parser/DRLParserTest.java

@@ -5,6 +5,7 @@
 import org.drools.drl.ast.descr.*;
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.drools.parser.DRLParserHelper.*;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -60,7 +61,7 @@ public void testParse() {
         ExprConstraintDescr expr = (ExprConstraintDescr) constraints.get(0);
         assertEquals("age >= 18", expr.getExpression());
 
-        assertEquals("inta=4;System.out.println($p.getName());", ruleDescr.getConsequence());
+        assertThat(ruleDescr.getConsequence().toString()).isEqualToIgnoringWhitespace("int a = 4; System.out.println($p.getName());");
     }
 
     @Test