From ca900fbd37e95a2786e7f43041f414103dced479 Mon Sep 17 00:00:00 2001 From: Eric Milles <eric.milles@thomsonreuters.com> Date: Fri, 14 May 2021 15:42:14 -0500 Subject: [PATCH] GROOVY-6782, GROOVY-8974, GROOVY-9033, GROOVY-10089 --- .../tests/xform/StaticCompilationTests.java | 46 +++++++++++++++++++ .../stc/StaticTypeCheckingVisitor.java | 42 ++++++++++------- .../stc/StaticTypeCheckingVisitor.java | 42 ++++++++++------- .../stc/StaticTypeCheckingVisitor.java | 42 ++++++++++------- 4 files changed, 121 insertions(+), 51 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java index 3541c6283f..347b0fef9d 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java @@ -788,6 +788,25 @@ public void testCompileStatic6610() { runConformTest(sources, "42"); } + @Test + public void testCompileStatic6782() { + //@formatter:off + String[] sources = { + "Main.groovy", + "@groovy.transform.CompileStatic\n" + + "void test() {\n" + + " String[] array = [123]\n" + + " def temp = array\n" + + " def x = temp[0]\n" + + " temp = [:]\n" + // works if this line is removed + "}\n" + + "test()\n", + }; + //@formatter:on + + runConformTest(sources); + } + @Test public void testCompileStatic6904() { //@formatter:off @@ -6206,4 +6225,31 @@ public void testCompileStatic10072() { runConformTest(sources); } + + @Test + public void testCompileStatic10089() { + //@formatter:off + String[] sources = { + "Main.groovy", + "@groovy.transform.CompileStatic\n" + + "void test(... attributes) {\n" + + " List one = [\n" + + " [id:'x', options:[count:1]]\n" + + " ]\n" + + " List two = attributes.collect {\n" + + " def node = Collections.singletonMap('children', one)\n" + + " if (node) {\n" + + " node = node.get('children').find { child -> child['id'] == 'x' }\n" + + " }\n" + + " [id: it['id'], name: node['name'], count: node['options']['count']]\n" + + // ^^^^^^^^^^^^^^^ GroovyCastException (map ctor for Collection) + " }\n" + + " print two\n" + + "}\n" + + "test( [id:'x'] )\n", + }; + //@formatter:on + + runConformTest(sources, "[[id:x, name:null, count:1]]"); + } } diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 33bf96f533..f2758f534c 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -935,7 +935,7 @@ else if (op == LOGICAL_OR) { } } - /* GRECLIPSE edit -- GROOVY-8974, GROOVY-9033 + /* GRECLIPSE edit -- GROOVY-8974, et al. if (lType.isUsingGenerics() && missesGenericsTypes(resultType) && isAssignment(op)) { // unchecked assignment // examples: @@ -950,19 +950,6 @@ else if (op == LOGICAL_OR) { resultType = completedType; } */ - if (isAssignment(op)) { - if (rightExpression instanceof ConstructorCallExpression) { - inferDiamondType((ConstructorCallExpression) rightExpression, lType); - } - if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { - if (!resultType.isGenericsPlaceHolder()) // plain reference loses information - resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); - } else if (lType.equals(OBJECT_TYPE) && GenericsUtils.hasUnresolvedGenerics(resultType)) { // def list = [] - Map<GenericsTypeName, GenericsType> placeholders = extractGenericsParameterMapOfThis(typeCheckingContext); - resultType = fullyResolveType(resultType, Optional.ofNullable(placeholders).orElseGet(Collections::emptyMap)); - } - } else - // GRECLIPSE end if (isArrayOp(op) && !lType.isArray() @@ -985,13 +972,20 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { boolean isEmptyDeclaration = expression instanceof DeclarationExpression && rightExpression instanceof EmptyExpression; if (!isEmptyDeclaration && isAssignment(op)) { - /* GRECLIPSE edit -- GROOVY-8974 if (rightExpression instanceof ConstructorCallExpression) { inferDiamondType((ConstructorCallExpression) rightExpression, lType); } - */ + // GRECLIPSE add -- unchecked assignment + if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { + // the inferred type of the binary expression is the type of the RHS + // "completed" with generics type information available from the LHS + if (!resultType.isGenericsPlaceHolder()) // plain reference loses information + resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); + } + // GRECLIPSE end ClassNode originType = getOriginalDeclarationType(leftExpression); typeCheckAssignment(expression, leftExpression, originType, rightExpression, resultType); + /* GRECLIPSE edit // if assignment succeeds but result type is not a subtype of original type, then we are in a special cast handling // and we must update the result type if (!implementsInterfaceOrIsSubclassOf(getWrapper(resultType), getWrapper(originType))) { @@ -999,6 +993,13 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { } else if (lType.isUsingGenerics() && !lType.isEnum() && hasRHSIncompleteGenericTypeInfo(resultType)) { // for example, LHS is List<ConcreteClass> and RHS is List<T> where T is a placeholder resultType = lType; + */ + // check for implicit conversion like "String a = 123", "int[] b = [1,2,3]", "List c = [].stream()", etc. + if (!implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(resultType), wrapTypeIfNecessary(originType))) { + resultType = originType; + } else if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { + resultType = originType; // retain primitive semantics + // GRECLIPSE end } else { // GROOVY-7549: RHS type may not be accessible to enclosing class int modifiers = resultType.getModifiers(); @@ -1008,12 +1009,19 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { && (Modifier.isPrivate(modifiers) || !Objects.equals(enclosingType.getPackageName(), resultType.getPackageName()))) { resultType = originType; // TODO: Find accesible type in hierarchy of resultType? } + // GRECLIPSE add -- GROOVY-9033, GROOVY-10089 + else if (GenericsUtils.hasUnresolvedGenerics(resultType)) { + Map<GenericsTypeName, GenericsType> enclosing = extractGenericsParameterMapOfThis(typeCheckingContext); + resultType = fullyResolveType(resultType, Optional.ofNullable(enclosing).orElseGet(Collections::emptyMap)); + } + // GRECLIPSE end } - + /* GRECLIPSE edit // make sure we keep primitive types if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { resultType = originType; } + */ // if we are in an if/else branch, keep track of assignment if (typeCheckingContext.ifElseForWhileAssignmentTracker != null && leftExpression instanceof VariableExpression diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index bdd22f1500..cb61c3b622 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -817,7 +817,7 @@ else if (op == LOGICAL_OR) { } boolean isAssignment = isAssignment(expression.getOperation().getType()); - /* GRECLIPSE edit -- GROOVY-8974, GROOVY-9033 + /* GRECLIPSE edit -- GROOVY-8974, et al. if (isAssignment && lType.isUsingGenerics() && missesGenericsTypes(resultType)) { // unchecked assignment // examples: @@ -832,19 +832,6 @@ else if (op == LOGICAL_OR) { resultType = completedType; } */ - if (isAssignment) { - if (rightExpression instanceof ConstructorCallExpression) { - inferDiamondType((ConstructorCallExpression) rightExpression, lType); - } - if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { - if (!resultType.isGenericsPlaceHolder()) // plain reference loses information - resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); - } else if (lType.equals(OBJECT_TYPE) && GenericsUtils.hasUnresolvedGenerics(resultType)) { // def list = [] - Map<GenericsTypeName, GenericsType> placeholders = extractGenericsParameterMapOfThis(typeCheckingContext); - resultType = fullyResolveType(resultType, Optional.ofNullable(placeholders).orElseGet(Collections::emptyMap)); - } - } else - // GRECLIPSE end if (isArrayOp(op) && !lType.isArray() @@ -867,13 +854,20 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { boolean isEmptyDeclaration = (expression instanceof DeclarationExpression && rightExpression instanceof EmptyExpression); if (isAssignment && !isEmptyDeclaration) { - /* GRECLIPSE edit -- GROOVY-8974 if (rightExpression instanceof ConstructorCallExpression) { inferDiamondType((ConstructorCallExpression) rightExpression, lType); } - */ + // GRECLIPSE add -- unchecked assignment + if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { + // the inferred type of the binary expression is the type of the RHS + // "completed" with generics type information available from the LHS + if (!resultType.isGenericsPlaceHolder()) // plain reference loses information + resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); + } + // GRECLIPSE end ClassNode originType = getOriginalDeclarationType(leftExpression); typeCheckAssignment(expression, leftExpression, originType, rightExpression, resultType); + /* GRECLIPSE edit // if assignment succeeds but result type is not a subtype of original type, then we are in a special cast handling // and we must update the result type if (!implementsInterfaceOrIsSubclassOf(getWrapper(resultType), getWrapper(originType))) { @@ -881,6 +875,13 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { } else if (lType.isUsingGenerics() && !lType.isEnum() && hasRHSIncompleteGenericTypeInfo(resultType)) { // for example, LHS is List<ConcreteClass> and RHS is List<T> where T is a placeholder resultType = lType; + */ + // check for implicit conversion like "String a = 123", "int[] b = [1,2,3]", "List c = [].stream()", etc. + if (!implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(resultType), wrapTypeIfNecessary(originType))) { + resultType = originType; + } else if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { + resultType = originType; // retain primitive semantics + // GRECLIPSE end } else { // GROOVY-7549: RHS type may not be accessible to enclosing class int modifiers = resultType.getModifiers(); @@ -890,12 +891,19 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { && (Modifier.isPrivate(modifiers) || !Objects.equals(enclosingType.getPackageName(), resultType.getPackageName()))) { resultType = originType; // TODO: Find accesible type in hierarchy of resultType? } + // GRECLIPSE add -- GROOVY-9033, GROOVY-10089 + else if (GenericsUtils.hasUnresolvedGenerics(resultType)) { + Map<GenericsTypeName, GenericsType> enclosing = extractGenericsParameterMapOfThis(typeCheckingContext); + resultType = fullyResolveType(resultType, Optional.ofNullable(enclosing).orElseGet(Collections::emptyMap)); + } + // GRECLIPSE end } - + /* GRECLIPSE edit // make sure we keep primitive types if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { resultType = originType; } + */ // track conditional assignment if (!isNullConstant(rightExpression) diff --git a/base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index b57fdbd615..3ba0d6a8e9 100644 --- a/base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -807,7 +807,7 @@ else if (op == LOGICAL_OR) { if (resultType == null) { resultType = lType; - /* GRECLIPSE edit -- GROOVY-8974, GROOVY-9033 + /* GRECLIPSE edit -- GROOVY-8974, et al. } else if (lType.isUsingGenerics() && isAssignment(op) && missesGenericsTypes(resultType)) { // unchecked assignment // List<Type> list = new LinkedList() @@ -817,21 +817,8 @@ else if (op == LOGICAL_OR) { // the inferred type of the binary expression is the type of the RHS // "completed" with generics type information available from the LHS resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); - } */ - } else if (isAssignment(op)) { - if (rightExpression instanceof ConstructorCallExpression) { - inferDiamondType((ConstructorCallExpression) rightExpression, lType); - } - if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { - if (!resultType.isGenericsPlaceHolder()) // plain reference loses information - resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); - } else if (lType.equals(OBJECT_TYPE) && GenericsUtils.hasUnresolvedGenerics(resultType)) { // def list = [] - Map<GenericsTypeName, GenericsType> placeholders = extractGenericsParameterMapOfThis(typeCheckingContext); - resultType = fullyResolveType(resultType, Optional.ofNullable(placeholders).orElseGet(Collections::emptyMap)); - } } - // GRECLIPSE end // GROOVY-5874: if left expression is a closure shared variable, a second pass should be done if (leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isClosureSharedVariable()) { @@ -859,13 +846,20 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { boolean isEmptyDeclaration = (expression instanceof DeclarationExpression && rightExpression instanceof EmptyExpression); if (!isEmptyDeclaration && isAssignment(op)) { - /* GRECLIPSE edit -- GROOVY-8974 if (rightExpression instanceof ConstructorCallExpression) { inferDiamondType((ConstructorCallExpression) rightExpression, lType); } - */ + // GRECLIPSE add -- unchecked assignment + if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) { + // the inferred type of the binary expression is the type of the RHS + // "completed" with generics type information available from the LHS + if (!resultType.isGenericsPlaceHolder()) // plain reference loses information + resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference()); + } + // GRECLIPSE end ClassNode originType = getOriginalDeclarationType(leftExpression); typeCheckAssignment(expression, leftExpression, originType, rightExpression, resultType); + /* GRECLIPSE edit // if assignment succeeds but result type is not a subtype of original type, then we are in a special cast handling // and we must update the result type if (!implementsInterfaceOrIsSubclassOf(getWrapper(resultType), getWrapper(originType))) { @@ -873,6 +867,13 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { } else if (lType.isUsingGenerics() && !lType.isEnum() && hasRHSIncompleteGenericTypeInfo(resultType)) { // for example, LHS is List<ConcreteClass> and RHS is List<T> where T is a placeholder resultType = lType; + */ + // check for implicit conversion like "String a = 123", "int[] b = [1,2,3]", "List c = [].stream()", etc. + if (!implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(resultType), wrapTypeIfNecessary(originType))) { + resultType = originType; + } else if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { + resultType = originType; // retain primitive semantics + // GRECLIPSE end } else { // GROOVY-7549: RHS type may not be accessible to enclosing class int modifiers = resultType.getModifiers(); @@ -882,12 +883,19 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { && (Modifier.isPrivate(modifiers) || !Objects.equals(enclosingType.getPackageName(), resultType.getPackageName()))) { resultType = originType; // TODO: Find accesible type in hierarchy of resultType? } + // GRECLIPSE add -- GROOVY-9033, GROOVY-10089 + else if (GenericsUtils.hasUnresolvedGenerics(resultType)) { + Map<GenericsTypeName, GenericsType> enclosing = extractGenericsParameterMapOfThis(typeCheckingContext); + resultType = fullyResolveType(resultType, Optional.ofNullable(enclosing).orElseGet(Collections::emptyMap)); + } + // GRECLIPSE end } - + /* GRECLIPSE edit // make sure we keep primitive types if (isPrimitiveType(originType) && resultType.equals(getWrapper(originType))) { resultType = originType; } + */ // track conditional assignment if (!isNullConstant(rightExpression)