You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/05/30 14:45:24 UTC
[groovy] branch master updated: GROOVY-9562: same top-level type,
not same module for private field read
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 508479e GROOVY-9562: same top-level type, not same module for private field read
508479e is described below
commit 508479ea011481ba580ee05e334218e00883ca22
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed May 27 20:29:37 2020 -0500
GROOVY-9562: same top-level type, not same module for private field read
---
.../transform/stc/StaticTypeCheckingVisitor.java | 45 +++++--------
src/test/groovy/bugs/Groovy9292.groovy | 77 ++++++++++++++--------
.../stc/FieldsAndPropertiesSTCTest.groovy | 27 ++++++++
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 13 ++--
4 files changed, 98 insertions(+), 64 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 09584b5..67e1d99 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -98,7 +98,6 @@ import org.codehaus.groovy.ast.tools.GenericsUtils;
import org.codehaus.groovy.ast.tools.WideningCategories;
import org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
import org.codehaus.groovy.classgen.ReturnAdder;
-import org.codehaus.groovy.classgen.Verifier;
import org.codehaus.groovy.classgen.asm.InvocationWriter;
import org.codehaus.groovy.control.CompilationUnit;
import org.codehaus.groovy.control.ErrorCollector;
@@ -485,47 +484,35 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private static ClassNode getOutermost(ClassNode cn) {
+ while (cn.getOuterClass() != null) {
+ cn = cn.getOuterClass();
+ }
+ return cn;
+ }
+
private static void addPrivateFieldOrMethodAccess(final Expression source, final ClassNode cn, final StaticTypesMarker key, final ASTNode accessedMember) {
cn.getNodeMetaData(key, x -> new LinkedHashSet<>()).add(accessedMember);
source.putNodeMetaData(key, accessedMember);
}
/**
- * Given a field node, checks if we are accessing or setting a private field from an inner class.
+ * Checks for private field access from inner or outer class.
*/
private void checkOrMarkPrivateAccess(final Expression source, final FieldNode fn, final boolean lhsOfAssignment) {
- if (fn == null) return;
- ClassNode declaringClass = fn.getDeclaringClass(), enclosingClassNode = typeCheckingContext.getEnclosingClassNode();
- if (fn.isPrivate() && (declaringClass != enclosingClassNode || typeCheckingContext.getEnclosingClosure() != null)
- && declaringClass.getModule() == enclosingClassNode.getModule()) {
- if (!lhsOfAssignment && enclosingClassNode.isDerivedFrom(declaringClass)) {
- // check for a public/protected getter since JavaBean getters haven't been recognised as properties
- // at this point and we don't want private field access for that case which will be handled later
- String suffix = Verifier.capitalize(fn.getName());
- MethodNode getter = findValidGetter(enclosingClassNode, "get" + suffix);
- if (getter == null && boolean_TYPE.equals(fn.getOriginType())) {
- getter = findValidGetter(enclosingClassNode, "is" + suffix);
- }
- if (getter != null) {
- source.putNodeMetaData(INFERRED_TYPE, getter.getReturnType());
- return;
- }
- }
- StaticTypesMarker marker = lhsOfAssignment ? PV_FIELDS_MUTATION : PV_FIELDS_ACCESS;
- addPrivateFieldOrMethodAccess(source, declaringClass, marker, fn);
- }
- }
+ if (fn == null || !fn.isPrivate()) return;
+ ClassNode declaringClass = fn.getDeclaringClass();
+ ClassNode enclosingClass = typeCheckingContext.getEnclosingClassNode();
+ if (declaringClass == enclosingClass && typeCheckingContext.getEnclosingClosure() == null) return;
- private MethodNode findValidGetter(final ClassNode classNode, final String name) {
- MethodNode getterMethod = classNode.getGetterMethod(name);
- if (getterMethod != null && (getterMethod.isPublic() || getterMethod.isProtected())) {
- return getterMethod;
+ if (declaringClass == enclosingClass || getOutermost(declaringClass) == getOutermost(enclosingClass)) {
+ StaticTypesMarker accessKind = lhsOfAssignment ? PV_FIELDS_MUTATION : PV_FIELDS_ACCESS;
+ addPrivateFieldOrMethodAccess(source, declaringClass, accessKind, fn);
}
- return null;
}
/**
- * Given a method node, checks if we are calling a private method from an inner class.
+ * Checks for private method call from inner or outer class.
*/
private void checkOrMarkPrivateAccess(final Expression source, final MethodNode mn) {
if (mn == null) {
diff --git a/src/test/groovy/bugs/Groovy9292.groovy b/src/test/groovy/bugs/Groovy9292.groovy
index b2a0a93..3ffe823 100644
--- a/src/test/groovy/bugs/Groovy9292.groovy
+++ b/src/test/groovy/bugs/Groovy9292.groovy
@@ -18,7 +18,6 @@
*/
package groovy.bugs
-import groovy.test.NotYetImplemented
import org.junit.Test
import static groovy.test.GroovyAssert.shouldFail
@@ -27,27 +26,56 @@ final class Groovy9292 {
private final GroovyShell shell = new GroovyShell()
- @Test @NotYetImplemented
+ @Test
+ void 'test accessing a private super class field inside a closure - same module'() {
+ shouldFail(MissingPropertyException) {
+ shell.evaluate '''
+ package a
+
+ class A {
+ private String superField
+ }
+
+ class B extends A {
+ @groovy.transform.CompileStatic
+ def test() {
+ 'something'.with {
+ return superField
+ }
+ }
+ }
+
+ new B().test()
+ '''
+ }
+ }
+
+ @Test
void 'test accessing a private super class field inside a closure - same package'() {
shouldFail(MissingPropertyException) {
shell.evaluate '''
package a
class A {
- private String superField = 'works'
+ private String superField
}
+ assert true
+ '''
+
+ shell.evaluate '''
+ package a
+
class B extends A {
@groovy.transform.CompileStatic
def test() {
'something'.with {
- return superField // ClassCastException: a.B$_test_closure1 cannot be cast to a.A
+ return superField
}
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -59,7 +87,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
@@ -89,7 +117,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
class B extends A {
@@ -101,8 +129,7 @@ final class Groovy9292 {
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -114,7 +141,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
@@ -144,7 +171,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
class B extends A {
@@ -156,8 +183,7 @@ final class Groovy9292 {
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -169,7 +195,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
@@ -199,7 +225,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
class B extends A {
@@ -211,8 +237,7 @@ final class Groovy9292 {
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -224,7 +249,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
@@ -254,7 +279,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
class B extends A {
@@ -266,8 +291,7 @@ final class Groovy9292 {
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -279,7 +303,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
@@ -309,7 +333,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
class B extends A {
@@ -321,8 +345,7 @@ final class Groovy9292 {
}
}
- def obj = new B()
- assert obj.test() == "works"
+ new B().test()
'''
}
}
@@ -334,7 +357,7 @@ final class Groovy9292 {
package a
class A {
- private String superField = 'works'
+ private String superField
}
assert true
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index a873576..545cde5 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -423,6 +423,33 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-9562
+ void testSuperPropertyAccessInAIC() {
+ assertScript '''
+ abstract class One {
+ int prop = 1
+ }
+
+ abstract class Two {
+ int prop = 2
+
+ abstract baz()
+ }
+
+ class Foo extends One {
+ Two bar() {
+ new Two() {
+ def baz() {
+ prop
+ }
+ }
+ }
+ }
+
+ assert new Foo().bar().baz() == 2
+ '''
+ }
+
// GROOVY-5737
void testAccessGeneratedFieldFromClosure() {
assertScript '''
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 32f2eb0..a488532 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -283,7 +283,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
// no GETFIELD in getXX
assert (astTrees['B'][1] =~ 'GETFIELD A.x').collect().size() == 0
// getX in getXX
- assert (astTrees['B'][1] =~ 'INVOKEVIRTUAL A.getX').collect().size() == 1
+ assert (astTrees['B'][1] =~ 'INVOKEVIRTUAL B.getX').collect().size() == 1
}
void testUseDirectReadFieldAccess() {
@@ -470,12 +470,11 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
// GROOVY-5649
void testShouldNotThrowStackOverflowUsingThis() {
- new GroovyShell().evaluate '''class HaveOption {
+ new GroovyShell().evaluate '''
+ class HaveOption {
private String helpOption;
-
- @groovy.transform.CompileStatic
public void setHelpOption(String helpOption) {
this.helpOption = helpOption
}
@@ -488,12 +487,11 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
}
void testShouldNotThrowStackOverflow() {
- new GroovyShell().evaluate '''class HaveOption {
+ new GroovyShell().evaluate '''
+ class HaveOption {
private String helpOption;
-
- @groovy.transform.CompileStatic
public void setHelpOption(String ho) {
helpOption = ho
}
@@ -791,7 +789,6 @@ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
// GROOVY-8753
void testPrivateFieldWithPublicGetter() {
assertScript '''
- @groovy.transform.CompileStatic
class A {
private List<String> fooNames = []
public A(Collection<String> names) {