You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/02/26 19:28:34 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-10071: SC/STC: variadic closure parameter checks and errors
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new c22d10c GROOVY-10071: SC/STC: variadic closure parameter checks and errors
c22d10c is described below
commit c22d10c07fc3c5032434a31304cc4f8b55fb680c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun May 2 16:27:28 2021 -0500
GROOVY-10071: SC/STC: variadic closure parameter checks and errors
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
src/test/groovy/transform/stc/ClosuresSTCTest.groovy
src/test/groovy/transform/stc/GenericsSTCTest.groovy
---
.../classgen/asm/sc/StaticTypesClosureWriter.java | 26 +++--
.../transform/stc/StaticTypeCheckingSupport.java | 21 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 33 +++---
.../groovy/transform/stc/ClosuresSTCTest.groovy | 120 ++++++++++++++-------
.../groovy/transform/stc/GenericsSTCTest.groovy | 13 ++-
.../asm/sc/StaticCompileClosureCallTest.groovy | 96 +++++++----------
6 files changed, 165 insertions(+), 144 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java
index 6dd546c..555eaf7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java
@@ -24,6 +24,7 @@ import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.ArrayExpression;
import org.codehaus.groovy.ast.expr.ClosureExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
@@ -34,13 +35,14 @@ import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
import org.objectweb.asm.Opcodes;
+import java.util.Collections;
import java.util.List;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@@ -77,18 +79,30 @@ public class StaticTypesClosureWriter extends ClosureWriter {
}
private static void createDirectCallMethod(final ClassNode closureClass, final MethodNode doCallMethod) {
- // in case there is no "call" method on the closure, we can create a "fast invocation" paths
+ // in case there is no "call" method on the closure, create a "fast invocation" path
// to avoid going through ClosureMetaClass by call(Object...) method
- // we can't have a specialized version of call(Object...) because the dispatch logic in ClosureMetaClass
- // is too complex!
+ // we can't have a specialized version of call(Object...) because the dispatch logic
+ // in ClosureMetaClass is too complex!
// call(Object)
- Parameter args = param(ClassHelper.OBJECT_TYPE, "args");
+ Parameter doCallParam = doCallMethod.getParameters()[0];
+ Parameter args = new Parameter(doCallParam.getType(), "args");
addGeneratedCallMethod(closureClass, doCallMethod, varX(args), new Parameter[]{args});
// call()
- addGeneratedCallMethod(closureClass, doCallMethod, constX(null), Parameter.EMPTY_ARRAY);
+ addGeneratedCallMethod(closureClass, doCallMethod, defaultArgument(doCallParam), Parameter.EMPTY_ARRAY);
+ }
+
+ private static Expression defaultArgument(final Parameter parameter) {
+ Expression argument;
+ if (parameter.getType().isArray()) {
+ ClassNode elementType = parameter.getType().getComponentType();
+ argument = new ArrayExpression(elementType, null, Collections.<Expression>singletonList(constX(0, true)));
+ } else {
+ argument = nullX();
+ }
+ return argument;
}
private static void addGeneratedCallMethod(ClassNode closureClass, MethodNode doCallMethod, Expression expression, Parameter[] params) {
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index bfc702b..2e9d221 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -438,18 +438,19 @@ public abstract class StaticTypeCheckingSupport {
* @param args
* @return -1 if no match, 0 if the last argument is exactly the vararg type and 1 if of an assignable type
*/
- static int lastArgMatchesVarg(Parameter[] params, ClassNode... args) {
- if (!isVargs(params)) return -1;
- // case length ==0 handled already
- // we have now two cases,
+ static int lastArgMatchesVarg(final Parameter[] parameters, final ClassNode... argumentTypes) {
+ if (!isVargs(parameters)) return -1;
+ int lastParamIndex = parameters.length - 1;
+ if (lastParamIndex == argumentTypes.length) return 0;
+ // two cases remain:
// the argument is wrapped in the vargs array or
// the argument is an array that can be used for the vargs part directly
- // we test only the wrapping part, since the non wrapping is done already
- ClassNode lastParamType = params[params.length - 1].getType();
- ClassNode ptype = lastParamType.getComponentType();
- ClassNode arg = args[args.length - 1];
- if (isNumberType(ptype) && isNumberType(arg) && !getWrapper(ptype).equals(getWrapper(arg))) return -1;
- return isAssignableTo(arg, ptype) ? min(getDistance(arg, lastParamType), getDistance(arg, ptype)) : -1;
+ // testing only the wrapping case since the non-wrapping is done already
+ ClassNode arrayType = parameters[lastParamIndex].getType();
+ ClassNode elementType = arrayType.getComponentType();
+ ClassNode argumentType = argumentTypes[argumentTypes.length - 1];
+ if (isNumberType(elementType) && isNumberType(argumentType) && !getWrapper(elementType).equals(getWrapper(argumentType))) return -1;
+ return isAssignableTo(argumentType, elementType) ? min(getDistance(argumentType, arrayType), getDistance(argumentType, elementType)) : -1;
}
/**
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index ef7111b..2d4ce31 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3391,12 +3391,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
FieldNode field = typeCheckingContext.getEnclosingClassNode().getDeclaredField(name);
GenericsType[] genericsTypes = field.getType().getGenericsTypes();
if (genericsTypes != null) {
- ClassNode closureReturnType = genericsTypes[0].getType();
- Object data = field.getNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS);
- if (data != null) {
- Parameter[] parameters = (Parameter[]) data;
+ Parameter[] parameters = field.getNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS);
+ if (parameters != null) {
typeCheckClosureCall(callArguments, args, parameters);
}
+ ClassNode closureReturnType = genericsTypes[0].getType();
storeType(call, closureReturnType);
}
} else if (objectExpression instanceof VariableExpression) {
@@ -3424,10 +3423,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else if (objectExpression instanceof ClosureExpression) {
// we can get actual parameters directly
Parameter[] parameters = ((ClosureExpression) objectExpression).getParameters();
- if (parameters != null) typeCheckClosureCall(callArguments, args, parameters);
- ClassNode data = getInferredReturnType(objectExpression);
- if (data != null) {
- storeType(call, data);
+ if (parameters != null) {
+ typeCheckClosureCall(callArguments, args, parameters);
+ }
+ ClassNode type = getInferredReturnType(objectExpression);
+ if (type != null) {
+ storeType(call, type);
}
}
int nbOfArgs;
@@ -3792,17 +3793,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return (getType(objectExpression).equals(CLOSURE_TYPE));
}
- protected void typeCheckClosureCall(final Expression callArguments, final ClassNode[] args, final Parameter[] parameters) {
- if (allParametersAndArgumentsMatch(parameters, args) < 0 &&
- lastArgMatchesVarg(parameters, args) < 0) {
- StringBuilder sb = new StringBuilder("[");
- for (int i = 0, parametersLength = parameters.length; i < parametersLength; i++) {
- final Parameter parameter = parameters[i];
- sb.append(parameter.getType().getName());
- if (i < parametersLength - 1) sb.append(", ");
- }
- sb.append("]");
- addStaticTypeError("Closure argument types: " + sb + " do not match with parameter types: " + formatArgumentList(args), callArguments);
+ protected void typeCheckClosureCall(final Expression arguments, final ClassNode[] argumentTypes, final Parameter[] parameters) {
+ if (allParametersAndArgumentsMatch(parameters, argumentTypes) < 0 && lastArgMatchesVarg(parameters, argumentTypes) < 0) {
+ addStaticTypeError("Cannot call closure that accepts " + formatArgumentList(extractTypesFromParameters(parameters)) + "with " + formatArgumentList(argumentTypes), arguments);
}
}
@@ -5453,7 +5446,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
protected static String formatArgumentList(ClassNode[] nodes) {
- if (nodes == null || nodes.length == 0) return "[]";
+ if (nodes == null || nodes.length == 0) return "[] ";
StringBuilder sb = new StringBuilder(24 * nodes.length);
sb.append("[");
for (ClassNode node : nodes) {
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 149f965..c1869c6 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -23,90 +23,128 @@ package groovy.transform.stc
*/
class ClosuresSTCTest extends StaticTypeCheckingTestCase {
- void testClosureWithoutArguments() {
+ void testClosureWithoutArguments1() {
assertScript '''
- def clos = { println "hello!" }
-
- println "Executing the Closure:"
- clos() //prints "hello!"
+ def c = { return 'foo' }
+ assert c() == 'foo'
'''
}
- // GROOVY-9079: no params to statically type check but shouldn't get NPE
- void testClosureWithoutArgumentsExplicit() {
+ void testClosureWithoutArguments2() {
assertScript '''
- import java.util.concurrent.Callable
+ def c = { -> return 'foo' }
+ assert c() == 'foo'
+ '''
+ }
- String makeFoo() {
- Callable<String> call = { -> 'foo' }
- call()
- }
+ // GROOVY-9079
+ void testClosureWithoutArguments3() {
+ assertScript '''
+ java.util.concurrent.Callable<String> c = { -> return 'foo' }
+ assert c() == 'foo'
+ '''
+ }
- assert makeFoo() == 'foo'
+ // GROOVY-10071
+ void testClosureWithoutArguments4() {
+ assertScript '''
+ def c = { ... zeroOrMore -> return 'foo' + zeroOrMore }
+ assert c('bar', 'baz') == 'foo[bar, baz]'
+ assert c('bar') == 'foo[bar]'
+ assert c() == 'foo[]'
'''
}
- void testClosureWithArguments() {
+ void testClosureWithArguments1() {
assertScript '''
- def printSum = { int a, int b -> print a+b }
- printSum( 5, 7 ) //prints "12"
+ def c = { int a, int b -> a + b }
+ assert c(5, 7) == 12
'''
shouldFailWithMessages '''
- def printSum = { int a, int b -> print a+b }
- printSum( '5', '7' ) //prints "12"
- ''', 'Closure argument types: [int, int] do not match with parameter types: [java.lang.String, java.lang.String]'
+ def c = { int a, int b -> print a + b }
+ c('5', '7')
+ ''',
+ 'Cannot call closure that accepts [int, int] with [java.lang.String, java.lang.String]'
}
- void testClosureWithArgumentsAndNoDef() {
+ void testClosureWithArguments2() {
assertScript '''
- { int a, int b -> print a+b }(5,7)
+ def result = { int a, int b -> a + b }(5, 7)
+ assert result == 12
'''
- }
- void testClosureWithArgumentsNoDefAndWrongType() {
shouldFailWithMessages '''
- { int a, int b -> print a+b }('5',7)
- ''', 'Closure argument types: [int, int] do not match with parameter types: [java.lang.String, int]'
+ { int a, int b -> a + b }('5', 7)
+ ''',
+ 'Cannot call closure that accepts [int, int] with [java.lang.String, int]'
}
- void testClosureReturnTypeInferrence() {
+ // GROOVY-6365
+ void testClosureWithArguments3() {
assertScript '''
- def closure = { int x, int y -> return x+y }
- int total = closure(2,3)
+ def c = { Object[] args -> args.length }
+ assert c('one', 'two') == 2
'''
+ }
- shouldFailWithMessages '''
- def closure = { int x, int y -> return x+y }
- int total = closure('2',3)
- ''', 'Closure argument types: [int, int] do not match with parameter types: [java.lang.String, int]'
+ void testClosureReturnTypeInferrence() {
+ assertScript '''
+ def c = { int a, int b -> return a + b }
+ int total = c(2, 3)
+ assert total == 5
+ '''
}
- void testClosureReturnTypeInferrenceWithoutDef() {
+ void testClosureReturnTypeInference2() {
assertScript '''
- int total = { int x, int y -> return x+y }(2,3)
+ int total = { int a, int b -> return a + b }(2, 3)
'''
}
- void testClosureReturnTypeInference() {
+ void testClosureReturnTypeInference3() {
shouldFailWithMessages '''
- def cl = { int x ->
- if (x==0) {
- 1L
+ def c = { int x ->
+ if (x == 0) {
+ 1L // long
} else {
x // int
}
}
- byte res = cl(0) // should throw an error because return type inference should be a long
- ''', 'Possible loss of precision from long to byte'
+ byte res = c(0)
+ ''',
+ 'Possible loss of precision from long to byte'
}
- void testClosureWithoutParam() {
+ // GROOVY-9907
+ void testClosureReturnTypeInference4() {
assertScript '''
{ -> println 'Hello' }()
'''
}
+ // GROOVY-9971
+ void testClosureReturnTypeInference5() {
+ assertScript '''
+ import java.util.function.Consumer
+
+ class C {
+ static <T> void m(T a, Consumer<T> c) {
+ c.accept(a)
+ }
+ static void main(args) {
+ def c = { ->
+ int x = 0
+ m('') {
+ print 'void return'
+ }
+ }
+ c.call()
+ }
+ }
+ '''
+ }
+
// GROOVY-8427
void testClosureReturnTypeInference6() {
assertScript '''
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index a9d4d66..dabe166 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -74,7 +74,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
List<String> list = []
list << 1
- ''', '[Static type checking] - Cannot call <T> java.util.List <String>#leftShift(T) with arguments [int]'
+ ''', 'Cannot call <T> java.util.List <String>#leftShift(T) with arguments [int]'
}
void testAddOnList2UsingLeftShift() {
@@ -122,14 +122,14 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
List<Integer> list = new LinkedList<>()
list.add 'Hello'
- ''', '[Static type checking] - Cannot find matching method java.util.LinkedList#add(java.lang.String). Please check if the declared type is correct and if the method exists.'
+ ''', 'Cannot find matching method java.util.LinkedList#add(java.lang.String). Please check if the declared type is correct and if the method exists.'
}
void testAddOnListWithDiamondAndWrongTypeUsingLeftShift() {
shouldFailWithMessages '''
List<Integer> list = new LinkedList<>()
list << 'Hello'
- ''', '[Static type checking] - Cannot call <T> java.util.LinkedList <java.lang.Integer>#leftShift(T) with arguments [java.lang.String]'
+ ''', 'Cannot call <T> java.util.LinkedList <java.lang.Integer>#leftShift(T) with arguments [java.lang.String]'
}
void testAddOnListWithDiamondAndNullUsingLeftShift() {
@@ -1279,7 +1279,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
}
}
Baz.qux(['abc'])
- ''', 'Cannot call <T extends java.util.List<? super java.lang.CharSequence>> Foo#bar(T) with arguments [java.util.List <String>] '
+ ''', 'Cannot call <T extends java.util.List<? super java.lang.CharSequence>> Foo#bar(T) with arguments [java.util.List <String>]'
}
// GROOVY-5721
@@ -1703,8 +1703,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
}
}
GoodCodeRed.foo()
- ''',
- "Cannot call <T> GoodCodeRed <Long>#attach(GoodCodeRed <Long>) with arguments [GoodCodeRed <Integer>]"
+ ''', 'Cannot call <T> GoodCodeRed <Long>#attach(GoodCodeRed <Long>) with arguments [GoodCodeRed <Integer>]'
}
void testHiddenGenerics() {
@@ -1718,7 +1717,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
class Blah {}
class MyList extends LinkedList<Object> {}
List<Blah> o = new MyList()
- ''','Incompatible generic argument types. Cannot assign MyList to: java.util.List <Blah>'
+ ''', 'Incompatible generic argument types. Cannot assign MyList to: java.util.List <Blah>'
// Groovy-5873
assertScript """
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileClosureCallTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileClosureCallTest.groovy
index 5189c9f..c680d91 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileClosureCallTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileClosureCallTest.groovy
@@ -23,7 +23,8 @@ import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
/**
* Tests for static compilation: checks that closures are called properly.
*/
-class StaticCompileClosureCallTest extends AbstractBytecodeTestCase {
+final class StaticCompileClosureCallTest extends AbstractBytecodeTestCase {
+
void testShouldCallClosure() {
def bytecode = compile([method:'m'],'''
@groovy.transform.CompileStatic
@@ -117,61 +118,61 @@ class StaticCompileClosureCallTest extends AbstractBytecodeTestCase {
void testWriteSharedVariableInClosure() {
def bytecode = compile([method:'m'],'''
- @groovy.transform.CompileStatic
- void m() {
- String test = 'test'
- def cl = { test = 'TEST' }
- cl()
- assert test == 'TEST'
- }
+ @groovy.transform.CompileStatic
+ void m() {
+ String test = 'test'
+ def cl = { test = 'TEST' }
+ cl()
+ assert test == 'TEST'
+ }
''')
clazz.newInstance().main()
}
void testCallPrivateMethodFromClosure() {
assertScript '''
- @groovy.transform.CompileStatic
- class Foo {
- void m() {
- String test = 'test'
- def cl = { test = bar() }
- cl()
- assert test == 'TEST'
+ @groovy.transform.CompileStatic
+ class Foo {
+ void m() {
+ String test = 'test'
+ def cl = { test = bar() }
+ cl()
+ assert test == 'TEST'
+ }
+ private String bar() { 'TEST' }
}
- private String bar() { 'TEST' }
- }
- new Foo().m()
+ new Foo().m()
'''
}
void testCallStaticPrivateMethodFromClosure() {
assertScript '''
- @groovy.transform.CompileStatic
- class Foo {
- void m() {
- String test = 'test'
- def cl = { test = bar() }
- cl()
- assert test == 'TEST'
+ @groovy.transform.CompileStatic
+ class Foo {
+ void m() {
+ String test = 'test'
+ def cl = { test = bar() }
+ cl()
+ assert test == 'TEST'
+ }
+ private static String bar() { 'TEST' }
}
- private static String bar() { 'TEST' }
- }
- new Foo().m()
+ new Foo().m()
'''
}
void testCallMethodWithinClosure() {
assertScript '''
- @groovy.transform.CompileStatic
- class Foo {
- static void m(StackTraceElement[] trace) {
- trace.each { StackTraceElement stackTraceElement -> !stackTraceElement.className.startsWith('foo') }
+ @groovy.transform.CompileStatic
+ class Foo {
+ static void m(StackTraceElement[] trace) {
+ trace.each { StackTraceElement stackTraceElement -> !stackTraceElement.className.startsWith('foo') }
+ }
}
- }
- 1
+ 1
'''
}
-
+
// GROOVY-6199
void testCallClassMethodFromNestedClosure() {
assertScript '''
@@ -193,29 +194,4 @@ class StaticCompileClosureCallTest extends AbstractBytecodeTestCase {
assert mc.bool
'''
}
-
- //GROOVY-6365
- void testClosureDoCallNotWrapped() {
- assertScript """
- @groovy.transform.CompileStatic
- class MyClass {
- def method() {
- final cl = { Object[] args -> args.length }
- cl('c1-1', 'c1-2')
- }
- }
-
- assert new MyClass().method() == 2
- """
- assertScript """
- class MyClass {
- def method() {
- final cl = { Object[] args -> args.length }
- cl('c1-1', 'c1-2')
- }
- }
-
- assert new MyClass().method() == 2
- """
- }
}