GROOVY-11754: STC: prevent cast exception under imbalanced instanceof

Author: eric-milles

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<Object, List<ClassNode>> lhs,
+                                            final Map<Object, List<ClassNode>> rhs) {
+        // TODO: deal with (x !instanceof T)
+        lhs.keySet().removeIf(k -> k instanceof Object[]);
+        rhs.keySet().removeIf(k -> k instanceof Object[]);
+
+        Function<Object, List<ClassNode>> 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<ClassNode> 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<ClassNode> 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<ClassNode> 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;
         }

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 {

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
-    }
 }