From fc2c4b4f3618c8464220cd33fb0dfe470bcc98a6 Mon Sep 17 00:00:00 2001
From: Sam Snyder <sam@moderne.io>
Date: Sun, 28 Jul 2024 20:46:24 -0700
Subject: [PATCH] Make AddToTagVisitor more targeted in its formatting.

Previously it would sometimes format tags that came before the tag being inserted.
---
 .../org/openrewrite/maven/tree/Scope.java     |   3 +
 .../maven/ChangeDependencyClassifierTest.java | 212 +++++++++---------
 .../org/openrewrite/xml/AddToTagVisitor.java  |   8 +-
 3 files changed, 112 insertions(+), 111 deletions(-)

diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/Scope.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/Scope.java
index fb50ce0259c..249a6b1f000 100644
--- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/Scope.java
+++ b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/Scope.java
@@ -24,6 +24,7 @@ public enum Scope {
     Runtime,
     Test,
     System,
+    Import,
     Invalid;
 
     /**
@@ -110,6 +111,8 @@ public static Scope fromName(@Nullable String scope) {
                 return Test;
             case "system":
                 return System;
+            case "import":
+                return Import;
             default:
                 return Invalid;
         }
diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyClassifierTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyClassifierTest.java
index 285b0c3acfb..be97c47383c 100644
--- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyClassifierTest.java
+++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyClassifierTest.java
@@ -40,7 +40,7 @@ void noClassifierToClassifier() {
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -48,7 +48,7 @@ void noClassifierToClassifier() {
                     <version>3.10.0</version>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -67,7 +67,7 @@ void noClassifierToClassifier() {
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -76,7 +76,7 @@ void noClassifierToClassifier() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -103,11 +103,11 @@ void noClassifierToClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -115,7 +115,7 @@ void noClassifierToClassifierManagedDependency() {
                     <version>3.10.0</version>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -130,11 +130,11 @@ void noClassifierToClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -143,7 +143,7 @@ void noClassifierToClassifierManagedDependency() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -170,11 +170,11 @@ void addClassifierUsingGlobsExpressions() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -187,7 +187,7 @@ void addClassifierUsingGlobsExpressions() {
                     <version>3.10.0</version>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -207,11 +207,11 @@ void addClassifierUsingGlobsExpressions() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -226,7 +226,7 @@ void addClassifierUsingGlobsExpressions() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -256,80 +256,80 @@ void addClassifierUsingGlobsExpressionsManagedDependency() {
           pomXml(
             """
               <project>
-                <modelVersion>4.0.0</modelVersion>
-
-                <groupId>com.mycompany.app</groupId>
-                <artifactId>my-app</artifactId>
-                <version>1</version>
-
-                <dependencies>
-                  <dependency>
-                    <groupId>org.ehcache</groupId>
-                    <artifactId>ehcache</artifactId>
-                    <version>3.10.0</version>
-                  </dependency>
-                  <dependency>
-                    <groupId>org.ehcache</groupId>
-                    <artifactId>ehcache-transactions</artifactId>
-                    <version>3.10.0</version>
-                  </dependency>
-                </dependencies>
-
-                <dependencyManagement>
+                  <modelVersion>4.0.0</modelVersion>
+              
+                  <groupId>com.mycompany.app</groupId>
+                  <artifactId>my-app</artifactId>
+                  <version>1</version>
+              
                   <dependencies>
-                    <dependency>
-                        <groupId>org.ehcache</groupId>
-                        <artifactId>ehcache</artifactId>
-                        <version>3.10.0</version>
-                    </dependency>
-                    <dependency>
-                        <groupId>org.ehcache</groupId>
-                        <artifactId>ehcache-transactions</artifactId>
-                        <version>3.10.0</version>
-                    </dependency>
+                      <dependency>
+                          <groupId>org.ehcache</groupId>
+                          <artifactId>ehcache</artifactId>
+                          <version>3.10.0</version>
+                      </dependency>
+                      <dependency>
+                          <groupId>org.ehcache</groupId>
+                          <artifactId>ehcache-transactions</artifactId>
+                          <version>3.10.0</version>
+                      </dependency>
                   </dependencies>
-                </dependencyManagement>
+              
+                  <dependencyManagement>
+                      <dependencies>
+                          <dependency>
+                              <groupId>org.ehcache</groupId>
+                              <artifactId>ehcache</artifactId>
+                              <version>3.10.0</version>
+                          </dependency>
+                          <dependency>
+                              <groupId>org.ehcache</groupId>
+                              <artifactId>ehcache-transactions</artifactId>
+                              <version>3.10.0</version>
+                          </dependency>
+                      </dependencies>
+                  </dependencyManagement>
               </project>
               """,
             """
               <project>
-                <modelVersion>4.0.0</modelVersion>
-
-                <groupId>com.mycompany.app</groupId>
-                <artifactId>my-app</artifactId>
-                <version>1</version>
-
-                <dependencies>
-                  <dependency>
-                    <groupId>org.ehcache</groupId>
-                    <artifactId>ehcache</artifactId>
-                    <version>3.10.0</version>
-                    <classifier>jakarta</classifier>
-                  </dependency>
-                  <dependency>
-                    <groupId>org.ehcache</groupId>
-                    <artifactId>ehcache-transactions</artifactId>
-                    <version>3.10.0</version>
-                    <classifier>jakarta</classifier>
-                  </dependency>
-                </dependencies>
-
-                <dependencyManagement>
+                  <modelVersion>4.0.0</modelVersion>
+              
+                  <groupId>com.mycompany.app</groupId>
+                  <artifactId>my-app</artifactId>
+                  <version>1</version>
+              
                   <dependencies>
-                    <dependency>
-                      <groupId>org.ehcache</groupId>
-                      <artifactId>ehcache</artifactId>
-                      <version>3.10.0</version>
-                      <classifier>jakarta</classifier>
-                    </dependency>
-                    <dependency>
-                      <groupId>org.ehcache</groupId>
-                      <artifactId>ehcache-transactions</artifactId>
-                      <version>3.10.0</version>
-                      <classifier>jakarta</classifier>
-                    </dependency>
+                      <dependency>
+                          <groupId>org.ehcache</groupId>
+                          <artifactId>ehcache</artifactId>
+                          <version>3.10.0</version>
+                          <classifier>jakarta</classifier>
+                      </dependency>
+                      <dependency>
+                          <groupId>org.ehcache</groupId>
+                          <artifactId>ehcache-transactions</artifactId>
+                          <version>3.10.0</version>
+                          <classifier>jakarta</classifier>
+                      </dependency>
                   </dependencies>
-                </dependencyManagement>
+              
+                  <dependencyManagement>
+                      <dependencies>
+                          <dependency>
+                              <groupId>org.ehcache</groupId>
+                              <artifactId>ehcache</artifactId>
+                              <version>3.10.0</version>
+                              <classifier>jakarta</classifier>
+                          </dependency>
+                          <dependency>
+                              <groupId>org.ehcache</groupId>
+                              <artifactId>ehcache-transactions</artifactId>
+                              <version>3.10.0</version>
+                              <classifier>jakarta</classifier>
+                          </dependency>
+                      </dependencies>
+                  </dependencyManagement>
               </project>
               """
           )
@@ -343,11 +343,11 @@ void classifierToClassifier() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -356,7 +356,7 @@ void classifierToClassifier() {
                     <classifier>javax</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -372,11 +372,11 @@ void classifierToClassifier() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -385,7 +385,7 @@ void classifierToClassifier() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -412,11 +412,11 @@ void classifierToClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -425,7 +425,7 @@ void classifierToClassifierManagedDependency() {
                     <classifier>javax</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -441,11 +441,11 @@ void classifierToClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -454,7 +454,7 @@ void classifierToClassifierManagedDependency() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -479,11 +479,11 @@ void classifierToNoClassifier() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -492,7 +492,7 @@ void classifierToNoClassifier() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -508,11 +508,11 @@ void classifierToNoClassifier() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -520,7 +520,7 @@ void classifierToNoClassifier() {
                     <version>3.10.0</version>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -545,11 +545,11 @@ void classifierToNoClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -558,7 +558,7 @@ void classifierToNoClassifierManagedDependency() {
                     <classifier>jakarta</classifier>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
@@ -574,11 +574,11 @@ void classifierToNoClassifierManagedDependency() {
             """
               <project>
                 <modelVersion>4.0.0</modelVersion>
-
+              
                 <groupId>com.mycompany.app</groupId>
                 <artifactId>my-app</artifactId>
                 <version>1</version>
-
+              
                 <dependencies>
                   <dependency>
                     <groupId>org.ehcache</groupId>
@@ -586,7 +586,7 @@ void classifierToNoClassifierManagedDependency() {
                     <version>3.10.0</version>
                   </dependency>
                 </dependencies>
-
+              
                 <dependencyManagement>
                   <dependencies>
                     <dependency>
diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/AddToTagVisitor.java b/rewrite-xml/src/main/java/org/openrewrite/xml/AddToTagVisitor.java
index b7645ca09a0..e5e4c97c679 100755
--- a/rewrite-xml/src/main/java/org/openrewrite/xml/AddToTagVisitor.java
+++ b/rewrite-xml/src/main/java/org/openrewrite/xml/AddToTagVisitor.java
@@ -46,13 +46,11 @@ public AddToTagVisitor(Xml.Tag scope, Xml.Tag tagToAdd, @Nullable Comparator<Con
     @Override
     public Xml visitTag(Xml.Tag t, P p) {
         if (scope.isScope(t)) {
-            boolean formatParent = false;
             assert getCursor().getParent() != null;
             if (t.getClosing() == null) {
-                t = t.withClosing(new Xml.Tag.Closing(Tree.randomId(), "\n",
-                                Markers.EMPTY, t.getName(), ""))
+                t = t.withClosing(autoFormat(new Xml.Tag.Closing(Tree.randomId(), "\n",
+                                Markers.EMPTY, t.getName(), ""), null, p, getCursor()))
                         .withBeforeTagDelimiterPrefix("");
-                formatParent = true;
             }
 
             //noinspection ConstantConditions
@@ -64,6 +62,7 @@ public Xml visitTag(Xml.Tag t, P p) {
             if (!formattedTagToAdd.getPrefix().contains("\n")) {
                 formattedTagToAdd = formattedTagToAdd.withPrefix("\n");
             }
+            formattedTagToAdd = autoFormat(formattedTagToAdd, null, p, getCursor());
 
             List<Content> content = t.getContent() == null ? new ArrayList<>() : new ArrayList<>(t.getContent());
             if (tagComparator != null) {
@@ -82,7 +81,6 @@ public Xml visitTag(Xml.Tag t, P p) {
             }
 
             t = t.withContent(content);
-            t = autoFormat(t, formatParent ? null : formattedTagToAdd, p, getCursor().getParent());
         }
 
         return super.visitTag(t, p);