GROOVY-6277, GROOVY-10390: SC: ensure accessibility of getter method

3_0_X backport
apache · May 29, 2024 · 6d2b866 · 6d2b866
1 parent 0c08d52
commit 6d2b866
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 18 deletions.
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -26,7 +26,6 @@
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.MethodNode;
-import org.codehaus.groovy.ast.PackageNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.Variable;
@@ -77,6 +76,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Consumer;
 
@@ -991,17 +991,12 @@ public static boolean hasDeclaredMethod(final ClassNode cNode, final String name
     }
 
     public static boolean inSamePackage(final ClassNode first, final ClassNode second) {
-        PackageNode firstPackage = first.getPackage();
-        PackageNode secondPackage = second.getPackage();
-        return ((firstPackage == null && secondPackage == null)
-                || firstPackage != null && secondPackage != null && firstPackage.getName().equals(secondPackage.getName()));
+        return Objects.equals(first.getPackageName(), second.getPackageName());
     }
 
-    public static boolean inSamePackage(final Class<?> first, final Class<?> second) {
-        Package firstPackage = first.getPackage();
-        Package secondPackage = second.getPackage();
-        return ((firstPackage == null && secondPackage == null)
-                || firstPackage != null && secondPackage != null && firstPackage.getName().equals(secondPackage.getName()));
+    public static boolean inSamePackage(final Class<?>  first, final Class<?>  second) {
+        Package firstPackage = first.getPackage(), secondPackage = second.getPackage();
+        return (firstPackage == null ? secondPackage == null : firstPackage.getName().equals(secondPackage.getName()));
     }
 
     public static boolean isDefaultVisibility(final int modifiers) {

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -85,6 +85,7 @@
 import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.inSamePackage;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
@@ -475,6 +476,14 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
             getterNode.setDeclaringClass(receiverType);
         }
         if (getterNode != null) {
+            // GROOVY-6277, GROOVY-11390: ensure accessibility
+            ClassNode accessingClass = controller.getClassNode();
+            ClassNode declaringClass = getterNode.getDeclaringClass();
+            if (!getterNode.isPublic() && !accessingClass.equals(declaringClass)
+                    && !(getterNode.isProtected() && accessingClass.isDerivedFrom(declaringClass))
+                    && (getterNode.isPrivate() || !inSamePackage(accessingClass, declaringClass))) {
+                return false;
+            }
             MethodCallExpression call = callX(receiver, getterName);
             call.setImplicitThis(implicitThis);
             call.setMethodTarget(getterNode);

diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -764,7 +764,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
         }
     }
 
-    // GROOVY-8074
+    // GROOVY-8074, GROOVY-11390
     void testMapPropertyAccess6() {
         assertScript '''
             class C extends HashMap {
@@ -787,6 +787,26 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
             assert map.baz == 33
             assert map['baz'] == 33
         """
+        assertScript """ // map entry before super property
+            class C extends ${PogoType.name} implements Map {
+                @Delegate private Map impl = [:]
+                void test() {
+                    put('foo', 11)
+                  //assert this.foo    == 11 // public getter of super
+                    assert this['foo'] == 11
+                    put('bar', 22)
+                  //assert this.bar    == 22 // protected getter of super
+                    assert this['bar'] == 22
+                    put('baz', 33)
+                    assert this.baz    == 33 // package-private getter of super
+                    assert this['baz'] == 33
+                    put('xxx', 44)
+                    assert this.xxx    == 44 // private getter of super
+                    assert this['xxx'] == 44
+                }
+            }
+            new C().test()
+        """
     }
 
     // GROOVY-5001, GROOVY-5491, GROOVY-6144, GROOVY-8555
@@ -1902,6 +1922,13 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
         @PackageScope void setBaz(baz) {}
     }
 
+    static class PogoType {
+        public        getFoo() { 1 }
+        protected     getBar() { 2 }
+        @PackageScope getBaz() { 3 }
+        private       getXxx() { 4 }
+    }
+
     static class BaseClass {
         int x
     }

diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -18,7 +18,6 @@
  */
 package org.codehaus.groovy.classgen.asm.sc
 
-import groovy.test.NotYetImplemented
 import groovy.transform.stc.FieldsAndPropertiesSTCTest
 
 /**
@@ -822,12 +821,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
         '''
     }
 
-    // GROOVY-6277
-    @Override @NotYetImplemented
-    void testPublicFieldVersusPrivateGetter() {
-        super.testPublicFieldVersusPrivateGetter()
-    }
-
     // GROOVY-11369
     @Override
     void testMapPropertyAccess5a() {