diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java index d85c745f18cc..f66c3c959b1c 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java @@ -43,13 +43,20 @@ * each POM element's children and removes duplicates, keeping only the last * occurrence of each element name. * - *

This strategy only targets "scalar" elements — elements that should appear - * at most once within their parent according to the Maven POM schema. Elements - * inside list containers (e.g., {@code } inside {@code }, + *

This strategy only targets "scalar" elements at well-known POM schema + * positions — elements that should appear at most once within their parent + * according to the Maven POM schema. Elements inside list containers + * (e.g., {@code } inside {@code }, * {@code } inside {@code }) are not affected by this strategy * since duplicate dependencies and plugins are handled by * {@link DeduplicateDependenciesStrategy}. * + *

Plugin {@code } elements are skipped entirely, because they + * contain free-form, plugin-specific XML whose schema is not known to this tool. + * Treating same-named children as duplicates in configuration sections (e.g., + * multiple {@code } entries inside {@code }) would silently + * remove valid list entries and break builds. + * * @see #12530 */ @Named @@ -134,6 +141,14 @@ protected UpgradeResult doApply(UpgradeContext context, Map pomM return new UpgradeResult(processedPoms, modifiedPoms, errorPoms); } + /** + * Parent element names whose contents are free-form, plugin-specific XML. + * Deduplication is skipped entirely for these elements and their descendants, + * because same-named children (e.g., multiple {@code } in + * {@code }) are list entries, not schema-level duplicates. + */ + static final Set FREEFORM_ELEMENTS = Set.of("configuration"); + /** * Recursively scans an element's children for duplicates and removes them. * Uses last-wins semantics (consistent with Maven 3's behavior). @@ -145,6 +160,14 @@ protected UpgradeResult doApply(UpgradeContext context, Map pomM private boolean removeDuplicateElements(Element element, UpgradeContext context) { boolean removed = false; + // Skip free-form plugin configuration elements — their XML schema is + // plugin-specific and unknown to this tool. Treating same-named children + // as duplicates here would silently remove valid list entries (e.g., + // elements inside ) and break builds. + if (FREEFORM_ELEMENTS.contains(element.name())) { + return false; + } + // Skip list container elements — their children naturally repeat if (LIST_CONTAINER_ELEMENTS.contains(element.name())) { // Still recurse into each child to check for duplicates within them diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java index 602cc0626958..4ae2d795d93a 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java @@ -457,6 +457,296 @@ void shouldNotRemoveModulesInList() { } } + @Nested + @DisplayName("Plugin Configuration Skipping") + class PluginConfigurationSkippingTests { + + @Test + @DisplayName("should not remove multiple arg elements inside compilerArgs in configuration") + void shouldNotRemoveCompilerArgsInConfiguration() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -Xlint:all + -Xlint:-processing + -Xmaxwarns + 5 + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("-Xlint:all"), "Should keep -Xlint:all arg"); + assertTrue(xml.contains("-Xlint:-processing"), "Should keep -Xlint:-processing arg"); + assertTrue(xml.contains("-Xmaxwarns"), "Should keep -Xmaxwarns arg"); + assertTrue(xml.contains("5"), "Should keep 5 arg"); + } + + @Test + @DisplayName("should not remove elements inside execution configuration") + void shouldNotRemoveElementsInsideExecutionConfiguration() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + org.codehaus.mojo + exec-maven-plugin + + + + + -classpath + ${project.build.outputDirectory} + com.example.Main + + + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + String xml = DomUtils.toXml(document); + // Count argument occurrences + int count = xml.split("", -1).length - 1; + assertEquals(3, count, "Should still have 3 argument elements"); + } + + @Test + @DisplayName("should not remove checkstyle module elements inside configuration") + void shouldNotRemoveCheckstyleModulesInConfiguration() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + maven-checkstyle-plugin + + + + + + + + + + + + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + String xml = DomUtils.toXml(document); + // Count module occurrences (Checker, FileTabCharacter, TreeWalker, AvoidStarImport, NeedBraces = 5) + int moduleCount = xml.split(" + + 4.0.0 + test + old-name + new-name + 1.0.0 + + + + maven-compiler-plugin + + + -Xlint:all + -Xmaxwarns + 5 + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + // POM-level duplicate should be removed + assertTrue(xml.contains("new-name"), "Should keep last artifactId"); + assertFalse(xml.contains("old-name"), "Should remove first artifactId"); + // Configuration args should be preserved + assertTrue(xml.contains("-Xlint:all"), "Should keep -Xlint:all arg"); + assertTrue(xml.contains("-Xmaxwarns"), "Should keep -Xmaxwarns arg"); + assertTrue(xml.contains("5"), "Should keep 5 arg"); + } + + @Test + @DisplayName("should not remove exclude elements inside inputExcludes in configuration") + void shouldNotRemoveExcludesInConfiguration() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + apache-rat-plugin + + + .github/** + src/main/antlr4/Abnf.g4 + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains(".github/**"), "Should keep first exclude"); + assertTrue(xml.contains("src/main/antlr4/Abnf.g4"), "Should keep second exclude"); + } + + @Test + @DisplayName("should not remove pluginExecution elements inside lifecycleMappingMetadata in configuration") + void shouldNotRemovePluginExecutionsInConfiguration() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + + org.eclipse.m2e + lifecycle-mapping + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + + + org.codehaus.mojo + exec-maven-plugin + + + + + + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + String xml = DomUtils.toXml(document); + int count = xml.split("", -1).length - 1; + assertEquals(2, count, "Should still have 2 pluginExecution elements"); + } + } + @Nested @DisplayName("Strategy Description") class StrategyDescriptionTests { diff --git a/its/core-it-suite/src/test/resources/mng-12534-after-annotation/plugin/pom.xml b/its/core-it-suite/src/test/resources-filtered/mng-12534-after-annotation/plugin/pom.xml similarity index 96% rename from its/core-it-suite/src/test/resources/mng-12534-after-annotation/plugin/pom.xml rename to its/core-it-suite/src/test/resources-filtered/mng-12534-after-annotation/plugin/pom.xml index 7363c59c1979..e268977329ba 100644 --- a/its/core-it-suite/src/test/resources/mng-12534-after-annotation/plugin/pom.xml +++ b/its/core-it-suite/src/test/resources-filtered/mng-12534-after-annotation/plugin/pom.xml @@ -11,7 +11,7 @@ 17 - 4.1.0-SNAPSHOT + ${maven-version}