Skip to content

Commit

Permalink
GROOVY-5523, GROOVY-7753, GROOVY-9972, GROOVY-9983
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed May 17, 2021
1 parent 08a300c commit a35832d
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 51 deletions.
58 changes: 58 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 @@ -379,6 +379,26 @@ public void testTypeChecked15() {
runConformTest(sources, "test");
}

@Test
public void testTypeChecked5523() {
//@formatter:off
String[] sources = {
"Main.groovy",
"import static org.codehaus.groovy.ast.ClassHelper.*\n" +
"import static org.codehaus.groovy.transform.stc.StaticTypesMarker.*\n" +
"@groovy.transform.TypeChecked\n" +
"def findFile(String path) {\n" +
" @groovy.transform.ASTTest(phase=INSTRUCTION_SELECTION, value={\n" +
" assert node.getNodeMetaData(INFERRED_TYPE) == make(File)\n" +
" })\n" +
" File file = path ? null : null\n" + // edge case
"}",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked6232() {
//@formatter:off
Expand Down Expand Up @@ -2081,6 +2101,44 @@ public void testTypeChecked9977() {
runConformTest(sources, "-1");
}

@Test
public void testTypeChecked9983() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TupleConstructor(defaults=false)\n" +
"class A<T> {\n" +
" T p\n" +
"}\n" +
"class B {\n" +
"}\n" +
"class C {\n" +
" static m(A<B> a_of_b) {\n" +
" }\n" +
"}\n" +
"class D extends B {\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" boolean flag = true\n" +
" A<B> v = new A<>(null)\n" + // Cannot call C#m(A<B>) with arguments [A<#>]
" A<B> w = new A<>(new B())\n" +
" A<B> x = new A<>(new D())\n" +
" A<B> y = flag ? new A<>(new B()) : new A<>(new B())\n" +
" A<B> z = flag ? new A<>(new B()) : new A<>(new D())\n" +
" C.m(new A<>(null))\n" +
" C.m(new A<>(new B()))\n" +
" C.m(new A<>(new D()))\n" +
" C.m(flag ? new A<>(new B()) : new A<>(new B()))\n" +
" C.m(flag ? new A<>(new B()) : new A<>(new D()))\n" + // Cannot call m(A<B>) with arguments [A<? extends B>]\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked9984() {
//@formatter:off
Expand Down
86 changes: 59 additions & 27 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 @@ -1214,7 +1214,7 @@ protected void inferDiamondType(final ConstructorCallExpression cce, final Class
// check if constructor call expression makes use of the diamond operator
if (cceType.getGenericsTypes() != null && cceType.getGenericsTypes().length == 0) {
ArgumentListExpression argumentListExpression = InvocationWriter.makeArgumentList(cce.getArguments());
/* GRECLIPSE edit -- GROOVY-9948
/* GRECLIPSE edit -- GROOVY-9948, GROOVY-9983
if (argumentListExpression.getExpressions().isEmpty()) {
adjustGenerics(lType, cceType);
} else {
Expand All @@ -1232,7 +1232,7 @@ protected void inferDiamondType(final ConstructorCallExpression cce, final Class
type = inferReturnTypeGenerics(type, constructor, argumentListExpression);
if (type.isUsingGenerics()) {
// GROOVY-6232, GROOVY-9956: if cce not assignment compatible, process target as additional type witness
if (checkCompatibleAssignmentTypes(lType, type, cce) && !GenericsUtils.buildWildcardType(lType).isCompatibleWith(type)) {
if (GenericsUtils.hasUnresolvedGenerics(type) || checkCompatibleAssignmentTypes(lType, type, cce) && !GenericsUtils.buildWildcardType(lType).isCompatibleWith(type)) {
// allow covariance of each type parameter, but maintain semantics for nested generics

ClassNode pType = GenericsUtils.parameterizeType(lType, type);
Expand Down Expand Up @@ -2264,7 +2264,7 @@ public void visitField(final FieldNode node) {
}
}

// GRECLIPSE add
// GRECLIPSE add -- GROOVY-9977, GROOVY-9983, GROOVY-9995
private void visitInitialExpression(final Expression value, final Expression target, final ASTNode position) {
if (value != null) {
ClassNode lType = target.getType();
Expand All @@ -2273,21 +2273,25 @@ private void visitInitialExpression(final Expression value, final Expression tar
} else if (isClosureWithType(lType) && value instanceof ClosureExpression) {
storeInferredReturnType(value, getCombinedBoundType(lType.getGenericsTypes()[0]));
}

typeCheckingContext.pushEnclosingBinaryExpression(assignX(target, value, position));

value.visit(this);
ClassNode rType = getType(value);
if (value instanceof ConstructorCallExpression) {
inferDiamondType((ConstructorCallExpression) value, lType);
}

BinaryExpression bexp = binX(
target,
Token.newSymbol("=", position.getLineNumber(), position.getColumnNumber()),
value
);
bexp.setSourcePosition(position);
BinaryExpression bexp = typeCheckingContext.popEnclosingBinaryExpression();
typeCheckAssignment(bexp, target, lType, value, getResultType(lType, ASSIGN, rType, bexp));
}
}

private static BinaryExpression assignX(final Expression lhs, final Expression rhs, final ASTNode pos) {
BinaryExpression exp = (BinaryExpression) GeneralUtils.assignX(lhs, rhs);
exp.setSourcePosition(pos);
return exp;
}
// GRECLIPSE end

@Override
Expand Down Expand Up @@ -4742,9 +4746,9 @@ public void visitTernaryExpression(final TernaryExpression expression) {
falseExpression.visit(this);
ClassNode resultType;
ClassNode typeOfFalse = getType(falseExpression);
// GRECLIPSE edit
//ClassNode typeOfTrue = getType(trueExpression);
// GRECLIPSE end
/* GRECLIPSE edit
ClassNode typeOfTrue = getType(trueExpression);
*/
// handle instanceof cases
if (hasInferredReturnType(falseExpression)) {
typeOfFalse = falseExpression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
Expand Down Expand Up @@ -4772,7 +4776,7 @@ public void visitTernaryExpression(final TernaryExpression expression) {
}
*/
if (isNullConstant(trueExpression) && isNullConstant(falseExpression)) { // GROOVY-5523
resultType = checkForTargetType(expression, UNKNOWN_PARAMETER_TYPE);
resultType = checkForTargetType(trueExpression, UNKNOWN_PARAMETER_TYPE);
} else if (isNullConstant(trueExpression) || (isEmptyCollection(trueExpression)
&& isOrImplements(typeOfTrue, typeOfFalse))) { // [] : List/Collection/Iterable
resultType = wrapTypeIfNecessary(checkForTargetType(falseExpression, typeOfFalse));
Expand All @@ -4790,7 +4794,7 @@ && isOrImplements(typeOfFalse, typeOfTrue))) { // List/Collection/Iterable : []
}

private ClassNode checkForTargetType(final Expression expr, final ClassNode type) {
/* GRECLIPSE edit -- GROOVY-9972
/* GRECLIPSE edit -- GROOVY-9972, GROOVY-9983
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression instanceof DeclarationExpression
&& isEmptyCollection(expr) && isAssignment(enclosingBinaryExpression.getOperation().getType())) {
Expand All @@ -4812,23 +4816,19 @@ && isEmptyCollection(expr) && isAssignment(enclosingBinaryExpression.getOperatio
ClassNode sourceType = type, targetType = null;
MethodNode enclosingMethod = typeCheckingContext.getEnclosingMethod();
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression instanceof DeclarationExpression
if (enclosingBinaryExpression != null
&& isAssignment(enclosingBinaryExpression.getOperation().getType())
&& isTypeSource(expr, enclosingBinaryExpression.getRightExpression())) {
targetType = enclosingBinaryExpression.getLeftExpression().getType();
} else if (currentField != null
&& isTypeSource(expr, currentField.getInitialExpression())) {
targetType = currentField.getType();
} else if (currentProperty != null
&& isTypeSource(expr, currentProperty.getInitialExpression())) {
targetType = currentProperty.getType();
} else if (enclosingMethod != null) {
// TODO: try enclosingMethod's code with isTypeSource(expr, ...)
targetType = enclosingMethod.getReturnType();
} // TODO: closure parameter default expression
targetType = getDeclaredOrInferredType(enclosingBinaryExpression.getLeftExpression());
} else if (enclosingMethod != null
&& !enclosingMethod.isVoidMethod()
&& isTypeSource(expr, enclosingMethod)) {
targetType = enclosingMethod.getReturnType();
}

if (targetType == null) return sourceType;

if (expr instanceof ConstructorCallExpression) {
if (targetType == null) targetType = sourceType;
inferDiamondType((ConstructorCallExpression) expr, targetType);
} else if (targetType != null && !isPrimitiveType(getUnwrapper(targetType))
&& !targetType.equals(OBJECT_TYPE) && missesGenericsTypes(sourceType)) {
Expand Down Expand Up @@ -4860,6 +4860,32 @@ private static boolean isTypeSource(final Expression expr, final Expression righ
}
return expr == right;
}

private static boolean isTypeSource(final Expression expr, final MethodNode mNode) {
boolean[] returned = new boolean[1];

mNode.getCode().visit(new org.codehaus.groovy.ast.CodeVisitorSupport() {
@Override
public void visitReturnStatement(final ReturnStatement returnStatement) {
if (isTypeSource(expr, returnStatement.getExpression())) {
returned[0] = true;
}
}
@Override
public void visitClosureExpression(final ClosureExpression expression) {
}
});

if (!returned[0]) {
new ReturnAdder(returnStatement -> {
if (isTypeSource(expr, returnStatement.getExpression())) {
returned[0] = true;
}
}).visitMethod(mNode);
}

return returned[0];
}
// GRECLIPSE end

private static boolean isEmptyCollection(Expression expr) {
Expand Down Expand Up @@ -6176,6 +6202,12 @@ private void resolvePlaceholdersFromImplicitTypeHints(final ClassNode[] actuals,
actuals[i] = getLiteralResultType(pt, at, LINKEDHASHMAP_CLASSNODE);
} else if (a instanceof ConstructorCallExpression) {
inferDiamondType((ConstructorCallExpression) a, pt); // GROOVY-10086
} else if (a instanceof TernaryExpression && at.isUsingGenerics() && at.getGenericsTypes().length == 0) {
// GROOVY-9983: double diamond scenario -- "m(flag ? new Type<>(...) : new Type<>(...))"
typeCheckingContext.pushEnclosingBinaryExpression(assignX(varX(p), a, a));
a.visit(this); // re-visit with target type witness
typeCheckingContext.popEnclosingBinaryExpression();
actuals[i] = getType(a);
}

// check for method call with known target
Expand Down
71 changes: 51 additions & 20 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 @@ -1099,7 +1099,7 @@ protected void inferDiamondType(final ConstructorCallExpression cce, final Class
ClassNode cceType = cce.getType(), inferredType = lType;
// check if constructor call expression makes use of the diamond operator
if (cceType.getGenericsTypes() != null && cceType.getGenericsTypes().length == 0) {
/* GRECLIPSE edit -- GROOVY-9948
/* GRECLIPSE edit -- GROOVY-9948, GROOVY-9983
ArgumentListExpression argumentListExpression = InvocationWriter.makeArgumentList(cce.getArguments());
if (argumentListExpression.getExpressions().isEmpty()) {
adjustGenerics(lType, cceType);
Expand All @@ -1119,7 +1119,7 @@ protected void inferDiamondType(final ConstructorCallExpression cce, final Class
type = inferReturnTypeGenerics(type, constructor, argumentList);
if (type.isUsingGenerics()) {
// GROOVY-6232, GROOVY-9956: if cce not assignment compatible, process target as additional type witness
if (checkCompatibleAssignmentTypes(lType, type, cce) && !GenericsUtils.buildWildcardType(lType).isCompatibleWith(type)) {
if (GenericsUtils.hasUnresolvedGenerics(type) || checkCompatibleAssignmentTypes(lType, type, cce) && !GenericsUtils.buildWildcardType(lType).isCompatibleWith(type)) {
// allow covariance of each type parameter, but maintain semantics for nested generics

ClassNode pType = GenericsUtils.parameterizeType(lType, type);
Expand Down Expand Up @@ -2083,22 +2083,25 @@ public void visitField(final FieldNode node) {
}
}

// GRECLIPSE add
// GRECLIPSE add -- GROOVY-9977, GROOVY-9983, GROOVY-9995
private void visitInitialExpression(final Expression value, final Expression target, final ASTNode position) {
if (value != null) {
ClassNode lType = target.getType();
if (isFunctionalInterface(lType)) { // GROOVY-9977
if (isFunctionalInterface(lType)) {
processFunctionalInterfaceAssignment(lType, value);
} else if (isClosureWithType(lType) && value instanceof ClosureExpression) {
storeInferredReturnType(value, getCombinedBoundType(lType.getGenericsTypes()[0]));
}

typeCheckingContext.pushEnclosingBinaryExpression(assignX(target, value, position));

value.visit(this);
ClassNode rType = getType(value);
if (value instanceof ConstructorCallExpression) {
inferDiamondType((ConstructorCallExpression) value, lType);
}

BinaryExpression bexp = assignX(target, value, position);
BinaryExpression bexp = typeCheckingContext.popEnclosingBinaryExpression();
typeCheckAssignment(bexp, target, lType, value, getResultType(lType, ASSIGN, rType, bexp));
}
}
Expand Down Expand Up @@ -4541,7 +4544,7 @@ public void visitTernaryExpression(final TernaryExpression expression) {
}
*/
if (isNullConstant(trueExpression) && isNullConstant(falseExpression)) { // GROOVY-5523
resultType = checkForTargetType(expression, UNKNOWN_PARAMETER_TYPE);
resultType = checkForTargetType(trueExpression, UNKNOWN_PARAMETER_TYPE);
} else if (isNullConstant(trueExpression) || (isEmptyCollection(trueExpression)
&& isOrImplements(typeOfTrue, typeOfFalse))) { // [] : List/Collection/Iterable
resultType = wrapTypeIfNecessary(checkForTargetType(falseExpression, typeOfFalse));
Expand All @@ -4559,7 +4562,7 @@ && isOrImplements(typeOfFalse, typeOfTrue))) { // List/Collection/Iterable : []
}

private ClassNode checkForTargetType(final Expression expr, final ClassNode type) {
/* GRECLIPSE edit -- GROOVY-9972
/* GRECLIPSE edit -- GROOVY-9972, GROOVY-9983
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression instanceof DeclarationExpression
&& isEmptyCollection(expr) && isAssignment(enclosingBinaryExpression.getOperation().getType())) {
Expand All @@ -4581,23 +4584,19 @@ && isEmptyCollection(expr) && isAssignment(enclosingBinaryExpression.getOperatio
ClassNode sourceType = type, targetType = null;
MethodNode enclosingMethod = typeCheckingContext.getEnclosingMethod();
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression instanceof DeclarationExpression
if (enclosingBinaryExpression != null
&& isAssignment(enclosingBinaryExpression.getOperation().getType())
&& isTypeSource(expr, enclosingBinaryExpression.getRightExpression())) {
targetType = enclosingBinaryExpression.getLeftExpression().getType();
} else if (currentField != null
&& isTypeSource(expr, currentField.getInitialExpression())) {
targetType = currentField.getType();
} else if (currentProperty != null
&& isTypeSource(expr, currentProperty.getInitialExpression())) {
targetType = currentProperty.getType();
} else if (enclosingMethod != null) {
// TODO: try enclosingMethod's code with isTypeSource(expr, ...)
targetType = enclosingMethod.getReturnType();
} // TODO: closure parameter default expression
targetType = getDeclaredOrInferredType(enclosingBinaryExpression.getLeftExpression());
} else if (enclosingMethod != null
&& !enclosingMethod.isVoidMethod()
&& isTypeSource(expr, enclosingMethod)) {
targetType = enclosingMethod.getReturnType();
}

if (targetType == null) return sourceType;

if (expr instanceof ConstructorCallExpression) {
if (targetType == null) targetType = sourceType;
inferDiamondType((ConstructorCallExpression) expr, targetType);
} else if (targetType != null && !isPrimitiveType(getUnwrapper(targetType))
&& !targetType.equals(OBJECT_TYPE) && missesGenericsTypes(sourceType)) {
Expand Down Expand Up @@ -4629,6 +4628,32 @@ private static boolean isTypeSource(final Expression expr, final Expression righ
}
return expr == right;
}

private static boolean isTypeSource(final Expression expr, final MethodNode mNode) {
boolean[] returned = new boolean[1];

mNode.getCode().visit(new org.codehaus.groovy.ast.CodeVisitorSupport() {
@Override
public void visitReturnStatement(final ReturnStatement returnStatement) {
if (isTypeSource(expr, returnStatement.getExpression())) {
returned[0] = true;
}
}
@Override
public void visitClosureExpression(final ClosureExpression expression) {
}
});

if (!returned[0]) {
new ReturnAdder(returnStatement -> {
if (isTypeSource(expr, returnStatement.getExpression())) {
returned[0] = true;
}
}).visitMethod(mNode);
}

return returned[0];
}
// GRECLIPSE end

private static boolean isEmptyCollection(final Expression expr) {
Expand Down Expand Up @@ -5895,6 +5920,12 @@ private void resolvePlaceholdersFromImplicitTypeHints(final ClassNode[] actuals,
actuals[i] = getLiteralResultType(pt, at, LINKEDHASHMAP_CLASSNODE);
} else if (a instanceof ConstructorCallExpression) {
inferDiamondType((ConstructorCallExpression) a, pt); // GROOVY-10086
} else if (a instanceof TernaryExpression && at.isUsingGenerics() && at.getGenericsTypes().length == 0) {
// GROOVY-9983: double diamond scenario -- "m(flag ? new Type<>(...) : new Type<>(...))"
typeCheckingContext.pushEnclosingBinaryExpression(assignX(varX(p), a, a));
a.visit(this); // re-visit with target type witness
typeCheckingContext.popEnclosingBinaryExpression();
actuals[i] = getType(a);
}

// check for method call with known target
Expand Down
Loading

0 comments on commit a35832d

Please sign in to comment.