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/12/03 15:44:26 UTC

[groovy] branch master updated: Validate unknown clause in GINQ

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

sunlan 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 3f440bc  Validate unknown clause in GINQ
3f440bc is described below

commit 3f440bc89502fb46d35db463e8490d3cc8e0e9e7
Author: Daniel Sun <su...@apache.org>
AuthorDate: Thu Dec 3 23:06:41 2020 +0800

    Validate unknown clause in GINQ
---
 .../org/codehaus/groovy/ast/GroovyCodeVisitor.java |  5 ++++
 .../org/apache/groovy/ginq/dsl/GinqAstBuilder.java | 22 +++++++++++++-
 .../groovy/ginq/dsl/expression/GinqExpression.java | 35 ++++++++++++++++++++++
 .../groovy/ginq/dsl/expression/JoinExpression.java |  2 +-
 .../ginq/provider/collection/GinqAstWalker.groovy  | 10 ++++++-
 .../org/apache/groovy/ginq/GinqErrorTest.groovy    | 13 ++++++++
 6 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java b/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
index be45610..ba705d3 100644
--- a/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
@@ -201,4 +201,9 @@ public interface GroovyCodeVisitor {
     default void visitListOfExpressions(List<? extends Expression> list) {
         if (list != null) list.forEach(expr -> expr.visit(this));
     }
+
+    default void visit(ASTNode astNode) {
+        if (null == astNode) return;
+        astNode.visit(this);
+    }
 }
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
index 5ab1856..4d15e91 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
@@ -44,6 +44,7 @@ import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.syntax.Types;
 
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Deque;
 import java.util.HashSet;
@@ -64,6 +65,8 @@ public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorRep
         this.sourceUnit = sourceUnit;
     }
 
+    private final List<MethodCallExpression> ignoredMethodCallExpressionList = new ArrayList<>();
+
     public GinqExpression getGinqExpression() {
         if (null == latestGinqExpression && !ginqExpressionStack.isEmpty()) {
             GinqExpression latestGinqExpression = ginqExpressionStack.peek();
@@ -71,6 +74,20 @@ public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorRep
                     latestGinqExpression.getLineNumber(), latestGinqExpression.getColumnNumber()));
         }
 
+        latestGinqExpression.visit(new CodeVisitorSupport() {
+            @Override
+            public void visitMethodCallExpression(MethodCallExpression call) {
+                ignoredMethodCallExpressionList.remove(call);
+                super.visitMethodCallExpression(call);
+            }
+        });
+
+        if (!ignoredMethodCallExpressionList.isEmpty()) {
+            MethodCallExpression methodCallExpression = ignoredMethodCallExpressionList.get(0);
+            this.collectSyntaxError(new GinqSyntaxError("Unknown clause: " + methodCallExpression.getMethodAsString(),
+                    methodCallExpression.getLineNumber(), methodCallExpression.getColumnNumber()));
+        }
+
         return latestGinqExpression;
     }
 
@@ -93,7 +110,10 @@ public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorRep
         super.visitMethodCallExpression(call);
         final String methodName = call.getMethodAsString();
 
-        if (!KEYWORD_SET.contains(methodName)) return;
+        if (!KEYWORD_SET.contains(methodName)) {
+            ignoredMethodCallExpressionList.add(call);
+            return;
+        }
 
         if (KW_FROM.equals(methodName)) {
             final GinqExpression ginqExpression = new GinqExpression();
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/GinqExpression.java b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/GinqExpression.java
index 9163b5d..3e6b599 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/GinqExpression.java
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/GinqExpression.java
@@ -19,6 +19,7 @@
 package org.apache.groovy.ginq.dsl.expression;
 
 import org.apache.groovy.ginq.dsl.GinqAstVisitor;
+import org.codehaus.groovy.ast.GroovyCodeVisitor;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -52,6 +53,40 @@ public class GinqExpression extends AbstractGinqExpression {
     private SelectExpression selectExpression;
 
     @Override
+    public void visit(GroovyCodeVisitor visitor) {
+        visitor.visit(fromExpression.aliasExpr);
+        visitor.visit(fromExpression.dataSourceExpr);
+
+        for (JoinExpression joinExpression : joinExpressionList) {
+            visitor.visit((joinExpression.aliasExpr));
+            visitor.visit((joinExpression.dataSourceExpr));
+
+            if (null != joinExpression.getOnExpression()) {
+                visitor.visit((joinExpression.getOnExpression().filterExpr));
+            }
+        }
+
+        if (null != whereExpression) {
+            visitor.visit(whereExpression.filterExpr);
+        }
+        if (null != groupExpression) {
+            visitor.visit(groupExpression.getClassifierExpr());
+
+            if (null != groupExpression.getHavingExpression()) {
+                visitor.visit(groupExpression.getHavingExpression().filterExpr);
+            }
+        }
+        if (null != orderExpression) {
+            visitor.visit(orderExpression.getOrdersExpr());
+        }
+        if (null != limitExpression) {
+            visitor.visit(limitExpression.getOffsetAndSizeExpr());
+        }
+
+        visitor.visit(selectExpression.getProjectionExpr());
+    }
+
+    @Override
     public <R> R accept(GinqAstVisitor<R> visitor) {
         return visitor.visitGinqExpression(this);
     }
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
index 125558e..29492dc 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
@@ -78,7 +78,7 @@ public class JoinExpression extends DataSourceExpression implements DataSourceHo
 
     @Override
     public String getText() {
-        return joinName + " " + aliasExpr.getText() + " in " + dataSourceExpr.getText() + " " + onExpression.getText();
+        return joinName + " " + aliasExpr.getText() + " in " + dataSourceExpr.getText() + (null == onExpression ? "" : " " + onExpression.getText());
     }
 
     @Override
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
index 9dd8298..f71beb3 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
@@ -83,7 +83,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.params
 import static org.codehaus.groovy.ast.tools.GeneralUtils.propX
 import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX
-
 /**
  * Visit AST of GINQ to generate target method calls for GINQ
  *
@@ -218,6 +217,15 @@ class GinqAstWalker implements GinqAstVisitor<Expression>, SyntaxErrorReportable
                 }
                 super.visitMethodCallExpression(call)
             }
+
+            @Override
+            void visitListOfExpressions(List<? extends Expression> list) {
+                if (list != null)
+                    list.forEach(expr -> {
+                        if (expr instanceof AbstractGinqExpression) return // do not visit subquery
+                        expr.visit(this)
+                    })
+            }
         })
 
         if (hasAggFunctionInSelect) {
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 db553ae..1943328 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
@@ -365,4 +365,17 @@ class GinqErrorTest {
 
         assert err.toString().contains('sub-query could not be used in the `select` clause with `groupby` @ line 4, column 24.')
     }
+
+    @Test
+    void "testGinq - from unknown select - 1"() {
+        def err = shouldFail '''\
+            GQ {
+                from n in [1, 2, 3]
+                hello world > 0
+                select n
+            }
+        '''
+
+        assert err.toString().concat('Unknown clause: hello @ line 3, column 17.')
+    }
 }