CLIENT-4526 Fix error listener's silent omitting

src/main/antlr4/com/aerospike/dsl/Condition.g4

@@ -15,15 +15,7 @@ logicalOrExpression
     ;
 
 logicalAndExpression
-    : basicExpression ('and' basicExpression)*                                  # AndExpression
-    ;
-
-basicExpression
-    : 'not' '(' expression ')'                                                  # NotExpression
-    | 'exclusive' '(' expression (',' expression)+ ')'                          # ExclusiveExpression
-    | 'let' '(' variableDefinition (',' variableDefinition)* ')' 'then' '(' expression ')'      # LetExpression
-    | 'when' '(' expressionMapping (',' expressionMapping)* ',' 'default' '=>' expression ')'   # WhenExpression
-    | comparisonExpression                                                      # ComparisonExpressionWrapper
+    : comparisonExpression ('and' comparisonExpression)*                        # AndExpression
     ;
 
 comparisonExpression
@@ -78,7 +70,7 @@ unaryExpression
     ;
 
 variableDefinition
-    : stringOperand '=' expression
+    : NAME_IDENTIFIER '=' expression
     ;
 
 expressionMapping
@@ -97,8 +89,20 @@ operand
     | placeholder
     | '$.' pathOrMetadata
     | '(' expression ')'
+    | notExpression
+    | exclusiveExpression
+    | letExpression
+    | whenExpression
     ;
 
+notExpression: 'not' '(' expression ')';
+
+exclusiveExpression: 'exclusive' '(' expression (',' expression)+ ')';
+
+letExpression: 'let' '(' variableDefinition (',' variableDefinition)* ')' 'then' '(' expression ')';
+
+whenExpression: 'when' '(' expressionMapping (',' expressionMapping)* ',' 'default' '=>' expression ')';
+
 functionCall
     : NAME_IDENTIFIER '(' expression (',' expression)* ')'
     ;

src/main/java/com/aerospike/dsl/impl/DSLParserErrorListener.java

@@ -10,6 +10,9 @@
 
 class DSLParserErrorListener extends BaseErrorListener {
 
+    private String firstLexerError;
+    private String firstParserError;
+
     @Override
     public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol,
                             int line, int charPositionInLine,
@@ -18,6 +21,7 @@ public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol,
             // LEADING_DOT_FLOAT_HEX_OR_BINARY (.0xff/.0b101) is never accepted by the grammar;
             // LEADING_DOT_FLOAT is the offending token in cases like ".3.7" where ".7" is lexed
             // as a valid-looking token but appears where an operator was expected.
+            // These throw immediately to provide a specific error message.
             if (token.getType() == ConditionLexer.LEADING_DOT_FLOAT_HEX_OR_BINARY
                     || token.getType() == ConditionLexer.LEADING_DOT_FLOAT) {
                 throw new DslParseException("Invalid float literal: " + token.getText());
@@ -31,5 +35,23 @@ public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol,
                 throw new DslParseException("Invalid float literal: " + token.getText() + nextText);
             }
         }
+        if (recognizer instanceof Parser) {
+            if (firstParserError == null) {
+                firstParserError = msg + " at character " + charPositionInLine;
+            }
+        } else {
+            if (firstLexerError == null) {
+                firstLexerError = msg + " at character " + charPositionInLine;
+            }
+        }
+    }
+
+    String getErrorMessage() {
+        if (firstLexerError != null && firstParserError != null) {
+            return "[Lexer] " + firstLexerError + "; [Parser] " + firstParserError;
+        }
+        if (firstLexerError != null) return "[Lexer] " + firstLexerError;
+        if (firstParserError != null) return "[Parser] " + firstParserError;
+        return null;
     }
 }

src/main/java/com/aerospike/dsl/impl/DSLParserImpl.java

@@ -49,20 +49,35 @@ public CTX[] parseCTX(String pathToCtx) {
             throw new DslParseException("Path must not be null or empty");
         }
 
-        ParseTree parseTree = getParseTree(pathToCtx);
         try {
+            ParseTree parseTree = getParseTree(pathToCtx);
             return buildCtx(new ExpressionConditionVisitor().visit(parseTree));
         } catch (Exception e) {
             throw new DslParseException("Could not parse the given DSL path input", e);
         }
     }
 
-    private ParseTree getParseTree(String input) {
+    private ConditionParser createParser(String input, DSLParserErrorListener errorListener) {
         ConditionLexer lexer = new ConditionLexer(CharStreams.fromString(input));
+        lexer.removeErrorListeners();
+        lexer.addErrorListener(errorListener);
         CommonTokenStream tokenStream = new CommonTokenStream(lexer);
         ConditionParser parser = new ConditionParser(tokenStream);
-        parser.addErrorListener(new DSLParserErrorListener());
-        return parser.parse();
+        parser.removeErrorListeners();
+        parser.addErrorListener(errorListener);
+        return parser;
+    }
+
+    private ParseTree getParseTree(String input) {
+        DSLParserErrorListener errorListener = new DSLParserErrorListener();
+        ConditionParser parser = createParser(input, errorListener);
+        ParseTree tree = parser.parse();
+
+        String errorMessage = errorListener.getErrorMessage();
+        if (errorMessage != null) {
+            throw new DslParseException("Could not parse given DSL expression input: " + errorMessage);
+        }
+        return tree;
     }
 
     private ParsedExpression getParsedExpression(ParseTree parseTree, PlaceholderValues placeholderValues,

src/main/java/com/aerospike/dsl/visitor/ExpressionConditionVisitor.java

@@ -54,7 +54,7 @@ public AbstractPart visitLetExpression(ConditionParser.LetExpressionContext ctx)
             // iterate through each definition
             for (ConditionParser.VariableDefinitionContext vdc : ctx.variableDefinition()) {
                 AbstractPart part = visit(vdc.expression());
-                LetOperand letOperand = new LetOperand(part, vdc.stringOperand().getText());
+                LetOperand letOperand = new LetOperand(part, vdc.NAME_IDENTIFIER().getText());
                 expressions.add(letOperand);
             }
             // last expression is the action (described after "then")
@@ -86,14 +86,12 @@ public AbstractPart visitWhenExpression(ConditionParser.WhenExpressionContext ct
 
     @Override
     public AbstractPart visitAndExpression(ConditionParser.AndExpressionContext ctx) {
-        // If there's only one basicExpression and no 'and' operators, just pass through
-        if (ctx.basicExpression().size() == 1) {
-            return visit(ctx.basicExpression(0));
+        if (ctx.comparisonExpression().size() == 1) {
+            return visit(ctx.comparisonExpression(0));
         }
 
         List<ExpressionContainer> expressions = new ArrayList<>();
-        // iterate through each sub-expression
-        for (ConditionParser.BasicExpressionContext ec : ctx.basicExpression()) {
+        for (ConditionParser.ComparisonExpressionContext ec : ctx.comparisonExpression()) {
             ExpressionContainer expr = (ExpressionContainer) visit(ec);
             if (expr == null) return null;
 
@@ -146,12 +144,6 @@ public AbstractPart visitExclusiveExpression(ConditionParser.ExclusiveExpression
                 ExpressionContainer.ExprPartsOperation.EXCLUSIVE_STRUCTURE);
     }
 
-    @Override
-    public AbstractPart visitComparisonExpressionWrapper(ConditionParser.ComparisonExpressionWrapperContext ctx) {
-        // Pass through the wrapper
-        return visit(ctx.comparisonExpression());
-    }
-
     @Override
     public AbstractPart visitBitwiseExpressionWrapper(ConditionParser.BitwiseExpressionWrapperContext ctx) {
         // Pass through the wrapper

src/test/java/com/aerospike/dsl/expression/BinExpressionsTests.java

@@ -66,7 +66,9 @@ void binNotEquals() {
     void negativeStringBinEquals() {
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("$.strBin == yes")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Unexpected identifier");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] mismatched input '<EOF>'")
+                .hasMessageContaining("at character 15");
     }
 
     // TODO: Will be handled in FMWK-486

src/test/java/com/aerospike/dsl/expression/ControlStructuresTests.java

@@ -67,6 +67,48 @@ void whenWithMultipleDeclarations() {
                 expected);
     }
 
+    @Test
+    void whenWithArithmeticBranchesComparedToValue() {
+        Exp expected = Exp.eq(
+                Exp.cond(
+                        Exp.eq(Exp.intBin("A"), Exp.val(0)), Exp.add(Exp.intBin("D"), Exp.intBin("E")),
+                        Exp.eq(Exp.intBin("A"), Exp.val(1)), Exp.sub(Exp.intBin("D"), Exp.intBin("E")),
+                        Exp.eq(Exp.intBin("A"), Exp.val(2)), Exp.mul(Exp.intBin("D"), Exp.intBin("E")),
+                        Exp.val(-1)
+                ),
+                Exp.val(2)
+        );
+
+        TestUtils.parseFilterExpressionAndCompare(
+                ExpressionContext.of("when($.A == 0 => $.D + $.E, " +
+                        "$.A == 1 => $.D - $.E, " +
+                        "$.A == 2 => $.D * $.E, " +
+                        "default => -1) == 2"),
+                expected);
+    }
+
+    @Test
+    void notWhenWithArithmeticBranches() {
+        Exp expected = Exp.not(
+                Exp.eq(
+                        Exp.cond(
+                                Exp.eq(Exp.intBin("A"), Exp.val(0)), Exp.add(Exp.intBin("D"), Exp.intBin("E")),
+                                Exp.eq(Exp.intBin("A"), Exp.val(1)), Exp.sub(Exp.intBin("D"), Exp.intBin("E")),
+                                Exp.eq(Exp.intBin("A"), Exp.val(2)), Exp.mul(Exp.intBin("D"), Exp.intBin("E")),
+                                Exp.val(-1)
+                        ),
+                        Exp.val(2)
+                )
+        );
+
+        TestUtils.parseFilterExpressionAndCompare(
+                ExpressionContext.of("not(when($.A == 0 => $.D + $.E, " +
+                        "$.A == 1 => $.D - $.E, " +
+                        "$.A == 2 => $.D * $.E, " +
+                        "default => -1) == 2)"),
+                expected);
+    }
+
     @Test
     void withMultipleVariablesDefinitionAndUsage() {
         Exp expected = Exp.let(
@@ -81,7 +123,7 @@ void withMultipleVariablesDefinitionAndUsage() {
 
         TestUtils.parseFilterExpressionAndCompare(ExpressionContext.of("let (x = 1, y = ${x} + 1) then (${x} + ${y})"),
                 expected);
-        // different spacing style
+        // Different spacing style
         TestUtils.parseFilterExpressionAndCompare(ExpressionContext.of("let(x = 1, y = ${x} + 1) then(${x} + ${y})"),
                 expected);
     }

src/test/java/com/aerospike/dsl/expression/ExplicitTypesTests.java

@@ -48,7 +48,9 @@ void stringComparisonNegativeTest() {
                 TestUtils.parseFilterExpressionAndCompare(ExpressionContext.of("$.stringBin1.get(type: STRING) == yes"),
                 Exp.eq(Exp.stringBin("stringBin1"), Exp.val("yes"))))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Unexpected identifier");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] mismatched input '<EOF>'")
+                .hasMessageContaining("at character 37");
     }
 
     @Test
@@ -130,7 +132,9 @@ void listComparison_constantOnRightSide_NegativeTest() {
                         Exp.eq(Exp.listBin("listBin1"), Exp.val(List.of("yes", "of course"))))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Unexpected identifier");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] mismatched input ','")
+                .hasMessageContaining("at character 34");
     }
 
     @Test
@@ -173,7 +177,9 @@ void listComparison_constantOnLeftSide_NegativeTest() {
                         Exp.eq(Exp.val(List.of("yes", "of course")), Exp.listBin("listBin1")))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 4");
     }
 
     @SuppressWarnings("unchecked")
@@ -243,7 +249,9 @@ void mapComparison_constantOnRightSide_NegativeTest() {
                         Exp.eq(Exp.mapBin("mapBin1"), Exp.val(treeMapOf("yes", "of course"))))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Unable to parse map operand");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] extraneous input 'yes'")
+                .hasMessageContaining("at character 29");
 
         assertThatThrownBy(() ->
                 TestUtils.parseFilterExpressionAndCompare(ExpressionContext.of("$.mapBin1.get(type: MAP) == ['yes', 'of course']"),
@@ -258,7 +266,9 @@ void mapComparison_constantOnRightSide_NegativeTest() {
                         Exp.eq(Exp.mapBin("mapBin1"), Exp.val(List.of("yes", "of course"))))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Unable to parse map operand");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] extraneous input '['")
+                .hasMessageContaining("at character 29");
     }
 
     @Test
@@ -312,7 +322,9 @@ void mapComparison_constantOnLeftSide_NegativeTest() {
                         Exp.eq(Exp.mapBin("mapBin1"), Exp.val(treeMapOf("of course", "yes"))))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 1");
 
         assertThatThrownBy(() ->
                 TestUtils.parseFilterExpressionAndCompare(ExpressionContext.of("['yes', 'of course'] == $.mapBin1.get(type: MAP)"), // incorrect: must be {}
@@ -327,7 +339,9 @@ void mapComparison_constantOnLeftSide_NegativeTest() {
                         Exp.eq(Exp.val(List.of("yes", "of course")), Exp.mapBin("mapBin1")))
         )
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 1");
     }
 
     @Test

src/test/java/com/aerospike/dsl/expression/InNegativeTests.java

@@ -348,7 +348,7 @@ void negVarBoundToCountPath() {
     void negVarDefLetInScalarVar() {
         assertThatThrownBy(() -> parseFilterExp(
                 ExpressionContext.of(
-                        "let(x = 5, y = ($.bin.get(type: INT) in ${x})) then (y == true)")))
+                        "let(x = 5, y = ($.bin.get(type: INT) in ${x})) then (${y} == true)")))
                 .isInstanceOf(DslParseException.class)
                 .hasMessageContaining("IN operation requires a List as the right operand")
                 .hasMessageContaining("variable 'x'");

src/test/java/com/aerospike/dsl/expression/ListExpressionsTests.java

@@ -306,7 +306,9 @@ void listBinElementCount() {
     void negativeSyntaxList() {
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("$.listBin1.[stringValue] == 100")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 12");
     }
 
     //@Test

src/test/java/com/aerospike/dsl/expression/LogicalExpressionsTests.java

@@ -94,19 +94,27 @@ void flatHierarchyAnd() {
     void negativeSyntaxLogicalOperators() {
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("($.intBin1 > 100 and ($.intBin2 > 100) or")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 41");
 
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("and ($.intBin1 > 100 and ($.intBin2 > 100)")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] extraneous input 'and'")
+                .hasMessageContaining("at character 0");
 
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("($.intBin1 > 100 and ($.intBin2 > 100) not")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 39");
 
         assertThatThrownBy(() -> parseFilterExp(ExpressionContext.of("($.intBin1 > 100 and ($.intBin2 > 100) exclusive")))
                 .isInstanceOf(DslParseException.class)
-                .hasMessageContaining("Could not parse given DSL expression input");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 39");
     }
 
     @Test
@@ -115,6 +123,8 @@ void negativeBinLogicalExclusiveWithOneParam() {
                 Exp.exclusive(
                         Exp.eq(Exp.stringBin("hand"), Exp.val("hook")))))
                 .isInstanceOf(DslParseException.class)
-                .hasMessage("Exclusive logical operator requires 2 or more expressions");
+                .hasMessageContaining("Could not parse given DSL expression input")
+                .hasMessageContaining("[Parser] no viable alternative at input")
+                .hasMessageContaining("at character 26");
     }
 }