Skip to content

Commit

Permalink
GROOVY-11373: SC: update error for direct target of inaccessible method
Browse files Browse the repository at this point in the history 
3_0_X backport
  • Loading branch information
@eric-milles
eric-milles committed May 9, 2024
1 parent 9d41af9 commit 005a3ae
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 45 deletions.
44 changes: 24 additions & 20 deletions src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

import static org.apache.groovy.ast.tools.ClassNodeUtils.formatTypeName;
import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
Expand Down Expand Up @@ -355,16 +357,17 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
return true;
}

Expression fixedReceiver = receiver;
boolean fixedImplicitThis = implicitThis;
if (target.isProtected()) {
ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
Expression fixedReceiver = receiver;
boolean fixedImplicitThis = implicitThis;
if (target.isPackageScope()) { // GROOVY-11373
/*if (!samePackageName(target.getDeclaringClass(), classNode)) {
writeMethodAccessError(target, receiver != null ? receiver : args);
}*/
} else if (target.isProtected()) {
ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, classNode);
if (!implicitThis && !isThisOrSuper(receiver) && !samePackageName(node, classNode)
&& StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass())) {
controller.getSourceUnit().addError(new SyntaxException(
"Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
receiver != null ? receiver : args
));
&& StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node, target.getDeclaringClass())) {
writeMethodAccessError(target, receiver != null ? receiver : args);
} else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
}
Expand Down Expand Up @@ -395,28 +398,29 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
return super.writeDirectMethodCall(target, implicitThis, receiver, args);
}

private void writeMethodAccessError(final MethodNode target, final Expression origin) {
StringJoiner descriptor = new StringJoiner(", ", target.getName() + "(", ")");
for (Parameter parameter : target.getParameters()) {
descriptor.add(formatTypeName(parameter.getOriginType()));
}
String message = "Cannot access method: " + descriptor + " of class: " + formatTypeName(target.getDeclaringClass());

controller.getSourceUnit().addError(new SyntaxException(message, origin));
}

private boolean tryPrivateMethod(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args, final ClassNode classNode) {
ClassNode declaringClass = target.getDeclaringClass();
if ((isPrivateBridgeMethodsCallAllowed(declaringClass, classNode) || isPrivateBridgeMethodsCallAllowed(classNode, declaringClass))
&& declaringClass.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS) != null
&& !declaringClass.equals(classNode)) {
if (tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
} else {
checkAndAddCannotCallPrivateMethodError(target, receiver, classNode, declaringClass);
}
}
checkAndAddCannotCallPrivateMethodError(target, receiver, classNode, declaringClass);
return false;
}

private void checkAndAddCannotCallPrivateMethodError(final MethodNode target, final Expression receiver, final ClassNode classNode, final ClassNode declaringClass) {
if (declaringClass != classNode) {
controller.getSourceUnit().addError(new SyntaxException(
"Cannot call private method " + (target.isStatic() ? "static " : "") + declaringClass.toString(false) + "#" + target.getName() + " from class " + classNode.toString(false),
receiver
));
writeMethodAccessError(target, receiver);
}
return false;
}

protected static boolean isPrivateBridgeMethodsCallAllowed(final ClassNode receiver, final ClassNode caller) {
Expand Down
47 changes: 22 additions & 25 deletions src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,40 @@
* specific language governing permissions and limitations
* under the License.
*/


package org.codehaus.groovy.classgen.asm.sc.bugs

import groovy.transform.stc.StaticTypeCheckingTestCase
import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport

class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
final class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {

void testGenericIdentityWithClosure() {
assertScript '''
public static <T> T identity(T self) { self }
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
})
Integer i = identity(2)
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
})
Closure c = identity {'foo'}
'''
static <T> T itself(T self) { self }
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
})
Integer i = itself(2)
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
})
Closure c = itself {'foo'}
'''
}

void testShouldNotThrowIllegalAccessToProtectedData() {
shouldFailWithMessages('''
class Test {
final Set<String> HISTORY = [] as HashSet
Set<String> getHistory() {
return HISTORY.clone() as HashSet<String>
}
shouldFailWithMessages '''
class C {
private final Set<String> history = []
Set<String> getHistory() {
(Set<String>) history.clone()
}
}
Test test = new Test()
println test.history
''', 'Method clone is protected in java.lang.Object')
def set = new C().history
''',
'Cannot access method: clone() of class: java.lang.Object'
}
}

0 comments on commit 005a3ae

Please sign in to comment.