Skip to content

Commit

Permalink
GROOVY-7164, GROOVY-10787
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Oct 6, 2022
1 parent 83535fa commit 8ff2465
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 95 deletions.
47 changes: 47 additions & 0 deletions ...ovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,31 @@ public void testTypeChecked7128d() {
runConformTest(sources);
}

@Test
public void testTypeChecked7164() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class C {\n" +
" private long timestamp\n" +
" Date getTimestamp() {\n" +
" timestamp ? new Date(timestamp) : null\n" +
" }\n" +
" void setTimestamp(Date timestamp) {\n" +
" this.timestamp = timestamp.time\n" +
" }\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" new C(timestamp: new Date())\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked7247() {
//@formatter:off
Expand Down Expand Up @@ -6552,4 +6577,26 @@ public void testTypeChecked10767() {

runConformTest(sources);
}

@Test
public void testTypeChecked10787() {
//@formatter:off
String[] sources = {
"Main.groovy",
"abstract class A<X extends Serializable> {\n" +
" X x\n" +
"}\n" +
"class C<Y extends Serializable> extends A<Y> {\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"def <Z extends Number> C<Z> fn(List<Z> list_of_z) {\n" +
" new C<Z>(x: list_of_z.first())\n" +
"}\n" +
"def c = fn([42])\n" +
"assert c.x == 42\n",
};
//@formatter:on

runConformTest(sources);
}
}
111 changes: 66 additions & 45 deletions ...rg.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
import java.util.Set;
import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.stream.IntStream;
Expand Down Expand Up @@ -1493,23 +1492,22 @@ protected void typeCheckAssignment(final BinaryExpression assignmentExpression,
}

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)
Expand Down Expand Up @@ -1745,8 +1743,7 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
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);
Expand All @@ -1771,15 +1768,11 @@ && hasAccessToMember(first(enclosingTypes), getter.getDeclaringClass(), getter.g
} 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);
Expand Down Expand Up @@ -1923,37 +1916,29 @@ private ClassNode getTypeForSpreadExpression(ClassNode testClass, ClassNode obje
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)));
}
}
}
return null;
}

private ClassNode getTypeForListPropertyExpression(ClassNode testClass, ClassNode objectExpressionType, PropertyExpression pexp) {
if (!implementsInterfaceOrIsSubclassOf(testClass, LIST_TYPE)) return null;
/* GRECLIPSE edit -- GROOVY-9821
ClassNode intf = GenericsUtils.parameterizeType(objectExpressionType, LIST_TYPE.getPlainNodeReference());
GenericsType[] types = intf.getGenericsTypes();
if (types == null || types.length != 1) return OBJECT_TYPE;
PropertyExpression subExp = new PropertyExpression(varX("{}", types[0].getType()), pexp.getPropertyAsString());
*/
GenericsType[] types = (testClass.equals(LIST_TYPE) ? testClass : GenericsUtils.parameterizeType(testClass, LIST_TYPE)).getGenericsTypes();
ClassNode itemType = (types != null && types.length == 1 ? getCombinedBoundType(types[0]) : OBJECT_TYPE);

PropertyExpression subExp = new PropertyExpression(varX("{}", itemType), pexp.getPropertyAsString());
// GRECLIPSE end
AtomicReference<ClassNode> result = new AtomicReference<ClassNode>();
if (existsProperty(subExp, true, new PropertyLookupVisitor(result))) {
ClassNode intf = LIST_TYPE.getPlainNodeReference();
intf.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(result.get()))});
return intf;
if (implementsInterfaceOrIsSubclassOf(testClass, LIST_TYPE)) {
/* GRECLIPSE edit -- GROOVY-9821
ClassNode listType = GenericsUtils.parameterizeType(objectExpressionType, LIST_TYPE.getPlainNodeReference());
GenericsType[] gts = listType.getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? gts[0].getType() : OBJECT_TYPE);
*/
GenericsType[] gts = (testClass.equals(LIST_TYPE) ? testClass : GenericsUtils.parameterizeType(testClass, LIST_TYPE)).getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? getCombinedBoundType(gts[0]) : OBJECT_TYPE);
// GRECLIPSE end
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;
}
Expand Down Expand Up @@ -6792,12 +6777,48 @@ private SetterInfo(final ClassNode receiverType, final String name, final List<M
}
}

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}.
*/
private class ParameterVariableExpression extends VariableExpression {

private final Parameter parameter;

ParameterVariableExpression(final Parameter parameter) {
Expand Down
79 changes: 53 additions & 26 deletions ...rg.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,23 +1459,22 @@ else if (rightExpression instanceof MapExpression)
}

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: " + receiverType.getText(), 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)
Expand Down Expand Up @@ -1704,8 +1703,7 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
List<MethodNode> setters = findSetters(current, setterName, 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);
Expand All @@ -1726,15 +1724,11 @@ && hasAccessToMember(enclosingTypes.iterator().next(), getter.getDeclaringClass(
} 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);
Expand Down Expand Up @@ -1877,12 +1871,9 @@ private ClassNode getTypeForMultiValueExpression(final ClassNode compositeType,
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))) {
gts = new GenericsType[]{new GenericsType(wrapTypeIfNecessary(propertyType.get()))};
ClassNode listType = LIST_TYPE.getPlainNodeReference();
listType.setGenericsTypes(gts);
return listType;
PropertyLookup requestor = new PropertyLookup(itemType);
if (existsProperty(propX(varX("{}", itemType), prop), true, requestor)) {
return GenericsUtils.makeClassSafe0(LIST_TYPE, new GenericsType(wrapTypeIfNecessary(requestor.propertyType)));
}
return null;
}
Expand Down Expand Up @@ -6531,12 +6522,48 @@ private SetterInfo(final ClassNode receiverType, final String name, final List<M
}
}

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) {
Expand Down
Loading

0 comments on commit 8ff2465

Please sign in to comment.