Skip to content

Commit 909a22d

Browse files
JohannisKgithub-actions[bot]jevanlingen
authored
DefaultComesLast should correctly support fall through cases (#520)
* Reimplemented logic * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Refactored * cleanup * Review comments * refactored to ListUtils.map * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: Jacob van Lingen <[email protected]> * polish * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: Jacob van Lingen <[email protected]> * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * added nullcheck * Changed logic when fallthrough cases contain logic * Removed unused method --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jacob van Lingen <[email protected]> Co-authored-by: lingenj <[email protected]>
1 parent e0f1440 commit 909a22d

File tree

2 files changed

+334
-94
lines changed

2 files changed

+334
-94
lines changed

‎src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java

+104-93
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import lombok.EqualsAndHashCode;
1919
import lombok.Value;
20+
import org.jspecify.annotations.Nullable;
2021
import org.openrewrite.Tree;
2122
import org.openrewrite.internal.ListUtils;
2223
import org.openrewrite.java.JavaIsoVisitor;
@@ -29,7 +30,8 @@
2930
import java.util.ArrayList;
3031
import java.util.Collections;
3132
import java.util.List;
32-
import java.util.stream.Collectors;
33+
34+
import static java.util.stream.Collectors.toList;
3335

3436
@Value
3537
@EqualsAndHashCode(callSuper = false)
@@ -41,117 +43,126 @@ public J.Switch visitSwitch(J.Switch switch_, P p) {
4143
J.Switch s = visitAndCast(switch_, p, super::visitSwitch);
4244

4345
if (!isDefaultCaseLastOrNotPresent(switch_)) {
44-
List<J.Case> cases = s.getCases().getStatements().stream().map(J.Case.class::cast).collect(Collectors.toList());
45-
List<J.Case> fixedCases = new ArrayList<>(cases.size());
46-
47-
int defaultCaseIndex = -1;
48-
J.Case defaultCase = null;
49-
50-
for (int i = 0; i < cases.size(); i++) {
51-
J.Case aCase = cases.get(i);
52-
53-
// skip cases with bodies for now
54-
if (aCase.getBody() != null) {
55-
return s;
56-
}
57-
if (isDefaultCase(aCase)) {
58-
defaultCaseIndex = i;
59-
defaultCase = aCase;
46+
List<J.Case> cases = s.getCases().getStatements().stream().map(J.Case.class::cast).collect(toList());
47+
List<J.Case> casesWithDefaultLast = maybeReorderCases(cases, p);
48+
if (casesWithDefaultLast != null) {
49+
boolean changed = true;
50+
if (cases.size() == casesWithDefaultLast.size()) {
51+
changed = false;
52+
for (int i = 0; i < cases.size(); i++) {
53+
if (cases.get(i) != casesWithDefaultLast.get(i)) {
54+
changed = true;
55+
break;
56+
}
57+
}
6058
}
61-
}
6259

63-
List<J.Case> casesGroupedWithDefault = new ArrayList<>();
64-
boolean foundNonEmptyCase = false;
65-
for (int i = defaultCaseIndex - 1; i >= 0; i--) {
66-
J.Case aCase = cases.get(i);
67-
if (aCase.getStatements().isEmpty() && !foundNonEmptyCase) {
68-
casesGroupedWithDefault.add(0, aCase);
69-
} else {
70-
foundNonEmptyCase = true;
71-
fixedCases.add(0, aCase);
60+
if (changed) {
61+
s = s.withCases(s.getCases().withStatements(casesWithDefaultLast.stream().map(Statement.class::cast).collect(toList())));
7262
}
7363
}
64+
}
65+
return s;
66+
}
7467

75-
foundNonEmptyCase = false;
76-
for (int i = defaultCaseIndex + 1; i < cases.size(); i++) {
77-
J.Case aCase = cases.get(i);
78-
if (defaultCase != null && defaultCase.getStatements().isEmpty() &&
79-
aCase.getStatements().isEmpty() && !foundNonEmptyCase) {
80-
casesGroupedWithDefault.add(aCase);
81-
} else {
82-
if (defaultCase != null && defaultCase.getStatements().isEmpty() && !foundNonEmptyCase) {
83-
// the last case grouped with default can be non-empty. it will be flipped with
84-
// the default case, including its statements
85-
casesGroupedWithDefault.add(aCase);
86-
}
87-
foundNonEmptyCase = true;
88-
fixedCases.add(aCase);
89-
}
90-
}
68+
private @Nullable List<J.Case> maybeReorderCases(List<J.Case> cases, P p) {
69+
List<J.Case> fallThroughCases = new ArrayList<>();
70+
List<J.Case> defaultCases = new ArrayList<>();
71+
List<J.Case> casesWithDefaultLast = new ArrayList<>();
9172

92-
if (defaultCase != null && !casesGroupedWithDefault.isEmpty()) {
93-
J.Case lastGroupedWithDefault = casesGroupedWithDefault.get(casesGroupedWithDefault.size() - 1);
94-
if (!lastGroupedWithDefault.getStatements().isEmpty()) {
95-
casesGroupedWithDefault.set(casesGroupedWithDefault.size() - 1,
96-
lastGroupedWithDefault.withStatements(Collections.emptyList()));
97-
defaultCase = defaultCase.withStatements(lastGroupedWithDefault.getStatements());
98-
}
73+
boolean defaultCaseFound = false;
74+
for (int i = 0; i < cases.size(); i++) {
75+
J.Case aCase = cases.get(i);
76+
if (aCase.getBody() != null) {
77+
return null;
9978
}
100-
101-
J.Case lastNotGroupedWithDefault = fixedCases.get(fixedCases.size() - 1);
102-
if (!lastNotGroupedWithDefault.getStatements().stream().reduce((s1, s2) -> s2)
103-
.map(stat -> stat instanceof J.Break || stat instanceof J.Continue ||
104-
stat instanceof J.Return || stat instanceof J.Throw)
105-
.orElse(false)) {
106-
107-
// add a break statement since this case is now no longer last and would fall through
108-
List<Statement> statementsOfCaseBeingMoved = new ArrayList<>(lastNotGroupedWithDefault.getStatements());
109-
J.Break breakStatement = autoFormat(
110-
new J.Break(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null),
111-
p
112-
);
113-
statementsOfCaseBeingMoved.add(breakStatement);
114-
115-
lastNotGroupedWithDefault = lastNotGroupedWithDefault.withStatements(
116-
ListUtils.map(statementsOfCaseBeingMoved, stmt -> autoFormat(stmt, p, getCursor()))
117-
);
118-
fixedCases.set(fixedCases.size() - 1, lastNotGroupedWithDefault);
79+
fallThroughCases.add(aCase);
80+
if (isDefaultCase(aCase)) {
81+
defaultCaseFound = true;
11982
}
120-
121-
fixedCases.addAll(casesGroupedWithDefault);
122-
if (defaultCase != null) {
123-
if (defaultCase.getStatements().stream().reduce((s1, s2) -> s2)
124-
.map(stat -> stat instanceof J.Break || stat instanceof J.Continue || isVoidReturn(stat))
125-
.orElse(false)) {
126-
List<Statement> fixedDefaultStatements = new ArrayList<>(defaultCase.getStatements());
127-
fixedDefaultStatements.remove(fixedDefaultStatements.size() - 1);
128-
fixedCases.add(defaultCase.withStatements(fixedDefaultStatements));
83+
if (i == cases.size() - 1 || !isFallthroughCase(aCase)) {
84+
if (defaultCaseFound) {
85+
defaultCases.addAll(fallThroughCases);
86+
fallThroughCases.clear();
87+
defaultCaseFound = false;
12988
} else {
130-
fixedCases.add(defaultCase);
89+
casesWithDefaultLast.addAll(fallThroughCases);
90+
fallThroughCases.clear();
13191
}
13292
}
93+
}
13394

134-
boolean changed = true;
135-
if (cases.size() == fixedCases.size()) {
136-
changed = false;
137-
for (int i = 0; i < cases.size(); i++) {
138-
if (cases.get(i) != fixedCases.get(i)) {
139-
changed = true;
140-
break;
141-
}
95+
casesWithDefaultLast = addBreakToLastCase(casesWithDefaultLast, p);
96+
casesWithDefaultLast.addAll(maybeReorderFallthroughCases(defaultCases, p));
97+
casesWithDefaultLast = ListUtils.mapLast(casesWithDefaultLast, this::removeBreak);
98+
return casesWithDefaultLast;
99+
}
100+
101+
private List<J.Case> maybeReorderFallthroughCases(List<J.Case> cases, P p) {
102+
List<J.Case> preDefaultCases = new ArrayList<>();
103+
List<J.Case> postDefaultCases = new ArrayList<>();
104+
J.Case defaultCase = null;
105+
for (int i = 0; i < cases.size(); i++) {
106+
J.Case aCase = cases.get(i);
107+
if (isDefaultCase(aCase)) {
108+
if (!aCase.getStatements().isEmpty()) {
109+
return cases;
142110
}
111+
defaultCase = aCase;
112+
} else if (defaultCase != null) {
113+
if (!aCase.getStatements().isEmpty() && i != cases.size() - 1) {
114+
return cases;
115+
} else {
116+
postDefaultCases.add(aCase);
117+
}
118+
} else {
119+
preDefaultCases.add(aCase);
143120
}
121+
}
144122

145-
if (changed) {
146-
s = s.withCases(s.getCases().withStatements(fixedCases.stream().map(Statement.class::cast).collect(Collectors.toList())));
147-
}
123+
List<J.Case> fixedCases = new ArrayList<>();
124+
fixedCases.addAll(preDefaultCases);
125+
if (!postDefaultCases.isEmpty()) {
126+
List<Statement> statements = postDefaultCases.get(postDefaultCases.size() - 1).getStatements();
127+
defaultCase = defaultCase.withStatements(statements);
128+
fixedCases.addAll(ListUtils.mapLast(postDefaultCases, e -> e.withStatements(Collections.emptyList())));
148129
}
130+
fixedCases.add(defaultCase);
131+
return fixedCases;
132+
}
149133

150-
return s;
134+
private List<J.Case> addBreakToLastCase(List<J.Case> cases, P p) {
135+
return ListUtils.mapLast(cases, e -> {
136+
if (isFallthroughCase(e)) {
137+
return addBreak(e, p);
138+
}
139+
return e;
140+
});
151141
}
152142

153-
private boolean isVoidReturn(Statement stat) {
154-
return stat instanceof J.Return && ((J.Return) stat).getExpression() == null;
143+
private boolean isFallthroughCase(J.Case aCase) {
144+
return aCase.getStatements().isEmpty() ||
145+
!(aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Break ||
146+
aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Continue ||
147+
aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Return ||
148+
aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Throw);
149+
}
150+
151+
private J.Case addBreak(J.Case e, P p) {
152+
J.Break breakStatement = autoFormat(
153+
new J.Break(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null),
154+
p
155+
);
156+
List<Statement> statements = e.getStatements();
157+
statements.add(breakStatement);
158+
return e.withStatements(ListUtils.map(statements, stmt -> autoFormat(stmt, p, getCursor())));
159+
}
160+
161+
private J.Case removeBreak(J.Case aCase) {
162+
if (!aCase.getStatements().isEmpty() && aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Break) {
163+
aCase = aCase.withStatements(aCase.getStatements().subList(0, aCase.getStatements().size() - 1));
164+
}
165+
return aCase;
155166
}
156167

157168
private boolean isDefaultCaseLastOrNotPresent(J.Switch switch_) {

0 commit comments

Comments
 (0)