GROOVY-11373: SC: cross-package direct target of package-private method

apache · May 9, 2024 · 131992c · 131992c
1 parent 14bb43a
commit 131992c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 32 deletions.
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -73,6 +73,7 @@
 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;
@@ -347,16 +348,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;
             }
@@ -389,6 +391,16 @@ 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) {
+        var descriptor = new java.util.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 isClassWithSuper(Expression receiver) {
         if (receiver instanceof PropertyExpression) {
             PropertyExpression pexp = (PropertyExpression) receiver;
@@ -404,21 +416,12 @@ private boolean tryPrivateMethod(final MethodNode target, final boolean implicit
                 && !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) {

diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -940,7 +940,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
         '''
     }
 
-    // GROOVY-11223
+    // GROOVY-11223, GROOVY-11373
     @Override
     void testMapPropertyAccess10() {
         assertScript """
@@ -954,13 +954,12 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
             def map = new ${MapType.name}()
             map.bar = 22 // protected setter
         """,
-        "Method setBar is protected in ${MapType.name}"
-        /*
+        "Cannot access method: setBar(java.lang.Object) of class: ${MapType.name}"
+
         shouldFailWithMessages """
             def map = new ${MapType.name}()
             map.baz = 33 // package-private setter
         """,
-        "Method setBaz is package-private in ${MapType.name}"
-        */
+        "Cannot access method: setBaz(java.lang.Object) of class: ${MapType.name}"
     }
 }
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
@@ -41,17 +41,15 @@ final class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCo
 
     void testShouldNotThrowIllegalAccessToProtectedData() {
         shouldFailWithMessages '''
-            class Test {
-                final Set<String> HISTORY = [] as HashSet
-
+            class C {
+                private final Set<String> history = []
                 Set<String> getHistory() {
-                    return HISTORY.clone() as HashSet<String>
+                    (Set<String>) history.clone()
                 }
             }
 
-            Test test = new Test()
-            println test.history
+            def set = new C().history
         ''',
-        'Method clone is protected in java.lang.Object'
+        'Cannot access method: clone() of class: java.lang.Object'
     }
 }