Skip to content

Commit

Permalink
GROOVY-6782, GROOVY-8974, GROOVY-9033, GROOVY-10089
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed May 14, 2021
1 parent a05d059 commit ca900fb
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 51 deletions.
46 changes: 46 additions & 0 deletions ...re.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]]");
}
}
42 changes: 25 additions & 17 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 @@ -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:
Expand All @@ -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()
Expand All @@ -985,20 +972,34 @@ && 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))) {
resultType = originType;
} 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();
Expand All @@ -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
Expand Down
42 changes: 25 additions & 17 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 @@ -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:
Expand All @@ -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()
Expand All @@ -867,20 +854,34 @@ && 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))) {
resultType = originType;
} 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();
Expand All @@ -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)
Expand Down
42 changes: 25 additions & 17 deletions ...rg.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()) {
Expand Down Expand Up @@ -859,20 +846,34 @@ && 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))) {
resultType = originType;
} 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();
Expand All @@ -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)
Expand Down

0 comments on commit ca900fb

Please sign in to comment.