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 2020/07/08 18:41:10 UTC
[groovy] 01/01: GROOVY-7996: STC: error for mismatched closure
resolve strategies
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-7996
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 05136a650604af51acdec6b9f18a374b9149b51d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jul 8 13:40:50 2020 -0500
GROOVY-7996: STC: error for mismatched closure resolve strategies
Provide the user some clue that runtime behavior will not match declared
behavior (in explicit or implicit @DelegatesTo metadata).
---
.../transform/stc/StaticTypeCheckingVisitor.java | 38 ++++++++++++++++++----
.../asm/sc/DelegatesToStaticCompileTest.groovy | 29 +++++++++++++++--
2 files changed, 58 insertions(+), 9 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 0dc82fc..da14d16 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3487,14 +3487,27 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
visitMethodCallArguments(receiver, argumentList, true, target);
}
if (target != null) {
- List<Expression> argExpressions = argumentList.getExpressions();
+ List<Expression> arguments = argumentList.getExpressions();
Parameter[] parameters = target.getParameters();
- for (int i = 0; i < argExpressions.size() && i < parameters.length; i += 1) {
- Expression arg = argExpressions.get(i);
- ClassNode aType = getType(arg), pType = parameters[i].getType();
- if (CLOSURE_TYPE.equals(aType) && CLOSURE_TYPE.equals(pType) && !isAssignableTo(aType, pType)) {
- addNoMatchingMethodError(receiver, name, getArgumentTypes(argumentList), call);
- call.removeNodeMetaData(DIRECT_METHOD_CALL_TARGET);
+ for (int i = 0, n = Math.min(arguments.size(), parameters.length); i < n; i += 1) {
+ Expression argument = arguments.get(i);
+ ClassNode aType = getType(argument), pType = parameters[i].getType();
+ if (CLOSURE_TYPE.equals(aType) && CLOSURE_TYPE.equals(pType)) {
+ // GROOVY-8310: check closure generics
+ if (!isAssignableTo(aType, pType) /*&& !extension.handleIncompatibleReturnType(getReturnStatement(argument), aType)*/) {
+ addNoMatchingMethodError(receiver, name, getArgumentTypes(argumentList), call);
+ call.removeNodeMetaData(DIRECT_METHOD_CALL_TARGET);
+ break;
+ }
+ // GROOVY-7996: check delegation metadata of closure parameter used as method call argument
+ if (argument instanceof VariableExpression && ((VariableExpression) argument).getAccessedVariable() instanceof Parameter) {
+ // TODO: Check additional delegation metadata like type (see checkClosureWithDelegatesTo).
+ int incomingStrategy = getResolveStrategy((Parameter) ((VariableExpression) argument).getAccessedVariable());
+ int outgoingStrategy = getResolveStrategy(parameters[i]);
+ if (incomingStrategy != outgoingStrategy) {
+ addStaticTypeError("Closure parameter with resolve strategy " + incomingStrategy + " passed to method with resolve strategy " + outgoingStrategy, argument);
+ }
+ }
}
}
}
@@ -3506,6 +3519,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private int getResolveStrategy(final Parameter parameter) {
+ List<AnnotationNode> annotations = parameter.getAnnotations(DELEGATES_TO);
+ if (annotations != null && !annotations.isEmpty()) {
+ Expression strategy = annotations.get(0).getMember("strategy");
+ if (strategy != null) {
+ return (Integer) evaluateExpression(castX(Integer_TYPE, strategy), getSourceUnit().getConfiguration());
+ }
+ }
+ return Closure.OWNER_FIRST;
+ }
+
private void inferMethodReferenceType(final MethodCallExpression call, final ClassNode receiver, final ArgumentListExpression argumentList) {
if (call == null) return;
if (receiver == null) return;
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/DelegatesToStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/DelegatesToStaticCompileTest.groovy
index f6ce151..d28d25f 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/DelegatesToStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/DelegatesToStaticCompileTest.groovy
@@ -92,7 +92,7 @@ class DelegatesToStaticCompileTest extends DelegatesToSTCTest implements StaticC
}
class Handler {
- def doWithEntity(@DelegatesTo(Entity) Closure c) {
+ def doWithEntity(@DelegatesTo(value=Entity, strategy=Closure.DELEGATE_FIRST) Closure c) {
new Entity().with(c)
}
@@ -111,4 +111,29 @@ class DelegatesToStaticCompileTest extends DelegatesToSTCTest implements StaticC
assert !bytecode.contains('INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.castToType')
}
}
-}
\ No newline at end of file
+
+ // GROOVY-7996
+ void testErrorForMismatchedClosureResolveStrategy() {
+ shouldFailWithMessages '''
+ class Foo {
+ def build(Closure x) { // resolve strategy OWNER_FIRST
+ this.with(x) // resolve strategy DELEGATE_FIRST
+ }
+ def propertyMissing(String name) {
+ 'something'
+ }
+ }
+
+ class Bar {
+ protected List bars = []
+ def baz() {
+ new Foo().build {
+ bars.isEmpty() // fails if executed; delegate's propertyMissing takes precedence
+ }
+ }
+ }
+
+ new Bar().baz()
+ ''', 'Closure parameter with resolve strategy 0 passed to method with resolve strategy 1'
+ }
+}