From c4a7f0c62e9880e58afd9be0e8e4142de897aa50 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Mon, 14 Jul 2025 14:54:10 -0400 Subject: [PATCH 01/46] refactor: extract helper method in util class --- .../migrate/lang/NullCheckAsSwitchCase.java | 32 +--- .../java/migrate/lang/SwitchUtils.java | 65 ++++++++ .../java/migrate/lang/SwitchUtilsTest.java | 146 ++++++++++++++++++ 3 files changed, 212 insertions(+), 31 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java create mode 100644 src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java b/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java index 9257b6feb6..86c13988d4 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java @@ -31,12 +31,12 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import static java.util.Objects.requireNonNull; import static org.openrewrite.java.migrate.lang.NullCheck.Matcher.nullCheck; +import static org.openrewrite.java.migrate.lang.SwitchUtils.coversAllPossibleValues; @EqualsAndHashCode(callSuper = false) @Value @@ -184,36 +184,6 @@ private J.Case createCaseStatement(J.Switch aSwitch, Statement whenNull, J.Case return nullCase.withStatements(ListUtils.mapFirst(nullCase.getStatements(), s -> s == null ? null : s.withPrefix(currentFirstCaseIndentation))); } - - private boolean coversAllPossibleValues(J.Switch switch_) { - List labels = new ArrayList<>(); - for (Statement statement : switch_.getCases().getStatements()) { - for (J j : ((J.Case) statement).getCaseLabels()) { - if (j instanceof J.Identifier && "default".equals(((J.Identifier) j).getSimpleName())) { - return true; - } - labels.add(j); - } - } - JavaType javaType = switch_.getSelector().getTree().getType(); - if (javaType instanceof JavaType.Class && ((JavaType.Class) javaType).getKind() == JavaType.FullyQualified.Kind.Enum) { - // Every enum value must be present in the switch - return ((JavaType.Class) javaType).getMembers().stream().allMatch(variable -> - labels.stream().anyMatch(label -> { - if (!(label instanceof TypeTree && TypeUtils.isOfType(((TypeTree) label).getType(), javaType))) { - return false; - } - J.Identifier enumName = null; - if (label instanceof J.Identifier) { - enumName = (J.Identifier) label; - } else if (label instanceof J.FieldAccess) { - enumName = ((J.FieldAccess) label).getName(); - } - return enumName != null && Objects.equals(variable.getName(), enumName.getSimpleName()); - })); - } - return false; - } }); } } diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java new file mode 100644 index 0000000000..8d22c6f99b --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -0,0 +1,65 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.lang; + +import org.openrewrite.java.tree.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +public class SwitchUtils { + /** + * Checks if a switch statement covers all possible values of its selector. + * This is typically used to determine if a switch statement is "exhaustive" as per the Java language specification. + * + * NOTE: Missing support for sealed classes/interfaces. + * + * @See Switch Expressions in Java 21 + * + * @param switch_ the switch statement to check + * @return true if the switch covers all possible values, false otherwise + */ + public static boolean coversAllPossibleValues(J.Switch switch_) { + List labels = new ArrayList<>(); + for (Statement statement : switch_.getCases().getStatements()) { + for (J j : ((J.Case) statement).getCaseLabels()) { + if (j instanceof J.Identifier && "default".equals(((J.Identifier) j).getSimpleName())) { + return true; + } + labels.add(j); + } + } + JavaType javaType = switch_.getSelector().getTree().getType(); + if (javaType instanceof JavaType.Class && ((JavaType.Class) javaType).getKind() == JavaType.FullyQualified.Kind.Enum) { + // Every enum value must be present in the switch + return ((JavaType.Class) javaType).getMembers().stream().allMatch(variable -> + labels.stream().anyMatch(label -> { + if (!(label instanceof TypeTree && TypeUtils.isOfType(((TypeTree) label).getType(), javaType))) { + return false; + } + J.Identifier enumName = null; + if (label instanceof J.Identifier) { + enumName = (J.Identifier) label; + } else if (label instanceof J.FieldAccess) { + enumName = ((J.FieldAccess) label).getName(); + } + return enumName != null && Objects.equals(variable.getName(), enumName.getSimpleName()); + })); + } + return false; + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java new file mode 100644 index 0000000000..726ade34f8 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java @@ -0,0 +1,146 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.lang; + +import org.intellij.lang.annotations.Language; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.openrewrite.ExecutionContext; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaParser; +import org.openrewrite.java.tree.J; + +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SwitchUtilsTest { + private static J.Switch getSwitchElement(@Language("java") String code) { + JavaParser parser = JavaParser.fromJavaVersion().build(); + J.CompilationUnit cu = (J.CompilationUnit) parser.parse(code).findFirst().get(); + + AtomicReference foundSwitch = new AtomicReference<>(); + new TreeVisitor() { + @Override + public J preVisit(J tree, ExecutionContext executionContext) { + if (foundSwitch.get() != null) { + return tree; + } + if (tree instanceof J.Switch sw) { + foundSwitch.set(sw); + } + return super.preVisit(tree, executionContext); + } + + + }.visit(cu, new InMemoryExecutionContext()); + + return foundSwitch.get(); + } + + @Test + void coversAllCasesAllEnums() { + assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + case GREEN -> System.out.println("go"); + } + } + enum TrafficLight { RED, YELLOW, GREEN } + } + """))); + } + + @Test + void coversAllCasesMissingEnums() { + assertFalse(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + } + } + enum TrafficLight { RED, YELLOW, GREEN } + } + """))); + } + + @Test + void coversAllCasesMissingEnumsWithDefault() { + assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + default -> System.out.println("unknown"); + } + } + enum TrafficLight { RED, YELLOW, GREEN } + } + """))); + } + + @Test + void coversAllCasesEnumOnlyDefault() { + assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + void method(TrafficLight light) { + switch (light) { + default -> System.out.println("unknown"); + } + } + enum TrafficLight { RED, YELLOW, GREEN } + } + """))); + } + + @Test + void coversAllCasesObjectOnlyDefault() { + assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + void method(Object obj) { + switch (obj) { + default -> System.out.println("default"); + } + } + } + """))); + } + + @Test + @Disabled("Unsupported yet") + void coversAllCasesAllSealedClasses() { + assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" + public class Test { + sealed abstract class Shape permits Circle, Square, Rectangle {} + void method(Shape shape) { + switch (shape) { + case Circle c -> System.out.println("circle"); + case Square s -> System.out.println("square"); + case Rectangle r -> System.out.println("rectangle"); + } + } + } + """))); + } +} From 09addfd0e8a650374452a78b0f8937a594b0af3e Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Mon, 14 Jul 2025 16:58:52 -0400 Subject: [PATCH 02/46] Tests --- ...chCaseAssigningToSwitchExpressionTest.java | 504 ++++++++++++++++++ 1 file changed, 504 insertions(+) create mode 100644 src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java new file mode 100644 index 0000000000..3151cd0789 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -0,0 +1,504 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.lang; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.version; + +class SwitchCaseAssigningToSwitchExpressionTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SwitchCaseAssigningToSwitchExpression()).allSources(source -> version(source, 21)); + } + + @DocumentExample + @Test + void convertSimpleArrowCasesAssignations() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> formatted = String.format("long %d", l); + default -> formatted = "unknown"; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> String.format("long %d", l); + default -> "unknown"; + }; + } + } + """)); + } + + @Test + void convertSimpleColonCasesAssignations() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: System.out.println("long"); formatted = String.format("long %d", l); break; + default: formatted = "unknown"; break; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i: yield String.format("int %d", i); + case Long l: { + System.out.println("long"); + yield String.format("long %d", l); + } + default: yield "unknown"; + }; + } + } + """)); + } + + @Test + void convertColonCasesSimpleAssignationInBlockToSingleYield() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: { + formatted = String.format("long %d", l); + break; + } + default: formatted = "unknown"; break; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i: yield String.format("int %d", i); + case Long l: yield String.format("long %d", l); + default: yield "unknown"; + }; + } + } + """)); + } + + @Test + void convertArrowCasesSimpleAssignationInBlockToSingleValue() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> { + formatted = String.format("long %d", l); + } + default -> formatted = "unknown"; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> String.format("long %d", l); + default -> "unknown"; + }; + } + } + """)); + } + + @Test + void convertArrowCasesAssignationInBlockToYieldInBlock() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> { + System.out.println("long!"); + formatted = String.format("long %d", l); + } + default -> formatted = "unknown"; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> { + System.out.println("long!"); + yield String.format("long %d", l); + } + default -> "unknown"; + }; + } + } + """)); + } + + @Test + void notConvertColonCasesWithMissingBreakStatement() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case null: formatted = "null"; + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertColonCasesWithMissingBreakStatementInBlock() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case null: { + System.out.println("null"); + formatted = "null"; + } + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertCasesWithMissingAssignment() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: System.out.println("Integer!"); break; + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertCasesWithAssignmentNotTheLastStatementOfStatementList() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: + formatted = String.format("Integer %d", i); + System.out.println("Integer!"); + break; + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertColonCasesWithAssignmentNotTheLastStatementOfBlock() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: { + formatted = String.format("Integer %d", i); + System.out.println("Integer!"); + break; + } + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertArrowCasesWithAssignmentNotTheLastStatementOfBlock() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s -> formatted = String.format("String %s", s); + case Integer i -> { + formatted = String.format("Integer %d", i); + System.out.println("Integer!"); + } + default: formatted = "unknown"; + } + } + } + """)); + } + + @Test + void notConvertCasesWithAssignmentToDifferentVariables() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + String formatted2 = "anotherInitialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: formatted2 = String.format("Integer %d", i); break; + default: formatted = "unknown"; break; + } + } + } + """)); + } + + @Test + void notConvertCasesWhenNonExhaustive() { + rewriteRun( + //language=java + java(""" + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + } + } + } + """)); + } + + @Test + void notConvertCasesWhenColonCaseHasNoStatementsAndNextCaseIsntAssignation() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String status = "initialValue"; + switch (obj) { + case null: + default: System.out.println("default"); break; + } + } + } + """)); + } + + @Test + void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignation() { + rewriteRun( + //language=java + java(""" + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: + case GREEN: + case YELLOW: status = "unsure"; break; + default: status = "unknown"; break; + } + } + } + """, """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: + case GREEN: + case YELLOW: yield "unsure"; + default: yield "unknown"; + }; + } + } + """)); + } + + // Is this always a correct thing to do? If yes we wouldn't need to check for exhaustiveness. + @Test + void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignationByAddedDefault() { + rewriteRun( + //language=java + java(""" + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: + case GREEN: + case YELLOW: status = "unsure"; break; + } + } + } + """, """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: + case GREEN: + case YELLOW: yield "unsure"; + default: yield "initialValue"; + }; + } + } + """)); + } + + // Is this always a correct thing to do? If yes we wouldn't need to check for exhaustiveness. + @Test + void convertCasesWithAddedDefault() { + rewriteRun( + //language=java + java(""" + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + } + } + } + """, """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: yield "stop"; + case GREEN: yield "go"; + default: yield "initialValue"; + }; + } + } + """)); + } + + @Test + void convertColonCasesWithMultipleBlocks() { + rewriteRun( + //language=java + java(""" + class Test { + void doFormat(Object obj) { + String status = "initialValue"; + switch (obj) { + case null: { + System.out.println("null case"); + } + status = "none"; + { + break; + } + default: status = "default status"; break; + } + } + } + """, """ + class Test { + void doFormat(Object obj) { + String status = switch (obj) { + case null: { + System.out.println("null case"); + } + yield "none"; + default: yield "default status"; + }; + } + } + """)); + } +} From c605a3673895f9dc03f39536bbd9e10be38082c1 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Wed, 16 Jul 2025 13:10:06 -0400 Subject: [PATCH 03/46] Cleaned up tests --- ...chCaseAssigningToSwitchExpressionTest.java | 221 ++++-------------- 1 file changed, 51 insertions(+), 170 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 3151cd0789..7b677778b3 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -68,7 +68,7 @@ void doFormat(Object obj) { String formatted = "initialValue"; switch (obj) { case Integer i: formatted = String.format("int %d", i); break; - case Long l: System.out.println("long"); formatted = String.format("long %d", l); break; + case Long l: formatted = String.format("long %d", l); break; default: formatted = "unknown"; break; } } @@ -78,10 +78,7 @@ class Test { void doFormat(Object obj) { String formatted = switch (obj) { case Integer i: yield String.format("int %d", i); - case Long l: { - System.out.println("long"); - yield String.format("long %d", l); - } + case Long l: yield String.format("long %d", l); default: yield "unknown"; }; } @@ -90,7 +87,8 @@ void doFormat(Object obj) { } @Test - void convertColonCasesSimpleAssignationInBlockToSingleYield() { + void notConvertSimpleColonCasesAssignationsWithExtraCodeInBlock() { + // Only one statement [+break;] per case is currently supported rewriteRun( //language=java java(""" @@ -99,29 +97,16 @@ void doFormat(Object obj) { String formatted = "initialValue"; switch (obj) { case Integer i: formatted = String.format("int %d", i); break; - case Long l: { - formatted = String.format("long %d", l); - break; - } + case Long l: System.out.println("long"); formatted = String.format("long %d", l); break; default: formatted = "unknown"; break; } } } - """, """ - class Test { - void doFormat(Object obj) { - String formatted = switch (obj) { - case Integer i: yield String.format("int %d", i); - case Long l: yield String.format("long %d", l); - default: yield "unknown"; - }; - } - } """)); } @Test - void convertArrowCasesSimpleAssignationInBlockToSingleValue() { + void convertColonCasesSimpleAssignationInBlockToSingleYield() { rewriteRun( //language=java java(""" @@ -129,11 +114,12 @@ class Test { void doFormat(Object obj) { String formatted = "initialValue"; switch (obj) { - case Integer i -> formatted = String.format("int %d", i); - case Long l -> { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: { formatted = String.format("long %d", l); + break; } - default -> formatted = "unknown"; + default: formatted = "unknown"; break; } } } @@ -141,9 +127,9 @@ void doFormat(Object obj) { class Test { void doFormat(Object obj) { String formatted = switch (obj) { - case Integer i -> String.format("int %d", i); - case Long l -> String.format("long %d", l); - default -> "unknown"; + case Integer i: yield String.format("int %d", i); + case Long l: yield String.format("long %d", l); + default: yield "unknown"; }; } } @@ -151,7 +137,7 @@ void doFormat(Object obj) { } @Test - void convertArrowCasesAssignationInBlockToYieldInBlock() { + void convertArrowCasesSimpleAssignationInBlockToSingleValue() { rewriteRun( //language=java java(""" @@ -161,7 +147,6 @@ void doFormat(Object obj) { switch (obj) { case Integer i -> formatted = String.format("int %d", i); case Long l -> { - System.out.println("long!"); formatted = String.format("long %d", l); } default -> formatted = "unknown"; @@ -173,10 +158,7 @@ class Test { void doFormat(Object obj) { String formatted = switch (obj) { case Integer i -> String.format("int %d", i); - case Long l -> { - System.out.println("long!"); - yield String.format("long %d", l); - } + case Long l -> String.format("long %d", l); default -> "unknown"; }; } @@ -184,45 +166,6 @@ void doFormat(Object obj) { """)); } - @Test - void notConvertColonCasesWithMissingBreakStatement() { - rewriteRun( - //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case null: formatted = "null"; - default: formatted = "unknown"; break; - } - } - } - """)); - } - - @Test - void notConvertColonCasesWithMissingBreakStatementInBlock() { - rewriteRun( - //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case null: { - System.out.println("null"); - formatted = "null"; - } - default: formatted = "unknown"; break; - } - } - } - """)); - } - @Test void notConvertCasesWithMissingAssignment() { rewriteRun( @@ -241,70 +184,6 @@ void doFormat(Object obj) { """)); } - @Test - void notConvertCasesWithAssignmentNotTheLastStatementOfStatementList() { - rewriteRun( - //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case Integer i: - formatted = String.format("Integer %d", i); - System.out.println("Integer!"); - break; - default: formatted = "unknown"; break; - } - } - } - """)); - } - - @Test - void notConvertColonCasesWithAssignmentNotTheLastStatementOfBlock() { - rewriteRun( - //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case Integer i: { - formatted = String.format("Integer %d", i); - System.out.println("Integer!"); - break; - } - default: formatted = "unknown"; break; - } - } - } - """)); - } - - @Test - void notConvertArrowCasesWithAssignmentNotTheLastStatementOfBlock() { - rewriteRun( - //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s -> formatted = String.format("String %s", s); - case Integer i -> { - formatted = String.format("Integer %d", i); - System.out.println("Integer!"); - } - default: formatted = "unknown"; - } - } - } - """)); - } - @Test void notConvertCasesWithAssignmentToDifferentVariables() { rewriteRun( @@ -324,26 +203,6 @@ void doFormat(Object obj) { """)); } - @Test - void notConvertCasesWhenNonExhaustive() { - rewriteRun( - //language=java - java(""" - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: status = "stop"; break; - case GREEN: status = "go"; break; - } - } - } - """)); - } - @Test void notConvertCasesWhenColonCaseHasNoStatementsAndNextCaseIsntAssignation() { rewriteRun( @@ -397,7 +256,6 @@ void doFormat(TrafficLight light) { """)); } - // Is this always a correct thing to do? If yes we wouldn't need to check for exhaustiveness. @Test void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignationByAddedDefault() { rewriteRun( @@ -426,14 +284,13 @@ void doFormat(TrafficLight light) { case RED: case GREEN: case YELLOW: yield "unsure"; - default: yield "initialValue"; + default: yield"initialValue"; }; } } """)); } - // Is this always a correct thing to do? If yes we wouldn't need to check for exhaustiveness. @Test void convertCasesWithAddedDefault() { rewriteRun( @@ -460,7 +317,7 @@ void doFormat(TrafficLight light) { String status = switch (light) { case RED: yield "stop"; case GREEN: yield "go"; - default: yield "initialValue"; + default: yield"initialValue"; }; } } @@ -468,7 +325,8 @@ void doFormat(TrafficLight light) { } @Test - void convertColonCasesWithMultipleBlocks() { + void notConvertColonCasesWithMultipleBlocks() { + // More than one block statement per case is not yet supported. rewriteRun( //language=java java(""" @@ -477,9 +335,8 @@ void doFormat(Object obj) { String status = "initialValue"; switch (obj) { case null: { - System.out.println("null case"); + status = "none"; } - status = "none"; { break; } @@ -487,15 +344,39 @@ void doFormat(Object obj) { } } } + """)); + } + + void convertCasesInstanceVariableAssignment() { + rewriteRun( + //language=java + java(""" + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + private String status = "initialValue"; + + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: this.status = "stop"; break; + case GREEN: this.status = "go"; break; + } + } + } """, """ class Test { - void doFormat(Object obj) { - String status = switch (obj) { - case null: { - System.out.println("null case"); - } - yield "none"; - default: yield "default status"; + enum TrafficLight { + RED, GREEN, YELLOW + } + private String status = "initialValue"; + + void doFormat(TrafficLight light) { + this.status = switch (light) { + case RED: yield "stop"; + case GREEN: yield "go"; + default: yield "initialValue"; }; } } From 8f52dd60af2c9436f1b96f2eadf88cec6e6af61e Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Wed, 16 Jul 2025 13:11:23 -0400 Subject: [PATCH 04/46] WIP - Working but yet to be cleaned up --- ...SwitchCaseAssigningToSwitchExpression.java | 197 ++++++++++++++++++ .../META-INF/rewrite/java-version-21.yml | 1 + 2 files changed, 198 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java new file mode 100644 index 0000000000..f6141527b8 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -0,0 +1,197 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.lang; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.*; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.marker.Markers; +import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; +import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Collections.singletonList; +import static org.openrewrite.Tree.randomId; + + +@Value +@EqualsAndHashCode(callSuper = false) +public class SwitchCaseAssigningToSwitchExpression extends Recipe { + @Override + public String getDisplayName() { + return "Convert assigning Switch statements to Switch expressions"; + } + + @Override + public String getDescription() { + return "Switch statements for which each case is assigning a value to the same variable can be converted to a switch expression that returns the value of the variable. " + + "This is only applicable for Java 21 and later, as switch expressions were introduced in Java 12, but this recipe requires the `yield` keyword."; + } + + @Override + public TreeVisitor getVisitor() { + TreeVisitor preconditions = Preconditions.and( + new UsesJavaVersion<>(21), + Preconditions.not(new KotlinFileChecker<>()), // necessary ? + Preconditions.not(new GroovyFileChecker<>()) // necessary ? + ); + + return Preconditions.check(preconditions, new JavaVisitor() { + @Override + public J visitBlock(J.Block block, ExecutionContext ctx) { + AtomicReference originalSwitch = new AtomicReference<>(); + + J.Block b = block.withStatements(ListUtils.map(block.getStatements(), (index, statement) -> { + if (statement == originalSwitch.getAndSet(null)) { + // We've already converted the switch/assignments to an assignment with a switch expression. + return null; + } + + if (index < block.getStatements().size() - 1 && + statement instanceof J.VariableDeclarations && + ((J.VariableDeclarations) statement).getVariables().size() == 1 && + block.getStatements().get(index + 1) instanceof J.Switch + ) { + J.VariableDeclarations vd = (J.VariableDeclarations) statement; + J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); + Optional newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, vd.getVariables().get(0), ctx); + if (newSwitchExpression.isPresent()) { + originalSwitch.set(nextStatementSwitch); + return vd.withVariables(singletonList(vd.getVariables().get(0).withInitializer(newSwitchExpression.get()))); + } + } + return statement; + })); + return super.visitBlock(b, ctx); + } + + private Optional buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable, ExecutionContext ctx) { + final String variableName = originalVariable.getSimpleName(); + AtomicBoolean isUnqualified = new AtomicBoolean(); + AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); + AtomicBoolean isUsingArrows = new AtomicBoolean(true); + + List updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), s -> { + if (isUnqualified.get()) { + return null; + } + + J.Case caseItem = (J.Case) s; + + if (caseItem != null && caseItem.getCaseLabels().get(0) instanceof J.Identifier && ((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName().equals("default")) { + isDefaultCaseAbsent.set(false); + } + + if (caseItem == null) { + return null; + } else if (caseItem.getBody() != null) { // arrow cases + if (caseItem.getBody() instanceof J.Block) { + J.Block block = (J.Block) caseItem.getBody(); + if (block.getStatements().size() == 1 && block.getStatements().get(0) instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) block.getStatements().get(0); + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName)) { + return caseItem.withBody(assignment.getAssignment()); + } + } + } + + } else if (caseItem.getBody() instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) caseItem.getBody(); + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName)) { + return caseItem.withBody(assignment.getAssignment()); + } + } + } + } else { // colon cases + isUsingArrows.set(false); + List caseStatements = caseItem.getStatements(); + if (caseStatements.isEmpty()) { + return caseItem; + } + + if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { + caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); + } + + if (caseStatements.size() == 2 && + caseStatements.get(0) instanceof J.Assignment && + caseStatements.get(1) instanceof J.Break) { + J.Assignment assignment = (J.Assignment) caseStatements.get(0); + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName)) { + J.Yield yieldStatement = new J.Yield( + randomId(), + assignment.getPrefix().withWhitespace(" "), // TODO: must be a better way to adjust the formatting when taken from a J.Block, see test convertColonCasesSimpleAssignationInBlockToSingleYield() + Markers.EMPTY, + false, + assignment.getAssignment() + ); + return caseItem.withStatements(singletonList(yieldStatement)); + } + } + } + } + + isUnqualified.set(true); + return null; + }); + if (isUnqualified.get()) { + return Optional.empty(); + } + + List statements = new ArrayList<>(); + statements.add(originalSwitch.getSelector().getTree()); + statements.add(originalVariable.getInitializer().withPrefix(Space.SINGLE_SPACE)); + + StringBuilder template = new StringBuilder( + "Object o = switch (#{any()}) {\n" + + (isUsingArrows.get() ? "default ->" : " default: yield") + " #{any()};\n" + + "}"); + J.VariableDeclarations vd = JavaTemplate.apply( + template.toString(), + new Cursor(getCursor(), originalSwitch), + originalSwitch.getCoordinates().replace(), //right coordinates? We don't want to replace the switch, we modify it and move it as an assignement one line up. + statements.toArray() + ); + + J.SwitchExpression initializer = (J.SwitchExpression) Objects.requireNonNull(vd.getVariables().get(0).getInitializer()); + if (isDefaultCaseAbsent.get()) { + updatedCases.add(initializer.getCases().getStatements().get(0)); + } + return Optional.of(initializer.withCases(initializer.getCases().withStatements(updatedCases))); + } + } + ); + } +} diff --git a/src/main/resources/META-INF/rewrite/java-version-21.yml b/src/main/resources/META-INF/rewrite/java-version-21.yml index 653d2a8d93..98ab2b94a3 100644 --- a/src/main/resources/META-INF/rewrite/java-version-21.yml +++ b/src/main/resources/META-INF/rewrite/java-version-21.yml @@ -145,3 +145,4 @@ recipeList: - org.openrewrite.java.migrate.lang.IfElseIfConstructToSwitch - org.openrewrite.java.migrate.lang.RefineSwitchCases - org.openrewrite.java.migrate.lang.SwitchCaseEnumGuardToLabel + - org.openrewrite.java.migrate.lang.SwitchCaseAssigningToSwitchExpression From 790fafe3b18236a31b7b227cd4f5a0fb4b9c527f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 12:17:34 +0200 Subject: [PATCH 05/46] Apply best practices to avoid repeated bot suggestions --- .../migrate/lang/NullCheckAsSwitchCase.java | 5 +- .../resources/META-INF/rewrite/examples.yml | 28 + ...chCaseAssigningToSwitchExpressionTest.java | 579 ++++++++++-------- 3 files changed, 339 insertions(+), 273 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java b/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java index 86c13988d4..aa2226fd7d 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/NullCheckAsSwitchCase.java @@ -24,7 +24,10 @@ import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.SemanticallyEqual; import org.openrewrite.java.search.UsesJavaVersion; -import org.openrewrite.java.tree.*; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 1af0177a1d..951bcb4e13 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -6235,6 +6235,34 @@ examples: language: java --- type: specs.openrewrite.org/v1beta/example +recipeName: org.openrewrite.java.migrate.lang.SwitchCaseAssigningToSwitchExpression +examples: +- description: '' + sources: + - before: | + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> formatted = String.format("long %d", l); + default -> formatted = "unknown"; + } + } + } + after: | + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> String.format("long %d", l); + default -> "unknown"; + }; + } + } + language: java +--- +type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.java.migrate.lang.SwitchCaseEnumGuardToLabel examples: - description: '' diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 7b677778b3..320cb34d09 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -15,7 +15,9 @@ */ package org.openrewrite.java.migrate.lang; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -34,56 +36,62 @@ public void defaults(RecipeSpec spec) { void convertSimpleArrowCasesAssignations() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case Integer i -> formatted = String.format("int %d", i); - case Long l -> formatted = String.format("long %d", l); - default -> formatted = "unknown"; - } - } - } - """, """ - class Test { - void doFormat(Object obj) { - String formatted = switch (obj) { - case Integer i -> String.format("int %d", i); - case Long l -> String.format("long %d", l); - default -> "unknown"; - }; - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> formatted = String.format("long %d", l); + default -> formatted = "unknown"; + } + } + } + """, + """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> String.format("long %d", l); + default -> "unknown"; + }; + } + } + """ + )); } @Test void convertSimpleColonCasesAssignations() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case Integer i: formatted = String.format("int %d", i); break; - case Long l: formatted = String.format("long %d", l); break; - default: formatted = "unknown"; break; - } - } - } - """, """ - class Test { - void doFormat(Object obj) { - String formatted = switch (obj) { - case Integer i: yield String.format("int %d", i); - case Long l: yield String.format("long %d", l); - default: yield "unknown"; - }; - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: formatted = String.format("long %d", l); break; + default: formatted = "unknown"; break; + } + } + } + """, + """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i: yield String.format("int %d", i); + case Long l: yield String.format("long %d", l); + default: yield "unknown"; + }; + } + } + """ + )); } @Test @@ -91,237 +99,260 @@ void notConvertSimpleColonCasesAssignationsWithExtraCodeInBlock() { // Only one statement [+break;] per case is currently supported rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case Integer i: formatted = String.format("int %d", i); break; - case Long l: System.out.println("long"); formatted = String.format("long %d", l); break; - default: formatted = "unknown"; break; - } - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: System.out.println("long"); formatted = String.format("long %d", l); break; + default: formatted = "unknown"; break; + } + } + } + """ + )); } @Test void convertColonCasesSimpleAssignationInBlockToSingleYield() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case Integer i: formatted = String.format("int %d", i); break; - case Long l: { - formatted = String.format("long %d", l); - break; - } - default: formatted = "unknown"; break; - } - } - } - """, """ - class Test { - void doFormat(Object obj) { - String formatted = switch (obj) { - case Integer i: yield String.format("int %d", i); - case Long l: yield String.format("long %d", l); - default: yield "unknown"; - }; - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: { + formatted = String.format("long %d", l); + break; + } + default: formatted = "unknown"; break; + } + } + } + """, + """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i: yield String.format("int %d", i); + case Long l: yield String.format("long %d", l); + default: yield "unknown"; + }; + } + } + """ + )); } @Test void convertArrowCasesSimpleAssignationInBlockToSingleValue() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case Integer i -> formatted = String.format("int %d", i); - case Long l -> { - formatted = String.format("long %d", l); - } - default -> formatted = "unknown"; - } - } - } - """, """ - class Test { - void doFormat(Object obj) { - String formatted = switch (obj) { - case Integer i -> String.format("int %d", i); - case Long l -> String.format("long %d", l); - default -> "unknown"; - }; - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i -> formatted = String.format("int %d", i); + case Long l -> { + formatted = String.format("long %d", l); + } + default -> formatted = "unknown"; + } + } + } + """, + """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i -> String.format("int %d", i); + case Long l -> String.format("long %d", l); + default -> "unknown"; + }; + } + } + """ + )); } @Test void notConvertCasesWithMissingAssignment() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case Integer i: System.out.println("Integer!"); break; - default: formatted = "unknown"; break; - } - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: System.out.println("Integer!"); break; + default: formatted = "unknown"; break; + } + } + } + """ + )); } @Test void notConvertCasesWithAssignmentToDifferentVariables() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String formatted = "initialValue"; - String formatted2 = "anotherInitialValue"; - switch (obj) { - case String s: formatted = String.format("String %s", s); break; - case Integer i: formatted2 = String.format("Integer %d", i); break; - default: formatted = "unknown"; break; - } - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + String formatted2 = "anotherInitialValue"; + switch (obj) { + case String s: formatted = String.format("String %s", s); break; + case Integer i: formatted2 = String.format("Integer %d", i); break; + default: formatted = "unknown"; break; + } + } + } + """ + )); } @Test void notConvertCasesWhenColonCaseHasNoStatementsAndNextCaseIsntAssignation() { rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String status = "initialValue"; - switch (obj) { - case null: - default: System.out.println("default"); break; - } - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String status = "initialValue"; + switch (obj) { + case null: + default: System.out.println("default"); break; + } + } + } + """ + )); } @Test void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignation() { rewriteRun( //language=java - java(""" - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: - case GREEN: - case YELLOW: status = "unsure"; break; - default: status = "unknown"; break; - } - } - } - """, """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = switch (light) { - case RED: - case GREEN: - case YELLOW: yield "unsure"; - default: yield "unknown"; - }; - } - } - """)); + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: + case GREEN: + case YELLOW: status = "unsure"; break; + default: status = "unknown"; break; + } + } + } + """, + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: + case GREEN: + case YELLOW: yield "unsure"; + default: yield "unknown"; + }; + } + } + """ + )); } @Test void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignationByAddedDefault() { rewriteRun( //language=java - java(""" - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: - case GREEN: - case YELLOW: status = "unsure"; break; - } - } - } - """, """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = switch (light) { - case RED: - case GREEN: - case YELLOW: yield "unsure"; - default: yield"initialValue"; - }; - } - } - """)); + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: + case GREEN: + case YELLOW: status = "unsure"; break; + } + } + } + """, + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: + case GREEN: + case YELLOW: yield "unsure"; + default: yield"initialValue"; + }; + } + } + """ + )); } @Test void convertCasesWithAddedDefault() { rewriteRun( //language=java - java(""" - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: status = "stop"; break; - case GREEN: status = "go"; break; - } - } - } - """, """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - void doFormat(TrafficLight light) { - String status = switch (light) { - case RED: yield "stop"; - case GREEN: yield "go"; - default: yield"initialValue"; - }; - } - } - """)); + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + } + } + } + """, + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: yield "stop"; + case GREEN: yield "go"; + default: yield"initialValue"; + }; + } + } + """ + )); } @Test @@ -329,57 +360,61 @@ void notConvertColonCasesWithMultipleBlocks() { // More than one block statement per case is not yet supported. rewriteRun( //language=java - java(""" - class Test { - void doFormat(Object obj) { - String status = "initialValue"; - switch (obj) { - case null: { - status = "none"; - } - { - break; - } - default: status = "default status"; break; - } - } - } - """)); + java( + """ + class Test { + void doFormat(Object obj) { + String status = "initialValue"; + switch (obj) { + case null: { + status = "none"; + } + { + break; + } + default: status = "default status"; break; + } + } + } + """ + )); } + @Test void convertCasesInstanceVariableAssignment() { rewriteRun( //language=java - java(""" - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - private String status = "initialValue"; + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: this.status = "stop"; break; - case GREEN: this.status = "go"; break; - } - } - } - """, """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - private String status = "initialValue"; + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + } + } + } + """, + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } - void doFormat(TrafficLight light) { - this.status = switch (light) { - case RED: yield "stop"; - case GREEN: yield "go"; - default: yield "initialValue"; - }; - } - } - """)); + void doFormat(TrafficLight light) { + String status = switch (light) { + case RED: yield "stop"; + case GREEN: yield "go"; + default: yield "initialValue"; + }; + } + } + """ + )); } } From d8769bc0f62ccdd01308f506c2cd4a9c442bf793 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 12:32:28 +0200 Subject: [PATCH 06/46] Use `reduce` and `visitSwitch` to extract Switch element --- .../java/migrate/lang/SwitchUtilsTest.java | 190 ++++++++++-------- 1 file changed, 106 insertions(+), 84 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java index 726ade34f8..82be2b0882 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java @@ -16,11 +16,9 @@ package org.openrewrite.java.migrate.lang; import org.intellij.lang.annotations.Language; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.openrewrite.ExecutionContext; -import org.openrewrite.InMemoryExecutionContext; -import org.openrewrite.TreeVisitor; +import org.junitpioneer.jupiter.ExpectedToFail; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaParser; import org.openrewrite.java.tree.J; @@ -30,117 +28,141 @@ import static org.junit.jupiter.api.Assertions.assertTrue; class SwitchUtilsTest { - private static J.Switch getSwitchElement(@Language("java") String code) { - JavaParser parser = JavaParser.fromJavaVersion().build(); - J.CompilationUnit cu = (J.CompilationUnit) parser.parse(code).findFirst().get(); - - AtomicReference foundSwitch = new AtomicReference<>(); - new TreeVisitor() { + private static J.Switch extractSwitch(@Language("java") String code) { + J.CompilationUnit cu = (J.CompilationUnit) JavaParser.fromJavaVersion().build().parse(code).findFirst().get(); + return new JavaIsoVisitor>() { @Override - public J preVisit(J tree, ExecutionContext executionContext) { - if (foundSwitch.get() != null) { - return tree; - } - if (tree instanceof J.Switch sw) { - foundSwitch.set(sw); - } - return super.preVisit(tree, executionContext); + public J.Switch visitSwitch(J.Switch _switch, AtomicReference switchAtomicReference) { + switchAtomicReference.set(_switch); + return _switch; } - - - }.visit(cu, new InMemoryExecutionContext()); - - return foundSwitch.get(); + }.reduce(cu, new AtomicReference<>()).get(); } @Test void coversAllCasesAllEnums() { - assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); - case GREEN -> System.out.println("go"); + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + case GREEN -> System.out.println("go"); + } } - } - enum TrafficLight { RED, YELLOW, GREEN } - } - """))); + enum TrafficLight { RED, YELLOW, GREEN } + } + """ + ) + ) + ); } @Test void coversAllCasesMissingEnums() { - assertFalse(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); + assertFalse( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + } } - } - enum TrafficLight { RED, YELLOW, GREEN } - } - """))); + enum TrafficLight { RED, YELLOW, GREEN } + } + """ + ) + ) + ); } @Test void coversAllCasesMissingEnumsWithDefault() { - assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); - default -> System.out.println("unknown"); + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + void method(TrafficLight light) { + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + default -> System.out.println("unknown"); + } } - } - enum TrafficLight { RED, YELLOW, GREEN } - } - """))); + enum TrafficLight { RED, YELLOW, GREEN } + } + """ + ) + ) + ); } @Test void coversAllCasesEnumOnlyDefault() { - assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - void method(TrafficLight light) { - switch (light) { - default -> System.out.println("unknown"); + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + void method(TrafficLight light) { + switch (light) { + default -> System.out.println("unknown"); + } } - } - enum TrafficLight { RED, YELLOW, GREEN } - } - """))); + enum TrafficLight { RED, YELLOW, GREEN } + } + """ + ) + ) + ); } @Test void coversAllCasesObjectOnlyDefault() { - assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - void method(Object obj) { - switch (obj) { - default -> System.out.println("default"); + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + void method(Object obj) { + switch (obj) { + default -> System.out.println("default"); + } } - } - } - """))); + } + """ + ) + ) + ); } + @ExpectedToFail("Not implemented yet for sealed classes") @Test - @Disabled("Unsupported yet") void coversAllCasesAllSealedClasses() { - assertTrue(SwitchUtils.coversAllPossibleValues(getSwitchElement(""" - public class Test { - sealed abstract class Shape permits Circle, Square, Rectangle {} - void method(Shape shape) { - switch (shape) { - case Circle c -> System.out.println("circle"); - case Square s -> System.out.println("square"); - case Rectangle r -> System.out.println("rectangle"); + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + sealed abstract class Shape permits Circle, Square, Rectangle {} + void method(Shape shape) { + switch (shape) { + case Circle c -> System.out.println("circle"); + case Square s -> System.out.println("square"); + case Rectangle r -> System.out.println("rectangle"); + } } - } - } - """))); + } + """ + ) + ) + ); } } From 5f1c4a3d95bd47aaa25a2faa6dbe3197187e9a62 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 12:33:15 +0200 Subject: [PATCH 07/46] Fix six space indentation --- .../java/migrate/lang/SwitchUtilsTest.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java index 82be2b0882..8c3697c8a8 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java @@ -47,11 +47,11 @@ void coversAllCasesAllEnums() { """ class Test { void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); - case GREEN -> System.out.println("go"); - } + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + case GREEN -> System.out.println("go"); + } } enum TrafficLight { RED, YELLOW, GREEN } } @@ -69,10 +69,10 @@ void coversAllCasesMissingEnums() { """ class Test { void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); - } + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + } } enum TrafficLight { RED, YELLOW, GREEN } } @@ -90,11 +90,11 @@ void coversAllCasesMissingEnumsWithDefault() { """ class Test { void method(TrafficLight light) { - switch (light) { - case RED -> System.out.println("stop"); - case YELLOW -> System.out.println("caution"); - default -> System.out.println("unknown"); - } + switch (light) { + case RED -> System.out.println("stop"); + case YELLOW -> System.out.println("caution"); + default -> System.out.println("unknown"); + } } enum TrafficLight { RED, YELLOW, GREEN } } @@ -112,9 +112,9 @@ void coversAllCasesEnumOnlyDefault() { """ class Test { void method(TrafficLight light) { - switch (light) { - default -> System.out.println("unknown"); - } + switch (light) { + default -> System.out.println("unknown"); + } } enum TrafficLight { RED, YELLOW, GREEN } } @@ -132,9 +132,9 @@ void coversAllCasesObjectOnlyDefault() { """ class Test { void method(Object obj) { - switch (obj) { - default -> System.out.println("default"); - } + switch (obj) { + default -> System.out.println("default"); + } } } """ @@ -153,11 +153,11 @@ void coversAllCasesAllSealedClasses() { class Test { sealed abstract class Shape permits Circle, Square, Rectangle {} void method(Shape shape) { - switch (shape) { - case Circle c -> System.out.println("circle"); - case Square s -> System.out.println("square"); - case Rectangle r -> System.out.println("rectangle"); - } + switch (shape) { + case Circle c -> System.out.println("circle"); + case Square s -> System.out.println("square"); + case Rectangle r -> System.out.println("rectangle"); + } } } """ From 4e56657ea7c9330a5ac723ea80c447295a8bd226 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 12:34:11 +0200 Subject: [PATCH 08/46] Reduce visibility of SwitchUtils --- .../java/org/openrewrite/java/migrate/lang/SwitchUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index 8d22c6f99b..0ab2e07aaa 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.Objects; -public class SwitchUtils { +class SwitchUtils { /** * Checks if a switch statement covers all possible values of its selector. * This is typically used to determine if a switch statement is "exhaustive" as per the Java language specification. From 56d9ba10876152440da31d5687266c198eb544b4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 12:35:28 +0200 Subject: [PATCH 09/46] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 320cb34d09..270872b770 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -15,9 +15,7 @@ */ package org.openrewrite.java.migrate.lang; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; From ccb54c241fc9d8b7acc170ded99df6c93b292841 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Thu, 17 Jul 2025 10:42:14 -0400 Subject: [PATCH 10/46] fix tests to work with updated rewrite-java --- .../migrate/lang/SwitchCaseAssigningToSwitchExpression.java | 2 +- .../lang/SwitchCaseAssigningToSwitchExpressionTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index f6141527b8..f611261cd5 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -176,7 +176,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS StringBuilder template = new StringBuilder( "Object o = switch (#{any()}) {\n" + - (isUsingArrows.get() ? "default ->" : " default: yield") + " #{any()};\n" + + "default" + (isUsingArrows.get() ? " ->" : ": yield") + " #{any()};\n" + "}"); J.VariableDeclarations vd = JavaTemplate.apply( template.toString(), diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 270872b770..2e2ef46fea 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -309,7 +309,7 @@ void doFormat(TrafficLight light) { case RED: case GREEN: case YELLOW: yield "unsure"; - default: yield"initialValue"; + default: yield "initialValue"; }; } } @@ -345,7 +345,7 @@ void doFormat(TrafficLight light) { String status = switch (light) { case RED: yield "stop"; case GREEN: yield "go"; - default: yield"initialValue"; + default: yield "initialValue"; }; } } From cfaa216036287aaec2c8717d78007bc259c1e043 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Thu, 17 Jul 2025 13:58:43 -0400 Subject: [PATCH 11/46] Java 17 precondition --- ...SwitchCaseAssigningToSwitchExpression.java | 4 +- ...chCaseAssigningToSwitchExpressionTest.java | 38 ------------------- 2 files changed, 2 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index f611261cd5..56ee6312e9 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -51,13 +51,13 @@ public String getDisplayName() { @Override public String getDescription() { return "Switch statements for which each case is assigning a value to the same variable can be converted to a switch expression that returns the value of the variable. " + - "This is only applicable for Java 21 and later, as switch expressions were introduced in Java 12, but this recipe requires the `yield` keyword."; + "This is only applicable for Java 17 and later."; } @Override public TreeVisitor getVisitor() { TreeVisitor preconditions = Preconditions.and( - new UsesJavaVersion<>(21), + new UsesJavaVersion<>(17), Preconditions.not(new KotlinFileChecker<>()), // necessary ? Preconditions.not(new GroovyFileChecker<>()) // necessary ? ); diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 2e2ef46fea..a943e9fa03 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -377,42 +377,4 @@ void doFormat(Object obj) { """ )); } - - @Test - void convertCasesInstanceVariableAssignment() { - rewriteRun( - //language=java - java( - """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - - void doFormat(TrafficLight light) { - String status = "initialValue"; - switch (light) { - case RED: status = "stop"; break; - case GREEN: status = "go"; break; - } - } - } - """, - """ - class Test { - enum TrafficLight { - RED, GREEN, YELLOW - } - - void doFormat(TrafficLight light) { - String status = switch (light) { - case RED: yield "stop"; - case GREEN: yield "go"; - default: yield "initialValue"; - }; - } - } - """ - )); - } } From 750fe6b493b741d8c279d97b2850a5815d534f98 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Thu, 17 Jul 2025 14:18:39 -0400 Subject: [PATCH 12/46] Don't add a `default` case if the switch is already exhaustive --- ...SwitchCaseAssigningToSwitchExpression.java | 6 +- ...chCaseAssigningToSwitchExpressionTest.java | 67 +++++++++---------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 56ee6312e9..783f005946 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -80,7 +80,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { ) { J.VariableDeclarations vd = (J.VariableDeclarations) statement; J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); - Optional newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, vd.getVariables().get(0), ctx); + Optional newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, vd.getVariables().get(0)); if (newSwitchExpression.isPresent()) { originalSwitch.set(nextStatementSwitch); return vd.withVariables(singletonList(vd.getVariables().get(0).withInitializer(newSwitchExpression.get()))); @@ -91,7 +91,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { return super.visitBlock(b, ctx); } - private Optional buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable, ExecutionContext ctx) { + private Optional buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { final String variableName = originalVariable.getSimpleName(); AtomicBoolean isUnqualified = new AtomicBoolean(); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); @@ -186,7 +186,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS ); J.SwitchExpression initializer = (J.SwitchExpression) Objects.requireNonNull(vd.getVariables().get(0).getInitializer()); - if (isDefaultCaseAbsent.get()) { + if (isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch)) { updatedCases.add(initializer.getCases().getStatements().get(0)); } return Optional.of(initializer.withCases(initializer.getCases().withStatements(updatedCases))); diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index a943e9fa03..fa369a9b45 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -280,7 +280,7 @@ void doFormat(TrafficLight light) { } @Test - void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignationByAddedDefault() { + void convertCasesWithAddedDefault() { rewriteRun( //language=java java( @@ -292,9 +292,8 @@ enum TrafficLight { void doFormat(TrafficLight light) { String status = "initialValue"; switch (light) { - case RED: - case GREEN: - case YELLOW: status = "unsure"; break; + case RED: status = "stop"; break; + case GREEN: status = "go"; break; } } } @@ -306,9 +305,8 @@ enum TrafficLight { } void doFormat(TrafficLight light) { String status = switch (light) { - case RED: - case GREEN: - case YELLOW: yield "unsure"; + case RED: yield "stop"; + case GREEN: yield "go"; default: yield "initialValue"; }; } @@ -318,7 +316,32 @@ void doFormat(TrafficLight light) { } @Test - void convertCasesWithAddedDefault() { + void notConvertColonCasesWithMultipleBlocks() { + // More than one block statement per case is not yet supported. + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(Object obj) { + String status = "initialValue"; + switch (obj) { + case null: { + status = "none"; + } + { + break; + } + default: status = "default status"; break; + } + } + } + """ + )); + } + + @Test + void noDefaultAddedIfAlreadyExhaustive() { rewriteRun( //language=java java( @@ -332,6 +355,7 @@ void doFormat(TrafficLight light) { switch (light) { case RED: status = "stop"; break; case GREEN: status = "go"; break; + case YELLOW: status = "unsure"; break; } } } @@ -345,36 +369,11 @@ void doFormat(TrafficLight light) { String status = switch (light) { case RED: yield "stop"; case GREEN: yield "go"; - default: yield "initialValue"; + case YELLOW: yield "unsure"; }; } } """ )); } - - @Test - void notConvertColonCasesWithMultipleBlocks() { - // More than one block statement per case is not yet supported. - rewriteRun( - //language=java - java( - """ - class Test { - void doFormat(Object obj) { - String status = "initialValue"; - switch (obj) { - case null: { - status = "none"; - } - { - break; - } - default: status = "default status"; break; - } - } - } - """ - )); - } } From 483ace0c76d75fb3987fa62c3a4a4fde405ce819 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Thu, 17 Jul 2025 15:34:53 -0400 Subject: [PATCH 13/46] Do not apply the recipe if a case statement/body assignment is referencing the original variable --- ...SwitchCaseAssigningToSwitchExpression.java | 37 +++++++++++++------ ...chCaseAssigningToSwitchExpressionTest.java | 32 ++++++++++++++++ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 783f005946..56b0c6ed8b 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -19,9 +19,11 @@ import lombok.Value; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.Space; import org.openrewrite.java.tree.Statement; @@ -51,7 +53,7 @@ public String getDisplayName() { @Override public String getDescription() { return "Switch statements for which each case is assigning a value to the same variable can be converted to a switch expression that returns the value of the variable. " + - "This is only applicable for Java 17 and later."; + "This is only applicable for Java 17 and later."; } @Override @@ -74,9 +76,9 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { } if (index < block.getStatements().size() - 1 && - statement instanceof J.VariableDeclarations && - ((J.VariableDeclarations) statement).getVariables().size() == 1 && - block.getStatements().get(index + 1) instanceof J.Switch + statement instanceof J.VariableDeclarations && + ((J.VariableDeclarations) statement).getVariables().size() == 1 && + block.getStatements().get(index + 1) instanceof J.Switch ) { J.VariableDeclarations vd = (J.VariableDeclarations) statement; J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); @@ -93,12 +95,12 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { private Optional buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { final String variableName = originalVariable.getSimpleName(); - AtomicBoolean isUnqualified = new AtomicBoolean(); + AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); AtomicBoolean isUsingArrows = new AtomicBoolean(true); List updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), s -> { - if (isUnqualified.get()) { + if (!isQualified.get()) { return null; } @@ -117,7 +119,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS J.Assignment assignment = (J.Assignment) block.getStatements().get(0); if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName)) { + if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { return caseItem.withBody(assignment.getAssignment()); } } @@ -144,12 +146,12 @@ private Optional buildNewSwitchExpression(J.Switch originalS } if (caseStatements.size() == 2 && - caseStatements.get(0) instanceof J.Assignment && - caseStatements.get(1) instanceof J.Break) { + caseStatements.get(0) instanceof J.Assignment && + caseStatements.get(1) instanceof J.Break) { J.Assignment assignment = (J.Assignment) caseStatements.get(0); if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName)) { + if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { J.Yield yieldStatement = new J.Yield( randomId(), assignment.getPrefix().withWhitespace(" "), // TODO: must be a better way to adjust the formatting when taken from a J.Block, see test convertColonCasesSimpleAssignationInBlockToSingleYield() @@ -163,10 +165,11 @@ private Optional buildNewSwitchExpression(J.Switch originalS } } - isUnqualified.set(true); + isQualified.set(false); return null; }); - if (isUnqualified.get()) { + + if (!isQualified.get()) { return Optional.empty(); } @@ -191,6 +194,16 @@ private Optional buildNewSwitchExpression(J.Switch originalS } return Optional.of(initializer.withCases(initializer.getCases().withStatements(updatedCases))); } + + private boolean containsIdentifier(String identifierName, Expression expression) { + return new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { + found.set(found.get() || id.getSimpleName().equals(identifierName)); + return super.visitIdentifier(id, found); + } + }.reduce(expression, new AtomicBoolean()).get(); + } } ); } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index fa369a9b45..69bb72ae6d 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -376,4 +376,36 @@ void doFormat(TrafficLight light) { """ )); } + + @Test + void notConvertWhenOriginalVariableIsUsedInCaseAssignment() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(int i) { + String orig = "initialValue"; + switch (i) { + case 1: orig = orig.toLowerCase(); break; + } + } + + void doFormat2(int i) { + String orig = "initialValue"; + switch (i) { + case 1: orig = String.format("%s %s", orig, "foo"); break; + } + } + + void doFormat3(int i) { + String orig = "initialValue"; + switch (i) { + case 1: orig = "foo" + orig; break; + } + } + } + """ + )); + } } From 8e75a6b48b42d157af6e68df80fe1991654e2ea8 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Thu, 17 Jul 2025 17:04:43 -0400 Subject: [PATCH 14/46] Do not apply the recipe if the original variable initializer is anything else than a literal, variable reference, field access or an expression composed of them only --- ...SwitchCaseAssigningToSwitchExpression.java | 35 +++++++-- ...chCaseAssigningToSwitchExpressionTest.java | 71 ++++++++++++++++++- 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 56b0c6ed8b..2833087fcd 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -17,16 +17,14 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesJavaVersion; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.Space; -import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -39,6 +37,7 @@ import java.util.concurrent.atomic.AtomicReference; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; import static org.openrewrite.Tree.randomId; @@ -78,6 +77,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { if (index < block.getStatements().size() - 1 && statement instanceof J.VariableDeclarations && ((J.VariableDeclarations) statement).getVariables().size() == 1 && + !canHaveSideEffects(((J.VariableDeclarations) statement).getVariables().get(0).getInitializer()) && block.getStatements().get(index + 1) instanceof J.Switch ) { J.VariableDeclarations vd = (J.VariableDeclarations) statement; @@ -188,7 +188,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS statements.toArray() ); - J.SwitchExpression initializer = (J.SwitchExpression) Objects.requireNonNull(vd.getVariables().get(0).getInitializer()); + J.SwitchExpression initializer = (J.SwitchExpression) requireNonNull(vd.getVariables().get(0).getInitializer()); if (isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch)) { updatedCases.add(initializer.getCases().getStatements().get(0)); } @@ -199,11 +199,34 @@ private boolean containsIdentifier(String identifierName, Expression expression) return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { - found.set(found.get() || id.getSimpleName().equals(identifierName)); + if (id.getSimpleName().equals(identifierName)) { + found.set(true); + return id; + } return super.visitIdentifier(id, found); } }.reduce(expression, new AtomicBoolean()).get(); } + + private boolean canHaveSideEffects(@Nullable Expression expression) { + if (expression == null) { + return false; + } + + return new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) { + found.set(true); + return method; + } + + @Override + public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { + found.set(true); + return newClass; + } + }.reduce(expression, new AtomicBoolean()).get(); + } } ); } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 69bb72ae6d..71dda2a83a 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -387,25 +387,90 @@ class Test { void doFormat(int i) { String orig = "initialValue"; switch (i) { - case 1: orig = orig.toLowerCase(); break; + default: orig = orig.toLowerCase(); break; } } void doFormat2(int i) { String orig = "initialValue"; switch (i) { - case 1: orig = String.format("%s %s", orig, "foo"); break; + default: orig = String.format("%s %s", orig, "foo"); break; } } void doFormat3(int i) { String orig = "initialValue"; switch (i) { - case 1: orig = "foo" + orig; break; + default: orig = "foo" + orig; break; } } } """ )); } + + @Test + void notConvertWhenOriginalVariableAssignationHasSideEffects() { + rewriteRun( + //language=java + java( + """ + class Test { + void methodInvocation(int i) { + String orig = "initialValue".toLowerCase(); + switch (i) { + default: orig = "hello"; break; + } + } + + void newClass(int i) { + String orig = new String("initialValue"); + switch (i) { + default: orig = "hello"; break; + } + } + + void newClassInBinaryExpression(int i) { + String orig = "initialValue" + new String("more"); + switch (i) { + default: orig = "hello"; break; + } + } + } + """ + )); + } + + @Test + void convertWhenOriginalVariableAssignationIsComplexExpressionButNoSideEffects() { + rewriteRun( + //language=java + java( + """ + class Test { + String field = "strawberry"; + + void doFormat(int i) { + String variable = "var"; + String orig = "initialValue" + "test" + 45 + true + field + this.field; + switch (i) { + default: orig = "hello"; break; + } + } + } + """, + """ + class Test { + String field = "strawberry"; + + void doFormat(int i) { + String variable = "var"; + String orig = switch (i) { + default: yield "hello"; + }; + } + } + """ + )); + } } From 0212743313bbcbc67e4d6354afff9a427c023f26 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 18 Jul 2025 11:58:59 +0200 Subject: [PATCH 15/46] Organize imports --- .../lang/SwitchCaseAssigningToSwitchExpression.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 2833087fcd..dc93d7c688 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -24,14 +24,16 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesJavaVersion; -import org.openrewrite.java.tree.*; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; import java.util.ArrayList; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -179,8 +181,8 @@ private Optional buildNewSwitchExpression(J.Switch originalS StringBuilder template = new StringBuilder( "Object o = switch (#{any()}) {\n" + - "default" + (isUsingArrows.get() ? " ->" : ": yield") + " #{any()};\n" + - "}"); + "default" + (isUsingArrows.get() ? " ->" : ": yield") + " #{any()};\n" + + "}"); J.VariableDeclarations vd = JavaTemplate.apply( template.toString(), new Cursor(getCursor(), originalSwitch), From 1bdd386ece24601f620b4afd1f2978cb130b4a13 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 09:39:02 -0400 Subject: [PATCH 16/46] Move the recipe to the right java migration declarative meta recipe --- src/main/resources/META-INF/rewrite/java-version-17.yml | 1 + src/main/resources/META-INF/rewrite/java-version-21.yml | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/META-INF/rewrite/java-version-17.yml b/src/main/resources/META-INF/rewrite/java-version-17.yml index 35ba237f18..06d3ebaea7 100644 --- a/src/main/resources/META-INF/rewrite/java-version-17.yml +++ b/src/main/resources/META-INF/rewrite/java-version-17.yml @@ -59,6 +59,7 @@ recipeList: artifactId: commons-codec newVersion: 1.17.x - org.openrewrite.java.migrate.AddLombokMapstructBinding + - org.openrewrite.java.migrate.lang.SwitchCaseAssigningToSwitchExpression --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/main/resources/META-INF/rewrite/java-version-21.yml b/src/main/resources/META-INF/rewrite/java-version-21.yml index 98ab2b94a3..653d2a8d93 100644 --- a/src/main/resources/META-INF/rewrite/java-version-21.yml +++ b/src/main/resources/META-INF/rewrite/java-version-21.yml @@ -145,4 +145,3 @@ recipeList: - org.openrewrite.java.migrate.lang.IfElseIfConstructToSwitch - org.openrewrite.java.migrate.lang.RefineSwitchCases - org.openrewrite.java.migrate.lang.SwitchCaseEnumGuardToLabel - - org.openrewrite.java.migrate.lang.SwitchCaseAssigningToSwitchExpression From dc63a4f8d01c4ebc6e0f5701f80672f48a420eb9 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 10:00:02 -0400 Subject: [PATCH 17/46] check for implicit toString() calls in the original variable initializer when checking for side effects --- ...SwitchCaseAssigningToSwitchExpression.java | 29 +++++++++-- ...chCaseAssigningToSwitchExpressionTest.java | 49 +++++++++++-------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index dc93d7c688..a031ec3532 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -24,10 +24,7 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesJavaVersion; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.Space; -import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -210,6 +207,7 @@ public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { }.reduce(expression, new AtomicBoolean()).get(); } + // Is any code elsewhere executed due to the provided expression private boolean canHaveSideEffects(@Nullable Expression expression) { if (expression == null) { return false; @@ -227,6 +225,29 @@ public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { found.set(true); return newClass; } + + private boolean isToStringImplicitelyCalled(Expression a, Expression b) { + // Assuming an implicit `.toString()` call could have a side effect, but excluding + // the java.lang.* classes from that rule. + if (TypeUtils.isAssignableTo("java.lang.String", a.getType()) && + TypeUtils.isAssignableTo("java.lang.String", b.getType())) { + return false; + } + + return a.getType() == JavaType.Primitive.String && + (!(b.getType() instanceof JavaType.Primitive || requireNonNull(b.getType()).toString().startsWith("java.lang")) && + !TypeUtils.isAssignableTo("java.lang.String", b.getType())); + } + + @Override + public J.Binary visitBinary(J.Binary binary, AtomicBoolean found) { + if (isToStringImplicitelyCalled(binary.getLeft(), binary.getRight()) || + isToStringImplicitelyCalled(binary.getRight(), binary.getLeft())) { + found.set(true); + return binary; + } + return super.visitBinary(binary, found); + } }.reduce(expression, new AtomicBoolean()).get(); } } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 71dda2a83a..c858ce5664 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -415,29 +415,36 @@ void notConvertWhenOriginalVariableAssignationHasSideEffects() { //language=java java( """ - class Test { - void methodInvocation(int i) { - String orig = "initialValue".toLowerCase(); - switch (i) { - default: orig = "hello"; break; - } - } + class Test { + void methodInvocation(int i) { + String orig = "initialValue".toLowerCase(); + switch (i) { + default: orig = "hello"; break; + } + } + + void newClass(int i) { + String orig = new String("initialValue"); + switch (i) { + default: orig = "hello"; break; + } + } - void newClass(int i) { - String orig = new String("initialValue"); - switch (i) { - default: orig = "hello"; break; - } - } + void newClassInBinaryExpression(int i) { + String orig = "initialValue" + new String("more"); + switch (i) { + default: orig = "hello"; break; + } + } - void newClassInBinaryExpression(int i) { - String orig = "initialValue" + new String("more"); - switch (i) { - default: orig = "hello"; break; - } - } - } - """ + void implicitToStringInvocation(int i, Test o) { + String orig = "initialValue" + o; + switch (i) { + default: orig = "hello"; break; + } + } + } + """ )); } From da7f8f432f9d395192ed7e2ebd3908f7a2cf119e Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 11:40:05 -0400 Subject: [PATCH 18/46] Do not apply the recipe if a colon-switch label has an empty statement list AND is the last case of the switch --- ...SwitchCaseAssigningToSwitchExpression.java | 9 +++++++- ...chCaseAssigningToSwitchExpressionTest.java | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index a031ec3532..c505d8a394 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -97,8 +97,9 @@ private Optional buildNewSwitchExpression(J.Switch originalS AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); AtomicBoolean isUsingArrows = new AtomicBoolean(true); + AtomicBoolean isLastCaseEmpty = new AtomicBoolean(false); - List updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), s -> { + List updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), (index, s) -> { if (!isQualified.get()) { return null; } @@ -137,6 +138,9 @@ private Optional buildNewSwitchExpression(J.Switch originalS isUsingArrows.set(false); List caseStatements = caseItem.getStatements(); if (caseStatements.isEmpty()) { + if (index + 1 == originalSwitch.getCases().getStatements().size()) { + isLastCaseEmpty.set(true); + } return caseItem; } @@ -189,7 +193,10 @@ private Optional buildNewSwitchExpression(J.Switch originalS J.SwitchExpression initializer = (J.SwitchExpression) requireNonNull(vd.getVariables().get(0).getInitializer()); if (isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch)) { + // add a `default:` case yielding/returning the originalVariable initializer updatedCases.add(initializer.getCases().getStatements().get(0)); + } else if (isLastCaseEmpty.get()) { + return Optional.empty(); } return Optional.of(initializer.withCases(initializer.getCases().withStatements(updatedCases))); } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index c858ce5664..f23667e4bd 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -480,4 +480,27 @@ void doFormat(int i) { """ )); } + + @Test + void notConvertColonSwitchWithEmptyLastCase() { + rewriteRun( + //language=java + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void doFormat(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + case YELLOW: + } + } + } + """ + )); + } } From 4bd2243eaa437e19fdbdc67041d878cf8cb08e97 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 12:54:22 -0400 Subject: [PATCH 19/46] Do not apply the recipe if the original variable has no initializer and the switch isn't exhaustive and each case isn't assigning the original variable a value as expected --- ...SwitchCaseAssigningToSwitchExpression.java | 53 +++++++++++-------- ...chCaseAssigningToSwitchExpressionTest.java | 33 ++++++++++++ 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index c505d8a394..7f3c599c55 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -29,7 +29,6 @@ import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; -import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; @@ -172,33 +171,43 @@ private Optional buildNewSwitchExpression(J.Switch originalS return null; }); - if (!isQualified.get()) { + boolean shouldAddDefaultCase = isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch); + Expression originalInitializer = originalVariable.getInitializer(); + + if (!isQualified.get() || + (originalInitializer == null && shouldAddDefaultCase) || + (isLastCaseEmpty.get() && !shouldAddDefaultCase) + ) { return Optional.empty(); } - List statements = new ArrayList<>(); - statements.add(originalSwitch.getSelector().getTree()); - statements.add(originalVariable.getInitializer().withPrefix(Space.SINGLE_SPACE)); + if (shouldAddDefaultCase) { + updatedCases.add( + createDefaultCase(isUsingArrows.get(), originalInitializer.withPrefix(Space.SINGLE_SPACE), originalSwitch) + ); + } - StringBuilder template = new StringBuilder( - "Object o = switch (#{any()}) {\n" + - "default" + (isUsingArrows.get() ? " ->" : ": yield") + " #{any()};\n" + - "}"); - J.VariableDeclarations vd = JavaTemplate.apply( - template.toString(), - new Cursor(getCursor(), originalSwitch), - originalSwitch.getCoordinates().replace(), //right coordinates? We don't want to replace the switch, we modify it and move it as an assignement one line up. - statements.toArray() + return Optional.of( + new J.SwitchExpression( + randomId(), + Space.SINGLE_SPACE, + Markers.EMPTY, + originalSwitch.getSelector(), + originalSwitch.getCases().withStatements(updatedCases), + originalVariable.getType() + ) ); + } - J.SwitchExpression initializer = (J.SwitchExpression) requireNonNull(vd.getVariables().get(0).getInitializer()); - if (isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch)) { - // add a `default:` case yielding/returning the originalVariable initializer - updatedCases.add(initializer.getCases().getStatements().get(0)); - } else if (isLastCaseEmpty.get()) { - return Optional.empty(); - } - return Optional.of(initializer.withCases(initializer.getCases().withStatements(updatedCases))); + private J.Case createDefaultCase(boolean arrow, Expression returnedExpression, J.Switch originalSwitch) { + String template = "switch(1) {\n" + "default" + (arrow ? " ->" : ": yield") + " #{any()};\n}"; + J.Switch switchStatement = JavaTemplate.apply( + template, + new Cursor(getCursor(), originalSwitch), + originalSwitch.getCoordinates().replace(), + returnedExpression + ); + return (J.Case) switchStatement.getCases().getStatements().get(0); } private boolean containsIdentifier(String identifierName, Expression expression) { diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index f23667e4bd..49a53ec317 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -503,4 +503,37 @@ void doFormat(TrafficLight light) { """ )); } + + @Test + void notConvertSwitchOnUninitializedOriginalVariableAndNonExhaustiveSwitch() { + rewriteRun( + //language=java + java( + """ + class Test { + enum TrafficLight { + RED, GREEN, YELLOW + } + void exhaustiveButCantAddDefaultAndMissingAssignment(TrafficLight light) { + String status; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + case YELLOW: + } + } + + void exhaustiveButMissingAssignment(TrafficLight light) { + String status; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + case YELLOW: + default: System.out.println("foo"); + } + } + } + """ + )); + } } From 4fb847962deadb63b0d6cd2fb2dbfb8753e70f13 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 13:07:51 -0400 Subject: [PATCH 20/46] Small cleanup --- .../lang/SwitchCaseAssigningToSwitchExpression.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 7f3c599c55..727d8d7c92 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -105,13 +105,12 @@ private Optional buildNewSwitchExpression(J.Switch originalS J.Case caseItem = (J.Case) s; - if (caseItem != null && caseItem.getCaseLabels().get(0) instanceof J.Identifier && ((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName().equals("default")) { + if (caseItem.getCaseLabels().get(0) instanceof J.Identifier && + ((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName().equals("default")) { isDefaultCaseAbsent.set(false); } - if (caseItem == null) { - return null; - } else if (caseItem.getBody() != null) { // arrow cases + if (caseItem.getBody() != null) { // arrow cases if (caseItem.getBody() instanceof J.Block) { J.Block block = (J.Block) caseItem.getBody(); if (block.getStatements().size() == 1 && block.getStatements().get(0) instanceof J.Assignment) { @@ -156,7 +155,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { J.Yield yieldStatement = new J.Yield( randomId(), - assignment.getPrefix().withWhitespace(" "), // TODO: must be a better way to adjust the formatting when taken from a J.Block, see test convertColonCasesSimpleAssignationInBlockToSingleYield() + assignment.getPrefix().withWhitespace(" "), Markers.EMPTY, false, assignment.getAssignment() From 44f47b4945248d68d0a3764e59edc949c3ea0f76 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 15:42:17 -0400 Subject: [PATCH 21/46] Inline the switch expression on the return statement when appropriate --- ...SwitchCaseAssigningToSwitchExpression.java | 54 ++++++-- ...chCaseAssigningToSwitchExpressionTest.java | 118 ++++++++++++++++++ 2 files changed, 159 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 727d8d7c92..4085543d70 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -30,7 +30,6 @@ import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; import java.util.List; -import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -64,15 +63,21 @@ public TreeVisitor getVisitor() { return Preconditions.check(preconditions, new JavaVisitor() { @Override public J visitBlock(J.Block block, ExecutionContext ctx) { - AtomicReference originalSwitch = new AtomicReference<>(); + AtomicReference originalSwitch = new AtomicReference<>(); + AtomicReference inlinedReturn = new AtomicReference<>(); + int lastIndex = block.getStatements().size() - 1; J.Block b = block.withStatements(ListUtils.map(block.getStatements(), (index, statement) -> { if (statement == originalSwitch.getAndSet(null)) { // We've already converted the switch/assignments to an assignment with a switch expression. return null; } - if (index < block.getStatements().size() - 1 && + if (index == lastIndex && inlinedReturn.get() != null) { + return inlinedReturn.get(); + } + + if (index < lastIndex && statement instanceof J.VariableDeclarations && ((J.VariableDeclarations) statement).getVariables().size() == 1 && !canHaveSideEffects(((J.VariableDeclarations) statement).getVariables().get(0).getInitializer()) && @@ -80,10 +85,19 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { ) { J.VariableDeclarations vd = (J.VariableDeclarations) statement; J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); - Optional newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, vd.getVariables().get(0)); - if (newSwitchExpression.isPresent()) { + + J.VariableDeclarations.NamedVariable originalVariable = vd.getVariables().get(0); + J.SwitchExpression newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, originalVariable); + + if (newSwitchExpression != null) { originalSwitch.set(nextStatementSwitch); - return vd.withVariables(singletonList(vd.getVariables().get(0).withInitializer(newSwitchExpression.get()))); + J.Return lastReturn = canSwitchBeReturnedInline(index, block.getStatements(), originalVariable.getSimpleName()); + if (lastReturn != null) { + inlinedReturn.set(lastReturn.withExpression(newSwitchExpression)); + return null; // We're inlining on return, remove the original variable declaration. + } else { + return vd.withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))); + } } } return statement; @@ -91,7 +105,23 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { return super.visitBlock(b, ctx); } - private Optional buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { + private J.@Nullable Return canSwitchBeReturnedInline(int currentStatementIndex, List blockStatements, String originalVariableName) { + if (currentStatementIndex + 3 == blockStatements.size()) { + Statement lastStatement = blockStatements.get(currentStatementIndex + 2); + if (lastStatement instanceof J.Return) { + J.Return lastReturn = (J.Return) lastStatement; + if (lastReturn.getExpression() instanceof J.Identifier) { + J.Identifier identifier = (J.Identifier) lastReturn.getExpression(); + if (identifier.getSimpleName().equals(originalVariableName)) { + return lastReturn; + } + } + } + } + return null; + } + + private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { final String variableName = originalVariable.getSimpleName(); AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); @@ -175,9 +205,8 @@ private Optional buildNewSwitchExpression(J.Switch originalS if (!isQualified.get() || (originalInitializer == null && shouldAddDefaultCase) || - (isLastCaseEmpty.get() && !shouldAddDefaultCase) - ) { - return Optional.empty(); + (isLastCaseEmpty.get() && !shouldAddDefaultCase)) { + return null; } if (shouldAddDefaultCase) { @@ -186,7 +215,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS ); } - return Optional.of( + return new J.SwitchExpression( randomId(), Space.SINGLE_SPACE, @@ -194,8 +223,7 @@ private Optional buildNewSwitchExpression(J.Switch originalS originalSwitch.getSelector(), originalSwitch.getCases().withStatements(updatedCases), originalVariable.getType() - ) - ); + ); } private J.Case createDefaultCase(boolean arrow, Expression returnedExpression, J.Switch originalSwitch) { diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 49a53ec317..caf2e3a737 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -16,6 +16,7 @@ package org.openrewrite.java.migrate.lang; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -536,4 +537,121 @@ void exhaustiveButMissingAssignment(TrafficLight light) { """ )); } + + @Test + void inlineWhenVariableOnlyToBeReturned() { + rewriteRun( + //language=java + java( + """ + class Test { + String doFormat() { + String formatted; + switch (1) { + default: formatted = "foo"; break; + } + return formatted; + } + } + """, + """ + class Test { + String doFormat() { + return switch (1) { + default: yield "foo"; + }; + } + } + """ + )); + } + + @Test + void doNotinlineWhenInappropriate() { + rewriteRun( + //language=java + java( + """ + class Test { + String originalVariableNotReturned() { + String formatted; + switch (1) { + default: formatted = "foo"; break; + } + return "string"; + } + + String codeBetweenSwitchAndReturn() { + String formatted; + switch (1) { + default: formatted = "foo"; break; + } + System.out.println("Hey"); + return formatted; + } + + void noReturnedExpression() { + String formatted; + switch (1) { + default: formatted = "foo"; break; + } + return; + } + } + """, + """ + class Test { + String originalVariableNotReturned() { + String formatted= switch (1) { + default: yield "foo"; + }; + return "string"; + } + + String codeBetweenSwitchAndReturn() { + String formatted= switch (1) { + default: yield "foo"; + }; + System.out.println("Hey"); + return formatted; + } + + void noReturnedExpression() { + String formatted= switch (1) { + default: yield "foo"; + }; + return; + } + } + """ + )); + } + + @Test + @ExpectedToFail + void failsToFormatWithASpaceWhenOriginalVariableHasNoInitializer() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat() { + String formatted; + switch (1) { + default: formatted = "foo"; break; + } + } + } + """, + """ + class Test { + void doFormat() { + String formatted = switch (1) { + default: yield "foo"; + }; + } + } + """ + )); + } } From e574f5814fde02b60018ff4ce5cbeb0c73420f7a Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Fri, 18 Jul 2025 16:35:54 -0400 Subject: [PATCH 22/46] Comments are preserved --- ...SwitchCaseAssigningToSwitchExpression.java | 13 +++- ...chCaseAssigningToSwitchExpressionTest.java | 78 +++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 4085543d70..32eb5c8317 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -93,10 +93,19 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { originalSwitch.set(nextStatementSwitch); J.Return lastReturn = canSwitchBeReturnedInline(index, block.getStatements(), originalVariable.getSimpleName()); if (lastReturn != null) { - inlinedReturn.set(lastReturn.withExpression(newSwitchExpression)); + inlinedReturn.set( + lastReturn + .withExpression(newSwitchExpression) + .withPrefix(lastReturn.getPrefix() + .withComments(ListUtils.concatAll(vd.getComments(), ListUtils.concatAll(nextStatementSwitch.getComments(), lastReturn.getComments()))) + .withWhitespace(vd.getPrefix().getWhitespace()) + ) + ); return null; // We're inlining on return, remove the original variable declaration. } else { - return vd.withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))); + return vd + .withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))) + .withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments())); } } } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index caf2e3a737..f65516223e 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -654,4 +654,82 @@ void doFormat() { """ )); } + + @Test + void commentsArePreserved() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(Object obj) { + // line before the original variable + String formatted = "initialValue"; // original variable after code + // line before the switch + switch (obj) { // first line of the switch + // before the cases + case Integer i -> formatted = String.format("int %d", i); // first case + // between the 1st and 2nd case + /* before the 2nd case */ case Long l -> formatted = String.format("long %d", l); + default -> formatted = "unknown"; + // after the last case + } // last line of the switch + } + } + """, + """ + class Test { + void doFormat(Object obj) { + // line before the original variable + // original variable after code + // line before the switch + String formatted = switch (obj) { // first line of the switch + // before the cases + case Integer i -> String.format("int %d", i); // first case + // between the 1st and 2nd case + /* before the 2nd case */ case Long l -> String.format("long %d", l); + default -> "unknown"; + // after the last case + }; // last line of the switch + } + } + """ + )); + } + + @Test + void commentsArePreservedWhenInlining() { + rewriteRun( + //language=java + java( + """ + class Test { + String doFormat() { + // line before original variable + String formatted; // original variable after code + // between the original variable and the switch + switch (1) { // on the switch after code + default: formatted = "foo"; break; + } // last line of the switch + // between switch and return + return formatted; // after return on the same line + } + } + """, + """ + class Test { + String doFormat() { + // line before original variable + // original variable after code + // between the original variable and the switch + // last line of the switch + // between switch and return + return switch (1) { // on the switch after code + default: yield "foo"; + }; // after return on the same line + } + } + """ + )); + } } From 6bb990804e4d42935e26d9f613b47b37039d728f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 13:44:55 +0200 Subject: [PATCH 23/46] Apply formatter --- ...SwitchCaseAssigningToSwitchExpression.java | 6 +- ...chCaseAssigningToSwitchExpressionTest.java | 75 ++++++++++++------- 2 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 32eb5c8317..656b39bdcc 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -37,7 +37,6 @@ import static java.util.Objects.requireNonNull; import static org.openrewrite.Tree.randomId; - @Value @EqualsAndHashCode(callSuper = false) public class SwitchCaseAssigningToSwitchExpression extends Recipe { @@ -56,10 +55,9 @@ public String getDescription() { public TreeVisitor getVisitor() { TreeVisitor preconditions = Preconditions.and( new UsesJavaVersion<>(17), - Preconditions.not(new KotlinFileChecker<>()), // necessary ? - Preconditions.not(new GroovyFileChecker<>()) // necessary ? + Preconditions.not(new KotlinFileChecker<>()), + Preconditions.not(new GroovyFileChecker<>()) ); - return Preconditions.check(preconditions, new JavaVisitor() { @Override public J visitBlock(J.Block block, ExecutionContext ctx) { diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index f65516223e..cb8f77a66c 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -27,12 +27,13 @@ class SwitchCaseAssigningToSwitchExpressionTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new SwitchCaseAssigningToSwitchExpression()).allSources(source -> version(source, 21)); + spec.recipe(new SwitchCaseAssigningToSwitchExpression()).allSources(source -> version(source, 21) + ); } @DocumentExample @Test - void convertSimpleArrowCasesAssignations() { + void convertSimpleArrowCasesAssignment() { rewriteRun( //language=java java( @@ -59,11 +60,12 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test - void convertSimpleColonCasesAssignations() { + void convertSimpleColonCasesAssignment() { rewriteRun( //language=java java( @@ -90,11 +92,12 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test - void notConvertSimpleColonCasesAssignationsWithExtraCodeInBlock() { + void notConvertSimpleColonCasesAssignmentWithExtraCodeInBlock() { // Only one statement [+break;] per case is currently supported rewriteRun( //language=java @@ -111,7 +114,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -145,7 +149,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -178,7 +183,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -198,7 +204,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -219,7 +226,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -238,7 +246,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -277,7 +286,8 @@ void doFormat(TrafficLight light) { } } """ - )); + ) + ); } @Test @@ -313,7 +323,8 @@ void doFormat(TrafficLight light) { } } """ - )); + ) + ); } @Test @@ -338,7 +349,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -375,7 +387,8 @@ void doFormat(TrafficLight light) { } } """ - )); + ) + ); } @Test @@ -407,7 +420,8 @@ void doFormat3(int i) { } } """ - )); + ) + ); } @Test @@ -446,7 +460,8 @@ void implicitToStringInvocation(int i, Test o) { } } """ - )); + ) + ); } @Test @@ -479,7 +494,8 @@ void doFormat(int i) { } } """ - )); + ) + ); } @Test @@ -502,7 +518,8 @@ void doFormat(TrafficLight light) { } } """ - )); + ) + ); } @Test @@ -535,7 +552,8 @@ void exhaustiveButMissingAssignment(TrafficLight light) { } } """ - )); + ) + ); } @Test @@ -563,7 +581,8 @@ String doFormat() { } } """ - )); + ) + ); } @Test @@ -624,7 +643,8 @@ void noReturnedExpression() { } } """ - )); + ) + ); } @Test @@ -652,7 +672,8 @@ void doFormat() { } } """ - )); + ) + ); } @Test @@ -694,7 +715,8 @@ void doFormat(Object obj) { } } """ - )); + ) + ); } @Test @@ -730,6 +752,7 @@ String doFormat() { } } """ - )); + ) + ); } } From 3d9664d57ff861513bd7c07384eb5660d15dfb2e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 13:48:33 +0200 Subject: [PATCH 24/46] Ignore warnings on test text blocks --- .../migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index cb8f77a66c..58710125c3 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -24,6 +24,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.version; +@SuppressWarnings({"EnhancedSwitchMigration", "RedundantLabeledSwitchRuleCodeBlock", "StringOperationCanBeSimplified", "SwitchStatementWithTooFewBranches", "UnnecessaryReturnStatement", "UnusedAssignment"}) class SwitchCaseAssigningToSwitchExpressionTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { From 77111a533a584c6004b3f771ed1a764d510019fb Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 14:02:13 +0200 Subject: [PATCH 25/46] Sort annotations --- .../migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 58710125c3..836b382478 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -648,8 +648,8 @@ void noReturnedExpression() { ); } - @Test @ExpectedToFail + @Test void failsToFormatWithASpaceWhenOriginalVariableHasNoInitializer() { rewriteRun( //language=java From 62d2ec3b549c01b3fc7c00a8dfb7c39d985fe026 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 14:42:24 +0200 Subject: [PATCH 26/46] Add a new test not yet covered and update expectations on formatting --- ...chCaseAssigningToSwitchExpressionTest.java | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 836b382478..4c0600ad53 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -120,7 +120,7 @@ void doFormat(Object obj) { } @Test - void convertColonCasesSimpleAssignationInBlockToSingleYield() { + void convertColonCasesSimpleAssignmentInBlockToSingleYield() { rewriteRun( //language=java java( @@ -154,8 +154,44 @@ void doFormat(Object obj) { ); } + @ExpectedToFail("Not implemented yet, but should be possible to support") @Test - void convertArrowCasesSimpleAssignationInBlockToSingleValue() { + void convertColonCasesSimpleAssignmentInBlockToSingleYieldWithoutFinalCaseBreak() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(Object obj) { + String formatted = "initialValue"; + switch (obj) { + case Integer i: formatted = String.format("int %d", i); break; + case Long l: { + formatted = String.format("long %d", l); + break; + } + default: formatted = "unknown"; + } + } + } + """, + """ + class Test { + void doFormat(Object obj) { + String formatted = switch (obj) { + case Integer i: yield String.format("int %d", i); + case Long l: yield String.format("long %d", l); + default: yield "unknown"; + }; + } + } + """ + ) + ); + } + + @Test + void convertArrowCasesSimpleAssignmentInBlockToSingleValue() { rewriteRun( //language=java java( @@ -232,7 +268,7 @@ void doFormat(Object obj) { } @Test - void notConvertCasesWhenColonCaseHasNoStatementsAndNextCaseIsntAssignation() { + void notConvertCasesWhenColonCaseHasNoStatementsAndNextCaseIsntAssignment() { rewriteRun( //language=java java( @@ -252,7 +288,7 @@ void doFormat(Object obj) { } @Test - void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignation() { + void convertCasesWhenColonCaseHasNoStatementsAndNextCaseIsAssignment() { rewriteRun( //language=java java( @@ -426,7 +462,7 @@ void doFormat3(int i) { } @Test - void notConvertWhenOriginalVariableAssignationHasSideEffects() { + void notConvertWhenOriginalVariableAssignmentHasSideEffects() { rewriteRun( //language=java java( @@ -466,7 +502,7 @@ void implicitToStringInvocation(int i, Test o) { } @Test - void convertWhenOriginalVariableAssignationIsComplexExpressionButNoSideEffects() { + void convertWhenOriginalVariableAssignmentIsComplexExpressionButNoSideEffects() { rewriteRun( //language=java java( @@ -475,7 +511,6 @@ class Test { String field = "strawberry"; void doFormat(int i) { - String variable = "var"; String orig = "initialValue" + "test" + 45 + true + field + this.field; switch (i) { default: orig = "hello"; break; @@ -488,7 +523,6 @@ class Test { String field = "strawberry"; void doFormat(int i) { - String variable = "var"; String orig = switch (i) { default: yield "hello"; }; @@ -622,14 +656,14 @@ void noReturnedExpression() { """ class Test { String originalVariableNotReturned() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; return "string"; } String codeBetweenSwitchAndReturn() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; System.out.println("Hey"); @@ -637,7 +671,7 @@ String codeBetweenSwitchAndReturn() { } void noReturnedExpression() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; return; @@ -648,9 +682,8 @@ void noReturnedExpression() { ); } - @ExpectedToFail @Test - void failsToFormatWithASpaceWhenOriginalVariableHasNoInitializer() { + void whitespaceAddedWhenNoOriginalAssignment() { rewriteRun( //language=java java( From cf6740f2c2166e058bc8465245e94eed5046d333 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 15:00:08 +0200 Subject: [PATCH 27/46] Use `JavaIsoVisitor` since we're not changing types --- .../migrate/lang/SwitchCaseAssigningToSwitchExpression.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 656b39bdcc..abc145867b 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -58,9 +58,9 @@ public TreeVisitor getVisitor() { Preconditions.not(new KotlinFileChecker<>()), Preconditions.not(new GroovyFileChecker<>()) ); - return Preconditions.check(preconditions, new JavaVisitor() { + return Preconditions.check(preconditions, new JavaIsoVisitor() { @Override - public J visitBlock(J.Block block, ExecutionContext ctx) { + public J.Block visitBlock(J.Block block, ExecutionContext ctx) { AtomicReference originalSwitch = new AtomicReference<>(); AtomicReference inlinedReturn = new AtomicReference<>(); From 50125b1e1335f3eab5ee766503652de651675d2f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 15:25:18 +0200 Subject: [PATCH 28/46] Update src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index abc145867b..698f45f68b 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -22,7 +22,6 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; From d2746b4995f0c069506b2229328fe1a2c50e8b7d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 15:47:23 +0200 Subject: [PATCH 29/46] For now expect missing whitespace --- .../lang/SwitchCaseAssigningToSwitchExpression.java | 13 ++++++------- .../SwitchCaseAssigningToSwitchExpressionTest.java | 7 ++++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 698f45f68b..183ef4c63d 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -99,11 +99,10 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { ) ); return null; // We're inlining on return, remove the original variable declaration. - } else { - return vd - .withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))) - .withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments())); } + return vd + .withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))) + .withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments())); } } return statement; @@ -275,7 +274,7 @@ public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { return newClass; } - private boolean isToStringImplicitelyCalled(Expression a, Expression b) { + private boolean isToStringImplicitlyCalled(Expression a, Expression b) { // Assuming an implicit `.toString()` call could have a side effect, but excluding // the java.lang.* classes from that rule. if (TypeUtils.isAssignableTo("java.lang.String", a.getType()) && @@ -290,8 +289,8 @@ private boolean isToStringImplicitelyCalled(Expression a, Expression b) { @Override public J.Binary visitBinary(J.Binary binary, AtomicBoolean found) { - if (isToStringImplicitelyCalled(binary.getLeft(), binary.getRight()) || - isToStringImplicitelyCalled(binary.getRight(), binary.getLeft())) { + if (isToStringImplicitlyCalled(binary.getLeft(), binary.getRight()) || + isToStringImplicitlyCalled(binary.getRight(), binary.getLeft())) { found.set(true); return binary; } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 4c0600ad53..59be00a044 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -656,14 +656,14 @@ void noReturnedExpression() { """ class Test { String originalVariableNotReturned() { - String formatted = switch (1) { + String formatted= switch (1) { default: yield "foo"; }; return "string"; } String codeBetweenSwitchAndReturn() { - String formatted = switch (1) { + String formatted= switch (1) { default: yield "foo"; }; System.out.println("Hey"); @@ -671,7 +671,7 @@ String codeBetweenSwitchAndReturn() { } void noReturnedExpression() { - String formatted = switch (1) { + String formatted= switch (1) { default: yield "foo"; }; return; @@ -682,6 +682,7 @@ void noReturnedExpression() { ); } + @ExpectedToFail("Missing whitespace before `=`") @Test void whitespaceAddedWhenNoOriginalAssignment() { rewriteRun( From d42a2d3a9748ff4e91c073376dde32de7f22f3ab Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 15:51:15 +0200 Subject: [PATCH 30/46] Add a space before `=` when there was no previous initializer --- .../lang/SwitchCaseAssigningToSwitchExpression.java | 3 ++- .../lang/SwitchCaseAssigningToSwitchExpressionTest.java | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 183ef4c63d..c3916e099a 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -101,7 +101,8 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { return null; // We're inlining on return, remove the original variable declaration. } return vd - .withVariables(singletonList(originalVariable.withInitializer(newSwitchExpression))) + .withVariables(singletonList(originalVariable.getPadding().withInitializer( + JLeftPadded.build(newSwitchExpression).withBefore(Space.SINGLE_SPACE)))) .withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments())); } } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 59be00a044..4c0600ad53 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -656,14 +656,14 @@ void noReturnedExpression() { """ class Test { String originalVariableNotReturned() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; return "string"; } String codeBetweenSwitchAndReturn() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; System.out.println("Hey"); @@ -671,7 +671,7 @@ String codeBetweenSwitchAndReturn() { } void noReturnedExpression() { - String formatted= switch (1) { + String formatted = switch (1) { default: yield "foo"; }; return; @@ -682,7 +682,6 @@ void noReturnedExpression() { ); } - @ExpectedToFail("Missing whitespace before `=`") @Test void whitespaceAddedWhenNoOriginalAssignment() { rewriteRun( From 86918c16f8995ec372baf899c5af728260ee1b59 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Mon, 21 Jul 2025 10:04:40 -0400 Subject: [PATCH 31/46] Add support for last colon case doing assignment without a break; --- ...SwitchCaseAssigningToSwitchExpression.java | 53 +++++++++++-------- ...chCaseAssigningToSwitchExpressionTest.java | 2 - 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index c3916e099a..488e3ca3fe 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -170,35 +170,26 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } } else { // colon cases isUsingArrows.set(false); + boolean isLastCase = index + 1 == originalSwitch.getCases().getStatements().size(); + List caseStatements = caseItem.getStatements(); if (caseStatements.isEmpty()) { - if (index + 1 == originalSwitch.getCases().getStatements().size()) { + if (isLastCase) { isLastCaseEmpty.set(true); } return caseItem; } - if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { - caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); - } - - if (caseStatements.size() == 2 && - caseStatements.get(0) instanceof J.Assignment && - caseStatements.get(1) instanceof J.Break) { - J.Assignment assignment = (J.Assignment) caseStatements.get(0); - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { - J.Yield yieldStatement = new J.Yield( - randomId(), - assignment.getPrefix().withWhitespace(" "), - Markers.EMPTY, - false, - assignment.getAssignment() - ); - return caseItem.withStatements(singletonList(yieldStatement)); - } - } + J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, variableName); + if (assignment != null) { + J.Yield yieldStatement = new J.Yield( + randomId(), + assignment.getPrefix().withWhitespace(" "), + Markers.EMPTY, + false, + assignment.getAssignment() + ); + return caseItem.withStatements(singletonList(yieldStatement)); } } @@ -232,6 +223,24 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { ); } + private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, String variableName) { + if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { + caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); + } + + if (((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) && + caseStatements.get(0) instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) caseStatements.get(0); + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { + return assignment; + } + } + } + return null; + } + private J.Case createDefaultCase(boolean arrow, Expression returnedExpression, J.Switch originalSwitch) { String template = "switch(1) {\n" + "default" + (arrow ? " ->" : ": yield") + " #{any()};\n}"; J.Switch switchStatement = JavaTemplate.apply( diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 4c0600ad53..810ea00dd1 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -16,7 +16,6 @@ package org.openrewrite.java.migrate.lang; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -154,7 +153,6 @@ void doFormat(Object obj) { ); } - @ExpectedToFail("Not implemented yet, but should be possible to support") @Test void convertColonCasesSimpleAssignmentInBlockToSingleYieldWithoutFinalCaseBreak() { rewriteRun( From f8a43e41d625366ba2075d3ed1f95b429ba35b83 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Mon, 21 Jul 2025 10:23:16 -0400 Subject: [PATCH 32/46] Make buildNewSwitchExpression more readable --- ...SwitchCaseAssigningToSwitchExpression.java | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 488e3ca3fe..10601b1eb0 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -147,26 +147,9 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } if (caseItem.getBody() != null) { // arrow cases - if (caseItem.getBody() instanceof J.Block) { - J.Block block = (J.Block) caseItem.getBody(); - if (block.getStatements().size() == 1 && block.getStatements().get(0) instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) block.getStatements().get(0); - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { - return caseItem.withBody(assignment.getAssignment()); - } - } - } - - } else if (caseItem.getBody() instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) caseItem.getBody(); - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName)) { - return caseItem.withBody(assignment.getAssignment()); - } - } + Expression assignedExpression = extractAssignedExpressionFromArrowCase(caseItem.getBody(), variableName); + if (assignedExpression != null) { + return caseItem.withBody(assignedExpression); } } else { // colon cases isUsingArrows.set(false); @@ -223,6 +206,32 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { ); } + private @Nullable Expression extractAssignedExpressionFromArrowCase(J caseBody, String variableName) { + if (caseBody instanceof J.Block) { + J.Block block = (J.Block) caseBody; + if (block.getStatements().size() == 1 && block.getStatements().get(0) instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) block.getStatements().get(0); + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { + return assignment.getAssignment(); + } + } + } + + } else if (caseBody instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) caseBody; + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (variable.getSimpleName().equals(variableName)) { + return assignment.getAssignment(); + } + } + } + + return null; + } + private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, String variableName) { if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); From f24117a25a3ab2d30c5875c7fb13acfbf8b93d0b Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Mon, 21 Jul 2025 10:57:54 -0400 Subject: [PATCH 33/46] clean-up the assignment validation and extraction code further Removed duplicated logic and fixed a bug where arrow cases with self-referencing assignment within a J.Block was still possible. --- ...SwitchCaseAssigningToSwitchExpression.java | 51 ++++++++----------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 10601b1eb0..0de78c5997 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -147,9 +147,9 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } if (caseItem.getBody() != null) { // arrow cases - Expression assignedExpression = extractAssignedExpressionFromArrowCase(caseItem.getBody(), variableName); - if (assignedExpression != null) { - return caseItem.withBody(assignedExpression); + J.Assignment assignment = extractValidAssignmentFromArrowCase(caseItem.getBody(), variableName); + if (assignment != null) { + return caseItem.withBody(assignment.getAssignment()); } } else { // colon cases isUsingArrows.set(false); @@ -163,7 +163,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { return caseItem; } - J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, variableName); + J.Assignment assignment = extractValidAssignmentFromColonCase(caseStatements, isLastCase, variableName); if (assignment != null) { J.Yield yieldStatement = new J.Yield( randomId(), @@ -206,40 +206,29 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { ); } - private @Nullable Expression extractAssignedExpressionFromArrowCase(J caseBody, String variableName) { - if (caseBody instanceof J.Block) { - J.Block block = (J.Block) caseBody; - if (block.getStatements().size() == 1 && block.getStatements().get(0) instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) block.getStatements().get(0); - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { - return assignment.getAssignment(); - } - } - } - - } else if (caseBody instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) caseBody; - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName)) { - return assignment.getAssignment(); - } - } + private J.@Nullable Assignment extractValidAssignmentFromArrowCase(J caseBody, String variableName) { + if (caseBody instanceof J.Block && ((J.Block)caseBody).getStatements().size() == 1) { + caseBody = ((J.Block) caseBody).getStatements().get(0); } - return null; + return validateAssignmentFromCase(caseBody, variableName); } - private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, String variableName) { + private J.@Nullable Assignment extractValidAssignmentFromColonCase(List caseStatements, boolean isLastCase, String variableName) { if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); } - if (((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) && - caseStatements.get(0) instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) caseStatements.get(0); + if ((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) { + return validateAssignmentFromCase(caseStatements.get(0), variableName); + } + + return null; + } + + private J.@Nullable Assignment validateAssignmentFromCase(J maybeAssignment, String variableName) { + if (maybeAssignment instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) maybeAssignment; if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { @@ -247,6 +236,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } } } + return null; } @@ -258,6 +248,7 @@ private J.Case createDefaultCase(boolean arrow, Expression returnedExpression, J originalSwitch.getCoordinates().replace(), returnedExpression ); + return (J.Case) switchStatement.getCases().getStatements().get(0); } From 0754a4c7b3484f286129b0eadae98423201a578e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 23:30:21 +0200 Subject: [PATCH 34/46] Delegate to `InlineVariable` to inline return --- ...SwitchCaseAssigningToSwitchExpression.java | 47 ++++--------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 0de78c5997..79a9131a29 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -25,6 +25,7 @@ import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; +import org.openrewrite.staticanalysis.InlineVariable; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -59,27 +60,24 @@ public TreeVisitor getVisitor() { ); return Preconditions.check(preconditions, new JavaIsoVisitor() { @Override - public J.Block visitBlock(J.Block block, ExecutionContext ctx) { + public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { + J.Block block = super.visitBlock(originalBlock, ctx); + AtomicReference originalSwitch = new AtomicReference<>(); - AtomicReference inlinedReturn = new AtomicReference<>(); int lastIndex = block.getStatements().size() - 1; - J.Block b = block.withStatements(ListUtils.map(block.getStatements(), (index, statement) -> { + return block.withStatements(ListUtils.map(block.getStatements(), (index, statement) -> { if (statement == originalSwitch.getAndSet(null)) { + doAfterVisit(new InlineVariable().getVisitor()); // We've already converted the switch/assignments to an assignment with a switch expression. return null; } - if (index == lastIndex && inlinedReturn.get() != null) { - return inlinedReturn.get(); - } - if (index < lastIndex && statement instanceof J.VariableDeclarations && ((J.VariableDeclarations) statement).getVariables().size() == 1 && !canHaveSideEffects(((J.VariableDeclarations) statement).getVariables().get(0).getInitializer()) && - block.getStatements().get(index + 1) instanceof J.Switch - ) { + block.getStatements().get(index + 1) instanceof J.Switch) { J.VariableDeclarations vd = (J.VariableDeclarations) statement; J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); @@ -88,18 +86,6 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { if (newSwitchExpression != null) { originalSwitch.set(nextStatementSwitch); - J.Return lastReturn = canSwitchBeReturnedInline(index, block.getStatements(), originalVariable.getSimpleName()); - if (lastReturn != null) { - inlinedReturn.set( - lastReturn - .withExpression(newSwitchExpression) - .withPrefix(lastReturn.getPrefix() - .withComments(ListUtils.concatAll(vd.getComments(), ListUtils.concatAll(nextStatementSwitch.getComments(), lastReturn.getComments()))) - .withWhitespace(vd.getPrefix().getWhitespace()) - ) - ); - return null; // We're inlining on return, remove the original variable declaration. - } return vd .withVariables(singletonList(originalVariable.getPadding().withInitializer( JLeftPadded.build(newSwitchExpression).withBefore(Space.SINGLE_SPACE)))) @@ -108,23 +94,6 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } return statement; })); - return super.visitBlock(b, ctx); - } - - private J.@Nullable Return canSwitchBeReturnedInline(int currentStatementIndex, List blockStatements, String originalVariableName) { - if (currentStatementIndex + 3 == blockStatements.size()) { - Statement lastStatement = blockStatements.get(currentStatementIndex + 2); - if (lastStatement instanceof J.Return) { - J.Return lastReturn = (J.Return) lastStatement; - if (lastReturn.getExpression() instanceof J.Identifier) { - J.Identifier identifier = (J.Identifier) lastReturn.getExpression(); - if (identifier.getSimpleName().equals(originalVariableName)) { - return lastReturn; - } - } - } - } - return null; } private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { @@ -207,7 +176,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } private J.@Nullable Assignment extractValidAssignmentFromArrowCase(J caseBody, String variableName) { - if (caseBody instanceof J.Block && ((J.Block)caseBody).getStatements().size() == 1) { + if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) { caseBody = ((J.Block) caseBody).getStatements().get(0); } From a02ace0f16c65589eebbab425cf2822f877c243b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 23:36:59 +0200 Subject: [PATCH 35/46] Use `SemanticallyEqual.areEqual` instead of comparing String name --- ...SwitchCaseAssigningToSwitchExpression.java | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 79a9131a29..8671e6b858 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -22,6 +22,7 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.search.SemanticallyEqual; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; @@ -83,7 +84,6 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { J.VariableDeclarations.NamedVariable originalVariable = vd.getVariables().get(0); J.SwitchExpression newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, originalVariable); - if (newSwitchExpression != null) { originalSwitch.set(nextStatementSwitch); return vd @@ -97,7 +97,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { } private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { - final String variableName = originalVariable.getSimpleName(); + final J.Identifier variableName = originalVariable.getName(); AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); AtomicBoolean isUsingArrows = new AtomicBoolean(true); @@ -160,72 +160,64 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { if (shouldAddDefaultCase) { updatedCases.add( - createDefaultCase(isUsingArrows.get(), originalInitializer.withPrefix(Space.SINGLE_SPACE), originalSwitch) + createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get()) ); } - return - new J.SwitchExpression( - randomId(), - Space.SINGLE_SPACE, - Markers.EMPTY, - originalSwitch.getSelector(), - originalSwitch.getCases().withStatements(updatedCases), - originalVariable.getType() - ); + return new J.SwitchExpression( + randomId(), + Space.SINGLE_SPACE, + Markers.EMPTY, + originalSwitch.getSelector(), + originalSwitch.getCases().withStatements(updatedCases), + originalVariable.getType()); } - private J.@Nullable Assignment extractValidAssignmentFromArrowCase(J caseBody, String variableName) { + private J.@Nullable Assignment extractValidAssignmentFromArrowCase(J caseBody, J.Identifier variableName) { if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) { caseBody = ((J.Block) caseBody).getStatements().get(0); } - return validateAssignmentFromCase(caseBody, variableName); } - private J.@Nullable Assignment extractValidAssignmentFromColonCase(List caseStatements, boolean isLastCase, String variableName) { + private J.@Nullable Assignment extractValidAssignmentFromColonCase(List caseStatements, boolean isLastCase, J.Identifier variableName) { if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); } - if ((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) { return validateAssignmentFromCase(caseStatements.get(0), variableName); } - return null; } - private J.@Nullable Assignment validateAssignmentFromCase(J maybeAssignment, String variableName) { + private J.@Nullable Assignment validateAssignmentFromCase(J maybeAssignment, J.Identifier variableName) { if (maybeAssignment instanceof J.Assignment) { J.Assignment assignment = (J.Assignment) maybeAssignment; if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (variable.getSimpleName().equals(variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { + if (SemanticallyEqual.areEqual(variable, variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { return assignment; } } } - return null; } - private J.Case createDefaultCase(boolean arrow, Expression returnedExpression, J.Switch originalSwitch) { - String template = "switch(1) {\n" + "default" + (arrow ? " ->" : ": yield") + " #{any()};\n}"; + private J.Case createDefaultCase(J.Switch originalSwitch, Expression returnedExpression, boolean arrow) { J.Switch switchStatement = JavaTemplate.apply( - template, + "switch(1) { default" + (arrow ? " ->" : ": yield") + " #{any()}; }", new Cursor(getCursor(), originalSwitch), originalSwitch.getCoordinates().replace(), returnedExpression ); - return (J.Case) switchStatement.getCases().getStatements().get(0); } - private boolean containsIdentifier(String identifierName, Expression expression) { + private boolean containsIdentifier(J.Identifier identifierName, Expression expression) { return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { - if (id.getSimpleName().equals(identifierName)) { + if (SemanticallyEqual.areEqual(id, identifierName)) { found.set(true); return id; } From fa9e5b5448561f86c0202ebcce03ef0a713024ad Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Jul 2025 23:55:01 +0200 Subject: [PATCH 36/46] Detect two more forms of side effects --- ...SwitchCaseAssigningToSwitchExpression.java | 12 ++++++++++++ ...chCaseAssigningToSwitchExpressionTest.java | 19 +++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 8671e6b858..1446abf4a3 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -233,6 +233,12 @@ private boolean canHaveSideEffects(@Nullable Expression expression) { } return new JavaIsoVisitor() { + @Override + public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean found) { + found.set(true); + return super.visitAssignment(assignment, found); + } + @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) { found.set(true); @@ -245,6 +251,12 @@ public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { return newClass; } + @Override + public J.Unary visitUnary(J.Unary unary, AtomicBoolean found) { + found.set(true); + return super.visitUnary(unary, found); + } + private boolean isToStringImplicitlyCalled(Expression a, Expression b) { // Assuming an implicit `.toString()` call could have a side effect, but excluding // the java.lang.* classes from that rule. diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 810ea00dd1..2c8720bf20 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -27,8 +27,9 @@ class SwitchCaseAssigningToSwitchExpressionTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new SwitchCaseAssigningToSwitchExpression()).allSources(source -> version(source, 21) - ); + spec + .recipe(new SwitchCaseAssigningToSwitchExpression()) + .allSources(source -> version(source, 17)); } @DocumentExample @@ -493,6 +494,20 @@ void implicitToStringInvocation(int i, Test o) { default: orig = "hello"; break; } } + + void incrementOperator(int i, Test o) { + int n = i++; + switch (i) { + default: n = 5; break; + } + } + + void assignment(int i, Test o) { + int n = ( i = 2 ); + switch (i) { + default: n = 5; break; + } + } } """ ) From ceac40285079aaf545e44cf89e56511cc3d74062 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Jul 2025 00:40:23 +0200 Subject: [PATCH 37/46] Show the difference between colon and arrow case for `null, default` --- ...SwitchCaseAssigningToSwitchExpression.java | 3 +- ...chCaseAssigningToSwitchExpressionTest.java | 54 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 1446abf4a3..8a9af30fd5 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -109,7 +109,6 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { } J.Case caseItem = (J.Case) s; - if (caseItem.getCaseLabels().get(0) instanceof J.Identifier && ((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName().equals("default")) { isDefaultCaseAbsent.set(false); @@ -226,7 +225,7 @@ public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { }.reduce(expression, new AtomicBoolean()).get(); } - // Is any code elsewhere executed due to the provided expression + // Might the initializer affect the input or output of the switch expression? private boolean canHaveSideEffects(@Nullable Expression expression) { if (expression == null) { return false; diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 2c8720bf20..0cc96277b2 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -634,7 +634,7 @@ String doFormat() { } @Test - void doNotinlineWhenInappropriate() { + void doNotInlineWhenInappropriate() { rewriteRun( //language=java java( @@ -695,6 +695,58 @@ void noReturnedExpression() { ); } + @Test + void defaultAsSecondLabelColonCase() { + rewriteRun( + //language=java + java( + """ + class A { + void doFormat(String str) { + String formatted = "initialValue"; + switch (str) { + case "foo": formatted = "Foo"; + case "bar": formatted = "Bar"; + case null, default: formatted = "unknown"; + } + } + } + """ + ) + ); + } + + @Test + void defaultAsSecondLabelArrowCase() { + rewriteRun( + java( + """ + class B { + void doFormat(String str) { + String formatted = "initialValue"; + switch (str) { + case "foo" -> formatted = "Foo"; + case "bar" -> formatted = "Bar"; + case null, default -> formatted = "Other"; + } + } + } + """, + """ + class B { + void doFormat(String str) { + String formatted = switch (str) { + case "foo" -> "Foo"; + case "bar" -> "Bar"; + case null, default -> "Other"; + }; + } + } + """ + ) + ); + } + @Test void whitespaceAddedWhenNoOriginalAssignment() { rewriteRun( From f5bb4f6b644aa028222a1714fe4ce5223e3eaf77 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Jul 2025 01:04:01 +0200 Subject: [PATCH 38/46] Add another test case that should not be converted --- .../lang/SwitchCaseAssigningToSwitchExpression.java | 3 +-- .../lang/SwitchCaseAssigningToSwitchExpressionTest.java | 9 +++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 8a9af30fd5..3903d7a514 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -159,8 +159,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { if (shouldAddDefaultCase) { updatedCases.add( - createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get()) - ); + createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get())); } return new J.SwitchExpression( diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 0cc96277b2..2c1a50c246 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -589,6 +589,15 @@ void exhaustiveButCantAddDefaultAndMissingAssignment(TrafficLight light) { } } + void exhaustiveButCantAddDefaultAfterEmptyLabel(TrafficLight light) { + String status = "initialValue"; + switch (light) { + case RED: status = "stop"; break; + case GREEN: status = "go"; break; + case YELLOW: + } + } + void exhaustiveButMissingAssignment(TrafficLight light) { String status; switch (light) { From bf98d52f730d5aae5d689c497b3cb1f36bb173b0 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Jul 2025 01:13:54 +0200 Subject: [PATCH 39/46] Polish --- ...SwitchCaseAssigningToSwitchExpression.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 3903d7a514..5faf4f8850 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -97,7 +97,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { } private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { - final J.Identifier variableName = originalVariable.getName(); + J.Identifier variableName = originalVariable.getName(); AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); AtomicBoolean isUsingArrows = new AtomicBoolean(true); @@ -115,7 +115,11 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { } if (caseItem.getBody() != null) { // arrow cases - J.Assignment assignment = extractValidAssignmentFromArrowCase(caseItem.getBody(), variableName); + J caseBody = caseItem.getBody(); + if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) { + caseBody = ((J.Block) caseBody).getStatements().get(0); + } + J.Assignment assignment = extractAssignmentOfVariable(caseBody, variableName); if (assignment != null) { return caseItem.withBody(assignment.getAssignment()); } @@ -131,7 +135,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { return caseItem; } - J.Assignment assignment = extractValidAssignmentFromColonCase(caseStatements, isLastCase, variableName); + J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, variableName); if (assignment != null) { J.Yield yieldStatement = new J.Yield( randomId(), @@ -171,29 +175,23 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { originalVariable.getType()); } - private J.@Nullable Assignment extractValidAssignmentFromArrowCase(J caseBody, J.Identifier variableName) { - if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) { - caseBody = ((J.Block) caseBody).getStatements().get(0); - } - return validateAssignmentFromCase(caseBody, variableName); - } - - private J.@Nullable Assignment extractValidAssignmentFromColonCase(List caseStatements, boolean isLastCase, J.Identifier variableName) { + private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, J.Identifier variableName) { if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); } if ((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) { - return validateAssignmentFromCase(caseStatements.get(0), variableName); + return extractAssignmentOfVariable(caseStatements.get(0), variableName); } return null; } - private J.@Nullable Assignment validateAssignmentFromCase(J maybeAssignment, J.Identifier variableName) { + private J.@Nullable Assignment extractAssignmentOfVariable(J maybeAssignment, J.Identifier variableName) { if (maybeAssignment instanceof J.Assignment) { J.Assignment assignment = (J.Assignment) maybeAssignment; if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (SemanticallyEqual.areEqual(variable, variableName) && !containsIdentifier(variableName, assignment.getAssignment())) { + if (SemanticallyEqual.areEqual(variable, variableName) && + !containsIdentifier(variableName, assignment.getAssignment())) { return assignment; } } From 4e3666e35ab4e5a712b82a68efd0d993826a21bb Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Tue, 22 Jul 2025 11:47:08 -0400 Subject: [PATCH 40/46] polish --- ...SwitchCaseAssigningToSwitchExpression.java | 20 +++++++++---------- ...chCaseAssigningToSwitchExpressionTest.java | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java index 5faf4f8850..6a0310a8d1 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java @@ -97,7 +97,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { } private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { - J.Identifier variableName = originalVariable.getName(); + J.Identifier originalVariableId = originalVariable.getName(); AtomicBoolean isQualified = new AtomicBoolean(true); AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); AtomicBoolean isUsingArrows = new AtomicBoolean(true); @@ -119,7 +119,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) { caseBody = ((J.Block) caseBody).getStatements().get(0); } - J.Assignment assignment = extractAssignmentOfVariable(caseBody, variableName); + J.Assignment assignment = extractAssignmentOfVariable(caseBody, originalVariableId); if (assignment != null) { return caseItem.withBody(assignment.getAssignment()); } @@ -135,7 +135,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { return caseItem; } - J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, variableName); + J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, originalVariableId); if (assignment != null) { J.Yield yieldStatement = new J.Yield( randomId(), @@ -175,23 +175,23 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { originalVariable.getType()); } - private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, J.Identifier variableName) { + private J.@Nullable Assignment extractAssignmentFromColonCase(List caseStatements, boolean isLastCase, J.Identifier variableId) { if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) { caseStatements = ((J.Block) caseStatements.get(0)).getStatements(); } if ((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) { - return extractAssignmentOfVariable(caseStatements.get(0), variableName); + return extractAssignmentOfVariable(caseStatements.get(0), variableId); } return null; } - private J.@Nullable Assignment extractAssignmentOfVariable(J maybeAssignment, J.Identifier variableName) { + private J.@Nullable Assignment extractAssignmentOfVariable(J maybeAssignment, J.Identifier variableId) { if (maybeAssignment instanceof J.Assignment) { J.Assignment assignment = (J.Assignment) maybeAssignment; if (assignment.getVariable() instanceof J.Identifier) { J.Identifier variable = (J.Identifier) assignment.getVariable(); - if (SemanticallyEqual.areEqual(variable, variableName) && - !containsIdentifier(variableName, assignment.getAssignment())) { + if (SemanticallyEqual.areEqual(variable, variableId) && + !containsIdentifier(variableId, assignment.getAssignment())) { return assignment; } } @@ -209,11 +209,11 @@ private J.Case createDefaultCase(J.Switch originalSwitch, Expression returnedExp return (J.Case) switchStatement.getCases().getStatements().get(0); } - private boolean containsIdentifier(J.Identifier identifierName, Expression expression) { + private boolean containsIdentifier(J.Identifier identifier, Expression expression) { return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { - if (SemanticallyEqual.areEqual(id, identifierName)) { + if (SemanticallyEqual.areEqual(id, identifier)) { found.set(true); return id; } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java index 2c1a50c246..534575e0ab 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java @@ -495,14 +495,14 @@ void implicitToStringInvocation(int i, Test o) { } } - void incrementOperator(int i, Test o) { + void incrementOperator(int i) { int n = i++; switch (i) { default: n = 5; break; } } - void assignment(int i, Test o) { + void assignment(int i) { int n = ( i = 2 ); switch (i) { default: n = 5; break; From c29b44c6f3760731bc777c9c1e985f7d81e764f8 Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Tue, 22 Jul 2025 16:00:33 -0400 Subject: [PATCH 41/46] filter out members that aren't Enum Constants when checking for switch exhaustiveness on an enum switch selector --- .../java/migrate/lang/SwitchUtils.java | 7 +++--- .../java/migrate/lang/SwitchUtilsTest.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index 0ab2e07aaa..94bf83bc89 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -25,13 +25,12 @@ class SwitchUtils { /** * Checks if a switch statement covers all possible values of its selector. * This is typically used to determine if a switch statement is "exhaustive" as per the Java language specification. - * + *

* NOTE: Missing support for sealed classes/interfaces. * - * @See Switch Expressions in Java 21 - * * @param switch_ the switch statement to check * @return true if the switch covers all possible values, false otherwise + * @See Switch Expressions in Java 21 */ public static boolean coversAllPossibleValues(J.Switch switch_) { List labels = new ArrayList<>(); @@ -46,7 +45,7 @@ public static boolean coversAllPossibleValues(J.Switch switch_) { JavaType javaType = switch_.getSelector().getTree().getType(); if (javaType instanceof JavaType.Class && ((JavaType.Class) javaType).getKind() == JavaType.FullyQualified.Kind.Enum) { // Every enum value must be present in the switch - return ((JavaType.Class) javaType).getMembers().stream().allMatch(variable -> + return ((JavaType.Class) javaType).getMembers().stream().filter(member -> member.hasFlags(Flag.Enum)).allMatch(variable -> labels.stream().anyMatch(label -> { if (!(label instanceof TypeTree && TypeUtils.isOfType(((TypeTree) label).getType(), javaType))) { return false; diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java index 8c3697c8a8..d4d1351fb6 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java @@ -165,4 +165,28 @@ void method(Shape shape) { ) ); } + + @Test + void coversAllCasesEnumWithExtraMembers() { + assertTrue( + SwitchUtils.coversAllPossibleValues( + extractSwitch( + """ + class Test { + enum EnumWithExtraMembers { + ONE, TWO; + public static final EnumWithExtraMembers THREE = ONE; + } + void method(EnumWithExtraMembers e) { + switch (e) { + case ONE -> System.out.println("one"); + case TWO -> System.out.println("two"); + } + } + } + """ + ) + ) + ); + } } From c17a0ccf42af4688052e8b8638aed47f17fde8b7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 17:58:45 +0200 Subject: [PATCH 42/46] Minor changes --- ...tchCaseAssignmentsToSwitchExpression.java} | 2 +- .../java/migrate/lang/SwitchUtils.java | 41 +++++++++++-------- .../META-INF/rewrite/java-version-17.yml | 2 +- ...aseAssignmentsToSwitchExpressionTest.java} | 4 +- 4 files changed, 27 insertions(+), 22 deletions(-) rename src/main/java/org/openrewrite/java/migrate/lang/{SwitchCaseAssigningToSwitchExpression.java => SwitchCaseAssignmentsToSwitchExpression.java} (99%) rename src/test/java/org/openrewrite/java/migrate/lang/{SwitchCaseAssigningToSwitchExpressionTest.java => SwitchCaseAssignmentsToSwitchExpressionTest.java} (99%) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java similarity index 99% rename from src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java rename to src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java index 6a0310a8d1..c453cb7a61 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java @@ -40,7 +40,7 @@ @Value @EqualsAndHashCode(callSuper = false) -public class SwitchCaseAssigningToSwitchExpression extends Recipe { +public class SwitchCaseAssignmentsToSwitchExpression extends Recipe { @Override public String getDisplayName() { return "Convert assigning Switch statements to Switch expressions"; diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index 94bf83bc89..4e1c86990a 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -17,9 +17,9 @@ import org.openrewrite.java.tree.*; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import java.util.*; + +import static java.util.stream.Collectors.toSet; class SwitchUtils { /** @@ -43,21 +43,26 @@ public static boolean coversAllPossibleValues(J.Switch switch_) { } } JavaType javaType = switch_.getSelector().getTree().getType(); - if (javaType instanceof JavaType.Class && ((JavaType.Class) javaType).getKind() == JavaType.FullyQualified.Kind.Enum) { - // Every enum value must be present in the switch - return ((JavaType.Class) javaType).getMembers().stream().filter(member -> member.hasFlags(Flag.Enum)).allMatch(variable -> - labels.stream().anyMatch(label -> { - if (!(label instanceof TypeTree && TypeUtils.isOfType(((TypeTree) label).getType(), javaType))) { - return false; - } - J.Identifier enumName = null; - if (label instanceof J.Identifier) { - enumName = (J.Identifier) label; - } else if (label instanceof J.FieldAccess) { - enumName = ((J.FieldAccess) label).getName(); - } - return enumName != null && Objects.equals(variable.getName(), enumName.getSimpleName()); - })); + if (javaType instanceof JavaType.Class) { + JavaType.Class javaTypeClass = (JavaType.Class) javaType; + if (javaTypeClass.hasFlags(Flag.Enum)) { + Collection labelValues = labels.stream() + .filter(label -> label instanceof J.Identifier || label instanceof J.FieldAccess) + .filter(label -> TypeUtils.isOfType(((TypeTree) label).getType(), javaType)) + .map(label -> label instanceof J.Identifier ? + ((J.Identifier) label).getSimpleName() : + ((J.FieldAccess) label).getName().getSimpleName()) + .collect(toSet()); + if (labelValues.isEmpty()) { + return false; + } + Collection enumValues = javaTypeClass.getMembers().stream() + .filter(member -> member.hasFlags(Flag.Enum)) + .map(JavaType.Variable::getName) + .collect(toSet()); + // Every enum value must be present in the switch + return !enumValues.isEmpty() && labelValues.containsAll(enumValues); + } } return false; } diff --git a/src/main/resources/META-INF/rewrite/java-version-17.yml b/src/main/resources/META-INF/rewrite/java-version-17.yml index 06d3ebaea7..20f9c20bc3 100644 --- a/src/main/resources/META-INF/rewrite/java-version-17.yml +++ b/src/main/resources/META-INF/rewrite/java-version-17.yml @@ -59,7 +59,7 @@ recipeList: artifactId: commons-codec newVersion: 1.17.x - org.openrewrite.java.migrate.AddLombokMapstructBinding - - org.openrewrite.java.migrate.lang.SwitchCaseAssigningToSwitchExpression + - org.openrewrite.java.migrate.lang.SwitchCaseAssignmentsToSwitchExpression --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java rename to src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java index 534575e0ab..c03ecde7e5 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java @@ -24,11 +24,11 @@ import static org.openrewrite.java.Assertions.version; @SuppressWarnings({"EnhancedSwitchMigration", "RedundantLabeledSwitchRuleCodeBlock", "StringOperationCanBeSimplified", "SwitchStatementWithTooFewBranches", "UnnecessaryReturnStatement", "UnusedAssignment"}) -class SwitchCaseAssigningToSwitchExpressionTest implements RewriteTest { +class SwitchCaseAssignmentsToSwitchExpressionTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { spec - .recipe(new SwitchCaseAssigningToSwitchExpression()) + .recipe(new SwitchCaseAssignmentsToSwitchExpression()) .allSources(source -> version(source, 17)); } From 1464af47e5cf8f3feaebb1aeaf5dc13117312634 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 18:10:57 +0200 Subject: [PATCH 43/46] Minimize which code paths are hit when --- .../SwitchCaseAssignmentsToSwitchExpression.java | 12 ++++++------ .../openrewrite/java/migrate/lang/SwitchUtils.java | 2 +- .../java/migrate/lang/SwitchUtilsTest.java | 6 ++---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java index c453cb7a61..a4f437afd0 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpression.java @@ -110,7 +110,7 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { J.Case caseItem = (J.Case) s; if (caseItem.getCaseLabels().get(0) instanceof J.Identifier && - ((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName().equals("default")) { + "default".equals(((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName())) { isDefaultCaseAbsent.set(false); } @@ -151,19 +151,19 @@ public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { isQualified.set(false); return null; }); + if (!isQualified.get()) { + return null; + } boolean shouldAddDefaultCase = isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch); Expression originalInitializer = originalVariable.getInitializer(); - - if (!isQualified.get() || - (originalInitializer == null && shouldAddDefaultCase) || + if ((originalInitializer == null && shouldAddDefaultCase) || (isLastCaseEmpty.get() && !shouldAddDefaultCase)) { return null; } if (shouldAddDefaultCase) { - updatedCases.add( - createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get())); + updatedCases.add(createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get())); } return new J.SwitchExpression( diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index 4e1c86990a..b89c72158e 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -30,7 +30,7 @@ class SwitchUtils { * * @param switch_ the switch statement to check * @return true if the switch covers all possible values, false otherwise - * @See Switch Expressions in Java 21 + * @see Switch Expressions in Java 21 */ public static boolean coversAllPossibleValues(J.Switch switch_) { List labels = new ArrayList<>(); diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java index d4d1351fb6..10904df328 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchUtilsTest.java @@ -17,7 +17,6 @@ import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaParser; import org.openrewrite.java.tree.J; @@ -143,10 +142,9 @@ void method(Object obj) { ); } - @ExpectedToFail("Not implemented yet for sealed classes") @Test void coversAllCasesAllSealedClasses() { - assertTrue( + assertFalse( SwitchUtils.coversAllPossibleValues( extractSwitch( """ @@ -162,7 +160,7 @@ void method(Shape shape) { } """ ) - ) + ), "Not implemented yet for sealed classes" ); } From dccd6ecbd26659c0c722be7d9e80723b4f6a0d85 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 18:27:40 +0200 Subject: [PATCH 44/46] Add explicit tests for fall through handling --- ...CaseAssignmentsToSwitchExpressionTest.java | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java index c03ecde7e5..cda324c392 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/SwitchCaseAssignmentsToSwitchExpressionTest.java @@ -714,12 +714,23 @@ class A { void doFormat(String str) { String formatted = "initialValue"; switch (str) { - case "foo": formatted = "Foo"; - case "bar": formatted = "Bar"; + case "foo": formatted = "Foo"; break; + case "bar": formatted = "Bar"; break; case null, default: formatted = "unknown"; } } } + """, + """ + class A { + void doFormat(String str) { + String formatted = switch (str) { + case "foo": yield "Foo"; + case "bar": yield "Bar"; + case null, default: yield "unknown"; + }; + } + } """ ) ); @@ -863,4 +874,48 @@ String doFormat() { ) ); } + + @Test + void notConvertWhenFallThrough() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(String str) { + String formatted = "initialValue"; + switch (str) { + case "A": formatted = "A"; // no break + case "B": formatted = "B"; // no break + case "C": formatted = "C"; // no break + default: formatted = "Z"; + } + } + } + """ + ) + ); + } + + @Test + void notConvertWhenFallThroughAppends() { + rewriteRun( + //language=java + java( + """ + class Test { + void doFormat(String str) { + String formatted = "initialValue"; + switch (str) { + case "A": formatted = "A"; + case "B": formatted = formatted + "B"; + case "C": formatted = formatted + "C"; + default: formatted = "Z"; break; + } + } + } + """ + ) + ); + } } From c9420014f642410951da1b8ebb46dfc26b10ead5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 18:41:27 +0200 Subject: [PATCH 45/46] Update src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/org/openrewrite/java/migrate/lang/SwitchUtils.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index b89c72158e..b96994c54d 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -17,8 +17,9 @@ import org.openrewrite.java.tree.*; -import java.util.*; - +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import static java.util.stream.Collectors.toSet; class SwitchUtils { From 3ca3e105ac03581dd27309eaf58394a95d9c5440 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 18:41:55 +0200 Subject: [PATCH 46/46] Add missing newline --- src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java index b96994c54d..baf2ce77b6 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; + import static java.util.stream.Collectors.toSet; class SwitchUtils {