-
Notifications
You must be signed in to change notification settings - Fork 109
Convert assigning Switch statements to Switch expressions #795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+1,499
−32
Merged
Changes from 41 commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
c4a7f0c
refactor: extract helper method in util class
pdelagrave 09addfd
Tests
pdelagrave c605a36
Cleaned up tests
pdelagrave 8f52dd6
WIP - Working but yet to be cleaned up
pdelagrave 790fafe
Apply best practices to avoid repeated bot suggestions
timtebeek d8769bc
Use `reduce` and `visitSwitch` to extract Switch element
timtebeek 5f1c4a3
Fix six space indentation
timtebeek 4e56657
Reduce visibility of SwitchUtils
timtebeek 56d9ba1
Apply suggestions from code review
timtebeek ccb54c2
fix tests to work with updated rewrite-java
pdelagrave cfaa216
Java 17 precondition
pdelagrave 750fe6b
Don't add a `default` case if the switch is already exhaustive
pdelagrave 483ace0
Do not apply the recipe if a case statement/body assignment is refere…
pdelagrave 8e75a6b
Do not apply the recipe if the original variable initializer is anyth…
pdelagrave 0212743
Organize imports
timtebeek 1bdd386
Move the recipe to the right java migration declarative meta recipe
pdelagrave dc63a4f
check for implicit toString() calls in the original variable initiali…
pdelagrave da7f8f4
Do not apply the recipe if a colon-switch label has an empty statemen…
pdelagrave 4bd2243
Do not apply the recipe if the original variable has no initializer a…
pdelagrave 4fb8479
Small cleanup
pdelagrave 44f47b4
Inline the switch expression on the return statement when appropriate
pdelagrave e574f58
Comments are preserved
pdelagrave 6bb9908
Apply formatter
timtebeek 3d9664d
Ignore warnings on test text blocks
timtebeek 77111a5
Sort annotations
timtebeek ced20f5
Merge branch 'main' into switchexpr
timtebeek 62d2ec3
Add a new test not yet covered and update expectations on formatting
timtebeek cf6740f
Use `JavaIsoVisitor` since we're not changing types
timtebeek 50125b1
Update src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssi…
timtebeek d2746b4
For now expect missing whitespace
timtebeek d42a2d3
Add a space before `=` when there was no previous initializer
timtebeek 86918c1
Add support for last colon case doing assignment without a break;
pdelagrave f8a43e4
Make buildNewSwitchExpression more readable
pdelagrave f24117a
clean-up the assignment validation and extraction code further
pdelagrave 0754a4c
Delegate to `InlineVariable` to inline return
timtebeek a02ace0
Use `SemanticallyEqual.areEqual` instead of comparing String name
timtebeek fa9e5b5
Detect two more forms of side effects
timtebeek ceac402
Show the difference between colon and arrow case for `null, default`
timtebeek f5bb4f6
Add another test case that should not be converted
timtebeek bf98d52
Polish
timtebeek 4e3666e
polish
pdelagrave c29b44c
filter out members that aren't Enum Constants when checking for switc…
pdelagrave f9637bc
Merge branch 'main' into switchexpr
pdelagrave c17a0cc
Minor changes
timtebeek 1464af4
Minimize which code paths are hit when
timtebeek dccd6ec
Add explicit tests for fall through handling
timtebeek c942001
Update src/main/java/org/openrewrite/java/migrate/lang/SwitchUtils.java
timtebeek 3ca3e10
Add missing newline
timtebeek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
283 changes: 283 additions & 0 deletions
283
src/main/java/org/openrewrite/java/migrate/lang/SwitchCaseAssigningToSwitchExpression.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,283 @@ | ||
| /* | ||
| * Copyright 2025 the original author or authors. | ||
| * <p> | ||
| * 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 | ||
| * <p> | ||
| * https://docs.moderne.io/licensing/moderne-source-available-license | ||
| * <p> | ||
| * 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.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.search.SemanticallyEqual; | ||
| 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; | ||
|
|
||
| import java.util.List; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
|
|
||
| import static java.util.Collections.singletonList; | ||
| import static java.util.Objects.requireNonNull; | ||
| 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 17 and later."; | ||
| } | ||
|
|
||
| @Override | ||
| public TreeVisitor<?, ExecutionContext> getVisitor() { | ||
| TreeVisitor<?, ExecutionContext> preconditions = Preconditions.and( | ||
| new UsesJavaVersion<>(17), | ||
| Preconditions.not(new KotlinFileChecker<>()), | ||
| Preconditions.not(new GroovyFileChecker<>()) | ||
| ); | ||
| return Preconditions.check(preconditions, new JavaIsoVisitor<ExecutionContext>() { | ||
| @Override | ||
| public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) { | ||
| J.Block block = super.visitBlock(originalBlock, ctx); | ||
|
|
||
| AtomicReference<J.@Nullable Switch> originalSwitch = new AtomicReference<>(); | ||
|
|
||
| int lastIndex = block.getStatements().size() - 1; | ||
| 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 && | ||
| 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; | ||
| J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1); | ||
|
|
||
| J.VariableDeclarations.NamedVariable originalVariable = vd.getVariables().get(0); | ||
| J.SwitchExpression newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, originalVariable); | ||
| if (newSwitchExpression != null) { | ||
| originalSwitch.set(nextStatementSwitch); | ||
| return vd | ||
| .withVariables(singletonList(originalVariable.getPadding().withInitializer( | ||
| JLeftPadded.<Expression>build(newSwitchExpression).withBefore(Space.SINGLE_SPACE)))) | ||
| .withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments())); | ||
| } | ||
| } | ||
| return statement; | ||
| })); | ||
| } | ||
|
|
||
| private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) { | ||
| J.Identifier originalVariableId = originalVariable.getName(); | ||
| AtomicBoolean isQualified = new AtomicBoolean(true); | ||
| AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true); | ||
| AtomicBoolean isUsingArrows = new AtomicBoolean(true); | ||
| AtomicBoolean isLastCaseEmpty = new AtomicBoolean(false); | ||
|
|
||
| List<Statement> updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), (index, s) -> { | ||
| if (!isQualified.get()) { | ||
| return null; | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
|
|
||
| if (caseItem.getBody() != null) { // arrow cases | ||
| 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, originalVariableId); | ||
| if (assignment != null) { | ||
| return caseItem.withBody(assignment.getAssignment()); | ||
| } | ||
| } else { // colon cases | ||
| isUsingArrows.set(false); | ||
| boolean isLastCase = index + 1 == originalSwitch.getCases().getStatements().size(); | ||
|
|
||
| List<Statement> caseStatements = caseItem.getStatements(); | ||
| if (caseStatements.isEmpty()) { | ||
| if (isLastCase) { | ||
| isLastCaseEmpty.set(true); | ||
| } | ||
| return caseItem; | ||
| } | ||
|
|
||
| J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, originalVariableId); | ||
| if (assignment != null) { | ||
| J.Yield yieldStatement = new J.Yield( | ||
| randomId(), | ||
| assignment.getPrefix().withWhitespace(" "), | ||
| Markers.EMPTY, | ||
| false, | ||
| assignment.getAssignment() | ||
| ); | ||
| return caseItem.withStatements(singletonList(yieldStatement)); | ||
| } | ||
| } | ||
|
|
||
| isQualified.set(false); | ||
| return null; | ||
| }); | ||
|
|
||
| boolean shouldAddDefaultCase = isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch); | ||
| Expression originalInitializer = originalVariable.getInitializer(); | ||
|
|
||
| if (!isQualified.get() || | ||
| (originalInitializer == null && shouldAddDefaultCase) || | ||
| (isLastCaseEmpty.get() && !shouldAddDefaultCase)) { | ||
| return null; | ||
| } | ||
|
|
||
| if (shouldAddDefaultCase) { | ||
| updatedCases.add( | ||
| 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()); | ||
| } | ||
|
|
||
| private J.@Nullable Assignment extractAssignmentFromColonCase(List<Statement> 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), variableId); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| 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, variableId) && | ||
| !containsIdentifier(variableId, assignment.getAssignment())) { | ||
| return assignment; | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private J.Case createDefaultCase(J.Switch originalSwitch, Expression returnedExpression, boolean arrow) { | ||
| J.Switch switchStatement = JavaTemplate.apply( | ||
| "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(J.Identifier identifier, Expression expression) { | ||
| return new JavaIsoVisitor<AtomicBoolean>() { | ||
| @Override | ||
| public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) { | ||
| if (SemanticallyEqual.areEqual(id, identifier)) { | ||
| found.set(true); | ||
| return id; | ||
| } | ||
| return super.visitIdentifier(id, found); | ||
| } | ||
| }.reduce(expression, new AtomicBoolean()).get(); | ||
| } | ||
|
|
||
| // Might the initializer affect the input or output of the switch expression? | ||
| private boolean canHaveSideEffects(@Nullable Expression expression) { | ||
| if (expression == null) { | ||
| return false; | ||
| } | ||
|
|
||
| return new JavaIsoVisitor<AtomicBoolean>() { | ||
| @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); | ||
| return method; | ||
| } | ||
|
|
||
| @Override | ||
| public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { | ||
| found.set(true); | ||
| 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. | ||
| 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 (isToStringImplicitlyCalled(binary.getLeft(), binary.getRight()) || | ||
| isToStringImplicitlyCalled(binary.getRight(), binary.getLeft())) { | ||
| found.set(true); | ||
| return binary; | ||
| } | ||
| return super.visitBinary(binary, found); | ||
| } | ||
| }.reduce(expression, new AtomicBoolean()).get(); | ||
| } | ||
| } | ||
| ); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.