Do not unwrap negated assignment

Fixes #3913
openrewrite · Jan 14, 2024 · 670dd5c · 670dd5c
1 parent 7d3655a
commit 670dd5c
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 96 deletions.
diff --git a/IDE.properties.tmp b/IDE.properties.tmp
@@ -21,6 +21,7 @@ rewrite-java
 rewrite-java-test
 rewrite-java-tck
 rewrite-java-17
+rewrite-java-21
 
 # Other language modules
 

diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/UnnecessaryParenthesesTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/UnnecessaryParenthesesTest.java
@@ -16,6 +16,7 @@
 package org.openrewrite.java.cleanup;
 
 import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.Issue;
@@ -798,57 +799,6 @@ void test(String s) {
         );
     }
 
-    @Test
-    void doNotUnwrapIfNoParens() {
-        rewriteRun(
-          java(
-            """
-              class Test {
-                  void test(String s) {
-                      if (s == null || s.isEmpty()) {
-                          System.out.println("empty");
-                      }
-                  }
-              }
-              """
-          )
-        );
-    }
-
-    @Test
-    void doNotUnwrapNegatedIfParens() {
-        rewriteRun(
-          java(
-            """
-              class Test {
-                  void test(String s) {
-                      if (!(s == null || s.isEmpty())) {
-                          System.out.println("empty");
-                      }
-                  }
-              }
-              """
-          )
-        );
-    }
-
-    @Test
-    void doNotUnwrapIfParens() {
-        rewriteRun(
-          java(
-            """
-              class Test {
-                  void test(String s) {
-                      if ((s == null || s.isEmpty()) || false) {
-                          System.out.println("empty");
-                      }
-                  }
-              }
-              """
-          )
-        );
-    }
-
     @SuppressWarnings("all")
     @Test
     void unwrapWhileParens() {
@@ -951,36 +901,6 @@ boolean test(String s) {
         );
     }
 
-    @Test
-    void requiredCast() {
-        rewriteRun(
-          java(
-            """
-              class Test {
-                  int test(Object o) {
-                      return ((int[]) o).length;
-                  }
-              }
-              """
-          )
-        );
-    }
-
-    @Test
-    void negatedTernaryNotUnwrapped() {
-        rewriteRun(
-          java(
-            """
-              public class A {
-                  void foo(String s, String other) {
-                      boolean a = !(s == null ? other == null : s.equalsIgnoreCase(other));
-                  }
-              }
-              """
-          )
-        );
-    }
-
     @Issue("https://github.com/openrewrite/rewrite/issues/3883")
     @Test
     void unwrapNotMethodInvocation() {
@@ -1011,20 +931,124 @@ boolean falseMethod() {
         );
     }
 
-    @Issue("https://github.com/openrewrite/rewrite/issues/3904")
-    @Test
-    void negatedInstanceOf() {
-        rewriteRun(
-          unnecessaryParentheses(style -> style),
-          java(
-            """
-              class Test {
-                  boolean isNotString(Object o) {
-                      return !(o instanceof String);
+    @Nested
+    class DoNotUnwrap {
+        @Test
+        @Issue("https://github.com/openrewrite/rewrite/issues/3913")
+        void negatedAssignment() {
+            rewriteRun(
+              java(
+                """
+                  class Test {
+                      void test(char ch) {
+                          boolean sign;
+                          if (!(sign = ch == '-')) {
+                              System.out.println("not signed");
+                          }
+                      }
                   }
-              }
-              """
-          )
-        );
+                  """
+              )
+            );
+        }
+
+        @Test
+        void doNotUnwrapIfNoParens() {
+            rewriteRun(
+              java(
+                """
+                  class Test {
+                      void test(String s) {
+                          if (s == null || s.isEmpty()) {
+                              System.out.println("empty");
+                          }
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Test
+        void doNotUnwrapNegatedIfParens() {
+            rewriteRun(
+              java(
+                """
+                  class Test {
+                      void test(String s) {
+                          if (!(s == null || s.isEmpty())) {
+                              System.out.println("empty");
+                          }
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Test
+        void doNotUnwrapIfParens() {
+            rewriteRun(
+              java(
+                """
+                  class Test {
+                      void test(String s) {
+                          if ((s == null || s.isEmpty()) || false) {
+                              System.out.println("empty");
+                          }
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Test
+        void requiredCast() {
+            rewriteRun(
+              java(
+                """
+                  class Test {
+                      int test(Object o) {
+                          return ((int[]) o).length;
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Test
+        void negatedTernaryNotUnwrapped() {
+            rewriteRun(
+              java(
+                """
+                  public class A {
+                      void foo(String s, String other) {
+                          boolean a = !(s == null ? other == null : s.equalsIgnoreCase(other));
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Issue("https://github.com/openrewrite/rewrite/issues/3904")
+        @Test
+        void negatedInstanceOf() {
+            rewriteRun(
+              unnecessaryParentheses(style -> style),
+              java(
+                """
+                  class Test {
+                      boolean isNotString(Object o) {
+                          return !(o instanceof String);
+                      }
+                  }
+                  """
+              )
+            );
+        }
+
     }
 }
diff --git a/rewrite-java/src/main/java/org/openrewrite/java/UnwrapParentheses.java b/rewrite-java/src/main/java/org/openrewrite/java/UnwrapParentheses.java
@@ -58,7 +58,10 @@ public static boolean isUnwrappable(Cursor parensScope) {
             return !(parensScope.getValue() == ((J.DoWhileLoop) parent).getWhileCondition());
         } else if (parent instanceof J.Unary) {
             J innerJ = ((J.Parentheses<?>) parensScope.getValue()).getTree();
-            return !(innerJ instanceof J.Binary) && !(innerJ instanceof J.Ternary) && !(innerJ instanceof J.InstanceOf);
+            return !(innerJ instanceof J.Assignment) &&
+                   !(innerJ instanceof J.Binary) &&
+                   !(innerJ instanceof J.Ternary) &&
+                   !(innerJ instanceof J.InstanceOf);
         }
         return true;
     }