You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/02/07 18:18:30 UTC

[groovy] branch master updated: GROOVY-5726: source range of method call in command expression

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 72ecf0a  GROOVY-5726: source range of method call in command expression
72ecf0a is described below

commit 72ecf0a2eb4d055fc7d406fe2f239d06eaeb8df7
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Feb 7 12:12:09 2022 -0600

    GROOVY-5726: source range of method call in command expression
    
    range of call only covered argument(s)
    
    `foo bar baz` --> `this.foo(bar).baz`
---
 .../apache/groovy/parser/antlr4/AstBuilder.java    | 49 ++++++++--------------
 .../org/apache/groovy/ginq/GinqErrorTest.groovy    | 12 +++---
 2 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index ae248a8..702b962 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -2539,7 +2539,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
         boolean hasArgumentList = asBoolean(ctx.enhancedArgumentListInPar());
         boolean hasCommandArgument = asBoolean(ctx.commandArgument());
 
-        if (visitingArrayInitializerCount > 0 && (hasArgumentList || hasCommandArgument)) {
+        if ((hasArgumentList || hasCommandArgument) && visitingArrayInitializerCount > 0) {
             // To avoid ambiguities, command chain expression should not be used in array initializer
             // the old parser does not support either, so no breaking changes
             // SEE http://groovy.329449.n5.nabble.com/parrot-Command-expressions-in-array-initializer-tt5752273.html
@@ -2548,12 +2548,9 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
 
         Expression baseExpr = (Expression) this.visit(ctx.expression());
 
-        if (hasArgumentList || hasCommandArgument) {
-            if (baseExpr instanceof BinaryExpression) {
-                if (!"[".equals(((BinaryExpression) baseExpr).getOperation().getText()) && !isInsideParentheses(baseExpr)) {
-                    throw createParsingFailedException("Unexpected input: '" + getOriginalText(ctx.expression()) + "'", ctx.expression());
-                }
-            }
+        if ((hasArgumentList || hasCommandArgument) && !isInsideParentheses(baseExpr)
+                && baseExpr instanceof BinaryExpression && !"[".equals(((BinaryExpression) baseExpr).getOperation().getText())) {
+            throw createParsingFailedException("Unexpected input: '" + getOriginalText(ctx.expression()) + "'", ctx.expression());
         }
 
         MethodCallExpression methodCallExpression = null;
@@ -2562,54 +2559,44 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
             Expression arguments = this.visitEnhancedArgumentListInPar(ctx.enhancedArgumentListInPar());
 
             if (baseExpr instanceof PropertyExpression) { // e.g. obj.a 1, 2
-                methodCallExpression =
-                        configureAST(
-                                this.createMethodCallExpression(
-                                        (PropertyExpression) baseExpr, arguments),
-                                arguments);
+                methodCallExpression = configureAST(this.createMethodCallExpression((PropertyExpression) baseExpr, arguments), ctx.expression(), arguments);
 
             } else if (baseExpr instanceof MethodCallExpression && !isInsideParentheses(baseExpr)) { // e.g. m {} a, b  OR  m(...) a, b
                 if (asBoolean(arguments)) {
                     // The error should never be thrown.
                     throw new GroovyBugError("When baseExpr is a instance of MethodCallExpression, which should follow NO argumentList");
                 }
-
                 methodCallExpression = (MethodCallExpression) baseExpr;
-            } else if (
-                    !isInsideParentheses(baseExpr)
-                            && (baseExpr instanceof VariableExpression /* e.g. m 1, 2 */
+            } else {
+                if (!isInsideParentheses(baseExpr)
+                        && (baseExpr instanceof VariableExpression /* e.g. m 1, 2 */
                             || baseExpr instanceof GStringExpression /* e.g. "$m" 1, 2 */
-                            || (baseExpr instanceof ConstantExpression && isTrue(baseExpr, IS_STRING)) /* e.g. "m" 1, 2 */)
-            ) {
-                validateInvalidMethodDefinition(baseExpr, arguments);
-
-                methodCallExpression =
-                        configureAST(
-                                this.createMethodCallExpression(baseExpr, arguments),
-                                arguments);
-            } else { // e.g. a[x] b, new A() b, etc.
-                methodCallExpression = configureAST(this.createCallMethodCallExpression(baseExpr, arguments), arguments);
+                            || (baseExpr instanceof ConstantExpression && isTrue(baseExpr, IS_STRING)) /* e.g. "m" 1, 2 */)) {
+                    validateInvalidMethodDefinition(baseExpr, arguments);
+                } else {
+                    // e.g. a[x] b, new A() b, etc.
+                }
+                methodCallExpression = configureAST(this.createMethodCallExpression(baseExpr, arguments), ctx.expression(), arguments);
             }
 
-            methodCallExpression.putNodeMetaData(IS_COMMAND_EXPRESSION, true);
+            methodCallExpression.putNodeMetaData(IS_COMMAND_EXPRESSION, Boolean.TRUE);
 
             if (!hasCommandArgument) {
-                return configureAST(methodCallExpression, ctx);
+                return methodCallExpression;
             }
         }
 
         if (hasCommandArgument) {
-            baseExpr.putNodeMetaData(IS_COMMAND_EXPRESSION, true);
+            baseExpr.putNodeMetaData(IS_COMMAND_EXPRESSION, Boolean.TRUE);
         }
 
         return configureAST(
                 (Expression) ctx.commandArgument().stream()
                         .map(e -> (Object) e)
-                        .reduce(null == methodCallExpression ? baseExpr : methodCallExpression,
+                        .reduce(methodCallExpression != null ? methodCallExpression : baseExpr,
                                 (r, e) -> {
                                     CommandArgumentContext commandArgumentContext = (CommandArgumentContext) e;
                                     commandArgumentContext.putNodeMetaData(CMD_EXPRESSION_BASE_EXPR, r);
-
                                     return this.visitCommandArgument(commandArgumentContext);
                                 }
                         ),
diff --git a/subprojects/groovy-ginq/src/test/groovy/org/apache/groovy/ginq/GinqErrorTest.groovy b/subprojects/groovy-ginq/src/test/groovy/org/apache/groovy/ginq/GinqErrorTest.groovy
index 1c4e7d4..e9f572c 100644
--- a/subprojects/groovy-ginq/src/test/groovy/org/apache/groovy/ginq/GinqErrorTest.groovy
+++ b/subprojects/groovy-ginq/src/test/groovy/org/apache/groovy/ginq/GinqErrorTest.groovy
@@ -21,11 +21,11 @@ package org.apache.groovy.ginq
 import groovy.transform.CompileStatic
 import org.junit.Test
 
-import static groovy.test.GroovyAssert.assertScript
 import static groovy.test.GroovyAssert.shouldFail
 
 @CompileStatic
-class GinqErrorTest {
+final class GinqErrorTest {
+
     @Test
     void "testGinq - from - 1"() {
         def err = shouldFail '''\
@@ -65,9 +65,9 @@ class GinqErrorTest {
             GQ {
                 select n from n in [0, 1, 2]
             }
-        '''
+        ''' //  ^
 
-        assert err.toString().contains("One `from` is expected and must be the first clause @ line 2, column 24.")
+        assert err.toString().contains("One `from` is expected and must be the first clause @ line 2, column 17.")
     }
 
     @Test
@@ -462,9 +462,9 @@ class GinqErrorTest {
                 from n in [1, 1, 2, 2]
                 select n, (rowNumber() over(order by n))
             }.toList()
-        '''
+        ''' //                              ^
 
-        assert err.toString().contains('Unknown window clause: `order` @ line 3, column 51.')
+        assert err.toString().contains('Unknown window clause: `order` @ line 3, column 45.')
     }
 
     @Test