Switch mvnup to domtrip (#11432)

Author: gnodet

apache-maven/src/main/appended-resources/META-INF/LICENSE.vm

@@ -37,7 +37,8 @@ subject to the terms and conditions of the following licenses:
 #*      *##set ( $spdx = 'MIT' )
 #*    *##elseif ( $license.name == "Eclipse Public License, Version 1.0" )
 #*      *##set ( $spdx = 'EPL-1.0' )
-#*    *##elseif ( $license.name == "Eclipse Public License, Version 2.0" )
+#*    *##elseif ( $license.name == "Eclipse Public License, Version 2.0"
+                    || $license.url.contains( "https://www.eclipse.org/legal/epl-v20.html" ) )
 #*      *##set ( $spdx = 'EPL-2.0' )
 #*    *##elseif ( $license.url.contains( "www.apache.org/licenses/LICENSE-2.0" )
                     || $license.url.contains( "https://raw.github.com/hunterhacker/jdom/master/LICENSE.txt" ) )

impl/maven-cli/pom.xml

@@ -160,8 +160,13 @@ under the License.
     </dependency>
 
     <dependency>
-      <groupId>org.jdom</groupId>
-      <artifactId>jdom2</artifactId>
+      <groupId>eu.maveniverse.maven.domtrip</groupId>
+      <artifactId>domtrip-core</artifactId>
+    </dependency>
+
+    <dependency>
+      <groupId>eu.maveniverse.maven.domtrip</groupId>
+      <artifactId>domtrip-maven</artifactId>
     </dependency>
 
     <dependency>

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/UpgradeContext.java

@@ -30,7 +30,7 @@
 import org.jline.utils.AttributedStringBuilder;
 import org.jline.utils.AttributedStyle;
 
-import static org.apache.maven.cling.invoker.mvnup.goals.UpgradeConstants.Indentation;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Indentation;
 
 @SuppressWarnings("VisibilityModifier")
 public class UpgradeContext extends LookupContext {

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java

@@ -18,26 +18,22 @@
  */
 package org.apache.maven.cling.invoker.mvnup.goals;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
 
+import eu.maveniverse.domtrip.Document;
+import eu.maveniverse.domtrip.DomTripException;
+import eu.maveniverse.domtrip.maven.MavenPomElements;
 import org.apache.maven.api.cli.mvnup.UpgradeOptions;
 import org.apache.maven.api.di.Inject;
 import org.apache.maven.cling.invoker.mvnup.Goal;
 import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
-import org.jdom2.Document;
-import org.jdom2.JDOMException;
-import org.jdom2.output.Format;
-import org.jdom2.output.XMLOutputter;
 
-import static org.apache.maven.cling.invoker.mvnup.goals.UpgradeConstants.Files.MVN_DIRECTORY;
-import static org.apache.maven.cling.invoker.mvnup.goals.UpgradeConstants.ModelVersions.MODEL_VERSION_4_1_0;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Files.MVN_DIRECTORY;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.ModelVersions.MODEL_VERSION_4_1_0;
 
 /**
  * Base class for upgrade goals containing shared functionality.
@@ -160,7 +156,7 @@ public int execute(UpgradeContext context) throws Exception {
         } else if (options.all().orElse(false)) {
             targetModel = MODEL_VERSION_4_1_0;
         } else {
-            targetModel = UpgradeConstants.ModelVersions.MODEL_VERSION_4_0_0;
+            targetModel = MavenPomElements.ModelVersions.MODEL_VERSION_4_0_0;
         }
 
         if (!ModelVersionUtils.isValidModelVersion(targetModel)) {
@@ -176,7 +172,7 @@ public int execute(UpgradeContext context) throws Exception {
         Map<Path, Document> pomMap;
         try {
             pomMap = PomDiscovery.discoverPoms(startingDirectory);
-        } catch (IOException | JDOMException e) {
+        } catch (IOException | DomTripException e) {
             context.failure("Failed to discover POM files: " + e.getMessage());
             return 1;
         }
@@ -212,7 +208,7 @@ protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Do
             // This is needed for both 4.0.0 and 4.1.0 to help Maven find the project root
             createMvnDirectoryIfNeeded(context);
 
-            return result.success() ? 0 : 1;
+            return result.errorPoms().isEmpty() ? 0 : 1;
         } catch (Exception e) {
             context.failure("Strategy execution failed: " + e.getMessage());
             return 1;
@@ -226,7 +222,7 @@ protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Do
     protected abstract boolean shouldSaveModifications();
 
     /**
-     * Saves the modified documents to disk.
+     * Saves the modified documents to disk using domtrip's perfect formatting preservation.
      */
     protected void saveModifications(UpgradeContext context, Map<Path, Document> pomMap) {
         context.info("");
@@ -236,27 +232,16 @@ protected void saveModifications(UpgradeContext context, Map<Path, Document> pom
             Path pomPath = entry.getKey();
             Document document = entry.getValue();
             try {
-                String content = Files.readString(entry.getKey(), StandardCharsets.UTF_8);
-                int startIndex = content.indexOf("<" + document.getRootElement().getName());
-                String head = startIndex >= 0 ? content.substring(0, startIndex) : "";
-                String lastTag = document.getRootElement().getName() + ">";
-                int endIndex = content.lastIndexOf(lastTag);
-                String tail = endIndex >= 0 ? content.substring(endIndex + lastTag.length()) : "";
-                Format format = Format.getRawFormat();
-                format.setLineSeparator(System.lineSeparator());
-                XMLOutputter out = new XMLOutputter(format);
-                ByteArrayOutputStream output = new ByteArrayOutputStream();
-                try (OutputStream outputStream = output) {
-                    outputStream.write(head.getBytes(StandardCharsets.UTF_8));
-                    out.output(document.getRootElement(), outputStream);
-                    outputStream.write(tail.getBytes(StandardCharsets.UTF_8));
-                }
-                String newBody = output.toString(StandardCharsets.UTF_8);
-                Files.writeString(pomPath, newBody, StandardCharsets.UTF_8);
+                // Use domtrip for perfect formatting preservation
+                String xmlContent = DomUtils.toXml(document);
+                Files.writeString(pomPath, xmlContent);
+                context.detail("Saved: " + pomPath);
             } catch (Exception e) {
                 context.failure("Failed to save " + pomPath + ": " + e.getMessage());
             }
         }
+
+        context.success("All modifications saved successfully");
     }
 
     /**

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java

@@ -19,16 +19,30 @@
 package org.apache.maven.cling.invoker.mvnup.goals;
 
 import java.nio.file.Path;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import eu.maveniverse.domtrip.Document;
+import eu.maveniverse.domtrip.Element;
+import eu.maveniverse.domtrip.maven.Coordinates;
+import eu.maveniverse.domtrip.maven.MavenPomElements;
 import org.apache.maven.api.cli.mvnup.UpgradeOptions;
 import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
-import org.jdom2.Document;
+
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PARENT;
 
 /**
  * Abstract base class for upgrade strategies that provides common functionality
  * and reduces code duplication across strategy implementations.
+ *
+ * <p>Strategies work with domtrip Documents for perfect formatting preservation.
+ * Subclasses can create domtrip Editors from Documents as needed:
+ * <pre>
+ * Editor editor = new Editor(document);
+ * // ... perform domtrip operations ...
+ * // Document is automatically updated
+ * </pre>
  */
 public abstract class AbstractUpgradeStrategy implements UpgradeStrategy {
 
@@ -92,4 +106,82 @@ protected void logSummary(UpgradeContext context, UpgradeResult result) {
         }
         context.unindent();
     }
+
+    /**
+     * Extracts an Artifact from a POM document with parent resolution.
+     * If groupId or version are missing, attempts to resolve from parent.
+     *
+     * <p>This method handles Maven's inheritance mechanism where groupId and version
+     * can be inherited from the parent POM.
+     *
+     * @param context the upgrade context for logging
+     * @param pomDocument the POM document
+     * @return the Artifact or null if it cannot be determined
+     */
+    public static Coordinates extractArtifactCoordinatesWithParentResolution(
+            UpgradeContext context, Document pomDocument) {
+        Element root = pomDocument.root();
+
+        // Extract direct values
+        String groupId = root.childTextTrimmed(MavenPomElements.Elements.GROUP_ID);
+        String artifactId = root.childTextTrimmed(MavenPomElements.Elements.ARTIFACT_ID);
+        String version = root.childTextTrimmed(MavenPomElements.Elements.VERSION);
+
+        // If groupId or version is missing, try to get from parent
+        if (groupId == null || version == null) {
+            Element parentElement = root.child(PARENT).orElse(null);
+            if (parentElement != null) {
+                if (groupId == null) {
+                    groupId = parentElement.childTextTrimmed(MavenPomElements.Elements.GROUP_ID);
+                }
+                if (version == null) {
+                    version = parentElement.childTextTrimmed(MavenPomElements.Elements.VERSION);
+                }
+            }
+        }
+
+        // ArtifactId is required and cannot be inherited
+        if (artifactId == null || artifactId.isEmpty()) {
+            context.debug("Cannot determine artifactId for POM");
+            return null;
+        }
+
+        // GroupId and version can be inherited, but if still null, we can't create a valid Artifact
+        if (groupId == null || groupId.isEmpty() || version == null || version.isEmpty()) {
+            context.debug("Cannot determine complete GAV for artifactId: " + artifactId);
+            return null;
+        }
+
+        return Coordinates.of(groupId, artifactId, version);
+    }
+
+    /**
+     * Computes all artifacts from all POMs in a multi-module project.
+     * This includes resolving parent inheritance.
+     *
+     * @param context the upgrade context for logging
+     * @param pomMap map of all POM files in the project
+     * @return set of all Artifacts in the project
+     */
+    public static Set<Coordinates> computeAllArtifactCoordinates(UpgradeContext context, Map<Path, Document> pomMap) {
+        Set<Coordinates> coordinates = new HashSet<>();
+
+        context.info("Computing artifacts for inference from " + pomMap.size() + " POM(s)...");
+
+        // Extract artifact from all POMs in the project
+        for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
+            Path pomPath = entry.getKey();
+            Document pomDocument = entry.getValue();
+
+            Coordinates coordinate =
+                    AbstractUpgradeStrategy.extractArtifactCoordinatesWithParentResolution(context, pomDocument);
+            if (coordinate != null) {
+                coordinates.add(coordinate);
+                context.debug("Found artifact: " + coordinate.toGAV() + " from " + pomPath);
+            }
+        }
+
+        context.info("Computed " + coordinates.size() + " unique artifact(s) for inference");
+        return coordinates;
+    }
 }