Review comments applied

Author: Jenson3210

src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java

@@ -40,13 +40,13 @@
 public class IfElseIfConstructToSwitch extends Recipe {
     @Override
     public String getDisplayName() {
-        return "Use pattern matching in switch cases";
+        return "If-elseIf-else construct to switch";
     }
 
     @Override
     public String getDescription() {
-        return "Enhance the Java programming language with pattern matching for switch expressions and statements. " +
-                "Extending pattern matching to switch allows an expression to be tested against a number of patterns, each with a specific action, so that complex data-oriented queries can be expressed concisely and safely.";
+        return "Replace if-elseIf-else construct with switch statements. In order to be replaced with a switch, " +
+                "all conditions must be on the same variable and there must be at least 3 cases.";
     }
 
     @Override
@@ -100,50 +100,43 @@ private static class SwitchCandidate {
             }
         }
 
-        J.@Nullable If handleNullCheck(J.If ifPart, Cursor cursor) {
-            if (ifPart.getIfCondition().getTree() instanceof J.Binary) {
-                Optional<NullCheck> nullCheck = nullCheck().get(ifPart, cursor);
-                if (nullCheck.isPresent()) {
-                    nullCheckedParameter = nullCheck.get().getNullCheckedParameter();
-                    nullCheckedStatement = nullCheck.get().whenNull();
-                    Statement elsePart = nullCheck.get().whenNotNull();
-                    if (elsePart instanceof J.If) {
-                        ifPart = (J.If) elsePart;
-                    } else {
-                        elze = elsePart;
-                        ifPart = null;
-                    }
+        private J.@Nullable If handleNullCheck(J.If ifPart, Cursor cursor) {
+            Optional<NullCheck> nullCheck = nullCheck().get(ifPart, cursor);
+            if (nullCheck.isPresent()) {
+                nullCheckedParameter = nullCheck.get().getNullCheckedParameter();
+                nullCheckedStatement = nullCheck.get().whenNull();
+                Statement elsePart = nullCheck.get().whenNotNull();
+                if (elsePart instanceof J.If) {
+                    ifPart = (J.If) elsePart;
                 } else {
-                    noPotentialCandidate();
+                    elze = elsePart;
+                    ifPart = null;
                 }
-                return ifPart;
+            } else {
+                noPotentialCandidate();
             }
-            throw new IllegalArgumentException("Unsupported if type: " + ifPart.getIfCondition().getTree().getClass().getSimpleName());
+            return ifPart;
         }
 
-        J.@Nullable If handleInstanceOfCheck(J.If ifPart) {
-            if (ifPart.getIfCondition().getTree() instanceof J.InstanceOf) {
-                patternMatchers.put((J.InstanceOf) ifPart.getIfCondition().getTree(), ifPart.getThenPart());
-                J.If.Else elsePart = ifPart.getElsePart();
-                if (elsePart != null && elsePart.getBody() instanceof J.If) {
-                    ifPart = (J.If) elsePart.getBody();
-                } else {
-                    elze = elsePart != null ? elsePart.getBody() : null;
-                    ifPart = null;
-                }
-                return ifPart;
+        private J.@Nullable If handleInstanceOfCheck(J.If ifPart) {
+            patternMatchers.put((J.InstanceOf) ifPart.getIfCondition().getTree(), ifPart.getThenPart());
+            J.If.Else elsePart = ifPart.getElsePart();
+            if (elsePart != null && elsePart.getBody() instanceof J.If) {
+                ifPart = (J.If) elsePart.getBody();
+            } else {
+                elze = elsePart != null ? elsePart.getBody() : null;
+                ifPart = null;
             }
-            throw new IllegalArgumentException("Unsupported if type: " + ifPart.getIfCondition().getTree().getClass().getSimpleName());
+            return ifPart;
         }
 
         void noPotentialCandidate() {
             this.potentialCandidate = false;
         }
 
         boolean isValidCandidate() {
-            // all ifs in the chain must be on the same variable
+            // all ifs in the chain must be on the same variable in order to be a candidate for switch pattern matching
             if (potentialCandidate && patternMatchers.keySet().stream().map(J.InstanceOf::getExpression).map(expression -> ((J.Identifier) expression).getSimpleName()).distinct().count() != 1) {
-                // pattern matching in a switch can only happen if all if cases are on the same variable.
                 this.potentialCandidate = false;
                 return false;
             }

src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java

@@ -39,13 +39,14 @@
 public class NullCheckAsSwitchCase extends Recipe {
     @Override
     public String getDisplayName() {
-        return "Use pattern matching in switch cases";
+        return "Add null check to existing switch cases";
     }
 
     @Override
     public String getDescription() {
-        return "Enhance the Java programming language with pattern matching for switch expressions and statements. " +
-                "Extending pattern matching to switch allows an expression to be tested against a number of patterns, each with a specific action, so that complex data-oriented queries can be expressed concisely and safely.";
+        return "In later java versions, null checks are valid switch cases. " +
+                "This recipe will only add null checks to existing switch cases if there are no other statements in between them " +
+                "or if the block in the if statement is not impacting the flow of the switch.";
     }
 
     @Override

src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java

@@ -317,4 +317,36 @@ else if (obj instanceof Long) {
           )
         );
     }
+
+    @Test
+    void noSwitchBlockWithDifferentVariablesBeingChecked() {
+        rewriteRun(
+                //language=java
+                java(
+                        """
+                          class Test {
+                              static String formatter(Object obj) {
+                                  String formatted = "initialValue";
+                                  Object anotherObj = obj;
+                                  if (obj == null) {
+                                      formatted = "null";
+                                  } else if (obj instanceof Integer i)
+                                      formatted = String.format("int %d", i);
+                                  else if (obj instanceof Long l) {
+                                      formatted = String.format("long %d", l);
+                                  } else if (obj instanceof Double d) {
+                                      formatted = String.format("double %f", d);
+                                  } else if (anotherObj instanceof String s) {
+                                      String str = "String";
+                                      formatted = String.format("%s %s", str, s);
+                                  } else {
+                                      formatted = "unknown";
+                                  }
+                                  return formatted;
+                              }
+                          }
+                          """
+                )
+        );
+    }
 }