You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/05/28 01:30:08 UTC
[groovy] branch GROOVY-9562 created (now d62df6a)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9562
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at d62df6a GROOVY-9562: same top-level type, not same module for private field read
This branch includes the following new commits:
new d62df6a GROOVY-9562: same top-level type, not same module for private field read
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-9562: same top-level type,
not same module for private field read
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9562
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit d62df6ac7ba708a579bc256997032cc8973febde
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) {