Skip to content

Commit e775b10

Browse files
timtebeektimtebeek
committed
Polish MoveConditionsToWhile 
1 parent dbabed5 commit e775b10

File tree

2 files changed

+25
-38
lines changed

2 files changed

+25
-38
lines changed

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

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.openrewrite.TreeVisitor;
2121
import org.openrewrite.java.JavaTemplate;
2222
import org.openrewrite.java.JavaVisitor;
23+
import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor;
2324
import org.openrewrite.java.tree.J;
2425
import org.openrewrite.java.tree.Statement;
2526

@@ -35,7 +36,7 @@ public String getDisplayName() {
3536
@Override
3637
public String getDescription() {
3738
return "Simplifies `while (true)` loops where the first statement is an `if` statement that only contains a `break`. " +
38-
"The condition is inverted and moved to the loop condition for better readability.";
39+
"The condition is inverted and moved to the loop condition for better readability.";
3940
}
4041

4142
@Override
@@ -45,35 +46,25 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
4546
public J visitWhileLoop(J.WhileLoop whileLoop, ExecutionContext ctx) {
4647
J.WhileLoop wl = (J.WhileLoop) super.visitWhileLoop(whileLoop, ctx);
4748

48-
if (!(wl.getCondition().getTree() instanceof J.Literal)) {
49+
if (!(wl.getCondition().getTree() instanceof J.Literal) ||
50+
!J.Literal.isLiteralValue(wl.getCondition().getTree(), true) ||
51+
!(wl.getBody() instanceof J.Block)) {
4952
return wl;
5053
}
5154

52-
J.Literal condition = (J.Literal) wl.getCondition().getTree();
53-
if (!Boolean.TRUE.equals(condition.getValue())) {
54-
return wl;
55-
}
56-
57-
if (!(wl.getBody() instanceof J.Block)) {
58-
return wl;
59-
}
60-
61-
J.Block body = (J.Block) wl.getBody();
62-
List<Statement> statements = body.getStatements();
63-
55+
List<Statement> statements = ((J.Block) wl.getBody()).getStatements();
6456
if (statements.isEmpty() || !(statements.get(0) instanceof J.If)) {
6557
return wl;
6658
}
6759

6860
J.If ifStatement = (J.If) statements.get(0);
69-
7061
if (ifStatement.getElsePart() != null) {
7162
// Actually in some cases it would be safe to proceed, but let's skip for now. Can be amended later.
7263
return wl;
7364
}
7465

7566
Statement thenBody = ifStatement.getThenPart();
76-
J.Break breakStatement = null;
67+
final J.Break breakStatement;
7768
if (thenBody instanceof J.Block) {
7869
J.Block thenBlock = (J.Block) thenBody;
7970
if (thenBlock.getStatements().size() != 1 || !(thenBlock.getStatements().get(0) instanceof J.Break)) {
@@ -82,21 +73,22 @@ public J visitWhileLoop(J.WhileLoop whileLoop, ExecutionContext ctx) {
8273
breakStatement = (J.Break) thenBlock.getStatements().get(0);
8374
} else if (thenBody instanceof J.Break) {
8475
breakStatement = (J.Break) thenBody;
76+
} else {
77+
return wl;
8578
}
8679

8780
// Check that the break has no label
88-
if (breakStatement == null || breakStatement.getLabel() != null) {
81+
if (breakStatement.getLabel() != null) {
8982
return wl;
9083
}
9184

92-
JavaTemplate whileTemplate = JavaTemplate.builder("while (!(#{any()})) #{}")
93-
.build();
94-
95-
List<Statement> remainingStatements = statements.subList(1, statements.size());
96-
J.Block newBody = body.withStatements(remainingStatements);
97-
98-
return whileTemplate.apply(getCursor(), wl.getCoordinates().replace(),
99-
ifStatement.getIfCondition().getTree(), newBody);
85+
doAfterVisit(new SimplifyBooleanExpressionVisitor());
86+
return JavaTemplate.apply(
87+
"while (!(#{any()})) #{}",
88+
getCursor(),
89+
wl.getCoordinates().replace(),
90+
ifStatement.getIfCondition().getTree(),
91+
((J.Block) wl.getBody()).withStatements(statements.subList(1, statements.size())));
10092
}
10193
};
10294
}

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,23 @@
1717

1818
import org.junit.jupiter.api.Test;
1919
import org.openrewrite.DocumentExample;
20+
import org.openrewrite.test.RecipeSpec;
2021
import org.openrewrite.test.RewriteTest;
2122

2223
import static org.openrewrite.java.Assertions.java;
2324

2425
@SuppressWarnings("ConstantConditions")
2526
class MoveConditionsToWhileTest implements RewriteTest {
2627

28+
@Override
29+
public void defaults(RecipeSpec spec) {
30+
spec.recipe(new MoveConditionsToWhile());
31+
}
32+
2733
@DocumentExample
2834
@Test
2935
void basicTransformation() {
3036
rewriteRun(
31-
spec -> spec.recipe(new MoveConditionsToWhile()),
3237
java(
3338
"""
3439
class Test {
@@ -46,7 +51,7 @@ void foo(int counter) {
4651
"""
4752
class Test {
4853
void foo(int counter) {
49-
while (!(counter >= 5)) {
54+
while (counter < 5) {
5055
System.out.println("Counter: " + counter);
5156
counter++;
5257
}
@@ -60,7 +65,6 @@ void foo(int counter) {
6065
@Test
6166
void withComplexCondition() {
6267
rewriteRun(
63-
spec -> spec.recipe(new MoveConditionsToWhile()),
6468
java(
6569
"""
6670
class Test {
@@ -92,7 +96,6 @@ void foo(int a, int b, String s) {
9296
@Test
9397
void withBlockInIf() {
9498
rewriteRun(
95-
spec -> spec.recipe(new MoveConditionsToWhile()),
9699
java(
97100
"""
98101
import java.util.concurrent.atomic.AtomicBoolean;
@@ -124,7 +127,6 @@ void foo(AtomicBoolean done) {
124127
@Test
125128
void doNotChangeWhenElsePresent() {
126129
rewriteRun(
127-
spec -> spec.recipe(new MoveConditionsToWhile()),
128130
java(
129131
"""
130132
class Test {
@@ -147,7 +149,6 @@ void foo(int counter) {
147149
@Test
148150
void doNotChangeWhenNotFirstStatement() {
149151
rewriteRun(
150-
spec -> spec.recipe(new MoveConditionsToWhile()),
151152
java(
152153
"""
153154
class Test {
@@ -169,7 +170,6 @@ void foo(int counter) {
169170
@Test
170171
void doNotChangeWhenMultipleStatementsInIf() {
171172
rewriteRun(
172-
spec -> spec.recipe(new MoveConditionsToWhile()),
173173
java(
174174
"""
175175
class Test {
@@ -191,7 +191,6 @@ void foo(int counter) {
191191
@Test
192192
void doNotChangeWhenNotWhileTrue() {
193193
rewriteRun(
194-
spec -> spec.recipe(new MoveConditionsToWhile()),
195194
java(
196195
"""
197196
class Test {
@@ -212,7 +211,6 @@ void foo(int counter, boolean condition) {
212211
@Test
213212
void doNotChangeWhenIfHasNoBreak() {
214213
rewriteRun(
215-
spec -> spec.recipe(new MoveConditionsToWhile()),
216214
java(
217215
"""
218216
class Test {
@@ -233,7 +231,6 @@ void foo(int counter) {
233231
@Test
234232
void emptyBodyAfterTransformation() {
235233
rewriteRun(
236-
spec -> spec.recipe(new MoveConditionsToWhile()),
237234
java(
238235
"""
239236
class Test {
@@ -261,7 +258,6 @@ void foo(boolean done) {
261258
@Test
262259
void doNotChangeWhenBreakHasLabel() {
263260
rewriteRun(
264-
spec -> spec.recipe(new MoveConditionsToWhile()),
265261
java(
266262
"""
267263
class Test {
@@ -282,7 +278,6 @@ void foo(int counter) {
282278
@Test
283279
void preserveComments() {
284280
rewriteRun(
285-
spec -> spec.recipe(new MoveConditionsToWhile()),
286281
java(
287282
"""
288283
class Test {
@@ -304,7 +299,7 @@ void foo(int counter) {
304299
class Test {
305300
void foo(int counter) {
306301
// Main loop
307-
while (!(counter >= 5)) {
302+
while (counter < 5) {
308303
// Process item
309304
System.out.println("Counter: " + counter);
310305
counter++;

0 commit comments

Comments
 (0)