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/05/08 18:27:20 UTC
[groovy] branch master updated: GROOVY-10597: STC: allow spread expression(s) for variadic parameter
This is an automated email from the ASF dual-hosted git repository.
emilles 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 e752790144 GROOVY-10597: STC: allow spread expression(s) for variadic parameter
e752790144 is described below
commit e7527901444e08c5ea1ea6443385033e051c4c14
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun May 8 12:35:11 2022 -0500
GROOVY-10597: STC: allow spread expression(s) for variadic parameter
---
.../transform/stc/StaticTypeCheckingVisitor.java | 89 ++++++++++++----------
.../groovy/transform/stc/MethodCallsSTCTest.groovy | 86 +++++++++++++++++++--
.../asm/sc/MethodCallsStaticCompilationTest.groovy | 7 ++
3 files changed, 133 insertions(+), 49 deletions(-)
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 24d281c14f..e4bfc5557c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -2272,7 +2272,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
Expression arguments = call.getArguments();
ArgumentListExpression argumentList = InvocationWriter.makeArgumentList(arguments);
- checkForbiddenSpreadArgument(argumentList);
visitMethodCallArguments(receiver, argumentList, false, null);
final ClassNode[] argumentTypes = getArgumentTypes(argumentList);
@@ -2285,23 +2284,19 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ctor = findMethodOrFail(call, receiver, "<init>", argumentTypes);
if (ctor != null) {
Parameter[] parameters = ctor.getParameters();
- if (looksLikeNamedArgConstructor(receiver, argumentTypes)
- && parameters.length == argumentTypes.length - 1) {
- ctor = typeCheckMapConstructor(call, receiver, arguments);
- } else {
- if (receiver.getGenericsTypes() != null) { // GROOVY-8909, GROOVY-9734, GROOVY-9844, GROOVY-9915, GROOVY-10482, et al.
- Map<GenericsTypeName, GenericsType> context = extractPlaceHoldersVisibleToDeclaration(receiver, ctor, argumentList);
- parameters = Arrays.stream(parameters).map(p -> new Parameter(applyGenericsContext(context, p.getType()), p.getName())).toArray(Parameter[]::new);
- }
- resolvePlaceholdersFromImplicitTypeHints(argumentTypes, argumentList, parameters);
- typeCheckMethodsWithGenericsOrFail(receiver, argumentTypes, ctor, call);
- visitMethodCallArguments(receiver, argumentList, true, ctor);
+ if (receiver.getGenericsTypes() != null) { // GROOVY-8909, GROOVY-9734, GROOVY-9844, GROOVY-9915, GROOVY-10482, et al.
+ Map<GenericsTypeName, GenericsType> context = extractPlaceHoldersVisibleToDeclaration(receiver, ctor, argumentList);
+ parameters = Arrays.stream(parameters).map(p -> new Parameter(applyGenericsContext(context, p.getType()), p.getName())).toArray(Parameter[]::new);
}
+ resolvePlaceholdersFromImplicitTypeHints(argumentTypes, argumentList, parameters);
+ typeCheckMethodsWithGenericsOrFail(receiver, argumentTypes, ctor, call);
+ visitMethodCallArguments(receiver, argumentList, true, ctor);
+ checkForbiddenSpreadArgument(argumentList, parameters);
+ } else {
+ checkForbiddenSpreadArgument(argumentList);
}
}
- if (ctor != null) {
- storeTargetMethod(call, ctor);
- }
+ if (ctor != null) storeTargetMethod(call, ctor);
}
// GROOVY-9327: check for AIC in STC method with non-STC enclosing class
@@ -2676,15 +2671,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return;
}
- Expression callArguments = call.getArguments();
-
- ArgumentListExpression argumentList = InvocationWriter.makeArgumentList(callArguments);
-
- checkForbiddenSpreadArgument(argumentList);
-
ClassNode receiver = call.getOwnerType();
- visitMethodCallArguments(receiver, argumentList, false, null);
+ ArgumentListExpression argumentList = InvocationWriter.makeArgumentList(call.getArguments());
+ visitMethodCallArguments(receiver, argumentList, false, null);
ClassNode[] args = getArgumentTypes(argumentList);
try {
@@ -2709,10 +2699,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
break;
}
}
- if (mn == null) {
- throw new GroovyBugError("Invalid state finding valid method: receivers should never be empty and findMethod should never return null");
- }
- if (mn.isEmpty()) {
+ if (mn == null || mn.isEmpty()) {
mn = extension.handleMissingMethod(receiver, name, argumentList, args, call);
}
boolean callArgsVisited = false;
@@ -2725,19 +2712,26 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode returnType = getType(directMethodCallCandidate);
if (returnType.isUsingGenerics() && !returnType.isEnum()) {
visitMethodCallArguments(receiver, argumentList, true, directMethodCallCandidate);
- ClassNode irtg = inferReturnTypeGenerics(chosenReceiver.getType(), directMethodCallCandidate, callArguments);
+ ClassNode irtg = inferReturnTypeGenerics(chosenReceiver.getType(), directMethodCallCandidate, argumentList);
returnType = irtg != null && implementsInterfaceOrIsSubclassOf(irtg, returnType) ? irtg : returnType;
callArgsVisited = true;
}
storeType(call, returnType);
storeTargetMethod(call, directMethodCallCandidate);
-
} else {
addAmbiguousErrorMessage(mn, name, args, call);
}
- if (!callArgsVisited) {
- visitMethodCallArguments(receiver, argumentList, true, call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET));
- }
+ }
+
+ MethodNode target = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
+ if (!callArgsVisited) {
+ visitMethodCallArguments(receiver, argumentList, true, target);
+ }
+ if (target != null) { Parameter[] params = target.getParameters();
+ checkClosureMetadata(argumentList.getExpressions(), params);
+ checkForbiddenSpreadArgument(argumentList, params);
+ } else {
+ checkForbiddenSpreadArgument(argumentList);
}
} finally {
extension.afterMethodCall(call);
@@ -3346,7 +3340,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
Expression callArguments = call.getArguments();
ArgumentListExpression argumentList = InvocationWriter.makeArgumentList(callArguments);
- checkForbiddenSpreadArgument(argumentList);
// visit closures *after* the method has been chosen
visitMethodCallArguments(receiver, argumentList, false, null);
@@ -3390,6 +3383,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (variable instanceof ASTNode) {
Parameter[] parameters = ((ASTNode) variable).getNodeMetaData(CLOSURE_ARGUMENTS);
if (parameters != null) {
+ checkForbiddenSpreadArgument(argumentList, parameters);
typeCheckClosureCall(callArguments, args, parameters);
}
ClassNode type = getType((ASTNode) variable);
@@ -3418,10 +3412,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
- int nArgs = 0;
- if (callArguments instanceof ArgumentListExpression) {
- nArgs = ((ArgumentListExpression) callArguments).getExpressions().size();
- }
+ List<Expression> list = argumentList.getExpressions();
+ int nArgs = list.stream().noneMatch(e -> e instanceof SpreadExpression) ? list.size() : Integer.MAX_VALUE;
storeTargetMethod(call, nArgs == 0 ? CLOSURE_CALL_NO_ARG : nArgs == 1 ? CLOSURE_CALL_ONE_ARG : CLOSURE_CALL_VARGS);
} else {
// method call receivers are :
@@ -3568,8 +3560,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (!callArgsVisited) {
visitMethodCallArguments(receiver, argumentList, true, target);
}
- if (target != null) {
- checkClosureMetadata(argumentList.getExpressions(), target.getParameters());
+ if (target != null) { Parameter[] params = target.getParameters();
+ checkClosureMetadata(argumentList.getExpressions(), params);
+ checkForbiddenSpreadArgument(argumentList, params);
+ } else {
+ checkForbiddenSpreadArgument(argumentList);
}
} finally {
typeCheckingContext.popEnclosingMethodCall();
@@ -3812,10 +3807,20 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
- protected void checkForbiddenSpreadArgument(final ArgumentListExpression argumentList) {
- for (Expression arg : argumentList.getExpressions()) {
- if (arg instanceof SpreadExpression) {
- addStaticTypeError("The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time", arg);
+ private void checkForbiddenSpreadArgument(final ArgumentListExpression arguments, final Parameter[] parameters) {
+ if (!isVargs(parameters)) {
+ checkForbiddenSpreadArgument(arguments);
+ } else { // GROOVY-10597: allow spread for the ... param
+ List<Expression> list = arguments.getExpressions();
+ list = list.subList(0, parameters.length - 1);
+ checkForbiddenSpreadArgument(args(list));
+ }
+ }
+
+ protected void checkForbiddenSpreadArgument(final ArgumentListExpression arguments) {
+ for (Expression argument : arguments) {
+ if (argument instanceof SpreadExpression) {
+ addStaticTypeError("The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time", argument);
}
}
}
diff --git a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
index 792b8e3b7d..78219b7644 100644
--- a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
@@ -1123,7 +1123,7 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testGetNameFromSuperInterfaceUsingConcreteImpl() {
+ void testGetNameFromSuperInterfaceViaConcreteType1() {
assertScript '''
interface Upper { String getName() }
interface Lower extends Upper {}
@@ -1135,7 +1135,7 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testGetNameFromSuperInterfaceUsingConcreteImplSubclass() {
+ void testGetNameFromSuperInterfaceViaConcreteType2() {
assertScript '''
interface Upper { String getName() }
interface Lower extends Upper {}
@@ -1148,7 +1148,25 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testSpreadArgsForbiddenInNonStaticMethodCall() {
+ void testSpreadArgsRestrictedInNonStaticMethodCall() {
+ // GROOVY-10597
+ assertScript '''
+ def m(int i, String... strings) {
+ '' + i + strings.join('')
+ }
+ List<String> strings() {['3','4']}
+ assert m(1, '2', *strings(), '5') == '12345'
+ '''
+
+ shouldFailWithMessages '''
+ def foo(String one, String... zeroOrMore) {
+ }
+ def bar(String[] strings) {
+ foo(*strings)
+ }
+ ''',
+ 'The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time'
+
shouldFailWithMessages '''
def foo(String a, String b, int c, double d, double e) {
}
@@ -1161,7 +1179,25 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'Cannot find matching method '
}
- void testSpreadArgsForbiddenInStaticMethodCall() {
+ void testSpreadArgsRestrictedInStaticMethodCall() {
+ // GROOVY-10597
+ assertScript '''
+ static m(int i, String... strings) {
+ return '' + i + strings.join('')
+ }
+ List<String> strings = ['3','4']
+ assert m(1,'2',*strings,'5') == '12345'
+ '''
+
+ shouldFailWithMessages '''
+ static foo(String one, String... zeroOrMore) {
+ }
+ static bar(String[] strings) {
+ foo(*strings)
+ }
+ ''',
+ 'The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time'
+
shouldFailWithMessages '''
static foo(String a, String b, int c, double d, double e) {
}
@@ -1174,7 +1210,27 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'Cannot find matching method '
}
- void testSpreadArgsForbiddenInConstructorCall() {
+ void testSpreadArgsRestrictedInConstructorCall() {
+ // GROOVY-10597
+ assertScript '''
+ class C {
+ C(String one, String... zeroOrMore) {
+ String result = one + zeroOrMore.join('')
+ assert result == 'ABC'
+ }
+ }
+ new C('A', *['B'], 'C')
+ '''
+
+ shouldFailWithMessages '''
+ class C {
+ C(String one, String... zeroOrMore) {
+ }
+ }
+ new C(*['A','B'])
+ ''',
+ 'The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time'
+
shouldFailWithMessages '''
class C {
C(String a, String b) {
@@ -1186,9 +1242,25 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
'Cannot find matching method '
}
- void testSpreadArgsForbiddenInClosureCall() {
+ void testSpreadArgsRestrictedInClosureCall() {
+ // GROOVY-10597
+ assertScript '''
+ def closure = { String one, String... zeroOrMore ->
+ return one + zeroOrMore.join('')
+ }
+ String result = closure('A', *['B','C'])
+ assert result == 'ABC'
+ '''
+
+ shouldFailWithMessages '''
+ def closure = { String one, String... zeroOrMore -> }
+ def strings = ['A','B','C']
+ closure(*strings)
+ ''',
+ 'The spread operator cannot be used as argument of method or closure calls with static type checking because the number of arguments cannot be determined at compile time'
+
shouldFailWithMessages '''
- def closure = { String a, String b, String c -> println "$a $b $c" }
+ def closure = { String a, String b, String c -> }
def strings = ['A','B','C']
closure(*strings)
''',
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
index e80c6586e3..ce90c09f71 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
@@ -36,6 +36,13 @@ public class MethodCallsStaticCompilationTest extends MethodCallsSTCTest impleme
}
}
+ @Override
+ void testSpreadArgsRestrictedInConstructorCall() {
+ shouldFail {
+ super.testSpreadArgsRestrictedInConstructorCall()
+ }
+ }
+
// GROOVY-7863
void testDoublyNestedPrivateMethodAccess() {
assertScript '''