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/11 09:58:58 UTC

[groovy] branch GROOVY-8258 updated: GROOVY-8258: 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 7926d1c  GROOVY-8258: add more error test cases
7926d1c is described below

commit 7926d1c3df860b3fb35e384ab8c478f094cb7b9f
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Oct 11 17:58:40 2020 +0800

    GROOVY-8258: add more error test cases
---
 .../org/apache/groovy/linq/dsl/GinqAstBuilder.java | 29 +++++++-----
 .../org/apache/groovy/linq/GinqErrorTest.groovy    | 54 ++++++++++++++++++++++
 2 files changed, 71 insertions(+), 12 deletions(-)

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 64b48a5..d3a1421 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
@@ -138,27 +138,32 @@ public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorRep
                 }
             }
 
-            FilterExpression filterExpression = null;
+            FilterExpression filterExpression;
             if ("where".equals(methodName)) {
                 filterExpression = new WhereExpression(filterExpr);
             } else {
                 filterExpression = new OnExpression(filterExpr);
             }
 
-            if (null == filterExpression) {
-                throw new GroovyBugError("Unknown method: " + methodName);
-            }
-
             filterExpression.setSourcePosition(call);
 
-            if (latestGinqExpressionClause instanceof DataSourceExpression) {
-                if (filterExpression instanceof WhereExpression) {
-                    ((DataSourceExpression) latestGinqExpressionClause).setWhereExpression((WhereExpression) filterExpression);
-                } else {
-                    ((JoinExpression) latestGinqExpressionClause).setOnExpression((OnExpression) filterExpression);
+            if (latestGinqExpressionClause instanceof JoinExpression && filterExpression instanceof OnExpression) {
+                ((JoinExpression) latestGinqExpressionClause).setOnExpression((OnExpression) filterExpression);
+            } else if (latestGinqExpressionClause instanceof DataSourceExpression && filterExpression instanceof WhereExpression) {
+                final DataSourceExpression dataSourceExpression = (DataSourceExpression) latestGinqExpressionClause;
+
+                if (null != dataSourceExpression.getGroupExpression() || null != dataSourceExpression.getOrderExpression() || null != dataSourceExpression.getLimitExpression()) {
+                    this.collectSyntaxError(new GinqSyntaxError(
+                            "The preceding clause of `" + methodName + "` should be `from`/" + "join clause",
+                            call.getLineNumber(), call.getColumnNumber()
+                    ));
                 }
-            } else {
-                throw new GroovyBugError("The preceding expression is not a DataSourceExpression: " + latestGinqExpressionClause);
+                dataSourceExpression.setWhereExpression((WhereExpression) filterExpression);
+            } else  {
+                this.collectSyntaxError(new GinqSyntaxError(
+                        "The preceding clause of `" + methodName + "` should be " + ("on".equals(methodName) ? "" : "`from`/") + "join clause",
+                        call.getLineNumber(), call.getColumnNumber()
+                ));
             }
 
             return;
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 8074604..ce52191 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
@@ -79,4 +79,58 @@ class GinqErrorTest {
         assert err.toString().contains('`on` clause is expected for `innerjoin` @ line 5, column 17.')
     }
 
+    @Test
+    void "testGinq - from on select - 1"() {
+        def err = shouldFail '''\
+            GINQ {
+                from n in [1, 2, 3]
+                on n > 1
+                select n
+            }
+        '''
+
+        assert err.toString().contains('The preceding clause of `on` should be join clause @ line 3, column 17.')
+    }
+
+    @Test
+    void "testGinq - from groupby where - 1"() {
+        def err = shouldFail '''\
+            GINQ {
+                from n in [1, 2, 3]
+                groupby n
+                where n > 1
+                select n
+            }
+        '''
+
+        assert err.toString().contains('The preceding clause of `where` should be `from`/join clause @ line 4, column 17.')
+    }
+
+    @Test
+    void "testGinq - from orderby where - 1"() {
+        def err = shouldFail '''\
+            GINQ {
+                from n in [1, 2, 3]
+                orderby n
+                where n > 1
+                select n
+            }
+        '''
+
+        assert err.toString().contains('The preceding clause of `where` should be `from`/join clause @ line 4, column 17.')
+    }
+
+    @Test
+    void "testGinq - from limit where - 1"() {
+        def err = shouldFail '''\
+            GINQ {
+                from n in [1, 2, 3]
+                limit 1
+                where n > 1
+                select n
+            }
+        '''
+
+        assert err.toString().contains('The preceding clause of `where` should be `from`/join clause @ line 4, column 17.')
+    }
 }