Skip to content

Commit 5906a02

Browse files
pdelagravetimtebeekgithub-actions[bot]
authored
Convert assigning Switch statements to Switch expressions (#795)
* refactor: extract helper method in util class * Tests * Cleaned up tests * WIP - Working but yet to be cleaned up * Apply best practices to avoid repeated bot suggestions * Use `reduce` and `visitSwitch` to extract Switch element * Fix six space indentation * Reduce visibility of SwitchUtils * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix tests to work with updated rewrite-java * Java 17 precondition * Don't add a `default` case if the switch is already exhaustive * Do not apply the recipe if a case statement/body assignment is referencing the original variable * 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 * Organize imports * Move the recipe to the right java migration declarative meta recipe * check for implicit toString() calls in the original variable initializer when checking for side effects * Do not apply the recipe if a colon-switch label has an empty statement list AND is the last case of the switch * 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 * Small cleanup * Inline the switch expression on the return statement when appropriate * Comments are preserved * Apply formatter * Ignore warnings on test text blocks * Sort annotations * Add a new test not yet covered and update expectations on formatting * Use `JavaIsoVisitor` since we're not changing types * 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> * For now expect missing whitespace * Add a space before `=` when there was no previous initializer * Add support for last colon case doing assignment without a break; * Make buildNewSwitchExpression more readable * 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. * Delegate to `InlineVariable` to inline return * Use `SemanticallyEqual.areEqual` instead of comparing String name * Detect two more forms of side effects * Show the difference between colon and arrow case for `null, default` * Add another test case that should not be converted * Polish * polish * filter out members that aren't Enum Constants when checking for switch exhaustiveness on an enum switch selector * Minor changes * Minimize which code paths are hit when * Add explicit tests for fall through handling * 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> * Add missing newline --------- Co-authored-by: Tim te Beek <tim@moderne.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 98c7a8a commit 5906a02

File tree

7 files changed

+1499
-32
lines changed

7 files changed

+1499
-32
lines changed

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

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,22 @@
2424
import org.openrewrite.java.JavaVisitor;
2525
import org.openrewrite.java.search.SemanticallyEqual;
2626
import org.openrewrite.java.search.UsesJavaVersion;
27-
import org.openrewrite.java.tree.*;
27+
import org.openrewrite.java.tree.Expression;
28+
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.java.tree.Space;
30+
import org.openrewrite.java.tree.Statement;
2831
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
2932
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
3033

3134
import java.time.Duration;
3235
import java.util.ArrayList;
3336
import java.util.List;
34-
import java.util.Objects;
3537
import java.util.Optional;
3638
import java.util.concurrent.atomic.AtomicReference;
3739

3840
import static java.util.Objects.requireNonNull;
3941
import static org.openrewrite.java.migrate.lang.NullCheck.Matcher.nullCheck;
42+
import static org.openrewrite.java.migrate.lang.SwitchUtils.coversAllPossibleValues;
4043

4144
@EqualsAndHashCode(callSuper = false)
4245
@Value
@@ -184,36 +187,6 @@ private J.Case createCaseStatement(J.Switch aSwitch, Statement whenNull, J.Case
184187

185188
return nullCase.withStatements(ListUtils.mapFirst(nullCase.getStatements(), s -> s == null ? null : s.withPrefix(currentFirstCaseIndentation)));
186189
}
187-
188-
private boolean coversAllPossibleValues(J.Switch switch_) {
189-
List<J> labels = new ArrayList<>();
190-
for (Statement statement : switch_.getCases().getStatements()) {
191-
for (J j : ((J.Case) statement).getCaseLabels()) {
192-
if (j instanceof J.Identifier && "default".equals(((J.Identifier) j).getSimpleName())) {
193-
return true;
194-
}
195-
labels.add(j);
196-
}
197-
}
198-
JavaType javaType = switch_.getSelector().getTree().getType();
199-
if (javaType instanceof JavaType.Class && ((JavaType.Class) javaType).getKind() == JavaType.FullyQualified.Kind.Enum) {
200-
// Every enum value must be present in the switch
201-
return ((JavaType.Class) javaType).getMembers().stream().allMatch(variable ->
202-
labels.stream().anyMatch(label -> {
203-
if (!(label instanceof TypeTree && TypeUtils.isOfType(((TypeTree) label).getType(), javaType))) {
204-
return false;
205-
}
206-
J.Identifier enumName = null;
207-
if (label instanceof J.Identifier) {
208-
enumName = (J.Identifier) label;
209-
} else if (label instanceof J.FieldAccess) {
210-
enumName = ((J.FieldAccess) label).getName();
211-
}
212-
return enumName != null && Objects.equals(variable.getName(), enumName.getSimpleName());
213-
}));
214-
}
215-
return false;
216-
}
217190
});
218191
}
219192
}
Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.java.migrate.lang;
17+
18+
import lombok.EqualsAndHashCode;
19+
import lombok.Value;
20+
import org.jspecify.annotations.Nullable;
21+
import org.openrewrite.*;
22+
import org.openrewrite.internal.ListUtils;
23+
import org.openrewrite.java.JavaIsoVisitor;
24+
import org.openrewrite.java.JavaTemplate;
25+
import org.openrewrite.java.search.SemanticallyEqual;
26+
import org.openrewrite.java.search.UsesJavaVersion;
27+
import org.openrewrite.java.tree.*;
28+
import org.openrewrite.marker.Markers;
29+
import org.openrewrite.staticanalysis.InlineVariable;
30+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
31+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
32+
33+
import java.util.List;
34+
import java.util.concurrent.atomic.AtomicBoolean;
35+
import java.util.concurrent.atomic.AtomicReference;
36+
37+
import static java.util.Collections.singletonList;
38+
import static java.util.Objects.requireNonNull;
39+
import static org.openrewrite.Tree.randomId;
40+
41+
@Value
42+
@EqualsAndHashCode(callSuper = false)
43+
public class SwitchCaseAssignmentsToSwitchExpression extends Recipe {
44+
@Override
45+
public String getDisplayName() {
46+
return "Convert assigning Switch statements to Switch expressions";
47+
}
48+
49+
@Override
50+
public String getDescription() {
51+
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. " +
52+
"This is only applicable for Java 17 and later.";
53+
}
54+
55+
@Override
56+
public TreeVisitor<?, ExecutionContext> getVisitor() {
57+
TreeVisitor<?, ExecutionContext> preconditions = Preconditions.and(
58+
new UsesJavaVersion<>(17),
59+
Preconditions.not(new KotlinFileChecker<>()),
60+
Preconditions.not(new GroovyFileChecker<>())
61+
);
62+
return Preconditions.check(preconditions, new JavaIsoVisitor<ExecutionContext>() {
63+
@Override
64+
public J.Block visitBlock(J.Block originalBlock, ExecutionContext ctx) {
65+
J.Block block = super.visitBlock(originalBlock, ctx);
66+
67+
AtomicReference<J.@Nullable Switch> originalSwitch = new AtomicReference<>();
68+
69+
int lastIndex = block.getStatements().size() - 1;
70+
return block.withStatements(ListUtils.map(block.getStatements(), (index, statement) -> {
71+
if (statement == originalSwitch.getAndSet(null)) {
72+
doAfterVisit(new InlineVariable().getVisitor());
73+
// We've already converted the switch/assignments to an assignment with a switch expression.
74+
return null;
75+
}
76+
77+
if (index < lastIndex &&
78+
statement instanceof J.VariableDeclarations &&
79+
((J.VariableDeclarations) statement).getVariables().size() == 1 &&
80+
!canHaveSideEffects(((J.VariableDeclarations) statement).getVariables().get(0).getInitializer()) &&
81+
block.getStatements().get(index + 1) instanceof J.Switch) {
82+
J.VariableDeclarations vd = (J.VariableDeclarations) statement;
83+
J.Switch nextStatementSwitch = (J.Switch) block.getStatements().get(index + 1);
84+
85+
J.VariableDeclarations.NamedVariable originalVariable = vd.getVariables().get(0);
86+
J.SwitchExpression newSwitchExpression = buildNewSwitchExpression(nextStatementSwitch, originalVariable);
87+
if (newSwitchExpression != null) {
88+
originalSwitch.set(nextStatementSwitch);
89+
return vd
90+
.withVariables(singletonList(originalVariable.getPadding().withInitializer(
91+
JLeftPadded.<Expression>build(newSwitchExpression).withBefore(Space.SINGLE_SPACE))))
92+
.withComments(ListUtils.concatAll(vd.getComments(), nextStatementSwitch.getComments()));
93+
}
94+
}
95+
return statement;
96+
}));
97+
}
98+
99+
private J.@Nullable SwitchExpression buildNewSwitchExpression(J.Switch originalSwitch, J.VariableDeclarations.NamedVariable originalVariable) {
100+
J.Identifier originalVariableId = originalVariable.getName();
101+
AtomicBoolean isQualified = new AtomicBoolean(true);
102+
AtomicBoolean isDefaultCaseAbsent = new AtomicBoolean(true);
103+
AtomicBoolean isUsingArrows = new AtomicBoolean(true);
104+
AtomicBoolean isLastCaseEmpty = new AtomicBoolean(false);
105+
106+
List<Statement> updatedCases = ListUtils.map(originalSwitch.getCases().getStatements(), (index, s) -> {
107+
if (!isQualified.get()) {
108+
return null;
109+
}
110+
111+
J.Case caseItem = (J.Case) s;
112+
if (caseItem.getCaseLabels().get(0) instanceof J.Identifier &&
113+
"default".equals(((J.Identifier) caseItem.getCaseLabels().get(0)).getSimpleName())) {
114+
isDefaultCaseAbsent.set(false);
115+
}
116+
117+
if (caseItem.getBody() != null) { // arrow cases
118+
J caseBody = caseItem.getBody();
119+
if (caseBody instanceof J.Block && ((J.Block) caseBody).getStatements().size() == 1) {
120+
caseBody = ((J.Block) caseBody).getStatements().get(0);
121+
}
122+
J.Assignment assignment = extractAssignmentOfVariable(caseBody, originalVariableId);
123+
if (assignment != null) {
124+
return caseItem.withBody(assignment.getAssignment());
125+
}
126+
} else { // colon cases
127+
isUsingArrows.set(false);
128+
boolean isLastCase = index + 1 == originalSwitch.getCases().getStatements().size();
129+
130+
List<Statement> caseStatements = caseItem.getStatements();
131+
if (caseStatements.isEmpty()) {
132+
if (isLastCase) {
133+
isLastCaseEmpty.set(true);
134+
}
135+
return caseItem;
136+
}
137+
138+
J.Assignment assignment = extractAssignmentFromColonCase(caseStatements, isLastCase, originalVariableId);
139+
if (assignment != null) {
140+
J.Yield yieldStatement = new J.Yield(
141+
randomId(),
142+
assignment.getPrefix().withWhitespace(" "),
143+
Markers.EMPTY,
144+
false,
145+
assignment.getAssignment()
146+
);
147+
return caseItem.withStatements(singletonList(yieldStatement));
148+
}
149+
}
150+
151+
isQualified.set(false);
152+
return null;
153+
});
154+
if (!isQualified.get()) {
155+
return null;
156+
}
157+
158+
boolean shouldAddDefaultCase = isDefaultCaseAbsent.get() && !SwitchUtils.coversAllPossibleValues(originalSwitch);
159+
Expression originalInitializer = originalVariable.getInitializer();
160+
if ((originalInitializer == null && shouldAddDefaultCase) ||
161+
(isLastCaseEmpty.get() && !shouldAddDefaultCase)) {
162+
return null;
163+
}
164+
165+
if (shouldAddDefaultCase) {
166+
updatedCases.add(createDefaultCase(originalSwitch, originalInitializer.withPrefix(Space.SINGLE_SPACE), isUsingArrows.get()));
167+
}
168+
169+
return new J.SwitchExpression(
170+
randomId(),
171+
Space.SINGLE_SPACE,
172+
Markers.EMPTY,
173+
originalSwitch.getSelector(),
174+
originalSwitch.getCases().withStatements(updatedCases),
175+
originalVariable.getType());
176+
}
177+
178+
private J.@Nullable Assignment extractAssignmentFromColonCase(List<Statement> caseStatements, boolean isLastCase, J.Identifier variableId) {
179+
if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.Block) {
180+
caseStatements = ((J.Block) caseStatements.get(0)).getStatements();
181+
}
182+
if ((caseStatements.size() == 2 && caseStatements.get(1) instanceof J.Break) || (caseStatements.size() == 1 && isLastCase)) {
183+
return extractAssignmentOfVariable(caseStatements.get(0), variableId);
184+
}
185+
return null;
186+
}
187+
188+
private J.@Nullable Assignment extractAssignmentOfVariable(J maybeAssignment, J.Identifier variableId) {
189+
if (maybeAssignment instanceof J.Assignment) {
190+
J.Assignment assignment = (J.Assignment) maybeAssignment;
191+
if (assignment.getVariable() instanceof J.Identifier) {
192+
J.Identifier variable = (J.Identifier) assignment.getVariable();
193+
if (SemanticallyEqual.areEqual(variable, variableId) &&
194+
!containsIdentifier(variableId, assignment.getAssignment())) {
195+
return assignment;
196+
}
197+
}
198+
}
199+
return null;
200+
}
201+
202+
private J.Case createDefaultCase(J.Switch originalSwitch, Expression returnedExpression, boolean arrow) {
203+
J.Switch switchStatement = JavaTemplate.apply(
204+
"switch(1) { default" + (arrow ? " ->" : ": yield") + " #{any()}; }",
205+
new Cursor(getCursor(), originalSwitch),
206+
originalSwitch.getCoordinates().replace(),
207+
returnedExpression
208+
);
209+
return (J.Case) switchStatement.getCases().getStatements().get(0);
210+
}
211+
212+
private boolean containsIdentifier(J.Identifier identifier, Expression expression) {
213+
return new JavaIsoVisitor<AtomicBoolean>() {
214+
@Override
215+
public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean found) {
216+
if (SemanticallyEqual.areEqual(id, identifier)) {
217+
found.set(true);
218+
return id;
219+
}
220+
return super.visitIdentifier(id, found);
221+
}
222+
}.reduce(expression, new AtomicBoolean()).get();
223+
}
224+
225+
// Might the initializer affect the input or output of the switch expression?
226+
private boolean canHaveSideEffects(@Nullable Expression expression) {
227+
if (expression == null) {
228+
return false;
229+
}
230+
231+
return new JavaIsoVisitor<AtomicBoolean>() {
232+
@Override
233+
public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean found) {
234+
found.set(true);
235+
return super.visitAssignment(assignment, found);
236+
}
237+
238+
@Override
239+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) {
240+
found.set(true);
241+
return method;
242+
}
243+
244+
@Override
245+
public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) {
246+
found.set(true);
247+
return newClass;
248+
}
249+
250+
@Override
251+
public J.Unary visitUnary(J.Unary unary, AtomicBoolean found) {
252+
found.set(true);
253+
return super.visitUnary(unary, found);
254+
}
255+
256+
private boolean isToStringImplicitlyCalled(Expression a, Expression b) {
257+
// Assuming an implicit `.toString()` call could have a side effect, but excluding
258+
// the java.lang.* classes from that rule.
259+
if (TypeUtils.isAssignableTo("java.lang.String", a.getType()) &&
260+
TypeUtils.isAssignableTo("java.lang.String", b.getType())) {
261+
return false;
262+
}
263+
264+
return a.getType() == JavaType.Primitive.String &&
265+
(!(b.getType() instanceof JavaType.Primitive || requireNonNull(b.getType()).toString().startsWith("java.lang")) &&
266+
!TypeUtils.isAssignableTo("java.lang.String", b.getType()));
267+
}
268+
269+
@Override
270+
public J.Binary visitBinary(J.Binary binary, AtomicBoolean found) {
271+
if (isToStringImplicitlyCalled(binary.getLeft(), binary.getRight()) ||
272+
isToStringImplicitlyCalled(binary.getRight(), binary.getLeft())) {
273+
found.set(true);
274+
return binary;
275+
}
276+
return super.visitBinary(binary, found);
277+
}
278+
}.reduce(expression, new AtomicBoolean()).get();
279+
}
280+
}
281+
);
282+
}
283+
}

0 commit comments

Comments
 (0)