GROOVY-11369: STC: map entry before non-public access method
- `isEmpty()`, `getClass()`, and `getMetaClass()` are exempt
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 e7ef925..698d9bc 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -264,6 +264,7 @@
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;
@@ -1522,7 +1523,7 @@
protected boolean existsProperty(final PropertyExpression pexp, final boolean readMode, final ClassCodeVisitorSupport visitor) {
super.visitPropertyExpression(pexp);
- String propertyName = pexp.getPropertyAsString();
+ final String propertyName = pexp.getPropertyAsString();
if (propertyName == null) return false;
Expression objectExpression = pexp.getObjectExpression();
@@ -1565,8 +1566,11 @@
}
}
+ final String isserName = getGetterName(propertyName, Boolean.TYPE);
+ final String getterName = getGetterName(propertyName);
+ final String setterName = getSetterName(propertyName);
+
boolean foundGetterOrSetter = false;
- String capName = capitalize(propertyName);
Set<ClassNode> handledNodes = new HashSet<>();
List<Receiver<String>> receivers = new ArrayList<>();
addReceivers(receivers, makeOwnerList(objectExpression), pexp.isImplicitThis());
@@ -1623,12 +1627,22 @@
}
}
- MethodNode getter = current.getGetterMethod("is" + capName);
+ MethodNode getter = current.getGetterMethod(isserName);
getter = allowStaticAccessToMember(getter, staticOnly);
- if (getter == null) getter = current.getGetterMethod(getGetterName(propertyName));
+ if (getter == null) getter = current.getGetterMethod(getterName);
getter = allowStaticAccessToMember(getter, staticOnly);
- List<MethodNode> setters = findSetters(current, getSetterName(propertyName), /*voidOnly:*/false);
+ if (getter != null
+ // GROOVY-11319:
+ && (!hasAccessToMember(typeCheckingContext.getEnclosingClassNode(), getter.getDeclaringClass(), getter.getModifiers())
+ // GROOVY-11369:
+ || (!isThisExpression(objectExpression) && !isSuperExpression(objectExpression) && isOrImplements(objectExpressionType, MAP_TYPE)
+ && (!getter.isPublic() || (propertyName.matches("empty|class|metaClass") && !List.of(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE)))))) {
+ getter = null;
+ }
+ List<MethodNode> setters = findSetters(current, setterName, /*voidOnly:*/false);
setters = allowStaticAccessToMember(setters, staticOnly);
+ // GROOVY-11319:
+ setters.removeIf(setter -> !hasAccessToMember(typeCheckingContext.getEnclosingClassNode(), setter.getDeclaringClass(), setter.getModifiers()));
if (readMode && getter != null && visitor != null) visitor.visitMethod(getter);
@@ -1636,9 +1650,8 @@
property = allowStaticAccessToMember(property, staticOnly);
// prefer explicit getter or setter over property if receiver is not 'this'
if (property == null || !enclosingTypes.contains(receiverType)) {
- ClassNode enclosingType = enclosingTypes.iterator().next();
if (readMode) {
- if (getter != null && hasAccessToMember(enclosingType, getter.getDeclaringClass(), getter.getModifiers())) {
+ if (getter != null) {
ClassNode returnType = inferReturnTypeGenerics(receiverType, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
storeInferredTypeForPropertyExpression(pexp, returnType);
storeTargetMethod(pexp, getter);
@@ -1647,11 +1660,9 @@
pexp.putNodeMetaData(IMPLICIT_RECEIVER, delegationData);
}
return true;
- } else {
- getter = null; // GROOVY-11319
}
} else {
- if (setters.stream().anyMatch(setter -> hasAccessToMember(enclosingType, setter.getDeclaringClass(), setter.getModifiers()))) {
+ if (!setters.isEmpty()) {
if (visitor != null) {
for (MethodNode setter : setters) {
// visiting setter will not infer the property type since return type is void, so visit a dummy field instead
@@ -1660,7 +1671,7 @@
visitor.visitField(virtual);
}
}
- SetterInfo info = new SetterInfo(current, getSetterName(propertyName), setters);
+ SetterInfo info = new SetterInfo(current, setterName, setters);
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression != null) {
putSetterInfo(enclosingBinaryExpression.getLeftExpression(), info);
@@ -1671,10 +1682,8 @@
}
pexp.removeNodeMetaData(READONLY_PROPERTY);
return true;
- } else if (field == null && getter != null && hasAccessToMember(enclosingType, getter.getDeclaringClass(), getter.getModifiers())) {
+ } else if (getter != null && (field == null || field.isFinal())) {
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE); // GROOVY-9127
- } else {
- setters.clear(); // GROOVY-11319
}
}
}
@@ -1688,8 +1697,8 @@
// GROOVY-5568: the property may be defined by DGM
for (ClassNode dgmReceiver : isPrimitiveType(receiverType) ? new ClassNode[]{receiverType, getWrapper(receiverType)} : new ClassNode[]{receiverType}) {
- Set<MethodNode> methods = findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, "get" + capName);
- for (MethodNode method : findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, "is" + capName)) {
+ Set<MethodNode> methods = findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, getterName);
+ for (MethodNode method : findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, isserName)) {
if (isPrimitiveBoolean(method.getReturnType())) methods.add(method);
}
if (staticOnlyAccess && receiver.getData() == null && !isClassType(receiver.getType())) {
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index d478df1..92df8ba 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -682,6 +682,27 @@
'''
}
+ // GROOVY-11369
+ void testMapPropertyAccess5() {
+ assertScript '''
+ def map = [:]
+ assert map.entry == null
+ assert map.empty == null
+ assert map.class == null
+ assert map.metaClass == null // TODO
+
+ map.entry = null
+ map.empty = null // not read-only property
+ map.class = null // not read-only property
+ map.metaClass = null // not read-only property
+
+ assert map.containsKey('entry')
+ assert map.containsKey('empty')
+ assert map.containsKey('class')
+ assert !map.containsKey('metaClass')
+ '''
+ }
+
// GROOVY-8074
void testMapPropertyAccess6() {
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 fbaec97..3b50ef7 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -859,7 +859,57 @@
'''
}
- // GROOVY-11368
+ // GROOVY-5001, GROOVY-5491, GROOVY-11367, GROOVY-11369
+ 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) == Number_TYPE
+ })
+ def d = map.something
+ '''
+ }
+
+ // 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 // TODO: GROOVY-6549 made this "put" (SC only)!
+ assert map.metaClass != null
+ assert map.containsKey('metaClass')
+ '''
+ }
+
+ // GROOVY-11367, GROOVY-11368
@Override
void testMapPropertyAccess9() {
String type = '''
