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 <eric.mil...@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