Skip to content

Commit d86353a

Browse files
JohannisKgithub-actions[bot]jevanlingenJenson3210timtebeek
authored
Fixes for DefaultComesLast (#523)
* 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 * Added support for labelled breaks * Added support for Block * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: Jacob van Lingen <[email protected]> * Cleanup from review * Update src/main/java/org/openrewrite/staticanalysis/DefaultComesLastVisitor.java Co-authored-by: Jente Sondervorst <[email protected]> * Added correct formatting * Add Yield as a non fall through case already * Minor polish --------- 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]> Co-authored-by: Jente Sondervorst <[email protected]> Co-authored-by: Tim te Beek <[email protected]>
1 parent ad9563a commit d86353a

File tree

2 files changed

+266
-20
lines changed

2 files changed

+266
-20
lines changed

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

+38-19
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@
2828
import org.openrewrite.marker.Markers;
2929

3030
import java.util.ArrayList;
31-
import java.util.Collections;
3231
import java.util.List;
3332

33+
import static java.util.Collections.emptyList;
3434
import static java.util.stream.Collectors.toList;
3535

3636
@Value
@@ -80,7 +80,7 @@ public J.Switch visitSwitch(J.Switch switch_, P p) {
8080
if (isDefaultCase(aCase)) {
8181
defaultCaseFound = true;
8282
}
83-
if (i == cases.size() - 1 || !isFallthroughCase(aCase)) {
83+
if (i == cases.size() - 1 || !isFallthroughCase(aCase.getStatements())) {
8484
if (defaultCaseFound) {
8585
defaultCases.addAll(fallThroughCases);
8686
fallThroughCases.clear();
@@ -120,46 +120,65 @@ private List<J.Case> maybeReorderFallthroughCases(List<J.Case> cases, P p) {
120120
}
121121
}
122122

123-
List<J.Case> fixedCases = new ArrayList<>();
124-
fixedCases.addAll(preDefaultCases);
123+
List<J.Case> fixedCases = new ArrayList<>(preDefaultCases);
125124
if (!postDefaultCases.isEmpty()) {
126125
List<Statement> statements = postDefaultCases.get(postDefaultCases.size() - 1).getStatements();
127126
defaultCase = defaultCase.withStatements(statements);
128-
fixedCases.addAll(ListUtils.mapLast(postDefaultCases, e -> e.withStatements(Collections.emptyList())));
127+
fixedCases.addAll(ListUtils.mapLast(postDefaultCases, e -> e.withStatements(emptyList())));
129128
}
129+
assert defaultCase != null;
130130
fixedCases.add(defaultCase);
131131
return fixedCases;
132132
}
133133

134134
private List<J.Case> addBreakToLastCase(List<J.Case> cases, P p) {
135135
return ListUtils.mapLast(cases, e -> {
136-
if (isFallthroughCase(e)) {
136+
if (isFallthroughCase(e.getStatements())) {
137137
return addBreak(e, p);
138138
}
139139
return e;
140140
});
141141
}
142142

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);
143+
private boolean isFallthroughCase(List<Statement> statements) {
144+
if (statements.isEmpty()) {
145+
return true;
146+
}
147+
Statement lastStatement = statements.get(statements.size() - 1);
148+
if (lastStatement instanceof J.Block) {
149+
return isFallthroughCase(((J.Block) lastStatement).getStatements());
150+
}
151+
return !(lastStatement instanceof J.Break ||
152+
lastStatement instanceof J.Continue ||
153+
lastStatement instanceof J.Return ||
154+
lastStatement instanceof J.Throw ||
155+
lastStatement instanceof J.Yield);
149156
}
150157

151158
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-
);
156159
List<Statement> statements = e.getStatements();
157-
statements.add(breakStatement);
158-
return e.withStatements(ListUtils.map(statements, stmt -> autoFormat(stmt, p, getCursor())));
160+
J.Switch switchStatement = getCursor().getValue();
161+
int switchIndent = switchStatement.getPrefix().getIndent().length();
162+
int caseIndent = switchStatement.getCases().getStatements().get(0).getPrefix().getIndent().length();
163+
int breakIndent = caseIndent + (caseIndent - switchIndent);
164+
Space prefix = breakIndent > 0 ? Space.build(String.format("\n%" + breakIndent + "s", ""), emptyList()) : Space.EMPTY;
165+
J.Break breakStatement = new J.Break(Tree.randomId(), prefix, Markers.EMPTY, null);
166+
if (!statements.isEmpty() && statements.get(statements.size() - 1) instanceof J.Block) {
167+
statements = ListUtils.mapLast(statements, s -> {
168+
J.Block block = (J.Block) s;
169+
List<Statement> blockStatements = block.getStatements();
170+
blockStatements.add(breakStatement);
171+
return block.withStatements(blockStatements);
172+
});
173+
} else {
174+
statements.add(breakStatement);
175+
}
176+
return e.withStatements(statements);
159177
}
160178

161179
private J.Case removeBreak(J.Case aCase) {
162-
if (!aCase.getStatements().isEmpty() && aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Break) {
180+
if (!aCase.getStatements().isEmpty() && aCase.getStatements().get(aCase.getStatements().size() - 1) instanceof J.Break &&
181+
((J.Break) aCase.getStatements().get(aCase.getStatements().size() - 1)).getLabel() == null) {
163182
aCase = aCase.withStatements(aCase.getStatements().subList(0, aCase.getStatements().size() - 1));
164183
}
165184
return aCase;

‎src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java

+228-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import static java.util.Collections.singletonList;
3333
import static org.openrewrite.java.Assertions.java;
3434

35-
@SuppressWarnings({"ConstantConditions", "EnhancedSwitchMigration", "SwitchStatementWithTooFewBranches"})
35+
@SuppressWarnings({"ConstantConditions", "EnhancedSwitchMigration", "SwitchStatementWithTooFewBranches", "DefaultNotLastCaseInSwitch"})
3636
class DefaultComesLastTest implements RewriteTest {
3737

3838
@Override
@@ -549,4 +549,231 @@ class Test {
549549
)
550550
);
551551
}
552+
553+
@Test
554+
void breakOuterLoop() {
555+
rewriteRun(
556+
//language=java
557+
java(
558+
"""
559+
class Test {
560+
int n;
561+
{
562+
loop: for (;;) {
563+
switch (n) {
564+
default:
565+
break loop;
566+
case 1:
567+
case 2:
568+
break;
569+
case 3:
570+
case 4:
571+
}
572+
}
573+
}
574+
}
575+
""",
576+
"""
577+
class Test {
578+
int n;
579+
{
580+
loop: for (;;) {
581+
switch (n) {
582+
case 1:
583+
case 2:
584+
break;
585+
case 3:
586+
case 4:
587+
break;
588+
default:
589+
break loop;
590+
}
591+
}
592+
}
593+
}
594+
"""
595+
)
596+
);
597+
}
598+
599+
@Test
600+
void casesContainBlocks() {
601+
//language=java
602+
rewriteRun(
603+
java(
604+
"""
605+
class Test {
606+
void test (int state) {
607+
switch (state) {
608+
default:
609+
case 1: {
610+
break;
611+
}
612+
case 2:
613+
break;
614+
}
615+
}
616+
}
617+
""",
618+
"""
619+
class Test {
620+
void test (int state) {
621+
switch (state) {
622+
case 2:
623+
break;
624+
case 1:
625+
default: {
626+
break;
627+
}
628+
}
629+
}
630+
}
631+
"""
632+
)
633+
);
634+
}
635+
636+
@Test
637+
void addBreakToBlockCase() {
638+
//language=java
639+
rewriteRun(
640+
java(
641+
"""
642+
class Test {
643+
void test (int state) {
644+
switch (state) {
645+
default:
646+
System.out.println();
647+
break;
648+
case 1: {
649+
System.out.println();
650+
}
651+
}
652+
}
653+
}
654+
""",
655+
"""
656+
class Test {
657+
void test (int state) {
658+
switch (state) {
659+
case 1: {
660+
System.out.println();
661+
break;
662+
}
663+
default:
664+
System.out.println();
665+
}
666+
}
667+
}
668+
"""
669+
)
670+
);
671+
}
672+
673+
@Test
674+
void differentIndentation() {
675+
//language=java
676+
rewriteRun(
677+
java(
678+
"""
679+
class Test {
680+
void test (int state) {
681+
switch (state) {
682+
default:
683+
System.out.println();
684+
break;
685+
case 1: {
686+
System.out.println();
687+
}
688+
}
689+
}
690+
}
691+
""",
692+
"""
693+
class Test {
694+
void test (int state) {
695+
switch (state) {
696+
case 1: {
697+
System.out.println();
698+
break;
699+
}
700+
default:
701+
System.out.println();
702+
}
703+
}
704+
}
705+
"""
706+
)
707+
);
708+
}
709+
710+
@Test
711+
void groupedCases() {
712+
//language=java
713+
rewriteRun(
714+
java(
715+
"""
716+
class Test {
717+
void test (int state) {
718+
switch (state) {
719+
default:
720+
System.out.println();
721+
break;
722+
case 0: case 1: case 2:
723+
System.out.println();
724+
}
725+
}
726+
}
727+
""",
728+
"""
729+
class Test {
730+
void test (int state) {
731+
switch (state) {
732+
case 0: case 1: case 2:
733+
System.out.println();
734+
break;
735+
default:
736+
System.out.println();
737+
}
738+
}
739+
}
740+
"""
741+
)
742+
);
743+
}
744+
745+
@Test
746+
void defaultPartOfGroupedCases() {
747+
//language=java
748+
rewriteRun(
749+
java(
750+
"""
751+
class Test {
752+
void test (int state) {
753+
switch (state) {
754+
case 0: case 1: case 2: default: case 3:
755+
System.out.println();
756+
break;
757+
case 4: case 5: case 6:
758+
System.out.println();
759+
}
760+
}
761+
}
762+
""",
763+
"""
764+
class Test {
765+
void test (int state) {
766+
switch (state) {
767+
case 4: case 5: case 6:
768+
System.out.println();
769+
break;
770+
case 0: case 1: case 2: case 3: default:
771+
System.out.println();
772+
}
773+
}
774+
}
775+
"""
776+
)
777+
);
778+
}
552779
}

0 commit comments

Comments
 (0)