From 2c8539a063aac4c7b8c9b2e3a16753aa9d16018e Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 18 May 2026 18:19:01 +0200 Subject: [PATCH 1/4] Fix #12080: mvnup - comment out dependencies with undefined property expressions Add a new compatibility fix to CompatibilityFixStrategy that detects dependencies whose coordinate fields (groupId, artifactId, version) contain ${...} expressions referencing properties not defined in any project POM. These orphan dependencies cause Maven 4's validator to reject them with "Not fully interpolated dependency" errors. The fix collects all properties from sections across the reactor (including profiles), skips well-known built-in properties (project.*, env.*, settings.*, maven.*, revision, sha1, changelist, etc.), and comments out dependencies with truly undefined expressions using format. Co-Authored-By: Claude Opus 4.6 --- .../mvnup/goals/CompatibilityFixStrategy.java | 153 ++++++++++++++++ .../goals/CompatibilityFixStrategyTest.java | 170 ++++++++++++++++++ 2 files changed, 323 insertions(+) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java index eaaf888e48da..01a4952c4777 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java @@ -25,9 +25,13 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Stream; +import eu.maveniverse.domtrip.Comment; import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Editor; import eu.maveniverse.domtrip.Element; import eu.maveniverse.domtrip.maven.Coordinates; import eu.maveniverse.domtrip.maven.MavenPomElements; @@ -54,6 +58,7 @@ import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_REPOSITORY; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.RELATIVE_PATH; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORIES; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORY; @@ -70,6 +75,8 @@ @Priority(20) public class CompatibilityFixStrategy extends AbstractUpgradeStrategy { + private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{([^}]+)}"); + @Override public boolean isApplicable(UpgradeContext context) { UpgradeOptions options = getOptions(context); @@ -117,6 +124,9 @@ public UpgradeResult doApply(UpgradeContext context, Map pomMap) Set modifiedPoms = new HashSet<>(); Set errorPoms = new HashSet<>(); + // Collect all properties defined across all project POMs for cross-POM analysis + Set allDefinedProperties = collectAllDefinedProperties(pomMap); + for (Map.Entry entry : pomMap.entrySet()) { Path pomPath = entry.getKey(); Document pomDocument = entry.getValue(); @@ -135,6 +145,7 @@ public UpgradeResult doApply(UpgradeContext context, Map pomMap) hasIssues |= fixDuplicatePlugins(pomDocument, context); hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context); hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context); + hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context); if (hasIssues) { context.success("Maven 4 compatibility issues fixed"); @@ -345,6 +356,148 @@ private boolean fixIncorrectParentRelativePaths( return false; } + /** + * Collects all property names defined in properties sections across all project POMs. + */ + private Set collectAllDefinedProperties(Map pomMap) { + Set properties = new HashSet<>(); + + for (Document document : pomMap.values()) { + Element root = document.root(); + + root.child(PROPERTIES) + .ifPresent(propsElement -> propsElement.children().forEach(child -> properties.add(child.name()))); + + root.child(PROFILES) + .ifPresent(profiles -> profiles.children(PROFILE) + .forEach(profile -> profile.child(PROPERTIES) + .ifPresent(propsElement -> + propsElement.children().forEach(child -> properties.add(child.name()))))); + } + + return properties; + } + + /** + * Fixes dependencies with undefined property expressions by commenting them out. + */ + private boolean fixUndefinedPropertyExpressions( + Document pomDocument, Set allDefinedProperties, UpgradeContext context) { + Element root = pomDocument.root(); + + Stream dependencyContainers = Stream.concat( + Stream.of( + new DependencyContainer(root.child(DEPENDENCIES).orElse(null), DEPENDENCIES), + new DependencyContainer( + root.child(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.child(DEPENDENCIES)) + .orElse(null), + DEPENDENCY_MANAGEMENT)) + .filter(container -> container.element != null), + root.child(PROFILES).stream() + .flatMap(profiles -> profiles.children(PROFILE)) + .flatMap(profile -> Stream.of( + new DependencyContainer( + profile.child(DEPENDENCIES).orElse(null), "profile dependencies"), + new DependencyContainer( + profile.child(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.child(DEPENDENCIES)) + .orElse(null), + "profile dependencyManagement")) + .filter(container -> container.element != null))); + + return dependencyContainers + .map(container -> fixUndefinedPropertyExpressionsInSection( + container.element, allDefinedProperties, pomDocument, context, container.sectionName)) + .reduce(false, Boolean::logicalOr); + } + + /** + * Fixes undefined property expressions in a specific dependencies section. + */ + private boolean fixUndefinedPropertyExpressionsInSection( + Element dependenciesElement, + Set allDefinedProperties, + Document pomDocument, + UpgradeContext context, + String sectionName) { + boolean fixed = false; + List dependencies = dependenciesElement.children(DEPENDENCY).toList(); + Editor editor = new Editor(pomDocument); + + for (Element dependency : dependencies) { + Set undefinedProps = findUndefinedProperties(dependency, allDefinedProperties); + if (!undefinedProps.isEmpty()) { + String propLabel = undefinedProps.size() > 1 ? "properties" : "property"; + String propsStr = "'" + String.join("', '", undefinedProps) + "'"; + + Comment comment = editor.commentOutElement(dependency); + String elementXml = comment.content().trim(); + comment.content( + " mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " "); + + context.detail("Fixed: Commented out dependency with undefined " + propLabel + " " + propsStr + " in " + + sectionName); + fixed = true; + } + } + + return fixed; + } + + /** + * Finds undefined property expressions in a dependency's coordinate fields. + */ + private Set findUndefinedProperties(Element dependency, Set allDefinedProperties) { + Set undefinedProperties = new HashSet<>(); + + String groupId = dependency.childText(MavenPomElements.Elements.GROUP_ID); + String artifactId = dependency.childText(MavenPomElements.Elements.ARTIFACT_ID); + String version = dependency.childText(MavenPomElements.Elements.VERSION); + + collectUndefinedExpressions(groupId, allDefinedProperties, undefinedProperties); + collectUndefinedExpressions(artifactId, allDefinedProperties, undefinedProperties); + collectUndefinedExpressions(version, allDefinedProperties, undefinedProperties); + + return undefinedProperties; + } + + private void collectUndefinedExpressions(String value, Set allDefinedProperties, Set result) { + if (value == null) { + return; + } + Matcher matcher = EXPRESSION_PATTERN.matcher(value); + while (matcher.find()) { + String propertyName = matcher.group(1); + if (!isWellKnownProperty(propertyName) && !allDefinedProperties.contains(propertyName)) { + result.add(propertyName); + } + } + } + + private static boolean isWellKnownProperty(String propertyName) { + if (propertyName.startsWith("project.") + || propertyName.startsWith("pom.") + || propertyName.startsWith("env.") + || propertyName.startsWith("settings.") + || propertyName.startsWith("maven.")) { + return true; + } + if (propertyName.startsWith("java.") + || propertyName.startsWith("os.") + || propertyName.startsWith("user.") + || propertyName.startsWith("file.") + || propertyName.startsWith("line.") + || propertyName.startsWith("path.") + || propertyName.startsWith("sun.")) { + return true; + } + return "basedir".equals(propertyName) + || "revision".equals(propertyName) + || "sha1".equals(propertyName) + || "changelist".equals(propertyName); + } + /** * Recursively finds all elements with a specific attribute value. */ diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java index 6144f4ca3265..15ea0630168e 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java @@ -478,6 +478,176 @@ void shouldNotModifyUrlsWithoutDeprecatedExpressions() throws Exception { } } + @Nested + @DisplayName("Undefined Property Expression Fixes") + class UndefinedPropertyExpressionFixesTests { + + @Test + @DisplayName("should comment out dependency with undefined property expression") + void shouldCommentOutDependencyWithUndefinedProperty() throws Exception { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + com.google.guava + guava + ${guava-version} + + + + + """; + + 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(), "Compatibility fix should succeed"); + assertTrue(result.modifiedCount() > 0, "Should have commented out dependency"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker"); + assertTrue(xml.contains("guava-version"), "Should mention the undefined property"); + + Element root = document.root(); + Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement"); + Element deps = DomUtils.findChildElement(depMgmt, "dependencies"); + assertEquals(0, deps.childElements("dependency").count(), "Should have no dependency elements"); + } + + @Test + @DisplayName("should not comment out dependency with defined property") + void shouldNotCommentOutDependencyWithDefinedProperty() throws Exception { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + 30.0-jre + + + + + com.google.guava + guava + ${guava-version} + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = document.root(); + Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement"); + Element deps = DomUtils.findChildElement(depMgmt, "dependencies"); + assertEquals(1, deps.childElements("dependency").count(), "Dependency should still be present"); + } + + @Test + @DisplayName("should not comment out dependency with well-known built-in property") + void shouldNotCommentOutDependencyWithBuiltinProperty() throws Exception { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + test + test-dep + ${project.version} + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = document.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency with built-in property should still be present"); + } + + @Test + @DisplayName("should recognize property defined in another module POM") + void shouldRecognizePropertyFromOtherPom() throws Exception { + String parentPom = """ + + + 4.0.0 + test + parent + 1.0.0 + + 30.0-jre + + + """; + + String childPom = """ + + + 4.0.0 + + test + parent + 1.0.0 + + child + + + com.google.guava + guava + ${guava-version} + + + + """; + + Document parentDoc = Document.of(parentPom); + Document childDoc = Document.of(childPom); + Map pomMap = Map.of( + Paths.get("pom.xml"), parentDoc, + Paths.get("child/pom.xml"), childDoc); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = childDoc.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency should not be commented out when property is defined in another POM"); + } + } + @Nested @DisplayName("Strategy Description") class StrategyDescriptionTests { From c75b4179784a4a47b610b7bd43b3799c07290cfc Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 19 May 2026 23:04:25 +0200 Subject: [PATCH 2/4] Fix Spotless formatting in CompatibilityFixStrategy Co-Authored-By: Claude Opus 4.6 --- .../invoker/mvnup/goals/CompatibilityFixStrategy.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java index 01a4952c4777..b5897b97cf6a 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java @@ -369,10 +369,9 @@ private Set collectAllDefinedProperties(Map pomMap) { .ifPresent(propsElement -> propsElement.children().forEach(child -> properties.add(child.name()))); root.child(PROFILES) - .ifPresent(profiles -> profiles.children(PROFILE) - .forEach(profile -> profile.child(PROPERTIES) - .ifPresent(propsElement -> - propsElement.children().forEach(child -> properties.add(child.name()))))); + .ifPresent(profiles -> profiles.children(PROFILE).forEach(profile -> profile.child(PROPERTIES) + .ifPresent(propsElement -> + propsElement.children().forEach(child -> properties.add(child.name()))))); } return properties; From 8231330eda82505836f04a9c173d9804d335a569 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 20 May 2026 20:00:04 +0200 Subject: [PATCH 3/4] =?UTF-8?q?Fix=20domtrip=20API=20migration:=20child()?= =?UTF-8?q?=20=E2=86=92=20childElement(),=20children()=20=E2=86=92=20child?= =?UTF-8?q?Elements()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 --- .../mvnup/goals/AbstractUpgradeGoal.java | 2 +- .../mvnup/goals/CompatibilityFixStrategy.java | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java index a427ce595bca..0dc73c968c18 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java @@ -310,7 +310,7 @@ protected void fixIncompatibleExtensions(UpgradeContext context) { boolean modified = false; boolean needsNisseCompat = false; - List extensions = root.children("extension").toList(); + List extensions = root.childElements("extension").toList(); List toRemove = new ArrayList<>(); for (Element ext : extensions) { diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java index b5897b97cf6a..d5e55e9e568c 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java @@ -365,13 +365,14 @@ private Set collectAllDefinedProperties(Map pomMap) { for (Document document : pomMap.values()) { Element root = document.root(); - root.child(PROPERTIES) - .ifPresent(propsElement -> propsElement.children().forEach(child -> properties.add(child.name()))); + root.childElement(PROPERTIES) + .ifPresent(propsElement -> + propsElement.childElements().forEach(child -> properties.add(child.name()))); - root.child(PROFILES) - .ifPresent(profiles -> profiles.children(PROFILE).forEach(profile -> profile.child(PROPERTIES) + root.childElement(PROFILES).ifPresent(profiles -> profiles.childElements(PROFILE) + .forEach(profile -> profile.childElement(PROPERTIES) .ifPresent(propsElement -> - propsElement.children().forEach(child -> properties.add(child.name()))))); + propsElement.childElements().forEach(child -> properties.add(child.name()))))); } return properties; @@ -386,21 +387,24 @@ private boolean fixUndefinedPropertyExpressions( Stream dependencyContainers = Stream.concat( Stream.of( - new DependencyContainer(root.child(DEPENDENCIES).orElse(null), DEPENDENCIES), new DependencyContainer( - root.child(DEPENDENCY_MANAGEMENT) - .flatMap(dm -> dm.child(DEPENDENCIES)) + root.childElement(DEPENDENCIES).orElse(null), DEPENDENCIES), + new DependencyContainer( + root.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) .orElse(null), DEPENDENCY_MANAGEMENT)) .filter(container -> container.element != null), - root.child(PROFILES).stream() - .flatMap(profiles -> profiles.children(PROFILE)) + root.childElement(PROFILES).stream() + .flatMap(profiles -> profiles.childElements(PROFILE)) .flatMap(profile -> Stream.of( new DependencyContainer( - profile.child(DEPENDENCIES).orElse(null), "profile dependencies"), + profile.childElement(DEPENDENCIES) + .orElse(null), + "profile dependencies"), new DependencyContainer( - profile.child(DEPENDENCY_MANAGEMENT) - .flatMap(dm -> dm.child(DEPENDENCIES)) + profile.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) .orElse(null), "profile dependencyManagement")) .filter(container -> container.element != null))); @@ -421,7 +425,8 @@ private boolean fixUndefinedPropertyExpressionsInSection( UpgradeContext context, String sectionName) { boolean fixed = false; - List dependencies = dependenciesElement.children(DEPENDENCY).toList(); + List dependencies = + dependenciesElement.childElements(DEPENDENCY).toList(); Editor editor = new Editor(pomDocument); for (Element dependency : dependencies) { From 4dae3072d805440ac3bdf44888c094f589d7300e Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 20 May 2026 20:25:47 +0200 Subject: [PATCH 4/4] Trigger CI re-run