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 2024/06/17 15:37:15 UTC
(groovy) branch master updated: GROOVY-11403: STC: map property precedence: entry before method (`this`)
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 080277e23d GROOVY-11403: STC: map property precedence: entry before method (`this`)
080277e23d is described below
commit 080277e23d7374bd93d62d3a4706aab66d36a01d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jun 17 10:19:26 2024 -0500
GROOVY-11403: STC: map property precedence: entry before method (`this`)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 11 +-
.../stc/FieldsAndPropertiesSTCTest.groovy | 178 +++++++++++++++++----
2 files changed, 150 insertions(+), 39 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 dac66058ee..8b3a9a65f9 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1608,10 +1608,8 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
// in case of a lookup on java.lang.Class, look for instance methods on Class
// in case of static property access, Type (static) and Class<Type> are tried
boolean staticOnly = !receiver.isObject();
- // GROOVY-11367, GROOVY-11384: map entry before a non-public getter/setter
- boolean publicOnly = !staticOnly && isOrImplements(receiverType, MAP_TYPE)
- && !isSuperExpression(objectExpression) && !(isThisExpression(objectExpression)
- && (!pexp.isImplicitThis() || typeCheckingContext.getEnclosingClosure() == null));
+ // GROOVY-11367, GROOVY-11384, GROOVY-11387: entry before a non-public member
+ boolean publicOnly = !staticOnly && isOrImplements(receiverType, MAP_TYPE);
List<MethodNode> setters = new ArrayList<>(4);
for (MethodNode method : findMethodsWithGenerated(wrapTypeIfNecessary(receiverType), setterName)) {
@@ -1658,9 +1656,8 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
continue;
}
- if (field != null && !staticOnly && !field.isPublic() // GROOVY-11367, GROOVY-11387: map entry before a non-public instance field
- && !(isThisExpression(objectExpression) && (!pexp.isImplicitThis() || typeCheckingContext.getEnclosingClosure() == null))
- && isOrImplements(receiverType, MAP_TYPE)) {
+ if (field != null && publicOnly && !field.isPublic() && !(isThisExpression(objectExpression)
+ && (!pexp.isImplicitThis() || typeCheckingContext.getEnclosingClosure() == null))) {
field = null;
}
// skip property/accessor checks for "field", "this.field", "this.with { field }", etc. within the declaring class of the field
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index fac75589b1..f6a53d589a 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -19,6 +19,7 @@
package groovy.transform.stc
import groovy.transform.PackageScope
+import org.codehaus.groovy.control.customizers.ImportCustomizer
import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
/**
@@ -26,6 +27,13 @@ import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
*/
class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
+ @Override
+ void configure() {
+ config.addCompilationCustomizers(
+ new ImportCustomizer().addImports('groovy.transform.PackageScope')
+ )
+ }
+
void testAssignFieldValue() {
assertScript '''
class C { int x }
@@ -803,9 +811,9 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-11367
- void testMapPropertyAccess8a() {
+ void testMapPropertyAccess9() {
for (mode in ['','static']) {
- assertScript """import groovy.transform.*
+ assertScript """
class M {
@Delegate final Map m = [:].withDefault{ 'entry' }
def $mode v = 'field'
@@ -815,21 +823,21 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
private $mode z = 'field'
}
def map = new M()
- assert map.m === map.@m
- assert map.v == 'field'
- assert map.w == 'field'
- assert map.x == 'entry'
- assert map.y == 'entry'
- assert map.z == 'entry'
+ assert map.m === map.@m
+ assert map.v == 'field'
+ assert map.w == 'field'
+ assert map.x == 'entry'
+ assert map.y == 'entry'
+ assert map.z == 'entry'
map.with {
- assert v == 'field'
- assert w == 'field'
- assert x == 'entry'
- assert y == 'entry'
- assert z == 'entry'
+ assert v == 'field'
+ assert w == 'field'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry'
}
"""
- assertScript """import groovy.transform.*
+ assertScript """
class M {
@Delegate final Map m = [:].withDefault{ 'entry' }
def $mode getV() { 'getter' }
@@ -839,25 +847,131 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
private $mode getZ() { 'getter' }
}
def map = new M()
- assert map.m === map.@m
- assert map.v == 'getter'
- assert map.w == 'getter'
- assert map.x == 'entry'
- assert map.y == 'entry'
- assert map.z == 'entry'
+ assert map.v == 'getter'
+ assert map.w == 'getter'
+ assert map.x == 'entry'
+ assert map.y == 'entry'
+ assert map.z == 'entry'
map.with {
- assert v == 'getter'
- assert w == 'getter'
- assert x == 'entry'
- assert y == 'entry'
- assert z == 'entry'
+ assert v == 'getter'
+ assert w == 'getter'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry'
+ }
+ """
+ }
+ }
+
+ // GROOVY-11403
+ void testMapPropertyAccess10() {
+ for (mode in ['','static']) {
+ assertScript """
+ class M {
+ @Delegate final Map m = [:].withDefault{ 'entry' }
+ def $mode v = 'field'
+ public $mode w = 'field'
+ protected $mode x = 'field'
+ @PackageScope $mode y = 'field'
+ private $mode z = 'field'
+ void test() {
+ assert m === this.@m
+ assert v == 'field'
+ assert w == 'field'
+ assert x == 'field'
+ assert y == 'field'
+ assert z == 'field';
+ {->
+ assert v == 'field'
+ assert w == 'field'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry'
+ }()
+ }
+ }
+ new M().test()
+ """
+ assertScript """
+ class M {
+ @Delegate final Map m = [:].withDefault{ 'entry' }
+ def $mode getV() { 'getter' }
+ public $mode getW() { 'getter' }
+ protected $mode getX() { 'getter' }
+ @PackageScope $mode getY() { 'getter' }
+ private $mode getZ() { 'getter' }
+ void test() {
+ assert v == 'getter'
+ assert w == 'getter'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry';
+ {->
+ assert v == 'getter'
+ assert w == 'getter'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry'
+ }()
+ }
+ }
+ new M().test()
+ """
+ }
+ }
+
+ // GROOVY-11403
+ void testMapPropertyAccess11() {
+ for (mode in ['','static']) {
+ assertScript """
+ class M {
+ @Delegate final Map m = [:].withDefault{ 'entry' }
+ def $mode v = 'field'
+ public $mode w = 'field'
+ protected $mode x = 'field'
+ @PackageScope $mode y = 'field'
+ private $mode z = 'field'
+ void test() {
+ assert this.m === this.@m
+ assert this.v == 'field'
+ assert this.w == 'field'
+ assert this.x == 'field'
+ assert this.y == 'field'
+ assert this.z == 'field'
+ }
}
+ new M().test()
+ """
+ assertScript """
+ class M {
+ @Delegate final Map m = [:].withDefault{ 'entry' }
+ def $mode getV() { 'getter' }
+ public $mode getW() { 'getter' }
+ protected $mode getX() { 'getter' }
+ @PackageScope $mode getY() { 'getter' }
+ private $mode getZ() { 'getter' }
+ void test() {
+ assert this.v == 'getter'
+ assert this.w == 'getter'
+ assert this.x == 'entry'
+ assert this.y == 'entry'
+ assert this.z == 'entry';
+ {->
+ assert this.v == 'getter'
+ assert this.w == 'getter'
+ assert this.x == 'entry'
+ assert this.y == 'entry'
+ assert this.z == 'entry'
+ }()
+ }
+ }
+ new M().test()
"""
}
}
// GROOVY-11368
- void testMapPropertyAccess9() {
+ void testMapPropertyAccess12() {
String type = '''
class C implements Map<String,String> {
@Delegate Map<String,String> impl = [:]
@@ -882,7 +996,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-11367
- void testMapPropertyAccess10() {
+ void testMapPropertyAccess13() {
String map = "def map = new ${MapType.name}()"
assertScript map + '''
map.foo = 11 // public setter
@@ -903,7 +1017,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-11387
- void testMapPropertyAccess11() {
+ void testMapPropertyAccess14() {
assertScript '''
def map = new HashMap<String,String>()
@ASTTest(phase=INSTRUCTION_SELECTION, value={
@@ -925,7 +1039,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-11387
- void testMapPropertyAccess12() {
+ void testMapPropertyAccess15() {
assertScript '''
class HttpHeaders extends HashMap<String,List<String>> {
protected static final String ACCEPT = 'Accept'
@@ -939,7 +1053,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-11401
- void testMapPropertyAccess13() {
+ void testMapPropertyAccess16() {
assertScript '''
class C {
private Object obj = 'field'
@@ -1265,7 +1379,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
// GROOVY-10981
void testSuperPropertyAccess4() {
- for (mode in ['', 'public', 'private', 'protected', '@groovy.transform.PackageScope']) {
+ for (mode in ['', 'public', 'private', 'protected', '@PackageScope']) {
assertScript """
abstract class A {
$mode Object p = 'field'
@@ -1429,7 +1543,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
// GROOVY-11198
void testPrivateFieldAccessInEnumInit() {
- for (mode in ['public', 'private', 'protected', '@groovy.transform.PackageScope']) {
+ for (mode in ['public', 'private', 'protected', '@PackageScope']) {
assertScript """
class C {
$mode static int ONE = 1