[IfChainToSwitch] bugfix: when a guard is present and true, must be treated as an unguarded pattern

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

@@ -31,6 +31,7 @@
 import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSubtype;
 import static com.google.errorprone.util.ASTHelpers.sameVariable;
+import static com.google.errorprone.util.ASTHelpers.stripParentheses;
 import static com.sun.source.tree.Tree.Kind.EXPRESSION_STATEMENT;
 import static com.sun.source.tree.Tree.Kind.THROW;
 import static java.lang.Math.max;
@@ -418,6 +419,28 @@ private static boolean switchOnNullWouldImplicitlyThrow(
         && cases.stream().noneMatch(CaseIr::hasCaseNull);
   }
 
+  /**
+   * Determines whether the given case IR has an "unguarded" pattern. As defined in the JLS,
+   * "unguarded" means that either there is no guard or the guard is the boolean literal `true`.
+   */
+  private static boolean isUnguarded(CaseIr caseIr) {
+    // Not a pattern
+    if (caseIr.instanceOfOptional().isEmpty()) {
+      return false;
+    }
+
+    if (caseIr.guardOptional().isEmpty()) {
+      return true;
+    }
+
+    // Guard is present and is `true`
+    ExpressionTree guard = stripParentheses(caseIr.guardOptional().get());
+    return isBooleanLiteral(guard)
+        && guard instanceof LiteralTree literalTree
+        && literalTree.getValue() instanceof Boolean b
+        && b;
+  }
+
   /**
    * Analyzes the supplied case IRs for a switch statement for issues related default/unconditional
    * cases. If deemed necessary, this method injects a `default` and/or `case null` into the
@@ -452,7 +475,7 @@ private Optional<List<CaseIr>> maybeFixDefaultNullAndUnconditional(
             .filter(
                 caseIr ->
                     caseIr.instanceOfOptional().isPresent()
-                        && caseIr.guardOptional().isEmpty()
+                        && isUnguarded(caseIr)
                         && isSubtype(
                             getType(subject),
                             getType(caseIr.instanceOfOptional().get().type()),
@@ -1673,7 +1696,7 @@ public static boolean isDominatedBy(
         boolean isPrimitive = getType(constantExpression).isPrimitive();
         if (isPrimitive) {
           // Guarded patterns cannot dominate primitives
-          if (lhs.guardOptional().isPresent()) {
+          if (!isUnguarded(lhs)) {
             continue;
           }
           if (lhs.instanceOfOptional().isPresent()) {
@@ -1705,7 +1728,7 @@ public static boolean isDominatedBy(
         }
         boolean isEnum = isEnumValue(constantExpression, state);
         if (isEnum) {
-          if (lhs.guardOptional().isPresent()) {
+          if (!isUnguarded(lhs)) {
             // Guarded patterns cannot dominate enum values
             continue;
           }
@@ -1726,7 +1749,7 @@ public static boolean isDominatedBy(
         // RHS must be a reference
         // The rhs-reference code would be needed to support e.g. String literals.  It is included
         // for completeness.
-        if (lhs.guardOptional().isPresent()) {
+        if (!isUnguarded(lhs)) {
           // Guarded patterns cannot dominate references
           continue;
         }
@@ -1757,7 +1780,7 @@ public static boolean isDominatedBy(
     }
 
     // RHS must be a pattern
-    if (lhs.guardOptional().isPresent()) {
+    if (!isUnguarded(lhs)) {
       // LHS has a guard, so cannot dominate RHS
       return false;
     }

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

@@ -3036,7 +3036,8 @@ public void foo(Suit s) {
   }
 
   @Test
-  public void ifChain_domination3_error() {
+  public void ifChain_conflictingUnguardedTypePatterns_noError() {
+    // Integer and Number are conflicting; both unconditional for Integer
     helper
         .addSourceLines(
             "Test.java",
@@ -3067,7 +3068,7 @@ public void foo(Suit s) {
   }
 
   @Test
-  public void ifChain_domination4_noError() {
+  public void ifChain_conflictingUnguardedTypePatternsShort_noError() {
     // Number and Integer are conflicting (both unconditional for Integer)
     helper
         .addSourceLines(
@@ -3093,7 +3094,7 @@ public void foo(Suit s) {
   }
 
   @Test
-  public void ifChain_domination5_noError() {
+  public void ifChain_dominationDefaultAndUnconditionalConflict_noError() {
     // Both a default and unconditional conflict
     helper
         .addSourceLines(
@@ -3120,6 +3121,55 @@ public void foo(Suit s) {
         .doTest();
   }
 
+  @Test
+  public void ifChain_dominationWithTrueGuard_error() {
+    // Guard of `true` must be treated as unguarded, causing case to be pulled up
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            """
+            import java.lang.Number;
+
+            class Test {
+              public void foo(Suit s) {
+                Integer i = s == null ? 0 : 1;
+                if (i == 0) {
+                  System.out.println("It's 0!");
+                } else if (i instanceof Integer o && o.hashCode() == 17) {
+                  System.out.println("Its hashcode is 17");
+                } else if (i instanceof Integer in && (true)) {
+                  System.out.println("It's unguarded");
+                } else if (i == 23) {
+                  System.out.println("It's a 23");
+                } else if (i instanceof Integer in && i > 348) {
+                  System.out.println("It's a big integer!");
+                }
+              }
+            }
+            """)
+        .addOutputLines(
+            "Test.java",
+"""
+import java.lang.Number;
+
+class Test {
+  public void foo(Suit s) {
+    Integer i = s == null ? 0 : 1;
+    switch (i) {
+      case 0 -> System.out.println("It's 0!");
+      case Integer o when o.hashCode() == 17 -> System.out.println("Its hashcode is 17");
+      case 23 -> System.out.println("It's a 23");
+      case Integer in when i > 348 -> System.out.println("It's a big integer!");
+      case Integer in when (true) -> System.out.println("It's unguarded");
+    }
+  }
+}
+""")
+        .setArgs("-XepOpt:IfChainToSwitch:EnableMain")
+        .setFixChooser(IfChainToSwitchTest::assertOneFixAndChoose)
+        .doTest(TEXT_MATCH);
+  }
+
   @Test
   public void ifChain_javadocEnum_error() {
     refactoringHelper