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 16:13:38 UTC
[groovy] branch master 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 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 00d84da2f5 GROOVY-7164, GROOVY-10787: STC: resolve placeholders for property lookup
00d84da2f5 is described below
commit 00d84da2f5b16b2e262f2bc58c4482d76ab6abe3
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
---
.../transform/stc/PropertyLookupVisitor.java | 60 -----------------
.../transform/stc/StaticTypeCheckingVisitor.java | 77 +++++++++++++++-------
.../transform/stc/ConstructorsSTCTest.groovy | 30 ++++++---
3 files changed, 75 insertions(+), 92 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 b60c50f5f4..8adff29aa9 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -135,7 +135,6 @@ import java.util.Optional;
import java.util.Set;
import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.BiPredicate;
import java.util.function.Consumer;
@@ -1372,23 +1371,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(propX(varX("_", receiverType), pName), false, new PropertyLookupVisitor(pType))) {
- addStaticTypeError("No such property: " + pName + " for class: " + prettyPrintTypeName(receiverType), receiver);
+ String propName = keyExpression.getText();
+ PropertyLookup requestor = new PropertyLookup(receiverType);
+ if (!existsProperty(propX(varX("_", receiverType), propName), false, requestor)) {
+ addStaticTypeError("No such property: " + propName + " for class: " + prettyPrintTypeName(receiverType), receiver);
} else {
- ClassNode targetType = Optional.ofNullable(receiverType.getSetterMethod(getSetterName(pName), false))
- .map(setter -> setter.getParameters()[0].getType()).orElseGet(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)
@@ -1592,8 +1590,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
List<MethodNode> setters = findSetters(current, getSetterName(propertyName), /*enforce void:*/false);
setters = allowStaticAccessToMember(setters, staticOnly);
- // need to visit even if we only look for setters for compatibility
- if (visitor != null && getter != null) visitor.visitMethod(getter);
+ if (readMode && getter != null && visitor != null) visitor.visitMethod(getter);
PropertyNode property = current.getProperty(propertyName);
property = allowStaticAccessToMember(property, staticOnly);
@@ -1613,15 +1610,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else {
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, getSetterName(propertyName), setters);
@@ -1743,9 +1736,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
GenericsType[] gts = compositeType.getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? getCombinedBoundType(gts[0]) : OBJECT_TYPE);
- AtomicReference<ClassNode> propertyType = new AtomicReference<>();
- if (existsProperty(propX(varX("{}", itemType), prop), true, new PropertyLookupVisitor(propertyType))) {
- return extension.buildListType(propertyType.get());
+ PropertyLookup requestor = new PropertyLookup(itemType);
+ if (existsProperty(propX(varX("{}", itemType), prop), true, requestor)) {
+ return extension.buildListType(requestor.propertyType);
}
return null;
}
@@ -6094,12 +6087,48 @@ 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(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}.
*/
private 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 a6897a8983..353b0cc4b2 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -496,25 +496,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(x: list_of_z.first())
+ }
+
+ def c = fn([42])
+ assert c.x == 42
'''
}