GROOVY-6323, GROOVY-6325, GROOVY-6332: make Closure not sue static scope and add transformer for VariableExpression in the static compiler
diff --git a/src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 5f84cfa..36fe89a 100644
--- a/src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -422,6 +422,7 @@
currentScope.putDeclaredVariable(var);
}
+ currentScope.setInStaticContext(false);
super.visitClosureExpression(expression);
markClosureSharedVariables();
diff --git a/src/main/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java b/src/main/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
index 1effdc9..93708c2 100644
--- a/src/main/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
+++ b/src/main/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
@@ -59,6 +59,7 @@
private final BinaryExpressionTransformer binaryExpressionTransformer = new BinaryExpressionTransformer(this);
private final ClosureExpressionTransformer closureExpressionTransformer = new ClosureExpressionTransformer(this);
private final BooleanExpressionTransformer booleanExpressionTransformer = new BooleanExpressionTransformer(this);
+ private final VariableExpressionTransformer variableExpressionTransformer = new VariableExpressionTransformer();
public StaticCompilationTransformer(final SourceUnit unit) {
this.unit = unit;
@@ -102,6 +103,9 @@
if (expr instanceof BooleanExpression) {
return booleanExpressionTransformer.transformBooleanExpression((BooleanExpression)expr);
}
+ if (expr instanceof VariableExpression) {
+ return variableExpressionTransformer.transformVariableExpression((VariableExpression)expr);
+ }
return super.transform(expr);
}
diff --git a/src/main/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java b/src/main/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
new file mode 100644
index 0000000..b475bde
--- /dev/null
+++ b/src/main/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2003-2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.codehaus.groovy.transform.sc.transformers;
+
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.transform.stc.StaticTypesMarker;
+
+/**
+ * Transformer for VariableExpression the bytecode backend wouldn't be able to
+ * handle otherwise.
+ * @author <a href="mailto:blackdrag@gmx.org">Jochen "blackdrag" Theodorou</a>
+ */
+public class VariableExpressionTransformer {
+
+ public Expression transformVariableExpression(VariableExpression expr) {
+ // we need to transform variable expressions that go to a delegate
+ // to a property expression, as ACG would loose the information
+ // in processClassVariable before it reaches any makeCall, that could
+ // handle it
+ Object val = expr.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+ if (val==null) return expr;
+ PropertyExpression pexp = new PropertyExpression(new VariableExpression("this"), expr.getName());
+ pexp.copyNodeMetaData(expr);
+ pexp.setImplicitThis(true);
+ return pexp;
+ }
+}
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 072a6da..63f3a69 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -345,7 +345,10 @@
pe.setImplicitThis(true);
if (visitPropertyExpressionSilent(pe, vexp)) {
storeType(vexp, getType(pe));
- vexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, pe.getNodeMetaData(StaticTypesMarker.READONLY_PROPERTY));
+ Object val = pe.getNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
+ if (val!=null) vexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY,val);
+ val = pe.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+ if (val!=null) vexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER,val);
return;
}
@@ -957,15 +960,15 @@
FieldNode field = current.getDeclaredField(propertyName);
field = allowStaticAccessToMember(field, staticOnly);
- if (storeField(field, isAttributeExpression, pexp, objectExpressionType, visitor)) return true;
+ if (storeField(field, isAttributeExpression, pexp, objectExpressionType, visitor, receiver.getData())) return true;
PropertyNode propertyNode = current.getProperty(propertyName);
propertyNode = allowStaticAccessToMember(propertyNode, staticOnly);
- if (storeProperty(propertyNode, pexp, objectExpressionType, visitor)) return true;
+ if (storeProperty(propertyNode, pexp, objectExpressionType, visitor, receiver.getData())) return true;
boolean isThisExpression = objectExpression instanceof VariableExpression &&
((VariableExpression)objectExpression).isThisExpression();
- if (storeField(field, isThisExpression, pexp, objectExpressionType, visitor)) return true;
+ if (storeField(field, isThisExpression, pexp, objectExpressionType, visitor, receiver.getData())) return true;
MethodNode getter = current.getGetterMethod("get" + capName);
getter = allowStaticAccessToMember(getter, staticOnly);
@@ -983,6 +986,8 @@
ClassNode cn = inferReturnTypeGenerics(current, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
storeInferredTypeForPropertyExpression(pexp, cn);
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
+ String delegationData = receiver.getData();
+ if (delegationData!=null) pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
return true;
}
} else {
@@ -996,6 +1001,8 @@
SetterInfo info = new SetterInfo(current, setter);
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression!=null) putSetterInfo(enclosingBinaryExpression.getLeftExpression(), info);
+ String delegationData = receiver.getData();
+ if (delegationData!=null) pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
return true;
} else if (getter!=null) {
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, true);
@@ -1003,7 +1010,7 @@
}
foundGetterOrSetter = foundGetterOrSetter || setter!=null || getter!=null;
- if (storeField(field, true, pexp, objectExpressionType, visitor)) return true;
+ if (storeField(field, true, pexp, objectExpressionType, visitor, receiver.getData())) return true;
// if the property expression is an attribute expression (o.@attr), then
// we stop now, otherwise we must check the parent class
if (/*!isAttributeExpression && */current.getSuperClass() != null) {
@@ -1034,11 +1041,13 @@
if (propertyType==null) propertyType = getTypeForSpreadExpression(testClass, objectExpressionType, pexp);
if (propertyType==null) continue;
if (visitor!=null) {
- // todo : type inferrence on maps and lists, if possible
+ // todo : type inference on maps and lists, if possible
PropertyNode node = new PropertyNode(propertyName, Opcodes.ACC_PUBLIC, propertyType, objectExpressionType, null, null, null);
visitor.visitProperty(node);
}
storeType(pexp, propertyType);
+ String delegationData = receiver.getData();
+ if (delegationData!=null) pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
return true;
}
return foundGetterOrSetter;
@@ -1144,18 +1153,24 @@
storeType(expressionToStoreOn, type);
}
- private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, PropertyExpression expressionToStoreOn, ClassNode receiver, ClassCodeVisitorSupport visitor) {
+ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, PropertyExpression expressionToStoreOn, ClassNode receiver, ClassCodeVisitorSupport visitor, String delegationData) {
if (field==null || !returnTrueIfFieldExists) return false;
if (visitor != null) visitor.visitField(field);
storeWithResolve(field.getOriginType(), receiver, field.getDeclaringClass(), field.isStatic(), expressionToStoreOn);
checkOrMarkPrivateAccess(field);
+ if (delegationData!=null) {
+ expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
+ }
return true;
}
- private boolean storeProperty(PropertyNode propertyNode, PropertyExpression expressionToStoreOn, ClassNode receiver, ClassCodeVisitorSupport visitor) {
+ private boolean storeProperty(PropertyNode propertyNode, PropertyExpression expressionToStoreOn, ClassNode receiver, ClassCodeVisitorSupport visitor, String delegationData) {
if (propertyNode == null) return false;
if (visitor != null) visitor.visitProperty(propertyNode);
storeWithResolve(propertyNode.getOriginType(), receiver, propertyNode.getDeclaringClass(), propertyNode.isStatic(), expressionToStoreOn);
+ if (delegationData!=null) {
+ expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
+ }
return true;
}
diff --git a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
index b6f4af0..d472a05 100644
--- a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
@@ -13,18 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
-
-
-
package groovy.transform.stc
/**
- * Units tests aimed at testing the behaviour of {@link DelegatesTo} in combination
+ * Units tests aimed at testing the behavior of {@link DelegatesTo} in combination
* with static type checking.
*
* @author Cedric Champeau
+ * @author <a href="mailto:blackdrag@gmx.org">Jochen "blackdrag" Theodorou</a>
*/
class DelegatesToSTCTest extends StaticTypeCheckingTestCase {
void testShouldChooseMethodFromOwner() {
@@ -633,4 +629,105 @@
''', 'Cannot find matching method'
}
+ // GROOVY-6323, GROOVY-6325, GROOVY-6332
+ void testStaticContextAndProperty() {
+ assertScript '''
+ class MyCar {
+ String brand
+ String model
+ }
+
+ class MyCarMain {
+ MyCar configureCar(@DelegatesTo(MyCar) Closure closure) {
+ def car = new MyCar()
+ closure.delegate = car
+ closure.resolveStrategy = Closure.DELEGATE_FIRST
+ closure()
+ car
+ }
+ static void main(String[] args) {
+ def main = new MyCarMain()
+ def car = main.configureCar {
+ brand = "BMW"
+ model = brand + " X5"
+ }
+ assert car.model == "BMW X5"
+ }
+ }
+ MyCarMain.main()
+ '''
+
+ assertScript '''
+ class MyCar {
+ private String _brand
+ private String _model
+
+ String getBrand() {
+ return _brand
+ }
+
+ void setBrand(String brand) {
+ _brand = brand
+ }
+
+ String getModel() {
+ return _model
+ }
+
+ void setModel(String model) {
+ _model = model
+ }
+ }
+
+ class MyCarMain {
+ MyCar configureCar(@DelegatesTo(value = MyCar, strategy = Closure.DELEGATE_FIRST) Closure closure) {
+ def car = new MyCar()
+ closure.delegate = car
+ closure.resolveStrategy = Closure.DELEGATE_FIRST
+ closure()
+ car
+ }
+
+ static void main(String[] args) {
+ def main = new MyCarMain()
+ def car = main.configureCar {
+ brand = "BMW"
+ model = brand
+ }
+ assert car.model == "BMW"
+ }
+ }
+ MyCarMain.main()
+ '''
+
+ assertScript '''
+ class Car {
+ private String _brand
+ String getBrand() { _brand }
+ void setBrand(String brand) { _brand = brand }
+ }
+
+ class Builder {
+ def <T> T configure(@DelegatesTo.Target Class<T> target, @DelegatesTo(genericTypeIndex=0) Closure cl) {
+ def obj = target.newInstance()
+ cl.delegate = obj
+ cl.resolveStrategy = Closure.DELEGATE_FIRST
+ cl.call()
+ obj
+ }
+ }
+
+ class Main {
+ void run() {
+ def builder = new Builder()
+ def car = builder.configure(Car) {
+ brand = brand
+ }
+ }
+ }
+
+ new Main().run()
+ '''
+ }
+
}
