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 2022/10/06 18:29:59 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-7164, GROOVY-10787: STC: resolve placeholders for property lookup
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new 9c0a32669b GROOVY-7164, GROOVY-10787: STC: resolve placeholders for property lookup
9c0a32669b is described below
commit 9c0a32669b8307344a4d9014265b3d205462cae0
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Oct 6 10:58:09 2022 -0500
GROOVY-7164, GROOVY-10787: STC: resolve placeholders for property lookup
2_5_X backport
---
.../transform/stc/PropertyLookupVisitor.java | 60 -------------
.../transform/stc/StaticTypeCheckingVisitor.java | 99 ++++++++++++++--------
.../transform/stc/ConstructorsSTCTest.groovy | 30 +++++--
3 files changed, 84 insertions(+), 105 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/PropertyLookupVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/PropertyLookupVisitor.java
deleted file mode 100644
index 83f1d2db64..0000000000
--- a/src/main/java/org/codehaus/groovy/transform/stc/PropertyLookupVisitor.java
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.codehaus.groovy.transform.stc;
-
-import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
-import org.codehaus.groovy.ast.ClassNode;
-import org.codehaus.groovy.ast.FieldNode;
-import org.codehaus.groovy.ast.MethodNode;
-import org.codehaus.groovy.ast.PropertyNode;
-import org.codehaus.groovy.control.SourceUnit;
-
-import java.util.concurrent.atomic.AtomicReference;
-
-/**
- * A visitor used as a callback to {@link org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor#existsProperty(org.codehaus.groovy.ast.expr.PropertyExpression, boolean, org.codehaus.groovy.ast.ClassCodeVisitorSupport)}
- * which will return set the type of the found property in the provided reference.
- */
-class PropertyLookupVisitor extends ClassCodeVisitorSupport {
- private final AtomicReference<ClassNode> result;
-
- public PropertyLookupVisitor(final AtomicReference<ClassNode> result) {
- this.result = result;
- }
-
- @Override
- protected SourceUnit getSourceUnit() {
- return null;
- }
-
- @Override
- public void visitMethod(final MethodNode node) {
- result.set(node.getReturnType());
- }
-
- @Override
- public void visitProperty(final PropertyNode node) {
- result.set(node.getType());
- }
-
- @Override
- public void visitField(final FieldNode field) {
- result.set(field.getType());
- }
-}
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 686fd047c3..0bb117352f 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -134,7 +134,6 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
@@ -268,6 +267,7 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isWild
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.lastArgMatchesVarg;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.missesGenericsTypes;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.prettyPrintType;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.prettyPrintTypeName;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.resolveClassNodeGenerics;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.toMethodParametersString;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.typeCheckMethodArgumentWithGenerics;
@@ -1330,23 +1330,22 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
protected void checkGroovyConstructorMap(final Expression receiver, final ClassNode receiverType, final MapExpression mapExpression) {
- // workaround for map-style checks putting setter info on wrong AST nodes
+ // workaround for map-style checks putting setter info on wrong AST node
typeCheckingContext.pushEnclosingBinaryExpression(null);
for (MapEntryExpression entryExpression : mapExpression.getMapEntryExpressions()) {
Expression keyExpression = entryExpression.getKeyExpression();
if (!(keyExpression instanceof ConstantExpression)) {
addStaticTypeError("Dynamic keys in map-style constructors are unsupported in static type checking", keyExpression);
} else {
- String pName = keyExpression.getText();
- AtomicReference<ClassNode> pType = new AtomicReference<>();
- if (!existsProperty(new PropertyExpression(varX("_", receiverType), pName), false, new PropertyLookupVisitor(pType))) {
- addStaticTypeError("No such property: " + pName + " for class: " + receiverType.getText(), receiver);
+ String propName = keyExpression.getText();
+ PropertyLookup requestor = new PropertyLookup(receiverType);
+ if (!existsProperty(new PropertyExpression(varX("_", receiverType), propName), false, requestor)) {
+ addStaticTypeError("No such property: " + propName + " for class: " + prettyPrintTypeName(receiverType), receiver);
} else {
- MethodNode setter = receiverType.getSetterMethod("set" + MetaClassHelper.capitalize(pName), false);
- ClassNode targetType = setter != null ? setter.getParameters()[0].getType() : pType.get();
Expression valueExpression = entryExpression.getValueExpression();
- ClassNode valueType = getType(valueExpression);
+ ClassNode valueType = getType(valueExpression);
+ ClassNode targetType = requestor.propertyType;
ClassNode resultType = getResultType(targetType, ASSIGN, valueType,
assignX(keyExpression, valueExpression, entryExpression));
if (!checkCompatibleAssignmentTypes(targetType, resultType, valueExpression)
@@ -1563,8 +1562,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
List<MethodNode> setters = findSetters(current, setterName, false);
setters = allowStaticAccessToMember(setters, staticOnly);
- // need to visit even if we only look for a setters for compatibility
- if (visitor != null && getter != null) visitor.visitMethod(getter);
+ if (readMode && getter != null && visitor != null) visitor.visitMethod(getter);
PropertyNode propertyNode = current.getProperty(propertyName);
propertyNode = allowStaticAccessToMember(propertyNode, staticOnly);
@@ -1586,15 +1584,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else if (!readMode && checkGetterOrSetter) {
if (!setters.isEmpty()) {
if (visitor != null) {
- if (field != null) {
- visitor.visitField(field);
- } else {
- for (MethodNode setter : setters) {
- // visiting setter will not infer the property type since return type is void, so visit a dummy field instead
- FieldNode virtual = new FieldNode(propertyName, 0, setter.getParameters()[0].getOriginType(), current, null);
- virtual.setDeclaringClass(setter.getDeclaringClass());
- visitor.visitField(virtual);
- }
+ for (MethodNode setter : setters) {
+ // visiting setter will not infer the property type since return type is void, so visit a dummy field instead
+ FieldNode virtual = new FieldNode(propertyName, 0, setter.getParameters()[0].getOriginType(), current, null);
+ virtual.setDeclaringClass(setter.getDeclaringClass());
+ visitor.visitField(virtual);
}
}
SetterInfo info = new SetterInfo(current, setterName, setters);
@@ -1719,12 +1713,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
GenericsType[] gts = iteratorType.getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? getCombinedBoundType(gts[0]) : OBJECT_TYPE);
- PropertyExpression subExp = new PropertyExpression(varX("{}", itemType), pexp.getPropertyAsString());
- AtomicReference<ClassNode> result = new AtomicReference<>();
- if (existsProperty(subExp, true, new PropertyLookupVisitor(result))) {
- ClassNode listType = LIST_TYPE.getPlainNodeReference();
- listType.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(result.get()))});
- return listType;
+ PropertyLookup requestor = new PropertyLookup(itemType);
+ if (existsProperty(new PropertyExpression(varX("{}", itemType), pexp.getPropertyAsString()), true, requestor)) {
+ return GenericsUtils.makeClassSafe0(LIST_TYPE, new GenericsType(wrapTypeIfNecessary(requestor.propertyType)));
}
}
}
@@ -1732,17 +1723,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private ClassNode getTypeForListPropertyExpression(ClassNode testClass, ClassNode objectExpressionType, PropertyExpression pexp) {
- if (!implementsInterfaceOrIsSubclassOf(testClass, LIST_TYPE)) return null;
- ClassNode intf = GenericsUtils.parameterizeType(objectExpressionType, LIST_TYPE.getPlainNodeReference());
- GenericsType[] types = intf.getGenericsTypes();
- if (types == null || types.length != 1) return OBJECT_TYPE;
+ if (implementsInterfaceOrIsSubclassOf(testClass, LIST_TYPE)) {
+ ClassNode listType = GenericsUtils.parameterizeType(objectExpressionType, LIST_TYPE.getPlainNodeReference());
+ GenericsType[] gts = listType.getGenericsTypes();
+ ClassNode itemType = (gts != null && gts.length == 1 ? gts[0].getType() : OBJECT_TYPE);
- PropertyExpression subExp = new PropertyExpression(varX("{}", types[0].getType()), pexp.getPropertyAsString());
- AtomicReference<ClassNode> result = new AtomicReference<ClassNode>();
- if (existsProperty(subExp, true, new PropertyLookupVisitor(result))) {
- intf = LIST_TYPE.getPlainNodeReference();
- intf.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(result.get()))});
- return intf;
+ PropertyLookup requestor = new PropertyLookup(itemType);
+ if (existsProperty(new PropertyExpression(varX("{}", itemType), pexp.getPropertyAsString()), true, requestor)) {
+ return GenericsUtils.makeClassSafe0(LIST_TYPE, new GenericsType(wrapTypeIfNecessary(requestor.propertyType)));
+ }
}
return null;
}
@@ -5876,6 +5865,43 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private class PropertyLookup extends ClassCodeVisitorSupport {
+ ClassNode propertyType, receiverType;
+
+ PropertyLookup(final ClassNode type) {
+ receiverType = type;
+ }
+
+ @Override
+ protected SourceUnit getSourceUnit() {
+ return StaticTypeCheckingVisitor.this.getSourceUnit();
+ }
+
+ @Override
+ public void visitField(final FieldNode node) {
+ storePropertyType(node.getType(), node.isStatic() ? null : node.getDeclaringClass());
+ }
+
+ @Override
+ public void visitMethod(final MethodNode node) {
+ storePropertyType(node.getReturnType(), node.isStatic() ? null : node.getDeclaringClass());
+ }
+
+ @Override
+ public void visitProperty(final PropertyNode node) {
+ storePropertyType(node.getOriginType(), node.isStatic() ? null : node.getDeclaringClass());
+ }
+
+ private void storePropertyType(ClassNode type, final ClassNode declaringClass) {
+ if (declaringClass != null && GenericsUtils.hasUnresolvedGenerics(type)) { // GROOVY-10787
+ Map<GenericsTypeName, GenericsType> spec = extractPlaceHolders(null, receiverType, declaringClass);
+ type = applyGenericsContext(spec, type);
+ }
+ // TODO: if (propertyType != null) merge types
+ propertyType = type;
+ }
+ }
+
/**
* Wrapper for a Parameter so it can be treated like a VariableExpression
* and tracked in the {@code ifElseForWhileAssignmentTracker}.
@@ -5885,7 +5911,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
* variable.
*/
private static class ParameterVariableExpression extends VariableExpression {
-
private final Parameter parameter;
ParameterVariableExpression(final Parameter parameter) {
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index 950fa79957..285b312cb8 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -460,25 +460,39 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
'''
}
- //GROOVY-7164
- void testDefaultConstructorWhenSetterParamAndFieldHaveDifferentTypes() {
+ // GROOVY-7164
+ void testMapStyleConstructorWhenSetterParamAndFieldHaveDifferentTypes() {
assertScript '''
- class Test {
+ class C {
private long timestamp
Date getTimestamp() {
return timestamp ? new Date(timestamp) : null
}
- void setTimestamp (Date timestamp) {
+ void setTimestamp(Date timestamp) {
this.timestamp = timestamp.time
}
+ }
+ new C(timestamp: new Date())
+ '''
+ }
- def main() {
- new Test(timestamp: new Date())
- }
+ // GROOVY-10787
+ void testMapStyleConstructorWithParameterizedProperty() {
+ assertScript '''
+ abstract class A<X extends Serializable> {
+ X x
+ }
+ class C<Y extends Serializable> extends A<Y> {
}
- new Test().main()
+
+ def <Z extends Number> C<Z> fn(List<Z> list_of_z) {
+ new C<Z>(x: list_of_z.first())
+ }
+
+ def c = fn([42])
+ assert c.x == 42
'''
}