Fix DeleteStatements improperly deleting certain statements

Closes https://github.com/moderneinc/support-public/issues/19 Related #2627
openrewrite · Jan 12, 2023 · 47e4e85 · 47e4e85
1 parent 69322ca
commit 47e4e85
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 1 deletion.
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteStatementTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteStatementTest.java
@@ -402,4 +402,31 @@ public void a() {
           )
         );
     }
+
+    @Test
+    void deleteStatementInNegation() {
+        rewriteRun(
+          spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
+              @Override
+              public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
+                  J.MethodInvocation m = super.visitMethodInvocation(method, executionContext);
+                  if (m.getSimpleName().equals("b")) {
+                      doAfterVisit(new DeleteStatement<>(m));
+                  }
+                  return m;
+              }
+          })),
+          java(
+            """
+              public abstract class A {
+                  public void a() {
+                      boolean a = !b();
+                  }
+                  
+                  abstract boolean b();
+              }
+              """
+          )
+        );
+    }
 }
diff --git a/rewrite-java/src/main/java/org/openrewrite/java/DeleteStatement.java b/rewrite-java/src/main/java/org/openrewrite/java/DeleteStatement.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java;
 
+import org.openrewrite.SourceFile;
 import org.openrewrite.internal.ListUtils;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.search.FindReferencedTypes;
@@ -25,7 +26,29 @@
 import static org.openrewrite.Tree.randomId;
 
 /**
- * Deletes standalone statements. Does not include deletion of control statements present in for loops.
+ * Deletes standalone statements.
+ * <p/>
+ * Does not include deletion of:
+ * <ul>
+ *     <li>control statements present in for loops.</li>
+ *     <li>control statements present in while loops.</li>
+ *     <li>control statements present in do while loops.</li>
+ *     <li>control statements present in if statements.</li>
+ *     <li>control statements present in switch statements.</li>
+ *     <li>statements that would render the closest parent non {@link J.Block} statement unable to be compiled</li>
+ * </ul>
+ * <p/>
+ *
+ * For example, the statement <code>isPotato()</code> would not be removed from any of the following code:
+ * <pre>
+ * {@code
+ *     if (isPotato()) { }
+ *     while (isPotato()) { }
+ *     do { } while (isPotato());
+ *     boolean potato = isPotato();
+ *     boolean notPotato = !isPotato();
+ * }
+ * </pre>
  */
 public class DeleteStatement<P> extends JavaIsoVisitor<P> {
     private final Statement statement;
@@ -38,6 +61,13 @@ public DeleteStatement(Statement statement) {
     public @Nullable Statement visitStatement(Statement statement, P p) {
         Statement s = super.visitStatement(statement, p);
 
+        // Only remove a statement directly if it appears as top-level statement in a script file
+        // Otherwise, allow this statement to be removed by #visitBlock
+        // This prevents the visitor from removing statements that are not top-level statements or direct children of a block.
+        if (!(getCursor().getParentOrThrow().getValue() instanceof SourceFile)) {
+            return s;
+        }
+
         if (this.statement.isScope(s)) {
             return s instanceof J.Block ? emptyBlock() : null;
         }