[IfChainToSwitch] bugfix to prevent invalid code generation by ensuring that a guard cannot be a false literal

core/src/main/java/com/google/errorprone/bugpatterns/IfChainToSwitch.java

@@ -59,6 +59,7 @@
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IfTree;
 import com.sun.source.tree.InstanceOfTree;
+import com.sun.source.tree.LiteralTree;
 import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.Tree.Kind;
@@ -958,40 +959,48 @@ private Optional<ExpressionTree> validatePredicateForSubject(
 
           return Optional.empty();
         }
-      } else if (predicateIsConditionalAnd) {
+      } else if (predicateIsConditionalAnd && !mustBeInstanceOf) {
         // Maybe the predicate is something like `a instanceof Foo && predicate`.  If so, recurse on
         // the left side, and attach the right side of the conditional and as a guard to the
         // resulting case.
-        if (!mustBeInstanceOf && binaryTree.getKind().equals(Kind.CONDITIONAL_AND)) {
-          int currentCasesSize = cases.size();
-          var rv =
-              validatePredicateForSubject(
-                  binaryTree.getLeftOperand(),
-                  subject,
-                  state,
-                  /* mustBeInstanceOf= */ true,
-                  cases,
-                  elseOptional,
-                  arrowRhsOptional,
-                  handledEnumValues,
-                  ifTreeRange,
-                  /* caseStartPosition= */ caseStartPosition);
-          if (rv.isPresent()) {
-            CaseIr oldLastCase = cases.get(currentCasesSize);
-            // Update last case to attach the guard
-            cases.set(
-                currentCasesSize,
-                new CaseIr(
-                    /* hasCaseNull= */ oldLastCase.hasCaseNull(),
-                    /* hasDefault= */ oldLastCase.hasDefault(),
-                    /* instanceOfOptional= */ oldLastCase.instanceOfOptional(),
-                    /* guardOptional= */ Optional.of(binaryTree.getRightOperand()),
-                    /* expressionsOptional= */ oldLastCase.expressionsOptional(),
-                    /* arrowRhsOptional= */ oldLastCase.arrowRhsOptional(),
-                    /* caseSourceCodeRange= */ oldLastCase.caseSourceCodeRange()));
-            return rv;
+        int currentCasesSize = cases.size();
+        var rv =
+            validatePredicateForSubject(
+                binaryTree.getLeftOperand(),
+                subject,
+                state,
+                /* mustBeInstanceOf= */ true,
+                cases,
+                elseOptional,
+                arrowRhsOptional,
+                handledEnumValues,
+                ifTreeRange,
+                /* caseStartPosition= */ caseStartPosition);
+        if (rv.isPresent()) {
+          CaseIr oldLastCase = cases.get(currentCasesSize);
+          ExpressionTree rightOperandNoParentheses =
+              ASTHelpers.stripParentheses(binaryTree.getRightOperand());
+          // A guard cannot just be `false` (not valid Java)
+          if (isBooleanLiteral(rightOperandNoParentheses)
+              && rightOperandNoParentheses instanceof LiteralTree literalTree
+              && literalTree.getValue() instanceof Boolean b
+              && !b) {
+            return Optional.empty();
           }
+          // Update last case to attach the guard
+          cases.set(
+              currentCasesSize,
+              new CaseIr(
+                  /* hasCaseNull= */ oldLastCase.hasCaseNull(),
+                  /* hasDefault= */ oldLastCase.hasDefault(),
+                  /* instanceOfOptional= */ oldLastCase.instanceOfOptional(),
+                  /* guardOptional= */ Optional.of(binaryTree.getRightOperand()),
+                  /* expressionsOptional= */ oldLastCase.expressionsOptional(),
+                  /* arrowRhsOptional= */ oldLastCase.arrowRhsOptional(),
+                  /* caseSourceCodeRange= */ oldLastCase.caseSourceCodeRange()));
+          return rv;
         }
+
       } else if (!mustBeInstanceOf && predicateIsConditionalOr) {
         // Maybe the predicate is something like `x == 1 || x == 2`.
         return validateConditionalOrsForSubject(binaryTree, params);
@@ -1006,6 +1015,10 @@ private Optional<ExpressionTree> validatePredicateForSubject(
     return Optional.empty();
   }
 
+  private static boolean isBooleanLiteral(ExpressionTree tree) {
+    return tree.getKind() == Kind.BOOLEAN_LITERAL;
+  }
+
   /**
    * Validates whether the {@code binaryTree} represents a series of conditional-ORs that can be
    * converted to a single switch case having multiple expressions, returning the subject if so.

core/src/test/java/com/google/errorprone/bugpatterns/IfChainToSwitchTest.java

@@ -2108,6 +2108,36 @@ public void foo(Suit s) {
         .doTest(TEXT_MATCH);
   }
 
+  @Test
+  public void ifChain_guardFalse_noError() {
+    // A guard cannot be the literal `false`.
+    helper
+        .addSourceLines(
+            "Test.java",
+            """
+            import java.lang.Number;
+
+            class Test {
+              public void foo(Suit s) {
+                Object suit = s;
+                System.out.println("yo");
+                if (suit == Suit.SPADE) {
+                  return;
+                } else if (suit == Suit.DIAMOND) {
+                  return;
+                } else if (suit == Suit.HEART) {
+                  return;
+                } else if (suit instanceof Suit su && (false)) {
+                  throw new NullPointerException();
+                }
+                return;
+              }
+            }
+            """)
+        .setArgs("-XepOpt:IfChainToSwitch:EnableMain", "-XepOpt:IfChainToSwitch:EnableSafe=false")
+        .doTest();
+  }
+
   @Test
   public void ifChain_pullUpSafe2_error() {