GROOVY-11983: STC: unsound smart-cast in else branch of if (cond && !(x instanceof Y))

Author: paulk-asert

src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

@@ -56,6 +56,7 @@
 import org.codehaus.groovy.ast.expr.AttributeExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.BitwiseNegationExpression;
+import org.codehaus.groovy.ast.expr.BooleanExpression;
 import org.codehaus.groovy.ast.expr.CastExpression;
 import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
@@ -268,6 +269,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_AND;
 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;
@@ -4636,13 +4638,15 @@ public void visitIfElse(final IfStatement ifElse) {
 
             // GROOVY-6429: reverse instanceof tracking
             typeCheckingContext.pushTemporaryTypeInfo();
-            tti.forEach(this::putNotInstanceOfTypeInfo);
+            // GROOVY-11983: only invert when the boolean's falsity pins down each instanceof
+            boolean canInvert = canInvertNarrowingForElseBranch(ifElse.getBooleanExpression());
+            if (canInvert) tti.forEach(this::putNotInstanceOfTypeInfo);
 
             elsePath.visit(this);
 
             typeCheckingContext.popTemporaryTypeInfo();
             // GROOVY-8523: propagate tracking to outer scope; keep simple for now
-            if (elsePath.isEmpty() && !GeneralUtils.maybeFallsThrough(thenPath)) {
+            if (canInvert && elsePath.isEmpty() && !GeneralUtils.maybeFallsThrough(thenPath)) {
                 tti.forEach(this::putNotInstanceOfTypeInfo);
             }
         } finally {
@@ -4837,7 +4841,11 @@ public void visitTernaryExpression(final TernaryExpression expression) {
         Map<Object, List<ClassNode>> tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop();
 
         typeCheckingContext.pushTemporaryTypeInfo();
-        tti.forEach(this::putNotInstanceOfTypeInfo); // GROOVY-8412
+        // GROOVY-8412 / GROOVY-11983: only invert when sound (no LOGICAL_AND in condition)
+        if (!(expression instanceof ElvisOperatorExpression)
+                && canInvertNarrowingForElseBranch(expression.getBooleanExpression())) {
+            tti.forEach(this::putNotInstanceOfTypeInfo);
+        }
         Expression falseExpression = expression.getFalseExpression();
         ClassNode typeOfFalse = visitValueExpression(falseExpression);
         typeCheckingContext.popTemporaryTypeInfo();
@@ -6645,6 +6653,38 @@ private void putNotInstanceOfTypeInfo(final Object key, final Collection<ClassNo
         tti.addAll(types); // stash negative type(s)
     }
 
+    /**
+     * GROOVY-11983: Whether the temporary type info captured while visiting {@code expr}
+     * may be soundly inverted for the else branch of {@code if (expr) ... else ...} (or
+     * the false branch of a ternary, or the surrounding scope after an early-returning
+     * {@code if (expr) return}). Inversion is only sound when {@code !expr} pins down
+     * the negation of every individual instanceof / !instanceof check inside it. A
+     * logical-AND breaks that: the else of {@code A && B} is reached when at least one
+     * of {@code A}, {@code B} is false, not necessarily both — so an instanceof inside
+     * either operand cannot be safely inverted. In particular, a {@code !instanceof}
+     * inside an AND would otherwise be unwrapped into a positive smart-cast, producing
+     * an unsound {@code checkcast} and a runtime ClassCastException.
+     */
+    private static boolean canInvertNarrowingForElseBranch(final Expression expr) {
+        if (expr instanceof BinaryExpression be) {
+            int op = be.getOperation().getType();
+            if (op == LOGICAL_AND) return false;
+            if (op == LOGICAL_OR) {
+                return canInvertNarrowingForElseBranch(be.getLeftExpression())
+                    && canInvertNarrowingForElseBranch(be.getRightExpression());
+            }
+            return true; // instanceof, ==, comparisons, etc.
+        }
+        if (expr instanceof BooleanExpression be) return canInvertNarrowingForElseBranch(be.getExpression());
+        if (expr instanceof NotExpression ne) return canInvertNarrowingForElseBranch(ne.getExpression());
+        if (expr instanceof TernaryExpression te) {
+            return canInvertNarrowingForElseBranch(te.getBooleanExpression())
+                && canInvertNarrowingForElseBranch(te.getTrueExpression())
+                && canInvertNarrowingForElseBranch(te.getFalseExpression());
+        }
+        return true;
+    }
+
     private boolean optInstanceOfTypeInfo(final Expression expression, final ClassNode type) {
         var tti = typeCheckingContext.temporaryIfBranchTypeInformation.peek().get(extractTemporaryTypeInfoKey(expression));
         if (tti != null) { assert !tti.isEmpty();

src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy

@@ -1031,6 +1031,54 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
         }
     }
 
+    // GROOVY-11983: !instanceof inside && must not produce a positive smart-cast in the else branch
+    @Test
+    void testNotInstanceof7() {
+        for (test in ['!(x instanceof String)', 'x !instanceof String']) {
+            // if/else form: a non-String x with cond=false lands in the else; if x were
+            // smart-cast to String, the body would emit a checkcast and throw CCE
+            assertScript """
+                @groovy.transform.CompileStatic
+                class T {
+                    static int f(Object x, boolean cond) {
+                        if (cond && $test) {
+                            return 1
+                        } else {
+                            return x.hashCode()
+                        }
+                    }
+                }
+                assert T.f(42, false) == Integer.valueOf(42).hashCode()
+                assert T.f('s', false) == 's'.hashCode()
+                assert T.f('s', true)  == 's'.hashCode()
+                assert T.f(42, true)   == 1
+            """
+            // ternary form
+            assertScript """
+                @groovy.transform.CompileStatic
+                class T {
+                    static int g(Object x, boolean cond) {
+                        return (cond && $test) ? 1 : x.hashCode()
+                    }
+                }
+                assert T.g(42, false) == Integer.valueOf(42).hashCode()
+                assert T.g('s', false) == 's'.hashCode()
+            """
+            // early-return surrounding-scope form
+            assertScript """
+                @groovy.transform.CompileStatic
+                class T {
+                    static int h(Object x, boolean cond) {
+                        if (cond && $test) return 1
+                        return x.hashCode()
+                    }
+                }
+                assert T.h(42, false) == Integer.valueOf(42).hashCode()
+                assert T.h('s', false) == 's'.hashCode()
+            """
+        }
+    }
+
     // GROOVY-10217
     @Test
     void testInstanceOfThenSubscriptOperator() {