This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new d22e9a11f1 GROOVY-6277, GROOVY-10390: SC: ensure accessibility of
getter method
d22e9a11f1 is described below
commit d22e9a11f15500ea8e37b7839e917bab8f0987ed
Author: Eric Milles <[email protected]>
AuthorDate: Wed May 29 15:02:14 2024 -0500
GROOVY-6277, GROOVY-10390: SC: ensure accessibility of getter method
4_0_X backport
---
.../codehaus/groovy/ast/tools/GeneralUtils.java | 15 ++++-------
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 13 ++++++++--
.../stc/FieldsAndPropertiesSTCTest.groovy | 29 +++++++++++++++++++++-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 7 ------
4 files changed, 44 insertions(+), 20 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
index ee50cf1493..cc7535c426 100644
--- 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.ClassHelper;
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;
@@ -84,6 +83,7 @@ import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
@@ -1118,17 +1118,12 @@ public class GeneralUtils {
}
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
index ff5d635e87..d4b3608446 100644
---
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.callX;
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;
@@ -434,9 +435,9 @@ public class StaticTypesCallSiteWriter extends
CallSiteWriter {
if (makeGetField(receiver, receiverType, propertyName, safe,
implicitThis)) return;
boolean isScriptVariable = (receiverType.isScript() && receiver
instanceof VariableExpression && ((VariableExpression)
receiver).getAccessedVariable() == null);
- if (!isScriptVariable && controller.getClassNode().getOuterClass() ==
null) { // inner class still needs dynamic property sequence
+ // script variables, self entry lookup and outer class property
resolution still require the dynamic property sequence
+ if (!isScriptVariable && !isOrImplements(receiverType, MAP_TYPE) &&
controller.getClassNode().getOuterClass() == null)
addPropertyAccessError(receiver, propertyName, receiverType);
- }
MethodCallExpression call = callX(receiver, "getProperty",
args(constX(propertyName)));
call.setImplicitThis(implicitThis);
@@ -476,6 +477,14 @@ public class StaticTypesCallSiteWriter extends
CallSiteWriter {
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
index 2699733fec..efb3a90763 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -773,7 +773,7 @@ class FieldsAndPropertiesSTCTest extends
StaticTypeCheckingTestCase {
'''
}
- // GROOVY-8074
+ // GROOVY-8074, GROOVY-11390
void testMapPropertyAccess6() {
assertScript '''
class C extends HashMap {
@@ -796,6 +796,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
@@ -1895,6 +1915,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
index 64c9b4fc5d..359bcd19d8 100644
---
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
/**
@@ -860,12 +859,6 @@ final class FieldsAndPropertiesStaticCompileTest extends
FieldsAndPropertiesSTCT
'''
}
- // GROOVY-6277
- @Override @NotYetImplemented
- void testPublicFieldVersusPrivateGetter() {
- super.testPublicFieldVersusPrivateGetter()
- }
-
// GROOVY-11369
@Override
void testMapPropertyAccess5a() {
