Disable Java SpacesVisitor for non-Java sources (#3792)

* Disable java SpacesVisitor to make breaking changes on other code files like Groovy * Preserve prefix in `SimplifyBooleanExpressionVisitor` * Add another test case * Small cleanup to `UnwrapParentheses` * Further simplify `SimplifyBooleanExpressionVisitor` * Limit `MethodParamPad` to Java sources --------- Co-authored-by: Knut Wannheden <[email protected]>
openrewrite · Dec 8, 2023 · bee4a12 · bee4a12
1 parent 9ce0bde
commit bee4a12
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 24 deletions.
diff --git a/...test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java b/...test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java
@@ -405,6 +405,34 @@ public class A {
         );
     }
 
+    @Test
+    void preserveComments() {
+        rewriteRun(
+          java(
+            """
+              public class A {
+                  boolean m(boolean a) {
+                      if (/*a*/!!a) {
+                          return true;
+                      }
+                      return /*a*/!true || !true;
+                  }
+              }
+              """,
+            """
+              public class A {
+                  boolean m(boolean a) {
+                      if (/*a*/a) {
+                          return true;
+                      }
+                      return /*a*/false;
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @ParameterizedTest
     @Issue("https://github.com/openrewrite/rewrite-templating/issues/28")
     // Mimic what would be inserted by a Refaster template using two nullable parameters, with the second one a literal

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/UnwrapParentheses.java b/rewrite-java/src/main/java/org/openrewrite/java/UnwrapParentheses.java
@@ -18,6 +18,7 @@
 import org.openrewrite.Cursor;
 import org.openrewrite.Tree;
 import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.Space;
 
 public class UnwrapParentheses<P> extends JavaVisitor<P> {
     private final J.Parentheses<?> scope;
@@ -33,7 +34,7 @@ public <T extends J> J visitParentheses(J.Parentheses<T> parens, P p) {
             if (tree.getPrefix().isEmpty()) {
                 Object parent = getCursor().getParentOrThrow().getValue();
                 if (parent instanceof J.Return || parent instanceof J.Throw) {
-                    tree = tree.withPrefix(tree.getPrefix().withWhitespace(" "));
+                    tree = tree.withPrefix(Space.SINGLE_SPACE);
                 }
             }
             return tree;

diff --git a/...ite-java/src/main/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitor.java b/...ite-java/src/main/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitor.java
@@ -18,19 +18,16 @@
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Tree;
 import org.openrewrite.internal.lang.Nullable;
-import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.MethodMatcher;
+import org.openrewrite.java.search.SemanticallyEqual;
 import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaType;
-import org.openrewrite.java.tree.Space;
 
 import java.util.Collections;
 
 public class SimplifyBooleanExpressionVisitor extends JavaVisitor<ExecutionContext> {
-    private static final String MAYBE_AUTO_FORMAT_ME = "MAYBE_AUTO_FORMAT_ME";
-
     @Override
     public J visitBinary(J.Binary binary, ExecutionContext ctx) {
         J j = super.visitBinary(binary, ctx);
@@ -45,8 +42,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
                 j = asBinary.getRight();
             } else if (isLiteralTrue(asBinary.getRight())) {
                 j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(""));
-            } else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor())
-                    .equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) {
+            } else if (SemanticallyEqual.areEqual(asBinary.getLeft(), asBinary.getRight())) {
                 j = asBinary.getLeft();
             }
         } else if (asBinary.getOperator() == J.Binary.Type.Or) {
@@ -58,8 +54,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
                 j = asBinary.getRight();
             } else if (isLiteralFalse(asBinary.getRight())) {
                 j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(""));
-            } else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor())
-                    .equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) {
+            } else if (SemanticallyEqual.areEqual(asBinary.getLeft(), asBinary.getRight())) {
                 j = asBinary.getLeft();
             }
         } else if (asBinary.getOperator() == J.Binary.Type.Equal) {
@@ -88,7 +83,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
             }
         }
         if (asBinary != j) {
-            getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, "");
+            j = j.withPrefix(asBinary.getPrefix());
         }
         return j;
     }
@@ -99,9 +94,6 @@ public J postVisit(J tree, ExecutionContext ctx) {
         if (j instanceof J.Parentheses) {
             j = new UnnecessaryParenthesesVisitor<>().visit(j, ctx, getCursor().getParentOrThrow());
         }
-        if (j != null && getCursor().pollMessage(MAYBE_AUTO_FORMAT_ME) != null) {
-            j = autoFormat(j, ctx);
-        }
         return j;
     }
 
@@ -133,7 +125,7 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) {
             }
         }
         if (asUnary != j) {
-            getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, "");
+            j = j.withPrefix(asUnary.getPrefix());
         }
         return j;
     }
@@ -216,16 +208,6 @@ private J.Literal booleanLiteral(J j, boolean value) {
                 JavaType.Primitive.Boolean);
     }
 
-    private J removeAllSpace(J j) {
-        //noinspection ConstantConditions
-        return new JavaIsoVisitor<Integer>() {
-            @Override
-            public Space visitSpace(Space space, Space.Location loc, Integer integer) {
-                return Space.EMPTY;
-            }
-        }.visit(j, 0);
-    }
-
     /**
      * Override this method to disable simplification of equals expressions,
      * specifically for Kotlin while that is not yet part of the OpenRewrite/rewrite.

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/format/MethodParamPad.java b/rewrite-java/src/main/java/org/openrewrite/java/format/MethodParamPad.java
@@ -51,6 +51,12 @@ private static class MethodParamPadVisitor extends JavaIsoVisitor<ExecutionConte
         SpacesStyle spacesStyle;
         MethodParamPadStyle methodParamPadStyle;
 
+        @Override
+        public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
+            // This visitor causes problems for Groovy sources as the `SpacesVisitor` is not aware of Groovy
+            return sourceFile instanceof J.CompilationUnit;
+        }
+
         @Override
         public J visit(@Nullable Tree tree, ExecutionContext ctx) {
             if (tree instanceof JavaSourceFile) {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/format/SpacesVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/format/SpacesVisitor.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java.format;
 
+import org.openrewrite.SourceFile;
 import org.openrewrite.Tree;
 import org.openrewrite.internal.ListUtils;
 import org.openrewrite.internal.StringUtils;
@@ -54,6 +55,11 @@ public SpacesVisitor(SpacesStyle style, @Nullable EmptyForInitializerPadStyle em
         this.stopAfter = stopAfter;
     }
 
+    @Override
+    public boolean isAcceptable(SourceFile sourceFile, P p) {
+        return sourceFile instanceof J.CompilationUnit;
+    }
+
     <T extends J> T spaceBefore(T j, boolean spaceBefore) {
         if (!j.getComments().isEmpty()) {
             return j;