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:09 UTC
[groovy] branch GROOVY-7996 created (now 05136a6)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-7996
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at 05136a6 GROOVY-7996: STC: error for mismatched closure resolve strategies
This branch includes the following new commits:
new 05136a6 GROOVY-7996: STC: error for mismatched closure resolve strategies
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-7996: STC: error for mismatched closure
resolve strategies
Posted by em...@apache.org.
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'
+ }
+}