Skip to content

Commit d14df02

Browse files
Some compilation issues are occuring after running switch pattern matching recipe (#775)
* Some compilation issues are occuring after running switch pattern matching recipe. * no need for different template when working with block * Overrule formatting bug upstream * Apply formatter * Use a visitor to detect return at any nesting depth * No need to set empty block prefix twice --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 8afe410 commit d14df02

File tree

5 files changed

+458
-74
lines changed

5 files changed

+458
-74
lines changed

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

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.Map;
3636
import java.util.Objects;
3737
import java.util.Optional;
38+
import java.util.concurrent.atomic.AtomicBoolean;
3839

3940
import static org.openrewrite.java.migrate.lang.NullCheck.Matcher.nullCheck;
4041
import static org.openrewrite.java.tree.J.Block.createEmptyBlock;
@@ -75,7 +76,9 @@ public J.Case visitCase(J.Case case_, ExecutionContext ctx) {
7576
if (case_.getBody() instanceof J.Block &&
7677
((J.Block) case_.getBody()).getStatements().isEmpty() &&
7778
!((J.Block) case_.getBody()).getEnd().isEmpty()) {
78-
return case_.withBody(((J.Block) case_.getBody()).withEnd(Space.EMPTY));
79+
return case_.withBody(((J.Block) case_.getBody())
80+
.withPrefix(Space.SINGLE_SPACE)
81+
.withEnd(Space.EMPTY));
7982
}
8083
return case_.withBody(case_.getBody().withPrefix(Space.SINGLE_SPACE));
8184
}
@@ -100,6 +103,11 @@ private static class SwitchCandidate {
100103
private SwitchCandidate(J.If if_, Cursor cursor) {
101104
this.if_ = if_;
102105
this.cursor = cursor;
106+
Cursor parent = cursor.getParent(2);
107+
if (parent == null || parent.getValue() instanceof J.If.Else) {
108+
potentialCandidate = false;
109+
return;
110+
}
103111
J.If ifPart = if_;
104112
while (potentialCandidate && ifPart != null) {
105113
if (ifPart.getIfCondition().getTree() instanceof J.Binary) {
@@ -157,6 +165,11 @@ private boolean validatePotentialCandidate() {
157165
if (patternMatchers.keySet().stream().anyMatch(instanceOf -> instanceOf.getPattern() == null)) {
158166
return false;
159167
}
168+
// The blocks cannot do a return as that would lead to all blocks having to do a return,
169+
// the block/expression difference in return for switch statements / expressions being different...
170+
if (returns(nullCheckedStatement) || patternMatchers.values().stream().anyMatch(this::returns) || returns(else_)) {
171+
return false;
172+
}
160173
// Do no harm -> If we do not know how to replace(yet), do not replace
161174
if (patternMatchers.keySet().stream().anyMatch(instanceOf -> {
162175
J clazz = instanceOf.getClazz();
@@ -173,6 +186,16 @@ private boolean validatePotentialCandidate() {
173186
(hasLastElseBlock ? 1 : 0);
174187
}
175188

189+
private boolean returns(@Nullable Statement statement) {
190+
return statement != null && new JavaIsoVisitor<AtomicBoolean>() {
191+
@Override
192+
public J.Return visitReturn(J.Return return_, AtomicBoolean atomicBoolean) {
193+
atomicBoolean.set(true);
194+
return return_;
195+
}
196+
}.reduce(statement, new AtomicBoolean(false)).get();
197+
}
198+
176199
public J.@Nullable Switch buildSwitchTemplate() {
177200
Optional<Expression> switchOn = switchOn();
178201
if (!this.potentialCandidate || !switchOn.isPresent()) {
@@ -183,36 +206,20 @@ private boolean validatePotentialCandidate() {
183206
StringBuilder switchBody = new StringBuilder("switch (#{any()}) {\n");
184207
int i = 1;
185208
if (nullCheckedParameter != null) {
186-
Statement statement = getStatement(Objects.requireNonNull(nullCheckedStatement));
187-
if (statement instanceof J.Block) {
188-
switchBody.append("case null -> #{}\n");
189-
} else {
190-
switchBody.append("case null -> #{any()};\n");
191-
}
192-
arguments[i++] = statement;
209+
switchBody.append("case null -> #{any()};\n");
210+
arguments[i++] = getStatement(Objects.requireNonNull(nullCheckedStatement));
193211
}
194212
for (Map.Entry<J.InstanceOf, Statement> entry : patternMatchers.entrySet()) {
195213
J.InstanceOf instanceOf = entry.getKey();
196-
Statement statement = getStatement(entry.getValue());
197-
if (statement instanceof J.Block) {
198-
switchBody.append("case #{}#{} -> #{}\n");
199-
} else {
200-
switchBody.append("case #{}#{} -> #{any()};\n");
201-
}
214+
switchBody.append("case #{}#{} -> #{any()};\n");
202215
arguments[i++] = getClassName(instanceOf);
203216
arguments[i++] = getPattern(instanceOf);
204-
arguments[i++] = statement;
217+
arguments[i++] = getStatement(entry.getValue());
205218
}
219+
switchBody.append("default -> #{any()};\n");
206220
if (else_ != null) {
207-
Statement statement = getStatement(else_);
208-
if (statement instanceof J.Block) {
209-
switchBody.append("default -> #{}\n");
210-
} else {
211-
switchBody.append("default -> #{any()};\n");
212-
}
213-
arguments[i] = statement;
221+
arguments[i] = getStatement(else_);
214222
} else {
215-
switchBody.append("default -> #{}\n");
216223
arguments[i] = createEmptyBlock();
217224
}
218225
switchBody.append("}\n");
@@ -243,11 +250,13 @@ private String getPattern(J.InstanceOf statement) {
243250
}
244251

245252
private Statement getStatement(Statement statement) {
246-
Statement toAdd = statement;
247253
if (statement instanceof J.Block && ((J.Block) statement).getStatements().size() == 1) {
248-
toAdd = ((J.Block) statement).getStatements().get(0);
254+
Statement firstStatement = ((J.Block) statement).getStatements().get(0);
255+
if (firstStatement instanceof Expression || firstStatement instanceof J.Throw) {
256+
return firstStatement;
257+
}
249258
}
250-
return toAdd;
259+
return statement;
251260
}
252261
}
253262
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,10 @@ private boolean hasNullCase(J.Switch switch_) {
107107

108108
private J.Case createNullCase(J.Switch aSwitch, Statement whenNull) {
109109
if (whenNull instanceof J.Block && ((J.Block) whenNull).getStatements().size() == 1) {
110-
whenNull = ((J.Block) whenNull).getStatements().get(0);
110+
Statement firstStatement = ((J.Block) whenNull).getStatements().get(0);
111+
if (firstStatement instanceof Expression || firstStatement instanceof J.Throw) {
112+
whenNull = firstStatement;
113+
}
111114
}
112115
String semicolon = whenNull instanceof J.Block ? "" : ";";
113116
J.Switch switchWithNullCase = JavaTemplate.apply(

0 commit comments

Comments
 (0)