Skip to content

Commit d1a6999

Browse files
eric-milleseric-milles
committed
GROOVY-10294
1 parent 2132dfd commit d1a6999

File tree

4 files changed

+58
-29
lines changed

4 files changed

+58
-29
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4150,6 +4150,26 @@ public void testTypeChecked10283() {
41504150
runConformTest(sources);
41514151
}
41524152

4153+
@Test
4154+
public void testTypeChecked10294() {
4155+
//@formatter:off
4156+
String[] sources = {
4157+
"Main.groovy",
4158+
"@groovy.transform.TypeChecked\n" +
4159+
"CharSequence test() {\n" +
4160+
" def x = 'xx'\n" +
4161+
" if (false) {\n" +
4162+
" x = null\n" +
4163+
" }\n" +
4164+
" x\n" + // Cannot return value of type Object on method returning type CharSequence
4165+
"}\n" +
4166+
"test()\n",
4167+
};
4168+
//@formatter:on
4169+
4170+
runConformTest(sources);
4171+
}
4172+
41534173
@Test
41544174
public void testTypeChecked10295() {
41554175
//@formatter:off

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,8 +1037,8 @@ else if (GenericsUtils.hasUnresolvedGenerics(resultType)) {
10371037
}
10381038
*/
10391039
// track conditional assignment
1040-
if (!isNullConstant(rightExpression)
1041-
&& leftExpression instanceof VariableExpression
1040+
if (/*GRECLIPSE edit !isNullConstant(rightExpression)
1041+
&&*/ leftExpression instanceof VariableExpression
10421042
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
10431043
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
10441044
if (accessedVariable instanceof Parameter) {
@@ -4832,8 +4832,14 @@ protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<Var
48324832
// GROOVY-6099: First element of the list may be null, if no assignment was made before the branch
48334833
List<ClassNode> nonNullValues = new ArrayList<ClassNode>(allValues.size());
48344834
for (ClassNode value : allValues) {
4835+
// GRECLIPSE add -- GROOVY-10294
4836+
if (value != UNKNOWN_PARAMETER_TYPE)
4837+
// GRECLIPSE end
48354838
if (value != null) nonNullValues.add(value);
48364839
}
4840+
// GRECLIPSE add
4841+
if (nonNullValues.isEmpty()) continue;
4842+
// GRECLIPSE end
48374843
ClassNode cn = lowestUpperBound(nonNullValues);
48384844
storeType(key, cn);
48394845
assignments.put(key, cn);

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -923,8 +923,8 @@ else if (GenericsUtils.hasUnresolvedGenerics(resultType)) {
923923
*/
924924

925925
// track conditional assignment
926-
if (!isNullConstant(rightExpression)
927-
&& leftExpression instanceof VariableExpression
926+
if (/*GRECLIPSE edit !isNullConstant(rightExpression)
927+
&&*/ leftExpression instanceof VariableExpression
928928
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
929929
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
930930
if (accessedVariable instanceof Parameter) {
@@ -4559,10 +4559,18 @@ private void restoreTypeBeforeConditional() {
45594559
protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
45604560
Map<VariableExpression, ClassNode> assignments = new HashMap<>();
45614561
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
4562+
/* GRECLIPSE edit -- GROOVY-10294
45624563
ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
45634564
.reduce(WideningCategories::lowestUpperBound).get();
45644565
assignments.put(var, type);
45654566
storeType(var, type);
4567+
*/
4568+
types.stream().filter(t -> t != null && t != UNKNOWN_PARAMETER_TYPE)
4569+
.reduce(WideningCategories::lowestUpperBound).ifPresent(type -> {
4570+
assignments.put(var, type);
4571+
storeType(var, type);
4572+
});
4573+
// GRECLIPSE end
45664574
});
45674575
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
45684576
return assignments;

‎base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -904,8 +904,7 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) {
904904
}
905905

906906
// track conditional assignment
907-
if (!isNullConstant(rightExpression)
908-
&& leftExpression instanceof VariableExpression
907+
if (leftExpression instanceof VariableExpression
909908
&& typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
910909
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
911910
if (accessedVariable instanceof Parameter) {
@@ -1081,14 +1080,10 @@ private static Token getOpWithoutEqual(final Expression exp) {
10811080
protected ClassNode getOriginalDeclarationType(final Expression lhs) {
10821081
if (lhs instanceof VariableExpression) {
10831082
Variable var = findTargetVariable((VariableExpression) lhs);
1084-
if (var instanceof PropertyNode) {
1085-
// Do NOT trust the type of the property node!
1086-
return getType(lhs);
1083+
if (!(var instanceof DynamicVariable || var instanceof PropertyNode)) {
1084+
return var.getOriginType();
10871085
}
1088-
if (var instanceof DynamicVariable) return getType(lhs);
1089-
return var.getOriginType();
1090-
}
1091-
if (lhs instanceof FieldExpression) {
1086+
} else if (lhs instanceof FieldExpression) {
10921087
return ((FieldExpression) lhs).getField().getOriginType();
10931088
}
10941089
return getType(lhs);
@@ -4207,18 +4202,18 @@ private void recordAssignment(final VariableExpression lhsExpr, final ClassNode
42074202

42084203
private void restoreTypeBeforeConditional() {
42094204
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
4210-
ClassNode originType = types.get(0);
4211-
storeType(var, originType);
4205+
var.putNodeMetaData(INFERRED_TYPE, types.get(0));
42124206
});
42134207
}
42144208

42154209
protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
42164210
Map<VariableExpression, ClassNode> assignments = new HashMap<>();
42174211
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
4218-
ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
4219-
.reduce(WideningCategories::lowestUpperBound).get();
4220-
assignments.put(var, type);
4221-
storeType(var, type);
4212+
types.stream().filter(t -> t != null && t != UNKNOWN_PARAMETER_TYPE) // GROOVY-6099, GROOVY-10294
4213+
.reduce(WideningCategories::lowestUpperBound).ifPresent(type -> {
4214+
assignments.put(var, type);
4215+
storeType(var, type);
4216+
});
42224217
});
42234218
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
42244219
return assignments;
@@ -4468,15 +4463,15 @@ protected void storeType(final Expression exp, ClassNode cn) {
44684463
if (exp instanceof VariableExpression) {
44694464
VariableExpression var = (VariableExpression) exp;
44704465
Variable accessedVariable = var.getAccessedVariable();
4471-
if (accessedVariable != exp && accessedVariable instanceof VariableExpression) {
4472-
storeType((VariableExpression) accessedVariable, cn);
4473-
}
4474-
if (accessedVariable instanceof Parameter
4475-
|| (accessedVariable instanceof PropertyNode
4476-
&& ((PropertyNode) accessedVariable).getField().isSynthetic())) {
4477-
((ASTNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
4478-
}
4479-
if (var.isClosureSharedVariable() && cn != null) {
4466+
if (accessedVariable instanceof VariableExpression) {
4467+
if (accessedVariable != exp)
4468+
storeType((VariableExpression) accessedVariable, cn);
4469+
} else if (accessedVariable instanceof Parameter
4470+
|| accessedVariable instanceof PropertyNode
4471+
&& ((PropertyNode) accessedVariable).getField().isSynthetic()) {
4472+
((AnnotatedNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
4473+
}
4474+
if (cn != null && var.isClosureSharedVariable()) {
44804475
List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, k -> new LinkedList<ClassNode>());
44814476
assignedTypes.add(cn);
44824477
}
@@ -5132,7 +5127,7 @@ protected ClassNode getType(final ASTNode exp) {
51325127
return fieldType;
51335128
}
51345129
if (variable != vexp && variable instanceof VariableExpression) {
5135-
return getType((Expression) variable);
5130+
return getType((VariableExpression) variable);
51365131
}
51375132
if (variable instanceof Parameter) {
51385133
Parameter parameter = (Parameter) variable;

0 commit comments

Comments
 (0)