Skip to content

Commit dd67de6

Browse files
greg-at-modernegreg-at-moderne
authored
PreferIncrementOperator - more coverage - compound assignment operators (#680)
* Support for compound assignment operators * Support for non-literals too * Suprressing warnings * Shorter description * Removed unused import
1 parent 6eca13a commit dd67de6

File tree

2 files changed

+102
-60
lines changed

2 files changed

+102
-60
lines changed

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

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.openrewrite.TreeVisitor;
2222
import org.openrewrite.internal.ListUtils;
2323
import org.openrewrite.java.JavaIsoVisitor;
24+
import org.openrewrite.java.search.SemanticallyEqual;
2425
import org.openrewrite.java.tree.*;
2526
import org.openrewrite.marker.Markers;
2627

@@ -33,12 +34,12 @@ public class PreferIncrementOperator extends Recipe {
3334

3435
@Override
3536
public String getDisplayName() {
36-
return "Prefer increment and decrement operators";
37+
return "Prefer increment/decrement and compound assignment operators";
3738
}
3839

3940
@Override
4041
public String getDescription() {
41-
return "Prefer the use of increment and decrement operators (`++` and `--`) over their more verbose equivalents.";
42+
return "Prefer the use of increment and decrement operators (`++`, `--`, `+=`, `-=`) over their more verbose equivalents.";
4243
}
4344

4445
@Override
@@ -62,33 +63,45 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
6263
if (statement instanceof J.Assignment) {
6364
J.Assignment assignment = (J.Assignment) statement;
6465

65-
if (assignment.getVariable() instanceof J.Identifier && assignment.getAssignment() instanceof J.Binary) {
66-
J.Identifier variable = (J.Identifier) assignment.getVariable();
66+
if (assignment.getAssignment() instanceof J.Binary) {
67+
Expression variable = assignment.getVariable();
6768
J.Binary binary = (J.Binary) assignment.getAssignment();
6869

6970
if (binary.getOperator() == J.Binary.Type.Addition || binary.getOperator() == J.Binary.Type.Subtraction) {
70-
if (binary.getLeft() instanceof J.Identifier) {
71-
J.Identifier binaryLeft = (J.Identifier) binary.getLeft();
72-
if (variable.getSimpleName().equals(binaryLeft.getSimpleName()) &&
73-
TypeUtils.isOfType(variable.getType(), binaryLeft.getType())) {
74-
75-
if (binary.getRight() instanceof J.Literal) {
76-
J.Literal literal = (J.Literal) binary.getRight();
77-
if (literal.getValue() instanceof Integer && (Integer) literal.getValue() == 1) {
78-
J.Unary.Type unaryType = binary.getOperator() == J.Binary.Type.Addition ?
79-
J.Unary.Type.PostIncrement : J.Unary.Type.PostDecrement;
80-
81-
return new J.Unary(
82-
Tree.randomId(),
83-
assignment.getPrefix(),
84-
assignment.getMarkers(),
85-
new JLeftPadded<>(Space.EMPTY, unaryType, Markers.EMPTY),
86-
variable.withPrefix(Space.EMPTY),
87-
assignment.getType()
88-
);
89-
}
71+
Expression binaryLeft = binary.getLeft();
72+
73+
if (SemanticallyEqual.areEqual(variable, binaryLeft)) {
74+
Expression right = binary.getRight();
75+
76+
if (right instanceof J.Literal) {
77+
J.Literal literal = (J.Literal) right;
78+
if (literal.getValue() instanceof Integer && (Integer) literal.getValue() == 1) {
79+
J.Unary.Type unaryType = binary.getOperator() == J.Binary.Type.Addition ?
80+
J.Unary.Type.PostIncrement : J.Unary.Type.PostDecrement;
81+
82+
return new J.Unary(
83+
Tree.randomId(),
84+
assignment.getPrefix(),
85+
assignment.getMarkers(),
86+
new JLeftPadded<>(Space.EMPTY, unaryType, Markers.EMPTY),
87+
variable.withPrefix(Space.EMPTY),
88+
assignment.getType()
89+
);
9090
}
9191
}
92+
93+
J.AssignmentOperation.Type opType = binary.getOperator() == J.Binary.Type.Addition ?
94+
J.AssignmentOperation.Type.Addition : J.AssignmentOperation.Type.Subtraction;
95+
96+
return new J.AssignmentOperation(
97+
Tree.randomId(),
98+
assignment.getPrefix(),
99+
assignment.getMarkers(),
100+
variable.withPrefix(Space.EMPTY),
101+
new JLeftPadded<>(Space.SINGLE_SPACE, opType, Markers.EMPTY),
102+
right.withPrefix(Space.SINGLE_SPACE),
103+
assignment.getType()
104+
);
92105
}
93106
}
94107
}

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

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,13 @@
1616
package org.openrewrite.staticanalysis;
1717

1818
import org.junit.jupiter.api.Test;
19-
import org.junitpioneer.jupiter.ExpectedToFail;
2019
import org.openrewrite.DocumentExample;
2120
import org.openrewrite.test.RecipeSpec;
2221
import org.openrewrite.test.RewriteTest;
2322

2423
import static org.openrewrite.java.Assertions.java;
2524

26-
@SuppressWarnings("ConstantConditions")
25+
@SuppressWarnings({"ConstantConditions", "UnusedAssignment"})
2726
class PreferIncrementOperatorTest implements RewriteTest {
2827

2928
@Override
@@ -105,22 +104,20 @@ void test() {
105104
}
106105

107106
@Test
108-
void multipleIncrements() {
107+
void incrementByTwo() {
109108
rewriteRun(
110109
java(
111110
"""
112111
class Test {
113-
void test(int i, int j) {
114-
i = i + 1;
115-
j = j - 1;
112+
void test(int i) {
113+
i = i + 2;
116114
}
117115
}
118116
""",
119117
"""
120118
class Test {
121-
void test(int i, int j) {
122-
i++;
123-
j--;
119+
void test(int i) {
120+
i += 2;
124121
}
125122
}
126123
"""
@@ -129,13 +126,14 @@ void test(int i, int j) {
129126
}
130127

131128
@Test
132-
void doNotChangeNonLiteralOne() {
129+
void doNotChangeDifferentVariable() {
133130
rewriteRun(
134131
java(
135132
"""
136133
class Test {
137-
void test(int i) {
138-
i = i + 2;
134+
int i, j = 0;
135+
Test() {
136+
i = j + 1;
139137
}
140138
}
141139
"""
@@ -144,13 +142,14 @@ void test(int i) {
144142
}
145143

146144
@Test
147-
void doNotChangeDifferentVariable() {
145+
void doNotChangeIfOrderIsReversed() {
146+
// No strong feelings here, just documenting the current behavior of not applying the change here.
148147
rewriteRun(
149148
java(
150149
"""
151150
class Test {
152-
void test(int i, int j) {
153-
i = j + 1;
151+
void test(int i) {
152+
i = 1 + i;
154153
}
155154
}
156155
"""
@@ -159,13 +158,28 @@ void test(int i, int j) {
159158
}
160159

161160
@Test
162-
void doNotChangeIfOrderIsReversed() {
161+
void compoundAssignmentForVariousValues() {
163162
rewriteRun(
164163
java(
165164
"""
166165
class Test {
167-
void test(int i) {
168-
i = 1 + i;
166+
void test(int i, int j, long l, long n) {
167+
i = i + 1;
168+
j = j + 5;
169+
i = i + 100;
170+
l = l + 2L;
171+
n = n - 2;
172+
}
173+
}
174+
""",
175+
"""
176+
class Test {
177+
void test(int i, int j, long l, long n) {
178+
i++;
179+
j += 5;
180+
i += 100;
181+
l += 2L;
182+
n -= 2;
169183
}
170184
}
171185
"""
@@ -174,47 +188,62 @@ void test(int i) {
174188
}
175189

176190
@Test
177-
void longType() {
191+
void doNotChangeAssignmentInIfCondition() {
192+
// No strong feelings here, just documenting the current behavior of not applying the change here.
178193
rewriteRun(
179194
java(
180195
"""
181196
class Test {
182-
void test(long l) {
183-
l = l + 1;
184-
}
185-
}
186-
""",
187-
"""
188-
class Test {
189-
void test(long l) {
190-
l++;
197+
void test(int i) {
198+
if ((i = i + 1) > 10) {
199+
System.out.println(i);
200+
}
191201
}
192202
}
193203
"""
194204
)
195205
);
196206
}
197207

198-
@ExpectedToFail("Not implemented yet")
199208
@Test
200-
void fieldIncrement() {
209+
void compoundAssignmentWithNonLiterals() {
201210
rewriteRun(
202211
java(
203212
"""
204213
class Test {
205-
int count;
214+
int field = 4;
215+
int[] arr = new int[10];
216+
Test other;
206217
207-
void test() {
208-
this.count = this.count + 1;
218+
void test(int i, int j, int k, int size) {
219+
i = i + j;
220+
k = k - size;
221+
i = i + "alef".length();
222+
j = j - (k * 2);
223+
field = field + 4;
224+
this.field = this.field + 3;
225+
this.field = field + 6; // This is not changed as the logic to detect "this.field" is equivalent to "field" in this case is not implemented.
226+
arr/*comment*/[0] = arr/*other comment*/[0] + 1;
227+
other.field = other.field + 2;
209228
}
210229
}
211230
""",
212231
"""
213232
class Test {
214-
int count;
233+
int field = 4;
234+
int[] arr = new int[10];
235+
Test other;
215236
216-
void test() {
217-
this.count++;
237+
void test(int i, int j, int k, int size) {
238+
i += j;
239+
k -= size;
240+
i += "alef".length();
241+
j -= (k * 2);
242+
field += 4;
243+
this.field += 3;
244+
this.field = field + 6; // This is not changed as the logic to detect "this.field" is equivalent to "field" in this case is not implemented.
245+
arr/*comment*/[0]++;
246+
other.field += 2;
218247
}
219248
}
220249
"""

0 commit comments

Comments
 (0)