This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new 7984ab5 GROOVY-7996: STC: error for mismatched closure resolve
strategies (closes #1303)
7984ab5 is described below
commit 7984ab58dac954bfc4f7408e35edbd910228b9a3
Author: Eric Milles <[email protected]>
AuthorDate: Wed Jul 8 13:40:50 2020 -0500
GROOVY-7996: STC: error for mismatched closure resolve strategies (closes
#1303)
Provide the user some clue that runtime behavior will not match declared
behavior (in explicit or implicit @DelegatesTo metadata).
---
.../codehaus/groovy/ast/tools/ClosureUtils.java | 31 ++++++++++++++---
.../transform/stc/StaticTypeCheckingVisitor.java | 39 ++++++++++++++++++----
.../groovy/transform/stc/DelegatesToSTCTest.groovy | 25 ++++++++++++++
.../asm/sc/DelegatesToStaticCompileTest.groovy | 4 +--
4 files changed, 85 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/ClosureUtils.java
b/src/main/java/org/codehaus/groovy/ast/tools/ClosureUtils.java
index 761e8eb..f9e59d5 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/ClosureUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/ClosureUtils.java
@@ -37,7 +37,7 @@ public class ClosureUtils {
* @throws java.lang.IllegalArgumentException when expression is null
* @throws java.lang.Exception when closure can't be read from source
*/
- public static String convertClosureToSource(ReaderSource readerSource,
ClosureExpression expression) throws Exception {
+ public static String convertClosureToSource(final ReaderSource
readerSource, final ClosureExpression expression) throws Exception {
String source = GeneralUtils.convertASTToSource(readerSource,
expression);
if (!source.startsWith("{")) {
throw new Exception("Error converting ClosureExpression into
source code. Closures must start with {. Found: " + source);
@@ -50,7 +50,7 @@ public class ClosureUtils {
* @param c a Closure
* @return true if it has exactly one argument and the type is char or
Character
*/
- public static boolean hasSingleCharacterArg(Closure c) {
+ public static boolean hasSingleCharacterArg(final Closure c) {
if (c.getMaximumNumberOfParameters() != 1) return false;
String typeName = c.getParameterTypes()[0].getName();
return typeName.equals("char") ||
typeName.equals("java.lang.Character");
@@ -61,7 +61,7 @@ public class ClosureUtils {
* @param c a Closure
* @return true if it has exactly one argument and the type is String
*/
- public static boolean hasSingleStringArg(Closure c) {
+ public static boolean hasSingleStringArg(final Closure c) {
if (c.getMaximumNumberOfParameters() != 1) return false;
String typeName = c.getParameterTypes()[0].getName();
return typeName.equals("java.lang.String");
@@ -70,14 +70,35 @@ public class ClosureUtils {
/**
* @return true if the ClosureExpression has an implicit 'it' parameter
*/
- public static boolean hasImplicitParameter(ClosureExpression ce) {
+ public static boolean hasImplicitParameter(final ClosureExpression ce) {
return ce.getParameters() != null && ce.getParameters().length == 0;
}
/**
* @return the parameters for the ClosureExpression
*/
- public static Parameter[] getParametersSafe(ClosureExpression ce) {
+ public static Parameter[] getParametersSafe(final ClosureExpression ce) {
return ce.getParameters() != null ? ce.getParameters() :
Parameter.EMPTY_ARRAY;
}
+
+ /**
+ * Returns the constant name associated with the given resolve strategy.
+ *
+ * @see {@link Closure#DELEGATE_FIRST}, et al.
+ *
+ * @since 3.0.5
+ */
+ public static String getResolveStrategyName(final int resolveStrategy) {
+ switch (resolveStrategy) {
+ case Closure.DELEGATE_FIRST:
+ return "DELEGATE_FIRST";
+ case Closure.DELEGATE_ONLY:
+ return "DELEGATE_ONLY";
+ case Closure.OWNER_ONLY:
+ return "OWNER_ONLY";
+ case Closure.TO_SELF:
+ return "TO_SELF";
+ }
+ return "OWNER_FIRST";
+ }
}
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 cc8cfc1..f29513c 100644
---
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -177,6 +177,7 @@ import static org.codehaus.groovy.ast.ClassHelper.long_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.short_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.void_WRAPPER_TYPE;
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
+import static
org.codehaus.groovy.ast.tools.ClosureUtils.getResolveStrategyName;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
@@ -3487,14 +3488,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 " + getResolveStrategyName(incomingStrategy) + " passed to
method with resolve strategy " + getResolveStrategyName(outgoingStrategy),
argument);
+ }
+ }
}
}
}
@@ -3506,6 +3520,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/groovy/transform/stc/DelegatesToSTCTest.groovy
b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
index da59f91..8480ac5 100644
--- a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
@@ -823,4 +823,29 @@ class DelegatesToSTCTest extends
StaticTypeCheckingTestCase {
}
'''
}
+
+ // 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 OWNER_FIRST passed to
method with resolve strategy DELEGATE_FIRST'
+ }
}
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..73c9c53 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,4 @@ class DelegatesToStaticCompileTest extends
DelegatesToSTCTest implements StaticC
assert !bytecode.contains('INVOKESTATIC
org/codehaus/groovy/runtime/ScriptBytecodeAdapter.castToType')
}
}
-}
\ No newline at end of file
+}
