You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/10/05 15:36:26 UTC

[groovy] branch GROOVY-8258 updated: GROOVY-8258: Add node positions and error tests

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

sunlan pushed a commit to branch GROOVY-8258
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/GROOVY-8258 by this push:
     new b1650bb  GROOVY-8258: Add node positions and error tests
b1650bb is described below

commit b1650bbfdec0e994dcdd9d0870b2c238afe0ccf9
Author: Daniel Sun <su...@apache.org>
AuthorDate: Mon Oct 5 23:35:51 2020 +0800

    GROOVY-8258: Add node positions and error tests
---
 .../apache/groovy/linq/GinqGroovyMethods.groovy    |  2 +-
 .../org/apache/groovy/linq/dsl/GinqAstBuilder.java | 45 +++++++++++++++--
 .../apache/groovy/linq/dsl/GinqSyntaxError.java    | 43 +++++++++++++++++
 .../dsl/expression/AbstractGinqExpression.java     | 44 +++++++++++++++++
 .../org/apache/groovy/linq/GinqErrorTest.groovy    | 56 ++++++++++++++++++++++
 5 files changed, 186 insertions(+), 4 deletions(-)

diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/GinqGroovyMethods.groovy b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/GinqGroovyMethods.groovy
index 5e521bb..c3f0dcb 100644
--- a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/GinqGroovyMethods.groovy
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/GinqGroovyMethods.groovy
@@ -35,7 +35,7 @@ class GinqGroovyMethods {
     static Expression GINQ(MacroContext ctx, final ClosureExpression closureExpression) {
         Statement code = closureExpression.getCode()
 
-        GinqAstBuilder ginqAstBuilder = new GinqAstBuilder()
+        GinqAstBuilder ginqAstBuilder = new GinqAstBuilder(ctx.getSourceUnit())
         code.visit(ginqAstBuilder)
         SimpleGinqExpression simpleGinqExpression = ginqAstBuilder.getSimpleGinqExpression()
 
diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstBuilder.java b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstBuilder.java
index 9a365e5..c80e347 100644
--- a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstBuilder.java
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstBuilder.java
@@ -30,6 +30,10 @@ import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
+import org.codehaus.groovy.control.SourceUnit;
+import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
+import org.codehaus.groovy.syntax.SyntaxException;
+import org.codehaus.groovy.syntax.Types;
 
 /**
  * Build the AST for GINQ
@@ -39,6 +43,11 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression;
 public class GinqAstBuilder extends CodeVisitorSupport {
     private SimpleGinqExpression simpleGinqExpression = new SimpleGinqExpression(); // store the result
     private GinqExpression ginqExpression; // store the return value
+    private final SourceUnit sourceUnit;
+
+    public GinqAstBuilder(SourceUnit sourceUnit) {
+        this.sourceUnit = sourceUnit;
+    }
 
     public SimpleGinqExpression getSimpleGinqExpression() {
         return simpleGinqExpression;
@@ -52,13 +61,32 @@ public class GinqAstBuilder extends CodeVisitorSupport {
 
         if ("from".equals(methodName)) {
             ArgumentListExpression arguments = (ArgumentListExpression) call.getArguments();
-            BinaryExpression binaryExpression = (BinaryExpression) arguments.getExpression(0);
+            if (arguments.getExpressions().size() != 1) {
+                this.collectSyntaxError(
+                        new GinqSyntaxError(
+                                "Only 1 argument expected for `from`, e.g. `from n in nums`",
+                                call.getLineNumber(), call.getColumnNumber()
+                        )
+                );
+            }
+            final Expression expression = arguments.getExpression(0);
+            if (!(expression instanceof BinaryExpression
+                    && ((BinaryExpression) expression).getOperation().getType() == Types.KEYWORD_IN)) {
+                this.collectSyntaxError(
+                        new GinqSyntaxError(
+                                "`in` is expected for `from`, e.g. `from n in nums`",
+                                call.getLineNumber(), call.getColumnNumber()
+                        )
+                );
+            }
+            BinaryExpression binaryExpression = (BinaryExpression) expression;
             Expression aliasExpr = binaryExpression.getLeftExpression();
             Expression dataSourceExpr = binaryExpression.getRightExpression();
 
             FromExpression fromExpression = new FromExpression(aliasExpr, dataSourceExpr);
-            simpleGinqExpression.addFromExpression(fromExpression);
+            fromExpression.setSourcePosition(call);
 
+            simpleGinqExpression.addFromExpression(fromExpression);
             ginqExpression = fromExpression;
             return;
         }
@@ -66,6 +94,7 @@ public class GinqAstBuilder extends CodeVisitorSupport {
         if ("where".equals(methodName)) {
             Expression filterExpr = ((ArgumentListExpression) call.getArguments()).getExpression(0);
             WhereExpression whereExpression = new WhereExpression(filterExpr);
+            whereExpression.setSourcePosition(call);
 
             if (ginqExpression instanceof FilterableExpression) {
                 ((FilterableExpression) ginqExpression).setWhereExpression(whereExpression);
@@ -78,11 +107,21 @@ public class GinqAstBuilder extends CodeVisitorSupport {
 
         if ("select".equals(methodName)) {
             SelectExpression selectExpression = new SelectExpression(call.getArguments());
-            simpleGinqExpression.setSelectExpression(selectExpression);
+            selectExpression.setSourcePosition(call);
 
+            simpleGinqExpression.setSelectExpression(selectExpression);
             ginqExpression = selectExpression;
 
             return;
         }
     }
+
+    private void collectSyntaxError(GinqSyntaxError ginqSyntaxError) {
+        SyntaxException e = new SyntaxException(
+                ginqSyntaxError.getMessage(),
+                ginqSyntaxError,
+                ginqSyntaxError.getLine(),
+                ginqSyntaxError.getColumn());
+        sourceUnit.getErrorCollector().addFatalError(new SyntaxErrorMessage(e, sourceUnit));
+    }
 }
diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqSyntaxError.java b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqSyntaxError.java
new file mode 100644
index 0000000..652d835
--- /dev/null
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqSyntaxError.java
@@ -0,0 +1,43 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.linq.dsl;
+
+/**
+ * Represents GINQ syntax error
+ *
+ * @since 4.0.0
+ */
+public class GinqSyntaxError extends AssertionError {
+    private final int line;
+    private final int column;
+
+    public GinqSyntaxError(String message, int line, int column) {
+        super(message, null);
+        this.line = line;
+        this.column = column;
+    }
+
+    public int getLine() {
+        return line;
+    }
+
+    public int getColumn() {
+        return column;
+    }
+}
diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/expression/AbstractGinqExpression.java b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/expression/AbstractGinqExpression.java
index 25e20c9..5cc3c36 100644
--- a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/expression/AbstractGinqExpression.java
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/expression/AbstractGinqExpression.java
@@ -1,5 +1,6 @@
 package org.apache.groovy.linq.dsl.expression;
 
+import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.NodeMetaDataHandler;
 
 import java.util.LinkedHashMap;
@@ -12,6 +13,10 @@ import java.util.Map;
  */
 public abstract class AbstractGinqExpression implements GinqExpression, NodeMetaDataHandler {
     private Map<?, ?> metaDataMap = new LinkedHashMap<>();
+    private int lineNumber = -1;
+    private int columnNumber = -1;
+    private int lastLineNumber = -1;
+    private int lastColumnNumber = -1;
 
     @Override
     public Map<?, ?> getMetaDataMap() {
@@ -22,4 +27,43 @@ public abstract class AbstractGinqExpression implements GinqExpression, NodeMeta
     public void setMetaDataMap(Map<?, ?> metaDataMap) {
         this.metaDataMap = metaDataMap;
     }
+
+    public void setSourcePosition(ASTNode node) {
+        this.lineNumber = node.getLineNumber();
+        this.columnNumber = node.getColumnNumber();
+        this.lastLineNumber = node.getLastLineNumber();
+        this.lastColumnNumber = node.getLastColumnNumber();
+    }
+
+    public int getLineNumber() {
+        return lineNumber;
+    }
+
+    public void setLineNumber(int lineNumber) {
+        this.lineNumber = lineNumber;
+    }
+
+    public int getColumnNumber() {
+        return columnNumber;
+    }
+
+    public void setColumnNumber(int columnNumber) {
+        this.columnNumber = columnNumber;
+    }
+
+    public int getLastLineNumber() {
+        return lastLineNumber;
+    }
+
+    public void setLastLineNumber(int lastLineNumber) {
+        this.lastLineNumber = lastLineNumber;
+    }
+
+    public int getLastColumnNumber() {
+        return lastColumnNumber;
+    }
+
+    public void setLastColumnNumber(int lastColumnNumber) {
+        this.lastColumnNumber = lastColumnNumber;
+    }
 }
diff --git a/subprojects/groovy-linq/src/test/groovy/org/apache/groovy/linq/GinqErrorTest.groovy b/subprojects/groovy-linq/src/test/groovy/org/apache/groovy/linq/GinqErrorTest.groovy
new file mode 100644
index 0000000..cee78af
--- /dev/null
+++ b/subprojects/groovy-linq/src/test/groovy/org/apache/groovy/linq/GinqErrorTest.groovy
@@ -0,0 +1,56 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.linq
+
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+
+@CompileStatic
+class GinqErrorTest {
+    @Test
+    void "testGinq - from select - 1"() {
+        def err = shouldFail '''\
+            def numbers = [0, 1, 2]
+            GINQ {
+                from numbers
+                select n
+            }
+        '''
+
+        assert err.toString().contains('`in` is expected for `from`, e.g. `from n in nums` @ line 3, column 17.')
+    }
+
+    @Test
+    void "testGinq - from select - 2"() {
+        def err = shouldFail '''\
+            def numbers = [0, 1, 2]
+            GINQ {
+                from n, numbers
+                select n
+            }
+        '''
+
+        assert err.toString().contains('Only 1 argument expected for `from`, e.g. `from n in nums` @ line 3, column 17.')
+    }
+
+}