diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 07bb03ac47d..9cc82464c96 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -266,6 +266,7 @@ import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL; import static org.codehaus.groovy.syntax.Types.KEYWORD_IN; import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF; +import static org.codehaus.groovy.syntax.Types.LOGICAL_OR; import static org.codehaus.groovy.syntax.Types.MINUS_MINUS; import static org.codehaus.groovy.syntax.Types.MOD; import static org.codehaus.groovy.syntax.Types.MOD_EQUAL; @@ -848,10 +849,13 @@ public Expression transform(final Expression expr) { return; } + // GROOVY-7971, GROOVY-8965, GROOVY-10702, GROOVY-11754, et al. + if (op == LOGICAL_OR) typeCheckingContext.pushTemporaryTypeInfo(); + ClassNode lType; leftExpression.visit(this); var setterInfo = removeSetterInfo(leftExpression); - if (setterInfo != null) { + if (setterInfo != null) { assert op != LOGICAL_OR; if (ensureValidSetter(expression, leftExpression, rightExpression, setterInfo)) { return; } @@ -863,7 +867,16 @@ public Expression transform(final Expression expr) { } else { lType = getType(leftExpression); } - rightExpression.visit(this); + if (op != LOGICAL_OR) { + rightExpression.visit(this); + } else { + var lhs = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); + typeCheckingContext.pushTemporaryTypeInfo(); + rightExpression.visit(this); + + var rhs = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); + propagateTemporaryTypeInfo(lhs, rhs); // `instanceof` on either side? + } } ClassNode rType = isNullConstant(rightExpression) @@ -992,6 +1005,43 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) { } } + private void propagateTemporaryTypeInfo(final Map> lhs, + final Map> rhs) { + // TODO: deal with (x !instanceof T) + lhs.keySet().removeIf(k -> k instanceof Object[]); + rhs.keySet().removeIf(k -> k instanceof Object[]); + + Function> getOrAdd = (key) -> + typeCheckingContext.temporaryIfBranchTypeInformation.peek().computeIfAbsent(key, x -> new LinkedList<>()); + + for (var entry : lhs.entrySet()) { + if (rhs.containsKey(entry.getKey())) { + // main case: (x instanceof A || x instanceof B) produces A|B type + List types = getOrAdd.apply(entry.getKey()); + types.addAll(entry.getValue()); + types.addAll(rhs.get(entry.getKey())); + } else if (entry.getKey() instanceof Variable) { + // edge case: (x instanceof A || ...) produces A|typeof(x) type + List types = getOrAdd.apply(entry.getKey()); + types.addAll(entry.getValue()); + Variable v = (Variable) entry.getKey(); + types.add(v instanceof ASTNode ? getType((ASTNode) v) : v.getType()); + } + } + + rhs.keySet().removeAll(lhs.keySet()); + + for (var entry : rhs.entrySet()) { + if (entry.getKey() instanceof Variable) { + // edge case: (... || x instanceof B) produces typeof(x)|B type + List types = getOrAdd.apply(entry.getKey()); + Variable v = (Variable) entry.getKey(); + types.add(v instanceof ASTNode ? getType((ASTNode) v) : v.getType()); + types.addAll(entry.getValue()); + } + } + } + private void validateResourceInARM(final BinaryExpression expression, final ClassNode lType) { if (expression instanceof DeclarationExpression && TryCatchStatement.isResource(expression) @@ -1173,8 +1223,8 @@ private boolean ensureValidSetter(final Expression expression, final Expression } addStaticTypeError(message, leftExpression); } else { - ClassNode[] tergetTypes = visibleSetters.stream().map(setterType).toArray(ClassNode[]::new); - addAssignmentError(tergetTypes.length == 1 ? tergetTypes[0] : new UnionTypeClassNode(tergetTypes), getType(valueExpression), expression); + ClassNode[] targetTypes = visibleSetters.stream().map(setterType).toArray(ClassNode[]::new); + addAssignmentError(targetTypes.length == 1 ? targetTypes[0] : new UnionTypeClassNode(targetTypes), getType(valueExpression), expression); } return true; } diff --git a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy index 21f1e140121..5cc59f07a20 100644 --- a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy +++ b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy @@ -241,7 +241,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { ''' } - // GROOVY-11769 + // GROOVY-11754 void testInstanceOf10() { assertScript ''' abstract class Foo { diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy index 1e0aca53659..b7ae38f82ba 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy @@ -31,10 +31,4 @@ class TypeInferenceStaticCompileTest extends TypeInferenceSTCTest implements Sta void testInstanceOf9() { super.testInstanceOf9() // GROOVY-7971 } - - @Override - @NotYetImplemented - void testInstanceOf10() { - super.testInstanceOf10() // GROOVY-11769 - } }