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
commit b94d20f9b60255b344a1f74ecc379606f4ddb1d9 Author: Daniel Sun <[email protected]> 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.') + } + +}
