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/06 17:11:46 UTC

[groovy] branch GROOVY-8258 updated: GROOVY-8258: minor refactor and add more error test cases

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 8a7c91a  GROOVY-8258: minor refactor and add more error test cases
8a7c91a is described below

commit 8a7c91aad9999ed5307c0059664c06832e643deb
Author: Daniel Sun <su...@apache.org>
AuthorDate: Wed Oct 7 01:10:07 2020 +0800

    GROOVY-8258: minor refactor and add more error test cases
---
 .../apache/groovy/linq/GinqGroovyMethods.groovy    |  2 +-
 .../org/apache/groovy/linq/dsl/GinqAstBuilder.java | 14 ++-----
 .../apache/groovy/linq/dsl/GinqAstWalker.groovy    | 27 +++++++++++++-
 .../groovy/linq/dsl/SyntaxErrorReportable.java     | 43 ++++++++++++++++++++++
 .../org/apache/groovy/linq/GinqErrorTest.groovy    | 17 ++++++++-
 5 files changed, 88 insertions(+), 15 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 3650cb3..534810b 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
@@ -39,7 +39,7 @@ class GinqGroovyMethods {
         code.visit(ginqAstBuilder)
         SimpleGinqExpression simpleGinqExpression = ginqAstBuilder.getSimpleGinqExpression()
 
-        GinqAstWalker ginqBuilder = new GinqAstWalker()
+        GinqAstWalker ginqBuilder = new GinqAstWalker(ctx.getSourceUnit())
         MethodCallExpression selectMethodCallExpression = ginqBuilder.visitSimpleGinqExpression(simpleGinqExpression)
 
         return selectMethodCallExpression
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 9f972a9..ee37ce5 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
@@ -35,8 +35,6 @@ 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;
 
 /**
@@ -44,7 +42,7 @@ import org.codehaus.groovy.syntax.Types;
  *
  * @since 4.0.0
  */
-public class GinqAstBuilder extends CodeVisitorSupport {
+public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorReportable {
     private SimpleGinqExpression simpleGinqExpression = new SimpleGinqExpression(); // store the result
     private GinqExpression ginqExpression; // store the return value
     private final SourceUnit sourceUnit;
@@ -136,12 +134,8 @@ public class GinqAstBuilder extends CodeVisitorSupport {
         }
     }
 
-    private void collectSyntaxError(GinqSyntaxError ginqSyntaxError) {
-        SyntaxException e = new SyntaxException(
-                ginqSyntaxError.getMessage(),
-                ginqSyntaxError,
-                ginqSyntaxError.getLine(),
-                ginqSyntaxError.getColumn());
-        sourceUnit.getErrorCollector().addFatalError(new SyntaxErrorMessage(e, sourceUnit));
+    @Override
+    public SourceUnit getSourceUnit() {
+        return sourceUnit;
     }
 }
diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstWalker.groovy b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstWalker.groovy
index 0be2349..ad0f720 100644
--- a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstWalker.groovy
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/GinqAstWalker.groovy
@@ -38,6 +38,7 @@ import org.codehaus.groovy.ast.expr.ListExpression
 import org.codehaus.groovy.ast.expr.MethodCallExpression
 import org.codehaus.groovy.ast.expr.TupleExpression
 import org.codehaus.groovy.ast.expr.VariableExpression
+import org.codehaus.groovy.control.SourceUnit
 
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX
 import static org.codehaus.groovy.ast.tools.GeneralUtils.lambdaX
@@ -52,7 +53,13 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt
  * @since 4.0.0
  */
 @CompileStatic
-class GinqAstWalker implements GinqVisitor<Object> {
+class GinqAstWalker implements GinqVisitor<Object>, SyntaxErrorReportable {
+
+    private final SourceUnit sourceUnit;
+
+    public GinqAstWalker(SourceUnit sourceUnit) {
+        this.sourceUnit = sourceUnit;
+    }
 
     @Override
     MethodCallExpression visitSimpleGinqExpression(SimpleGinqExpression simpleGinqExpression) {
@@ -107,10 +114,21 @@ class GinqAstWalker implements GinqVisitor<Object> {
         DataSourceExpression dataSourceExpression = innerJoinExpression.getNodeMetaData(__DATA_SOURCE_EXPRESSION)
         Expression receiverAliasExpr = dataSourceExpression.aliasExpr
         List<FilterExpression> filterExpressionList = innerJoinExpression.getFilterExpressionList()
+        int filterExpressionListSize = filterExpressionList.size()
+
+        if (0 == filterExpressionListSize) {
+            this.collectSyntaxError(
+                    new GinqSyntaxError(
+                            "`on` clause is expected for `innerJoin`",
+                            innerJoinExpression.getLineNumber(), innerJoinExpression.getColumnNumber()
+                    )
+            );
+        }
+
         OnExpression onExpression = (OnExpression) filterExpressionList.get(0)
 
         WhereExpression whereExpression = null
-        if (filterExpressionList.size() > 1) {
+        if (filterExpressionListSize > 1) {
             whereExpression = (WhereExpression) filterExpressionList.get(1)
         }
 
@@ -257,6 +275,11 @@ class GinqAstWalker implements GinqVisitor<Object> {
         propX(new VariableExpression(__T), name)
     }
 
+    @Override
+    SourceUnit getSourceUnit() {
+        sourceUnit
+    }
+
     private static final String __DATA_SOURCE_EXPRESSION = "__dataSourceExpression"
     private static final String __METHOD_CALL_RECEIVER = "__methodCallReceiver"
     private static final String __T = "__t"
diff --git a/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/SyntaxErrorReportable.java b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/SyntaxErrorReportable.java
new file mode 100644
index 0000000..5628451
--- /dev/null
+++ b/subprojects/groovy-linq/src/main/groovy/org/apache/groovy/linq/dsl/SyntaxErrorReportable.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;
+
+import org.codehaus.groovy.control.SourceUnit;
+import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
+import org.codehaus.groovy.syntax.SyntaxException;
+
+/**
+ * Supports reporting the syntax error of GINQ
+ *
+ * @since 4.0.0
+ */
+public interface SyntaxErrorReportable {
+    SourceUnit getSourceUnit();
+
+    default void collectSyntaxError(GinqSyntaxError ginqSyntaxError) {
+        SourceUnit sourceUnit = getSourceUnit();
+
+        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/test/groovy/org/apache/groovy/linq/GinqErrorTest.groovy b/subprojects/groovy-linq/src/test/groovy/org/apache/groovy/linq/GinqErrorTest.groovy
index 40831db..0fedabd 100644
--- 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
@@ -18,13 +18,11 @@
  */
 package org.apache.groovy.linq
 
-
 import groovy.transform.CompileStatic
 import org.junit.Test
 
 import static groovy.test.GroovyAssert.shouldFail
 
-
 @CompileStatic
 class GinqErrorTest {
     @Test
@@ -66,4 +64,19 @@ class GinqErrorTest {
         assert err.toString().contains('Only 1 argument expected for `from`, e.g. `from n in nums` @ line 3, column 17.')
     }
 
+    @Test
+    void "testGinq - from innerJoin select - 1"() {
+        def err = shouldFail '''\
+            def nums1 = [1, 2, 3]
+            def nums2 = [1, 2, 3]
+            assert [[1, 1], [2, 2], [3, 3]] == GINQ {
+                from n1 in nums1
+                innerJoin n2 in nums2
+                select n1, n2
+            }.toList()
+        '''
+
+        assert err.toString().contains('`on` clause is expected for `innerJoin` @ line 5, column 17.')
+    }
+
 }