Fixed edge case where DeleteProperty left an extraneous newline if de…

…leting the first element in a document (openrewrite#1841)
nmck257 · Jun 1, 2022 · b0bac68 · b0bac68
1 parent 726b973
commit b0bac68
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 41 deletions.
diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/ChangePropertyKey.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/ChangePropertyKey.java
@@ -121,7 +121,7 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, P p) {
                                 propertyToTest,
                                 entry
                         ));
-                        doAfterVisit(new DeletePropertyVisitor<>(entry));
+                        doAfterVisit(new DeleteProperty.DeletePropertyVisitor<>(entry));
                         break;
                     }
                     propertyToTest = propertyToTest.substring(value.length() + 1);
@@ -168,47 +168,11 @@ public Yaml.Mapping visitMapping(Yaml.Mapping mapping, P p) {
                     }));
                 } else {
                     m = m.withEntries(ListUtils.concat(m.getEntries(), newEntry));
-                    doAfterVisit(new DeletePropertyVisitor<>(entryToReplace));
+                    doAfterVisit(new DeleteProperty.DeletePropertyVisitor<>(entryToReplace));
                 }
             }
 
             return m;
         }
     }
-
-    private static class DeletePropertyVisitor<P> extends YamlIsoVisitor<P> {
-        private final Yaml.Mapping.Entry scope;
-
-        private DeletePropertyVisitor(Yaml.Mapping.Entry scope) {
-            this.scope = scope;
-        }
-
-        @Override
-        public Yaml.Mapping visitMapping(Yaml.Mapping mapping, P p) {
-            Yaml.Mapping m = super.visitMapping(mapping, p);
-
-            boolean changed = false;
-            List<Yaml.Mapping.Entry> entries = new ArrayList<>();
-            for (Yaml.Mapping.Entry entry : m.getEntries()) {
-                if (entry == scope || (entry.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) entry.getValue()).getEntries().isEmpty())) {
-                    changed = true;
-                } else {
-                    entries.add(entry);
-                }
-            }
-
-            if (changed) {
-                m = m.withEntries(entries);
-
-                if (getCursor().getParentOrThrow().getValue() instanceof Yaml.Document) {
-                    Yaml.Document document = getCursor().getParentOrThrow().getValue();
-                    if (!document.isExplicit()) {
-                        m = m.withEntries(m.getEntries());
-                    }
-                }
-            }
-            return m;
-        }
-    }
-
 }
diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/DeleteProperty.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/DeleteProperty.java
@@ -111,10 +111,10 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC
         };
     }
 
-    private static class DeletePropertyVisitor<P> extends YamlVisitor<P> {
+    public static class DeletePropertyVisitor<P> extends YamlVisitor<P> {
         private final Yaml.Mapping.Entry scope;
 
-        private DeletePropertyVisitor(Yaml.Mapping.Entry scope) {
+        public DeletePropertyVisitor(Yaml.Mapping.Entry scope) {
             this.scope = scope;
         }
 
@@ -124,10 +124,16 @@ public Yaml visitMapping(Yaml.Mapping mapping, P p) {
 
             boolean changed = false;
             List<Yaml.Mapping.Entry> entries = new ArrayList<>();
+            String deletedPrefix = null;
             for (Yaml.Mapping.Entry entry : m.getEntries()) {
                 if (entry == scope || (entry.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) entry.getValue()).getEntries().isEmpty())) {
+                    deletedPrefix = entry.getPrefix();
                     changed = true;
                 } else {
+                    if (deletedPrefix != null) {
+                        entry = entry.withPrefix(deletedPrefix);
+                        deletedPrefix = null;
+                    }
                     entries.add(entry);
                 }
             }

diff --git a/rewrite-yaml/src/test/kotlin/org/openrewrite/yaml/ChangePropertyKeyTest.kt b/rewrite-yaml/src/test/kotlin/org/openrewrite/yaml/ChangePropertyKeyTest.kt
@@ -341,7 +341,6 @@ class ChangePropertyKeyTest : YamlRecipeTest {
               else: qwe
         """,
         after = """
-
             something:
               else: qwe
             x.y.z: abc

diff --git a/rewrite-yaml/src/test/kotlin/org/openrewrite/yaml/DeletePropertyKeyTest.kt b/rewrite-yaml/src/test/kotlin/org/openrewrite/yaml/DeletePropertyKeyTest.kt
@@ -33,6 +33,42 @@ class DeletePropertyKeyTest : YamlRecipeTest {
         after = ""
     )
 
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite/issues/1841")
+    fun firstItem() = assertChanged(
+        before = """
+          management.metrics.binders.files.enabled: true
+          server.port: 8080
+        """,
+        after = """
+          server.port: 8080
+        """
+    )
+
+    @Test
+    fun lastItem() = assertChanged(
+        before = """
+          server.port: 8080
+          management.metrics.binders.files.enabled: true
+        """,
+        after = """
+          server.port: 8080
+        """
+    )
+
+    @Test
+    fun middleItem() = assertChanged(
+        before = """
+          app.name: foo
+          management.metrics.binders.files.enabled: true
+          server.port: 8080
+        """,
+        after = """
+          app.name: foo
+          server.port: 8080
+        """
+    )
+
     @Test
     fun downDeeper() = assertChanged(
         before = """