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 2023/10/21 16:03:44 UTC
[groovy] branch master updated: GROOVY-7994, GROOVY-11198: STC: outer field or property inference
This is an automated email from the ASF dual-hosted git repository.
emilles 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 1f8072c628 GROOVY-7994, GROOVY-11198: STC: outer field or property inference
1f8072c628 is described below
commit 1f8072c6288ed7b8b84d161fbe7e49dc1634bcfa
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Oct 21 10:42:40 2023 -0500
GROOVY-7994, GROOVY-11198: STC: outer field or property inference
---
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 22 +--
.../transform/stc/StaticTypeCheckingVisitor.java | 16 ++
.../stc/AnonymousInnerClassSTCTest.groovy | 136 ++++++++++++++++-
.../stc/FieldsAndPropertiesSTCTest.groovy | 168 +++++++++++----------
.../sc/AnonymousInnerClassStaticCompileTest.groovy | 7 +-
.../ArraysAndCollectionsStaticCompileTest.groovy | 4 +-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 3 +
7 files changed, 262 insertions(+), 94 deletions(-)
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 e61d641782..dd78f2db5a 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
@@ -159,6 +159,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
makeDynamicGetProperty(receiver, propertyName, safe);
return;
}
+
boolean[] isClassReceiver = new boolean[1];
ClassNode receiverType = getPropertyOwnerType(receiver, isClassReceiver);
if (receiverType.isArray() && "length".equals(propertyName)) {
@@ -188,6 +189,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
writeMapDotProperty(receiver, propertyName, safe);
return;
}
+
if (makeGetField(receiver, receiverType, propertyName, safe, implicitThis)) return;
if (isThisExpression(receiver) && receiverType.getOuterClass() != null) { // GROOVY-11198: outer field
if (makeGetField(receiver, receiverType.getOuterClass(), propertyName, safe, implicitThis)) return;
@@ -436,7 +438,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
getterName = "get" + capitalize(propertyName);
getterNode = receiverType.getGetterMethod(getterName);
}
- if (getterNode != null && receiver instanceof ClassExpression && !isClassType(receiverType) && !getterNode.isStatic()) {
+ if (getterNode != null && !getterNode.isStatic() && receiver instanceof ClassExpression && !isClassType(receiverType)) {
return false;
}
@@ -467,22 +469,24 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
return true;
}
- if (receiverType instanceof InnerClassNode && !receiverType.isStaticClass()) {
- if (makeGetPropertyWithGetter(receiver, receiverType.getOuterClass(), propertyName, safe, implicitThis)) {
- return true;
- }
- }
-
// GROOVY-7149: check direct interfaces
for (ClassNode node : receiverType.getInterfaces()) {
if (makeGetPropertyWithGetter(receiver, node, propertyName, safe, implicitThis)) {
return true;
}
}
- // go upper level
+ // check super class
ClassNode superClass = receiverType.getSuperClass();
if (superClass != null) {
- return makeGetPropertyWithGetter(receiver, superClass, propertyName, safe, implicitThis);
+ if (makeGetPropertyWithGetter(receiver, superClass, propertyName, safe, implicitThis)) {
+ return true;
+ }
+ }
+ // check outer class
+ if (implicitThis && receiverType instanceof InnerClassNode && !receiverType.isStaticClass()) {
+ if (makeGetPropertyWithGetter(receiver, receiverType.getOuterClass(), propertyName, safe, implicitThis)) {
+ return true;
+ }
}
return false;
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 58ee1260ec..d41bb632e6 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -215,6 +215,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
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.ctorX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.defaultValueX;
@@ -1737,6 +1738,21 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
return true;
}
+ if (pexp.isImplicitThis() && isThisExpression(objectExpression)) {
+ Iterator<ClassNode> iter = enclosingTypes.iterator(); // first enclosing is "this" type
+ boolean staticOnly = Modifier.isStatic(iter.next().getModifiers()) || staticOnlyAccess;
+ while (iter.hasNext()) {
+ ClassNode outer = iter.next();
+ // GROOVY-7994, GROOVY-11198: try "this.propertyName" as "Outer.propertyName" or "Outer.this.propertyName"
+ PropertyExpression pe = propX(staticOnly ? classX(outer) : propX(classX(outer), "this"), pexp.getProperty());
+ if (existsProperty(pe, readMode, visitor)) {
+ pexp.copyNodeMetaData(pe);
+ return true;
+ }
+ staticOnly = staticOnly || Modifier.isStatic(outer.getModifiers());
+ }
+ }
+
return foundGetterOrSetter;
}
diff --git a/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy b/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
index 822a712995..8ae6966ba1 100644
--- a/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
+++ b/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
@@ -19,8 +19,7 @@
package groovy.transform.stc
/**
- * Test cases specifically aimed at testing the behaviour of the type checker
- * with regards to anonymous inner classes.
+ * Unit tests for static type checking : anonymous inner classes.
*/
class AnonymousInnerClassSTCTest extends StaticTypeCheckingTestCase {
@@ -224,4 +223,137 @@ class AnonymousInnerClassSTCTest extends StaticTypeCheckingTestCase {
assert A.create().answer() == 42
'''
}
+
+ void testPrivateFieldAccess() {
+ assertScript '''
+ class C {
+ private int x
+ void test() {
+ def aic = new Runnable() {
+ void run() { x = 666 }
+ }
+ aic.run()
+ assert x == 666
+ }
+ }
+
+ new C().test()
+ '''
+ }
+
+ // GROOVY-7994
+ void testOuterPropertyAccess() {
+ String other = '''
+ class Other {
+ public final String text
+ String toString() { text }
+ Other( ) { text = "" }
+ Other(Object object) { text = "Object:$object" }
+ Other(String string) { text = "String:$string" }
+ static String foo(Object object) { "Object:$object" }
+ static String foo(String string) { "String:$string" }
+ String bar(Object object) { "Object:$object" }
+ String bar(String string) { "String:$string" }
+ }
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ String test() {
+ [ new Other(p), new Other(getP()) ].join('|')
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ class Inner {
+ String test() { // unqualified "p" should resolve to outer property
+ [ new Other(p), new Other(Outer.this.p), new Other(getP()) ].join('|')
+ }
+ }
+ }
+
+ String result = new Outer.Inner(new Outer()).test()
+ assert result == 'String:x|String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ String test() {
+ new Object() {
+ String toString() {
+ [ new Other(p), new Other(Outer.this.p), new Other(getP()) ].join('|')
+ }
+ }
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String p = 'x'
+ String test() {
+ new Object() {
+ String toString() {
+ [ new Other(p), new Other(getP()) ].join('|')
+ }
+ }
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ String test() {
+ new Object() {
+ String toString() {
+ [ Other.foo(p), Other.foo(Outer.this.p), Other.foo(getP()) ].join('|')
+ }
+ }
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ String test() {
+ new Object() {
+ String toString() {
+ [ new Other().bar(p), new Other().bar(getP()) ].join('|')
+ }
+ }
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x'
+ '''
+ assertScript other + '''
+ class Outer {
+ String getP() { 'x' }
+ String test() {
+ new Object() {
+ String toString() {
+ new Other().with { [ bar(p), bar(getP()) ].join('|') }
+ }
+ }
+ }
+ }
+
+ String result = new Outer().test()
+ assert result == 'String:x|String:x'
+ '''
+ }
}
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 4d8765b508..d663cb76c4 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -783,6 +783,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
def p = 1
}
+
def i = new Outer.Inner(new Outer())
def x = i.p
''',
@@ -797,6 +798,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
def p = 1
}
+
def i = new Outer.Inner(new Outer())
def x = i.getP()
''',
@@ -850,30 +852,21 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
- // GROOVY-10981, GROOVY-10985
void testOuterPropertyAccess9() {
- for (propertySource in [
- 'def get(String name){if(name=="VALUE")return 2}',
- 'def getProperty(String name){if(name=="VALUE")return 2}',
- '@Delegate private Map<String,Object> map = [VALUE:2]']) {
- assertScript """
- class Outer {
- static private int VALUE = 1
- static class Inner {
- $propertySource
- def test(int i) {
- if (i > VALUE) {
- // ...
- }
- return VALUE
- }
+ assertScript '''
+ class Outer {
+ static final int ONE = 1
+ enum Inner {
+ CONST(1 + ONE)
+ final int value
+ Inner(int value) {
+ this.value = value
}
}
+ }
- Object value = new Outer.Inner().test(0)
- assert value == 2
- """
- }
+ assert Outer.Inner.CONST.value == 2
+ '''
}
void testOuterPropertyAccess10() {
@@ -894,84 +887,111 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
// GROOVY-11199
void testOuterPropertyAccess11() {
assertScript '''
- class C {
- class D {
+ class Outer {
+ class Inner {
def m() { p = 'method' }
}
String p = 'field'
String getP() { 'property' }
}
- String which = new C.D(new C()).m()
+ String which = new Outer.Inner(new Outer()).m()
assert which == 'method'
'''
}
+ // GROOVY-10981, GROOVY-10985
+ void testOuterPropertyAccess12() {
+ for (propertySource in [
+ 'def get(String name){if(name=="VALUE")return 2}',
+ 'def getProperty(String name){if(name=="VALUE")return 2}',
+ '@Delegate private Map<String,Object> map = [VALUE:2]']) {
+ assertScript """
+ class Outer {
+ static private int VALUE = 1
+ static class Inner {
+ $propertySource
+ def test(int i) {
+ if (i > VALUE) {
+ // ...
+ }
+ return VALUE
+ }
+ }
+ }
+
+ Object value = new Outer.Inner().test(0)
+ assert value == 2
+ """
+ }
+ }
+
// GROOVY-11029
void testSuperPropertyAccess1() {
assertScript '''
- class Foo {
- Object myThing
+ class One {
+ Object thing
}
- class Bar extends Foo {
+ class Two extends One {
@Override
- Object getMyThing() {
- super.myThing
+ Object getThing() {
+ super.thing
}
@Override
- void setMyThing(Object object) {
- super.myThing = object
+ void setThing(object) {
+ super.thing = object
}
}
- def bar = new Bar()
- def value = 'thing'
- bar.myThing = value
- assert bar.myThing === value
+ def two = new Two()
+ two.thing = 'value'
+ assert two.thing == 'value'
'''
}
- // GROOVY-9562
void testSuperPropertyAccess2() {
assertScript '''
- abstract class One {
- int prop = 1
+ abstract class One implements java.util.function.IntSupplier {
+ final int prop = 1
}
-
- abstract class Two {
- int prop = 2
-
- abstract baz()
- }
-
- class Foo extends One {
- Two bar() {
- new Two() {
- def baz() {
+ class Two {
+ final int prop = 2
+ One m() {
+ new One() {
+ int getAsInt() {
prop
}
}
}
}
- assert new Foo().bar().baz() == 2
+ Number which = new Two().m().getAsInt()
+ assert which == 1 // super before outer
'''
}
- void testPrivateFieldAccessInAIC() {
+ // GROOVY-9562
+ void testSuperPropertyAccess3() {
assertScript '''
- class C {
- private int x
- void test() {
- def aic = new Runnable() {
- void run() { x = 666 }
+ abstract class One {
+ final int prop = 1
+ }
+ abstract class Two {
+ final int prop = 2
+ abstract int baz()
+ }
+ class Foo extends One {
+ Two bar() {
+ new Two() {
+ int baz() {
+ prop
+ }
}
- aic.run()
- assert x == 666
}
}
- new C().test()
+ Number which = new Foo().bar().baz()
+ assert which == 2 // super before outer
'''
}
@@ -1008,7 +1028,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
// GROOVY-11198
void testPrivateFieldAccessInEnumInit() {
- for (mode in ['', 'public', 'private', 'protected', '@groovy.transform.PackageScope']) {
+ for (mode in ['public', 'private', 'protected', '@groovy.transform.PackageScope']) {
assertScript """
class C {
$mode static int ONE = 1
@@ -1308,7 +1328,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
assert md.parameters[0].originType == STRING_TYPE
}
})
- void testBody() {
+ void test() {
def a = new A()
test1:
a.x = 1
@@ -1317,7 +1337,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
a.x = "3"
assert a.x == "3"
}
- testBody()
+ test()
'''
}
@@ -1331,12 +1351,9 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
void setX(boolean b) { which = 'boolean' }
}
- void test() {
- def c = new C()
- c.x = 'value'
- assert c.which == 'String'
- }
- test()
+ def c = new C()
+ c.x = 'value'
+ assert c.which == 'String'
'''
}
@@ -1350,11 +1367,11 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
void setX(boolean b) { which = 'boolean' }
}
- void test(A a) {
- a.x = 'value'
- assert a.which == 'String'
+ A a = new A() {
+ void setX(String s) { which = 'String' }
}
- test(new A() { void setX(String s) { which = 'String' } })
+ a.x = 'value'
+ assert a.which == 'String'
'''
}
@@ -1368,12 +1385,9 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
void setX(boolean b) { which = 'boolean' }
}
- void test() {
- def c = new C()
- c.x = 'value'
- assert c.which == 'String'
- }
- test()
+ def c = new C()
+ c.x = 'value'
+ assert c.which == 'String'
'''
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/AnonymousInnerClassStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/AnonymousInnerClassStaticCompileTest.groovy
index c8bcb5875f..896e3e169a 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/AnonymousInnerClassStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/AnonymousInnerClassStaticCompileTest.groovy
@@ -21,8 +21,7 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.AnonymousInnerClassSTCTest
/**
- * Statically compiled the {@link AnonymousInnerClassSTCTest tests} and runs them
- * to ensure they are compatible with static compilation.
+ * Unit tests for static compilation : anonymous inner classes.
*/
-class AnonymousInnerClassStaticCompileTest extends AnonymousInnerClassSTCTest implements StaticCompilationTestSupport {}
-
+final class AnonymousInnerClassStaticCompileTest extends AnonymousInnerClassSTCTest implements StaticCompilationTestSupport {
+}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
index db6f9a5f55..2dba5ad2fa 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
@@ -21,9 +21,9 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.ArraysAndCollectionsSTCTest
/**
- * Unit tests for static type checking : miscellaneous tests.
+ * Unit tests for static compilation : arrays and collections.
*/
-class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest implements StaticCompilationTestSupport {
+final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest implements StaticCompilationTestSupport {
void testListStarWithMethodReturningVoid() {
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 23d5a8b20e..253cb88da4 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -20,6 +20,9 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.FieldsAndPropertiesSTCTest
+/**
+ * Unit tests for static compilation : fields and properties.
+ */
final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest implements StaticCompilationTestSupport {
// GROOVY-5561