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 0d56d711d1 GROOVY-11369: STC: map entry before non-public access method
0d56d711d1 is described below
commit 0d56d711d1cde45ed21afb02943d1c2ead6b6867
Author: Eric Milles <eric.mil...@thomsonreuters.com>
AuthorDate: Wed May 8 09:55:17 2024 -0500
GROOVY-11369: STC: map entry before non-public access method
- `isEmpty()`, `getClass()`, and `getMetaClass()` are exempt
---
.../transform/stc/StaticTypeCheckingVisitor.java | 41 ++++++++++-------
.../stc/FieldsAndPropertiesSTCTest.groovy | 21 +++++++++
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 52 +++++++++++++++++++++-
3 files changed, 97 insertions(+), 17 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 e7ef925c5a..698d9bcd26 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.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;
@@ -1522,7 +1523,7 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
}
}
+ 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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
}
}
- 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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
}
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 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
// 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 d478df1409..92df8ba54a 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -682,6 +682,27 @@ class FieldsAndPropertiesSTCTest extends
StaticTypeCheckingTestCase {
'''
}
+ // 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 fbaec973b2..3b50ef7948 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 @@ final class FieldsAndPropertiesStaticCompileTest extends
FieldsAndPropertiesSTCT
'''
}
- // 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 = '''
