diff --git a/src/main/java/org/openrewrite/staticanalysis/PreferIncrementOperator.java b/src/main/java/org/openrewrite/staticanalysis/PreferIncrementOperator.java index 6dcd2973a9..8f36c55774 100644 --- a/src/main/java/org/openrewrite/staticanalysis/PreferIncrementOperator.java +++ b/src/main/java/org/openrewrite/staticanalysis/PreferIncrementOperator.java @@ -21,6 +21,7 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.search.SemanticallyEqual; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; @@ -33,12 +34,12 @@ public class PreferIncrementOperator extends Recipe { @Override public String getDisplayName() { - return "Prefer increment and decrement operators"; + return "Prefer increment/decrement and compound assignment operators"; } @Override public String getDescription() { - return "Prefer the use of increment and decrement operators (`++` and `--`) over their more verbose equivalents."; + return "Prefer the use of increment and decrement operators (`++`, `--`, `+=`, `-=`) over their more verbose equivalents."; } @Override @@ -62,33 +63,45 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { if (statement instanceof J.Assignment) { J.Assignment assignment = (J.Assignment) statement; - if (assignment.getVariable() instanceof J.Identifier && assignment.getAssignment() instanceof J.Binary) { - J.Identifier variable = (J.Identifier) assignment.getVariable(); + if (assignment.getAssignment() instanceof J.Binary) { + Expression variable = assignment.getVariable(); J.Binary binary = (J.Binary) assignment.getAssignment(); if (binary.getOperator() == J.Binary.Type.Addition || binary.getOperator() == J.Binary.Type.Subtraction) { - if (binary.getLeft() instanceof J.Identifier) { - J.Identifier binaryLeft = (J.Identifier) binary.getLeft(); - if (variable.getSimpleName().equals(binaryLeft.getSimpleName()) && - TypeUtils.isOfType(variable.getType(), binaryLeft.getType())) { - - if (binary.getRight() instanceof J.Literal) { - J.Literal literal = (J.Literal) binary.getRight(); - if (literal.getValue() instanceof Integer && (Integer) literal.getValue() == 1) { - J.Unary.Type unaryType = binary.getOperator() == J.Binary.Type.Addition ? - J.Unary.Type.PostIncrement : J.Unary.Type.PostDecrement; - - return new J.Unary( - Tree.randomId(), - assignment.getPrefix(), - assignment.getMarkers(), - new JLeftPadded<>(Space.EMPTY, unaryType, Markers.EMPTY), - variable.withPrefix(Space.EMPTY), - assignment.getType() - ); - } + Expression binaryLeft = binary.getLeft(); + + if (SemanticallyEqual.areEqual(variable, binaryLeft)) { + Expression right = binary.getRight(); + + if (right instanceof J.Literal) { + J.Literal literal = (J.Literal) right; + if (literal.getValue() instanceof Integer && (Integer) literal.getValue() == 1) { + J.Unary.Type unaryType = binary.getOperator() == J.Binary.Type.Addition ? + J.Unary.Type.PostIncrement : J.Unary.Type.PostDecrement; + + return new J.Unary( + Tree.randomId(), + assignment.getPrefix(), + assignment.getMarkers(), + new JLeftPadded<>(Space.EMPTY, unaryType, Markers.EMPTY), + variable.withPrefix(Space.EMPTY), + assignment.getType() + ); } } + + J.AssignmentOperation.Type opType = binary.getOperator() == J.Binary.Type.Addition ? + J.AssignmentOperation.Type.Addition : J.AssignmentOperation.Type.Subtraction; + + return new J.AssignmentOperation( + Tree.randomId(), + assignment.getPrefix(), + assignment.getMarkers(), + variable.withPrefix(Space.EMPTY), + new JLeftPadded<>(Space.SINGLE_SPACE, opType, Markers.EMPTY), + right.withPrefix(Space.SINGLE_SPACE), + assignment.getType() + ); } } } diff --git a/src/test/java/org/openrewrite/staticanalysis/PreferIncrementOperatorTest.java b/src/test/java/org/openrewrite/staticanalysis/PreferIncrementOperatorTest.java index 537a68c6a1..6e5e128267 100644 --- a/src/test/java/org/openrewrite/staticanalysis/PreferIncrementOperatorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/PreferIncrementOperatorTest.java @@ -16,14 +16,13 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; -@SuppressWarnings("ConstantConditions") +@SuppressWarnings({"ConstantConditions", "UnusedAssignment"}) class PreferIncrementOperatorTest implements RewriteTest { @Override @@ -105,22 +104,20 @@ void test() { } @Test - void multipleIncrements() { + void incrementByTwo() { rewriteRun( java( """ class Test { - void test(int i, int j) { - i = i + 1; - j = j - 1; + void test(int i) { + i = i + 2; } } """, """ class Test { - void test(int i, int j) { - i++; - j--; + void test(int i) { + i += 2; } } """ @@ -129,13 +126,14 @@ void test(int i, int j) { } @Test - void doNotChangeNonLiteralOne() { + void doNotChangeDifferentVariable() { rewriteRun( java( """ class Test { - void test(int i) { - i = i + 2; + int i, j = 0; + Test() { + i = j + 1; } } """ @@ -144,13 +142,14 @@ void test(int i) { } @Test - void doNotChangeDifferentVariable() { + void doNotChangeIfOrderIsReversed() { + // No strong feelings here, just documenting the current behavior of not applying the change here. rewriteRun( java( """ class Test { - void test(int i, int j) { - i = j + 1; + void test(int i) { + i = 1 + i; } } """ @@ -159,13 +158,28 @@ void test(int i, int j) { } @Test - void doNotChangeIfOrderIsReversed() { + void compoundAssignmentForVariousValues() { rewriteRun( java( """ class Test { - void test(int i) { - i = 1 + i; + void test(int i, int j, long l, long n) { + i = i + 1; + j = j + 5; + i = i + 100; + l = l + 2L; + n = n - 2; + } + } + """, + """ + class Test { + void test(int i, int j, long l, long n) { + i++; + j += 5; + i += 100; + l += 2L; + n -= 2; } } """ @@ -174,20 +188,16 @@ void test(int i) { } @Test - void longType() { + void doNotChangeAssignmentInIfCondition() { + // No strong feelings here, just documenting the current behavior of not applying the change here. rewriteRun( java( """ class Test { - void test(long l) { - l = l + 1; - } - } - """, - """ - class Test { - void test(long l) { - l++; + void test(int i) { + if ((i = i + 1) > 10) { + System.out.println(i); + } } } """ @@ -195,26 +205,45 @@ void test(long l) { ); } - @ExpectedToFail("Not implemented yet") @Test - void fieldIncrement() { + void compoundAssignmentWithNonLiterals() { rewriteRun( java( """ class Test { - int count; + int field = 4; + int[] arr = new int[10]; + Test other; - void test() { - this.count = this.count + 1; + void test(int i, int j, int k, int size) { + i = i + j; + k = k - size; + i = i + "alef".length(); + j = j - (k * 2); + field = field + 4; + this.field = this.field + 3; + 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. + arr/*comment*/[0] = arr/*other comment*/[0] + 1; + other.field = other.field + 2; } } """, """ class Test { - int count; + int field = 4; + int[] arr = new int[10]; + Test other; - void test() { - this.count++; + void test(int i, int j, int k, int size) { + i += j; + k -= size; + i += "alef".length(); + j -= (k * 2); + field += 4; + this.field += 3; + 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. + arr/*comment*/[0]++; + other.field += 2; } } """