You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by cc...@apache.org on 2015/06/05 13:52:23 UTC
incubator-groovy git commit: GROOVY-7456: Do not transform method
calls in closures for traits unless they are private trait methods
Repository: incubator-groovy
Updated Branches:
refs/heads/master 1dd170db1 -> 6fe6b3181
GROOVY-7456: Do not transform method calls in closures for traits unless they are private trait methods
Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/6fe6b318
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/6fe6b318
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/6fe6b318
Branch: refs/heads/master
Commit: 6fe6b31816e43ae3408e2dfdbf2d2bad1a9791c0
Parents: 1dd170d
Author: Cedric Champeau <cc...@apache.org>
Authored: Fri Jun 5 13:51:31 2015 +0200
Committer: Cedric Champeau <cc...@apache.org>
Committed: Fri Jun 5 13:51:31 2015 +0200
----------------------------------------------------------------------
.../stc/StaticTypeCheckingVisitor.java | 22 +++++--
.../trait/TraitReceiverTransformer.java | 61 ++++++++++++--------
.../classgen/asm/sc/bugs/Groovy7242Bug.groovy | 18 ++++++
.../transform/traitx/Groovy7456Bug.groovy | 50 ++++++++++++++++
4 files changed, 121 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/6fe6b318/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 0c2cfec..f52f736 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -2696,6 +2696,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private static boolean isTraitHelper(ClassNode node) {
+ return node instanceof InnerClassNode && Traits.isTrait(node.getOuterClass());
+ }
+
protected void addReceivers(final List<Receiver<String>> receivers,
final Collection<Receiver<String>> owners,
final boolean implicitThis) {
@@ -2710,16 +2714,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
int strategy = dmd.getStrategy();
ClassNode delegate = dmd.getType();
dmd = dmd.getParent();
-
switch (strategy) {
case Closure.OWNER_FIRST:
receivers.addAll(owners);
path.append("delegate");
- receivers.add(new Receiver<String>(delegate, path.toString()));
+ doAddDelegateReceiver(receivers, path, delegate);
break;
case Closure.DELEGATE_FIRST:
path.append("delegate");
- receivers.add(new Receiver<String>(delegate, path.toString()));
+ doAddDelegateReceiver(receivers, path, delegate);
receivers.addAll(owners);
break;
case Closure.OWNER_ONLY:
@@ -2728,7 +2731,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
break;
case Closure.DELEGATE_ONLY:
path.append("delegate");
- receivers.add(new Receiver<String>(delegate, path.toString()));
+ doAddDelegateReceiver(receivers, path, delegate);
dmd = null;
break;
}
@@ -2736,6 +2739,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private void doAddDelegateReceiver(final List<Receiver<String>> receivers, final StringBuilder path, final ClassNode delegate) {
+ receivers.add(new Receiver<String>(delegate, path.toString()));
+ if (isTraitHelper(delegate)) {
+ receivers.add(new Receiver<String>(delegate.getOuterClass(), path.toString()));
+ }
+ }
+
@Override
public void visitMethodCallExpression(MethodCallExpression call) {
final String name = call.getMethodAsString();
@@ -3920,10 +3930,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return node;
} else if (exp instanceof VariableExpression) {
VariableExpression vexp = (VariableExpression) exp;
- if (vexp == VariableExpression.THIS_EXPRESSION) return makeThis();
- if (vexp == VariableExpression.SUPER_EXPRESSION) return makeSuper();
ClassNode selfTrait = isTraitSelf(vexp);
if (selfTrait!=null) return makeSelf(selfTrait);
+ if (vexp == VariableExpression.THIS_EXPRESSION) return makeThis();
+ if (vexp == VariableExpression.SUPER_EXPRESSION) return makeSuper();
final Variable variable = vexp.getAccessedVariable();
if (variable instanceof FieldNode) {
checkOrMarkPrivateAccess(vexp, (FieldNode) variable);
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/6fe6b318/src/main/org/codehaus/groovy/transform/trait/TraitReceiverTransformer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitReceiverTransformer.java b/src/main/org/codehaus/groovy/transform/trait/TraitReceiverTransformer.java
index fe1298e..4cd28f8 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitReceiverTransformer.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitReceiverTransformer.java
@@ -53,13 +53,10 @@ import java.util.Collection;
import java.util.List;
/**
- * This expression transformer is used internally by the {@link org.codehaus.groovy.transform.trait.TraitASTTransformation trait}
- * AST transformation to change the receiver of a message on "this" into a static method call on the trait helper class.
- * <p></p>
- * In a nutshell, code like this one in a trait:<p></p>
- * <code>void foo() { this.bar() }</code>
- * is transformed into:
- * <code>void foo() { TraitHelper$bar(this) }</code>
+ * This expression transformer is used internally by the {@link org.codehaus.groovy.transform.trait.TraitASTTransformation
+ * trait} AST transformation to change the receiver of a message on "this" into a static method call on the trait helper
+ * class. <p></p> In a nutshell, code like this one in a trait:<p></p> <code>void foo() { this.bar() }</code> is
+ * transformed into: <code>void foo() { TraitHelper$bar(this) }</code>
*
* @author Cedric Champeau
* @since 2.3.0
@@ -74,6 +71,8 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
private final ClassNode fieldHelper;
private final Collection<String> knownFields;
+ private boolean inClosure;
+
public TraitReceiverTransformer(VariableExpression thisObject, SourceUnit unit, final ClassNode traitClass, ClassNode fieldHelper, Collection<String> knownFields) {
this.weaved = thisObject;
this.unit = unit;
@@ -91,7 +90,7 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
public Expression transform(final Expression exp) {
ClassNode weavedType = weaved.getOriginType();
if (exp instanceof BinaryExpression) {
- return transformBinaryExpression((BinaryExpression)exp, weavedType);
+ return transformBinaryExpression((BinaryExpression) exp, weavedType);
} else if (exp instanceof StaticMethodCallExpression) {
StaticMethodCallExpression call = (StaticMethodCallExpression) exp;
ClassNode ownerType = call.getOwnerType();
@@ -116,7 +115,7 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
return transformSuperMethodCall(call);
}
} else if (exp instanceof FieldExpression) {
- return transformFieldExpression((FieldExpression)exp);
+ return transformFieldExpression((FieldExpression) exp);
} else if (exp instanceof VariableExpression) {
VariableExpression vexp = (VariableExpression) exp;
Variable accessedVariable = vexp.getAccessedVariable();
@@ -129,9 +128,9 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
receiver = createStaticReceiver(receiver);
}
mce = new MethodCallExpression(
- receiver,
- Traits.helperGetterName(fn),
- ArgumentListExpression.EMPTY_ARGUMENTS
+ receiver,
+ Traits.helperGetterName(fn),
+ ArgumentListExpression.EMPTY_ARGUMENTS
);
mce.setSourcePosition(exp);
mce.setImplicitThis(false);
@@ -198,7 +197,10 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
);
mce.setImplicitThis(false);
mce.setSourcePosition(exp);
+ boolean oldInClosure = inClosure;
+ inClosure = true;
((ClosureExpression) exp).getCode().visit(this);
+ inClosure = oldInClosure;
// The rewrite we do is causing some troubles with type checking, which will
// not be able to perform closure parameter type inference
// so we store the replacement, which will be done *after* type checking.
@@ -238,18 +240,18 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
&& (((PropertyExpression) leftExpression).isImplicitThis() || "this".equals(((PropertyExpression) leftExpression).getObjectExpression().getText()))) {
leftFieldName = ((PropertyExpression) leftExpression).getPropertyAsString();
FieldNode fn = tryGetFieldNode(weavedType, leftFieldName);
- if (fieldHelper == null || fn==null && !fieldHelper.hasPossibleMethod(Traits.helperSetterName(new FieldNode(leftFieldName, 0, ClassHelper.OBJECT_TYPE, weavedType, null)), rightExpression)) {
+ if (fieldHelper == null || fn == null && !fieldHelper.hasPossibleMethod(Traits.helperSetterName(new FieldNode(leftFieldName, 0, ClassHelper.OBJECT_TYPE, weavedType, null)), rightExpression)) {
return createAssignmentToField(rightExpression, operation, leftFieldName);
}
}
- if (leftFieldName!=null) {
+ if (leftFieldName != null) {
FieldNode fn = weavedType.getDeclaredField(leftFieldName);
FieldNode staticField = tryGetFieldNode(weavedType, leftFieldName);
- if (fn==null) {
+ if (fn == null) {
fn = new FieldNode(leftFieldName, 0, ClassHelper.OBJECT_TYPE, weavedType, null);
}
Expression receiver = createFieldHelperReceiver();
- boolean isStatic = staticField!=null && staticField.isStatic();
+ boolean isStatic = staticField != null && staticField.isStatic();
if (fn.isStatic()) { // DO NOT USE isStatic variable here!
receiver = new PropertyExpression(receiver, "class");
}
@@ -268,10 +270,10 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
Expression leftTransform = transform(leftExpression);
Expression rightTransform = transform(rightExpression);
Expression ret =
- exp instanceof DeclarationExpression ?new DeclarationExpression(
+ exp instanceof DeclarationExpression ? new DeclarationExpression(
leftTransform, operation, rightTransform
- ):
- new BinaryExpression(leftTransform, operation, rightTransform);
+ ) :
+ new BinaryExpression(leftTransform, operation, rightTransform);
ret.setSourcePosition(exp);
ret.copyNodeMetaData(exp);
return ret;
@@ -307,9 +309,9 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
private FieldNode tryGetFieldNode(final ClassNode weavedType, final String fieldName) {
FieldNode fn = weavedType.getDeclaredField(fieldName);
- if (fn==null && ClassHelper.CLASS_Type.equals(weavedType)) {
+ if (fn == null && ClassHelper.CLASS_Type.equals(weavedType)) {
GenericsType[] genericsTypes = weavedType.getGenericsTypes();
- if (genericsTypes !=null && genericsTypes.length==1) {
+ if (genericsTypes != null && genericsTypes.length == 1) {
// for static properties
fn = genericsTypes[0].getType().getDeclaredField(fieldName);
}
@@ -323,7 +325,7 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
private Expression transformSuperMethodCall(final MethodCallExpression call) {
String method = call.getMethodAsString();
- if (method==null) {
+ if (method == null) {
throwSuperError(call);
}
@@ -350,7 +352,6 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
}
-
private Expression transformMethodCallOnThis(final MethodCallExpression call) {
Expression method = call.getMethod();
Expression arguments = call.getArguments();
@@ -363,6 +364,18 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
}
}
}
+ if (inClosure) {
+ MethodCallExpression transformed = new MethodCallExpression(
+ (Expression) call.getReceiver(),
+ call.getMethod(),
+ transform(call.getArguments())
+ );
+ transformed.setSourcePosition(call);
+ transformed.setSafe(call.isSafe());
+ transformed.setSpreadSafe(call.isSpreadSafe());
+ transformed.setImplicitThis(call.isImplicitThis());
+ return transformed;
+ }
MethodCallExpression transformed = new MethodCallExpression(
weaved,
@@ -391,7 +404,7 @@ class TraitReceiverTransformer extends ClassCodeExpressionTransformer {
}
private Expression createFieldHelperReceiver() {
- return ClassHelper.CLASS_Type.equals(weaved.getOriginType())?weaved:new CastExpression(fieldHelper,weaved);
+ return ClassHelper.CLASS_Type.equals(weaved.getOriginType()) ? weaved : new CastExpression(fieldHelper, weaved);
}
private ArgumentListExpression createArgumentList(final Expression origCallArgs) {
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/6fe6b318/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
index 817443e..17e6d62 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
@@ -80,4 +80,22 @@ class Groovy7242Bug extends StaticTypeCheckingTestCase implements StaticCompilat
assert a.x == 1
'''
}
+
+ void testCallPrivateMethodOfTraitInsideClosure() {
+ assertScript '''
+ trait MyTrait {
+ def f() {
+ ['a'].collect {String it -> f2(it)}
+ }
+
+ private f2(String s) {
+ s.toUpperCase()
+ }
+ }
+
+ class A implements MyTrait {}
+ def a = new A()
+ assert a.f() == ['A']
+ '''
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/6fe6b318/src/test/org/codehaus/groovy/transform/traitx/Groovy7456Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/traitx/Groovy7456Bug.groovy b/src/test/org/codehaus/groovy/transform/traitx/Groovy7456Bug.groovy
new file mode 100644
index 0000000..ecc5c46
--- /dev/null
+++ b/src/test/org/codehaus/groovy/transform/traitx/Groovy7456Bug.groovy
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2003-2015 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.traitx
+
+class Groovy7456Bug extends GroovyTestCase {
+ void testShouldAllowBuilderInTrait() {
+ assertScript '''
+ import groovy.xml.*
+
+ trait MyBuilder {
+ def build2() {
+ def mkp = new MarkupBuilder()
+
+ mkp.foo {
+ bar()
+ }
+ }
+ }
+
+ class Foo implements MyBuilder {
+
+ def build1() {
+ def mkp = new MarkupBuilder()
+
+ mkp.foo {
+ bar()
+ }
+ }
+ }
+
+ def foo = new Foo()
+ foo.build1()
+ foo.build2()
+ '''
+ }
+}