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/12 21:28:18 UTC
(groovy) branch master updated: GROOVY-11367: map property precedence: `metaClass` member before get/put
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 833c307f88 GROOVY-11367: map property precedence: `metaClass` member before get/put
833c307f88 is described below
commit 833c307f8830486c8ea6680c99eb7b64ae73daaf
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jun 12 16:11:53 2024 -0500
GROOVY-11367: map property precedence: `metaClass` member before get/put
`class` and `empty` retain get and put shadowing of public access method
---
src/main/java/groovy/lang/MetaClassImpl.java | 4 +-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 2 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 3 +-
src/test/groovy/MapTest.groovy | 56 +++++-------
.../stc/FieldsAndPropertiesSTCTest.groovy | 36 +++++++-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 99 ----------------------
6 files changed, 58 insertions(+), 142 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 5b614cd5e4..19d0bc1cd3 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2104,10 +2104,10 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
/**
- * Object#getClass, Map#isEmpty, GroovyObject#getMetaClass
+ * Object#getClass and Map#isEmpty
*/
private boolean isSpecialProperty(final String name) {
- return "class".equals(name) || (isMap && ("empty".equals(name) || "metaClass".equals(name)));
+ return "class".equals(name) || (isMap && ("empty".equals(name)));
}
private Tuple2<MetaMethod, MetaProperty> createMetaMethodAndMetaProperty(final Class sender, final String name, final boolean useSuper, final boolean isStatic) {
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 c2eddd4f70..1eb288f8bd 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
@@ -472,7 +472,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
if (!getterNode.isStatic() && receiver instanceof ClassExpression && !isClassType(receiverType)) {
return false;
}
- if (!getterNode.isPublic() && isOrImplements(receiverType, MAP_TYPE)) { // entry before non-public getter
+ if ((!getterNode.isPublic() || propertyName.matches("class|empty")) && isOrImplements(receiverType, MAP_TYPE)) {
return false; // GROOVY-11367
}
if (!AsmClassGenerator.isMemberDirectlyAccessible(getterNode.getModifiers(), getterNode.getDeclaringClass(), controller.getClassNode())) {
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 58e548d2af..6cc41c3867 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -268,7 +268,6 @@ import static org.codehaus.groovy.syntax.Types.MOD_EQUAL;
import static org.codehaus.groovy.syntax.Types.PLUS_PLUS;
import static org.codehaus.groovy.syntax.Types.REMAINDER;
import static org.codehaus.groovy.syntax.Types.REMAINDER_EQUAL;
-import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.COMPILESTATIC_CLASSNODE;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.ArrayList_TYPE;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.Collection_TYPE;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.LinkedHashMap_TYPE;
@@ -1679,7 +1678,7 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
&& !staticOnly // GROOVY-10387: Map.foo
&& !isSuperExpression(objectExpression)
&& !(isThisExpression(objectExpression) && (!pexp.isImplicitThis() || typeCheckingContext.getEnclosingClosure() == null)) // GROOVY-11384
- && (!getter.isPublic() || (propertyName.matches("empty|class|metaClass") && !List.of(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE)))))) {
+ && (!getter.isPublic() || propertyName.equals("class") || propertyName.equals("empty"))))) { // GROOVY-11367: public (not class or empty)
getter = null;
}
diff --git a/src/test/groovy/MapTest.groovy b/src/test/groovy/MapTest.groovy
index c65b699c8a..ff7ec1788a 100644
--- a/src/test/groovy/MapTest.groovy
+++ b/src/test/groovy/MapTest.groovy
@@ -89,28 +89,24 @@ final class MapTest extends GroovyTestCase {
* Map "empty" property isn't Map#isEmpty.
*/
void testMapEmpty() {
- def m = [empty: 'no']
+ def m = [:].withDefault{ 'entry' }
- assert m.get('empty') == 'no'
- assert m['empty'] == 'no'
- assert m.empty == 'no'
+ assert m.get('empty') == 'entry'
+ assert m['empty'] == 'entry'
+ assert m.empty == 'entry'
+ assert m.Empty == false
}
/**
- * Map "class" and "metaClass" properties aren't Object#getClass or GroovyObject#getMetaClass.
+ * Map "class" property isn't Object#getClass.
*/
void testMapClass() {
- def m = [class: 'xx', metaClass: 'yy']
+ def m = [:].withDefault{ 'entry' }
- assert m.get('class') == 'xx'
- assert m['class'] == 'xx'
- assert m.class == 'xx'
- assert m.Class == null
-
- assert m.get('metaClass') == 'yy'
- assert m['metaClass'] == 'yy'
- assert m.metaClass == 'yy'
- assert m.MetaClass == null
+ assert m.get('class') == 'entry'
+ assert m['class'] == 'entry'
+ assert m.class == 'entry'
+ assert m.Class == 'entry'
}
// GROOVY-5001, GROOVY-11367
@@ -135,7 +131,7 @@ final class MapTest extends GroovyTestCase {
assert map.z == 'entry'
assert map.empty == 'entry'
assert map.class == 'entry'
- assert map.metaClass == 'entry'
+ assert map.metaClass instanceof MetaClass
map.with {
assert v == 'field'
@@ -159,25 +155,17 @@ final class MapTest extends GroovyTestCase {
}
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.empty == 'entry'
- assert map.class == 'entry'
- assert map.metaClass == '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 empty == 'entry'
- assert it.class == 'entry' // "class" cannot be a variable expression
- assert metaClass instanceof org.codehaus.groovy.runtime.metaclass.ClosureMetaClass
+ assert v == 'getter'
+ assert w == 'getter'
+ assert x == 'entry'
+ assert y == 'entry'
+ assert z == 'entry'
}
"""
}
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 08e3889e48..38cf1c5c89 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -641,14 +641,14 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
- // GROOVY-11369, GROOVY-11372, GROOVY-11384
+ // GROOVY-11367, GROOVY-11369, GROOVY-11372, GROOVY-11384
void testMapPropertyAccess5() {
assertScript '''
def map = [:]
assert map.entry == null
assert map.empty == null
assert map.class == null
- assert map.metaClass == null // TODO
+ assert map.metaClass != null
assert map.properties != null
map.entry = null
@@ -667,6 +667,34 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
''',
'Cannot set read-only property: properties'
+ assertScript '''
+ Map<String,Number> map = [:]
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ assert node.getNodeMetaData(INFERRED_TYPE) == Number_TYPE
+ })
+ def e = map.entry
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ assert node.getNodeMetaData(INFERRED_TYPE) == Number_TYPE
+ })
+ def a = map.empty
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ assert node.getNodeMetaData(INFERRED_TYPE) == Number_TYPE
+ })
+ def b = map.class
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ assert node.getNodeMetaData(INFERRED_TYPE) == METACLASS_TYPE
+ })
+ def c = map.metaClass
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ assert node.getNodeMetaData(INFERRED_TYPE) == MAP_TYPE
+ })
+ def d = map.properties
+ '''
assertScript '''
void test(Map map) {
def str = ''
@@ -840,14 +868,14 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
assert map.entry == null
assert map.empty == null
assert map.class == null
- assert map.metaClass == null
+ assert map.metaClass != null
'''
assertScript type + '''
def test(C map) { // no diff
assert map.entry == null
assert map.empty == null
assert map.class == null
- assert map.metaClass == null
+ assert map.metaClass != null
}
test(new C())
'''
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 0767c5ed30..54d382e265 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -821,105 +821,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
'''
}
- // GROOVY-5001, GROOVY-5491, GROOVY-11367, GROOVY-11369, GROOVY-11370
- void testMapPropertyAccess() {
- assertScript '''
- Map<String,Number> map = [:]
-
- @ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE) == boolean_TYPE
- })
- def a = map.empty
-
- @ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE) == CLASS_Type
- })
- def b = map.class
-
- @ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE) == METACLASS_TYPE
- })
- def c = map.metaClass
-
- @ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE) == MAP_TYPE
- })
- def d = map.properties
-
- @ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE) == Number_TYPE
- })
- def e = map.some_thing
- '''
- }
-
- // GROOVY-11367, GROOVY-11369
- @Override
- void testMapPropertyAccess5() {
- shouldFailWithMessages '''
- def map = [:]
- map.empty = null
- ''',
- 'Cannot set read-only property: empty'
-
- shouldFailWithMessages '''
- def map = [:]
- map.class = null
- ''',
- 'Cannot set read-only property: class'
-
- assertScript '''
- def map = [:]
- map.metaClass = null
- assert map.metaClass != null
- assert !map.containsKey('metaClass')
- '''
-
- shouldFailWithMessages '''
- def map = [:]
- map.properties = null
- ''',
- 'Cannot set read-only property: properties'
-
- assertScript '''
- void test(Map map) {
- def str = ''
- str += map.empty
- str += map.with{ empty }
- str += map.with{ delegate.empty }
- str += map.with{ {->owner.empty}() }
- assert str.equals('truetruetruetrue')
- }
- test( [:].withDefault{ 'entry' } )
- '''
- }
-
- // GROOVY-11367, GROOVY-11368
- @Override
- void testMapPropertyAccess9() {
- String type = '''
- class C implements Map<String,String> {
- @Delegate Map<String,String> impl = [:]
- }
- '''
- assertScript type + '''
- def map = new C()
- assert map.entry == null
- assert map.empty != null
- assert map.class != null
- assert map.metaClass != null
- '''
- assertScript type + '''
- def test(C map) { // no diff
- assert map.entry == null
- assert map.empty != null
- assert map.class != null
- assert map.metaClass != null
- }
- test(new C())
- '''
- }
-
// GROOVY-11223, GROOVY-11373
@Override
void testMapPropertyAccess10() {