Skip to content

Commit

Permalink
GROOVY-7970: Can't call private method from outer class when using an…
Browse files Browse the repository at this point in the history
…onymous inner classes and @cs (closes #452)
  • Loading branch information
paulk-asert committed Nov 10, 2016
1 parent 6f39e27 commit ad56669
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 10 deletions.
Expand Up @@ -22,6 +22,7 @@
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.MethodNode;
Expand Down Expand Up @@ -186,7 +187,16 @@ public void writeSpecialConstructorCall(final ConstructorCallExpression call) {
/**
* Attempts to make a direct method call on a bridge method, if it exists.
*/
@Deprecated
protected boolean tryBridgeMethod(MethodNode target, Expression receiver, boolean implicitThis, TupleExpression args) {
return tryBridgeMethod(target, receiver, implicitThis, args, null);
}

/**
* Attempts to make a direct method call on a bridge method, if it exists.
*/
protected boolean tryBridgeMethod(MethodNode target, Expression receiver, boolean implicitThis,
TupleExpression args, ClassNode thisClass) {
ClassNode lookupClassNode;
if (target.isProtected()) {
lookupClassNode = controller.getClassNode();
Expand All @@ -203,8 +213,22 @@ protected boolean tryBridgeMethod(MethodNode target, Expression receiver, boolea
MethodNode bridge = bridges==null?null:bridges.get(target);
if (bridge != null) {
Expression fixedReceiver = receiver;
if (implicitThis && !controller.isInClosure()) {
fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
if (implicitThis) {
if (!controller.isInClosure()) {
fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
} else if (thisClass != null) {
ClassNode current = thisClass.getOuterClass();
fixedReceiver = new VariableExpression("thisObject", current);
// adjust for multiple levels of nesting if needed
while (current != null && current instanceof InnerClassNode && !lookupClassNode.equals(current)) {
FieldNode thisField = current.getField("this$0");
current = current.getOuterClass();
if (thisField != null) {
fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
fixedReceiver.setType(current);
}
}
}
}
ArgumentListExpression newArgs = new ArgumentListExpression(target.isStatic()?new ConstantExpression(null):fixedReceiver);
for (Expression expression : args.getExpressions()) {
Expand Down Expand Up @@ -261,7 +285,7 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
&& controller.isInClosure()
&& !target.isPublic()
&& target.getDeclaringClass() != classNode) {
if (!tryBridgeMethod(target, receiver, implicitThis, args)) {
if (!tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
// replace call with an invoker helper call
ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
MethodCallExpression mce = new MethodCallExpression(
Expand All @@ -281,6 +305,8 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
}
return true;
}
Expression fixedReceiver = null;
boolean fixedImplicitThis = implicitThis;
if (target.isPrivate()) {
if (tryPrivateMethod(target, implicitThis, receiver, args, classNode)) return true;
} else if (target.isProtected()) {
Expand All @@ -295,16 +321,36 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
controller.getSourceUnit().addError(
new SyntaxException("Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
src.getLineNumber(), src.getColumnNumber(), src.getLastLineNumber(), src.getLastColumnNumber()));
} else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args)) {
} else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
}
} else if (target.isPublic() && receiver != null) {
if (implicitThis
&& !classNode.isDerivedFrom(target.getDeclaringClass())
&& !classNode.implementsInterface(target.getDeclaringClass())
&& classNode instanceof InnerClassNode && controller.isInClosure()) {
ClassNode current = classNode.getOuterClass();
fixedReceiver = new VariableExpression("thisObject", current);
// adjust for multiple levels of nesting if needed
while (current != null && current instanceof InnerClassNode && !classNode.equals(current)) {
FieldNode thisField = current.getField("this$0");
current = current.getOuterClass();
if (thisField != null) {
fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
fixedReceiver.setType(current);
fixedImplicitThis = false;
}
}
}
}
if (receiver != null) {
if (!(receiver instanceof VariableExpression) || !((VariableExpression) receiver).isSuperExpression()) {
boolean callToSuper = receiver instanceof VariableExpression && ((VariableExpression) receiver).isSuperExpression();
if (!callToSuper) {
fixedReceiver = fixedReceiver == null ? receiver : fixedReceiver;
// in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead
// then replace the top operand type
Expression checkCastReceiver = new CheckcastReceiverExpression(receiver, target);
return super.writeDirectMethodCall(target, implicitThis, checkCastReceiver, args);
Expression checkCastReceiver = new CheckcastReceiverExpression(fixedReceiver, target);
return super.writeDirectMethodCall(target, fixedImplicitThis, checkCastReceiver, args);
}
}
return super.writeDirectMethodCall(target, implicitThis, receiver, args);
Expand All @@ -316,7 +362,7 @@ private boolean tryPrivateMethod(final MethodNode target, final boolean implicit
if ((isPrivateBridgeMethodsCallAllowed(declaringClass, classNode) || isPrivateBridgeMethodsCallAllowed(classNode, declaringClass))
&& declaringClass.getNodeMetaData(PRIVATE_BRIDGE_METHODS) != null
&& !declaringClass.equals(classNode)) {
if (tryBridgeMethod(target, receiver, implicitThis, args)) {
if (tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
} else if (declaringClass != classNode) {
controller.getSourceUnit().addError(new SyntaxException("Cannot call private method " + (target.isStatic() ? "static " : "") +
Expand Down
Expand Up @@ -3849,8 +3849,11 @@ protected List<MethodNode> findMethod(
collectAllInterfaceMethodsByName(receiver, name, methods);
methods.addAll(OBJECT_TYPE.getMethods(name));
}
if (typeCheckingContext.getEnclosingClosure() == null) {
// not in a closure
// TODO: investigate the trait exclusion a bit further, needed otherwise
// CallMethodOfTraitInsideClosureAndClosureParamTypeInference fails saying
// not static method can't be called from a static context
if (typeCheckingContext.getEnclosingClosure() == null || (receiver instanceof InnerClassNode && !receiver.getName().endsWith("$Trait$Helper"))) {
// not in a closure or within an inner class
ClassNode parent = receiver;
while (parent instanceof InnerClassNode && !parent.isStaticClass()) {
parent = parent.getOuterClass();
Expand Down
125 changes: 125 additions & 0 deletions src/test/groovy/bugs/Groovy7970Bug.groovy
@@ -0,0 +1,125 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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 groovy.bugs

class Groovy7970Bug extends GroovyTestCase {

private static final String getScriptAIC(String visibility, boolean cs) {
"""
${ cs ? '@groovy.transform.CompileStatic' : ''}
class Bar {
$visibility String renderTemplate(String arg) { "dummy\$arg" }
def foo() {
new Object() {
String bar() {
'A'.with{ renderTemplate(it) }
}
}
}
}
assert new Bar().foo().bar() == 'dummyA'
"""
}

private static final String getScriptNestedAIC(String visibility, boolean cs) {
"""
${ cs ? '@groovy.transform.CompileStatic' : ''}
class Bar {
$visibility String renderTemplate(String arg) { "dummy\$arg" }
def foo() {
new Object() {
def bar() {
new Object() { String xxx() { 'B'.with{ renderTemplate(it) } }}
}
}
}
}
assert new Bar().foo().bar().xxx() == 'dummyB'
"""
}

private static final String getScriptInner(String visibility, boolean cs) {
"""
${ cs ? '@groovy.transform.CompileStatic' : ''}
class Bar {
$visibility String renderTemplate(String arg) { "dummy\$arg" }
class Inner {
String baz() {
'C'.with{ renderTemplate(it) }
}
}
}
def b = new Bar()
assert new Bar.Inner(b).baz() == 'dummyC'
"""
}

private static final String getScriptNestedInner(String visibility, boolean cs) {
"""
${ cs ? '@groovy.transform.CompileStatic' : ''}
class Bar {
$visibility String renderTemplate(String arg) { "dummy\$arg" }
class Inner {
class InnerInner {
String xxx() { 'D'.with{ renderTemplate(it) } }
}
}
}
def b = new Bar()
def bi = new Bar.Inner(b)
assert new Bar.Inner.InnerInner(bi).xxx() == 'dummyD'
"""
}

void testMethodAccessFromClosureWithinInnerClass() {
assertScript getScriptInner('public', false)
assertScript getScriptInner('protected', false)
assertScript getScriptInner('private', false)
assertScript getScriptNestedInner('public', false)
assertScript getScriptNestedInner('protected', false)
assertScript getScriptNestedInner('private', false)
}

void testMethodAccessFromClosureWithinInnerClass_CS() {
assertScript getScriptInner('public', true)
assertScript getScriptInner('protected', true)
assertScript getScriptInner('private', true)
assertScript getScriptNestedInner('public', true)
assertScript getScriptNestedInner('protected', true)
assertScript getScriptNestedInner('private', true)
}

void testMethodAccessFromClosureWithinAIC() {
assertScript getScriptAIC('public', false)
assertScript getScriptAIC('protected', false)
assertScript getScriptAIC('private', false)
assertScript getScriptNestedAIC('public', false)
assertScript getScriptNestedAIC('protected', false)
assertScript getScriptNestedAIC('private', false)
}

void testMethodAccessFromClosureWithinAIC_CS() {
assertScript getScriptAIC('public', true)
assertScript getScriptAIC('protected', true)
assertScript getScriptAIC('private', true)
assertScript getScriptNestedAIC('public', true)
assertScript getScriptNestedAIC('protected', true)
assertScript getScriptNestedAIC('private', true)
}
}

0 comments on commit ad56669

Please sign in to comment.