Merge branch 'main' into return-as-last-statement

openrewrite · Nov 21, 2024 · 8904648 · 8904648
2 parents 2381c7f + 6199c0b
commit 8904648
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 13 deletions.
diff --git a/src/main/java/org/openrewrite/staticanalysis/NoEqualityInForCondition.java b/src/main/java/org/openrewrite/staticanalysis/NoEqualityInForCondition.java
@@ -19,6 +19,7 @@
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaVisitor;
+import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaType;
 
@@ -54,28 +55,59 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             public J visitForControl(J.ForLoop.Control control, ExecutionContext ctx) {
                 if (control.getCondition() instanceof J.Binary) {
                     J.Binary condition = (J.Binary) control.getCondition();
-                    if (condition.getRight() instanceof J.Literal && condition.getRight().getType() == JavaType.Primitive.Null) {
-                        return super.visitForControl(control, ctx);
-                    }
 
-                    if (control.getUpdate().size() == 1 && control.getUpdate().get(0) instanceof J.Unary) {
+                    if (isNumericalType(condition) &&
+                        control.getUpdate().size() == 1 &&
+                        control.getUpdate().get(0) instanceof J.Unary) {
                         J.Unary update = (J.Unary) control.getUpdate().get(0);
-
-                        if (condition.getOperator() == J.Binary.Type.NotEqual) {
-                            switch (update.getOperator()) {
-                                case PreIncrement:
-                                case PostIncrement:
-                                    return control.withCondition(condition.withOperator(J.Binary.Type.LessThan));
-                                case PreDecrement:
-                                case PostDecrement:
-                                    return control.withCondition(condition.withOperator(J.Binary.Type.GreaterThan));
+                        if (updatedExpressionInConditional(update.getExpression(), condition)) {
+                            if (condition.getOperator() == J.Binary.Type.NotEqual) {
+                                switch (update.getOperator()) {
+                                    case PreIncrement:
+                                    case PostIncrement:
+                                        return control.withCondition(condition.withOperator(J.Binary.Type.LessThan));
+                                    case PreDecrement:
+                                    case PostDecrement:
+                                        return control.withCondition(condition.withOperator(J.Binary.Type.GreaterThan));
+                                }
                             }
                         }
+
                     }
                 }
 
                 return super.visitForControl(control, ctx);
             }
+
+            private boolean updatedExpressionInConditional(Expression updatedExpression, J.Binary condition) {
+                final String simpleName;
+                if (updatedExpression instanceof J.Identifier) {
+                    simpleName = ((J.Identifier) updatedExpression).getSimpleName();
+                } else if (updatedExpression instanceof J.FieldAccess) {
+                    simpleName = ((J.FieldAccess) updatedExpression).getSimpleName();
+                } else {
+                    return false;
+                }
+
+                if (condition.getLeft() instanceof J.Identifier) {
+                    return simpleName.equals(((J.Identifier) condition.getLeft()).getSimpleName());
+                } else if (condition.getLeft() instanceof J.FieldAccess) {
+                    return simpleName.equals(((J.FieldAccess) condition.getLeft()).getSimpleName());
+                } else if (condition.getRight() instanceof J.Identifier) {
+                    return simpleName.equals(((J.Identifier) condition.getRight()).getSimpleName());
+                } else if (condition.getRight() instanceof J.FieldAccess) {
+                    return simpleName.equals(((J.FieldAccess) condition.getRight()).getSimpleName());
+                }
+                return false;
+            }
+
+            private boolean isNumericalType(J.Binary condition) {
+                JavaType type = condition.getRight().getType();
+                return type == JavaType.Primitive.Short ||
+                       type == JavaType.Primitive.Byte ||
+                       type == JavaType.Primitive.Int ||
+                       type == JavaType.Primitive.Long;
+            }
         };
     }
 }
diff --git a/src/test/java/org/openrewrite/staticanalysis/NoEqualityInForConditionTest.java b/src/test/java/org/openrewrite/staticanalysis/NoEqualityInForConditionTest.java
@@ -78,4 +78,40 @@ void test() {
           )
         );
     }
+
+    @Test
+    void retainCharComparison() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  int[] arr;
+                  void test() {
+                      for (int i = 0; arr[i] != '\\n'; i++) {
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainMismatchedComparison() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  void test() {
+                      int y = 9;
+                      for (int x = 0; y != 10; x++) {
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
 }