Skip to content

Commit

Permalink
GROOVY-10294
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Oct 31, 2021
1 parent 2132dfd commit d1a6999
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 29 deletions.
20 changes: 20 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 @@ -4150,6 +4150,26 @@ public void testTypeChecked10283() {
runConformTest(sources);
}

@Test
public void testTypeChecked10294() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"CharSequence test() {\n" +
" def x = 'xx'\n" +
" if (false) {\n" +
" x = null\n" +
" }\n" +
" x\n" + // Cannot return value of type Object on method returning type CharSequence
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked10295() {
//@formatter:off
Expand Down
10 changes: 8 additions & 2 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 @@ -1037,8 +1037,8 @@ else if (GenericsUtils.hasUnresolvedGenerics(resultType)) {
}
*/
// track conditional assignment
if (!isNullConstant(rightExpression)
&& leftExpression instanceof VariableExpression
if (/*GRECLIPSE edit !isNullConstant(rightExpression)
&&*/ leftExpression instanceof VariableExpression
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
Expand Down Expand Up @@ -4832,8 +4832,14 @@ protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<Var
// GROOVY-6099: First element of the list may be null, if no assignment was made before the branch
List<ClassNode> nonNullValues = new ArrayList<ClassNode>(allValues.size());
for (ClassNode value : allValues) {
// GRECLIPSE add -- GROOVY-10294
if (value != UNKNOWN_PARAMETER_TYPE)
// GRECLIPSE end
if (value != null) nonNullValues.add(value);
}
// GRECLIPSE add
if (nonNullValues.isEmpty()) continue;
// GRECLIPSE end
ClassNode cn = lowestUpperBound(nonNullValues);
storeType(key, cn);
assignments.put(key, cn);
Expand Down
12 changes: 10 additions & 2 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 @@ -923,8 +923,8 @@ else if (GenericsUtils.hasUnresolvedGenerics(resultType)) {
*/

// track conditional assignment
if (!isNullConstant(rightExpression)
&& leftExpression instanceof VariableExpression
if (/*GRECLIPSE edit !isNullConstant(rightExpression)
&&*/ leftExpression instanceof VariableExpression
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
Expand Down Expand Up @@ -4559,10 +4559,18 @@ private void restoreTypeBeforeConditional() {
protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
Map<VariableExpression, ClassNode> assignments = new HashMap<>();
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
/* GRECLIPSE edit -- GROOVY-10294
ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
.reduce(WideningCategories::lowestUpperBound).get();
assignments.put(var, type);
storeType(var, type);
*/
types.stream().filter(t -> t != null && t != UNKNOWN_PARAMETER_TYPE)
.reduce(WideningCategories::lowestUpperBound).ifPresent(type -> {
assignments.put(var, type);
storeType(var, type);
});
// GRECLIPSE end
});
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
Expand Down
45 changes: 20 additions & 25 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 @@ -904,8 +904,7 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) {
}

// track conditional assignment
if (!isNullConstant(rightExpression)
&& leftExpression instanceof VariableExpression
if (leftExpression instanceof VariableExpression
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
Expand Down Expand Up @@ -1081,14 +1080,10 @@ private static Token getOpWithoutEqual(final Expression exp) {
protected ClassNode getOriginalDeclarationType(final Expression lhs) {
if (lhs instanceof VariableExpression) {
Variable var = findTargetVariable((VariableExpression) lhs);
if (var instanceof PropertyNode) {
// Do NOT trust the type of the property node!
return getType(lhs);
if (!(var instanceof DynamicVariable || var instanceof PropertyNode)) {
return var.getOriginType();
}
if (var instanceof DynamicVariable) return getType(lhs);
return var.getOriginType();
}
if (lhs instanceof FieldExpression) {
} else if (lhs instanceof FieldExpression) {
return ((FieldExpression) lhs).getField().getOriginType();
}
return getType(lhs);
Expand Down Expand Up @@ -4207,18 +4202,18 @@ private void recordAssignment(final VariableExpression lhsExpr, final ClassNode

private void restoreTypeBeforeConditional() {
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
ClassNode originType = types.get(0);
storeType(var, originType);
var.putNodeMetaData(INFERRED_TYPE, types.get(0));
});
}

protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
Map<VariableExpression, ClassNode> assignments = new HashMap<>();
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
.reduce(WideningCategories::lowestUpperBound).get();
assignments.put(var, type);
storeType(var, type);
types.stream().filter(t -> t != null && t != UNKNOWN_PARAMETER_TYPE) // GROOVY-6099, GROOVY-10294
.reduce(WideningCategories::lowestUpperBound).ifPresent(type -> {
assignments.put(var, type);
storeType(var, type);
});
});
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
Expand Down Expand Up @@ -4468,15 +4463,15 @@ protected void storeType(final Expression exp, ClassNode cn) {
if (exp instanceof VariableExpression) {
VariableExpression var = (VariableExpression) exp;
Variable accessedVariable = var.getAccessedVariable();
if (accessedVariable != exp && accessedVariable instanceof VariableExpression) {
storeType((VariableExpression) accessedVariable, cn);
}
if (accessedVariable instanceof Parameter
|| (accessedVariable instanceof PropertyNode
&& ((PropertyNode) accessedVariable).getField().isSynthetic())) {
((ASTNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
}
if (var.isClosureSharedVariable() && cn != null) {
if (accessedVariable instanceof VariableExpression) {
if (accessedVariable != exp)
storeType((VariableExpression) accessedVariable, cn);
} else if (accessedVariable instanceof Parameter
|| accessedVariable instanceof PropertyNode
&& ((PropertyNode) accessedVariable).getField().isSynthetic()) {
((AnnotatedNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
}
if (cn != null && var.isClosureSharedVariable()) {
List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, k -> new LinkedList<ClassNode>());
assignedTypes.add(cn);
}
Expand Down Expand Up @@ -5132,7 +5127,7 @@ protected ClassNode getType(final ASTNode exp) {
return fieldType;
}
if (variable != vexp && variable instanceof VariableExpression) {
return getType((Expression) variable);
return getType((VariableExpression) variable);
}
if (variable instanceof Parameter) {
Parameter parameter = (Parameter) variable;
Expand Down

0 comments on commit d1a6999

Please sign in to comment.