Skip to content

Commit 5583525

Browse files
gnodetclaude
andcommitted
Fix #12080: mvnup - comment out dependencies with undefined property expressions
Comment out dependencies whose version uses a property expression that is not defined in any POM in the reactor. Scans <properties> sections including those in profiles. Well-known properties (project.*, env.*, maven.*, etc.) are excluded from the check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b50f897 commit 5583525

2 files changed

Lines changed: 598 additions & 1 deletion

File tree

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

Lines changed: 155 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@
2525
import java.util.Map;
2626
import java.util.Objects;
2727
import java.util.Set;
28+
import java.util.regex.Matcher;
29+
import java.util.regex.Pattern;
2830
import java.util.stream.Stream;
2931

32+
import eu.maveniverse.domtrip.Comment;
3033
import eu.maveniverse.domtrip.Document;
34+
import eu.maveniverse.domtrip.Editor;
3135
import eu.maveniverse.domtrip.Element;
3236
import eu.maveniverse.domtrip.maven.Coordinates;
3337
import eu.maveniverse.domtrip.maven.MavenPomElements;
@@ -54,6 +58,7 @@
5458
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_REPOSITORY;
5559
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE;
5660
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES;
61+
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES;
5762
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.RELATIVE_PATH;
5863
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORIES;
5964
import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORY;
@@ -70,6 +75,8 @@
7075
@Priority(20)
7176
public class CompatibilityFixStrategy extends AbstractUpgradeStrategy {
7277

78+
private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{([^}]+)}");
79+
7380
@Override
7481
public boolean isApplicable(UpgradeContext context) {
7582
UpgradeOptions options = getOptions(context);
@@ -117,6 +124,8 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
117124
Set<Path> modifiedPoms = new HashSet<>();
118125
Set<Path> errorPoms = new HashSet<>();
119126

127+
Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap);
128+
120129
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
121130
Path pomPath = entry.getKey();
122131
Document pomDocument = entry.getValue();
@@ -128,13 +137,13 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
128137
try {
129138
boolean hasIssues = false;
130139

131-
// Apply all compatibility fixes
132140
hasIssues |= fixUnsupportedCombineChildrenAttributes(pomDocument, context);
133141
hasIssues |= fixUnsupportedCombineSelfAttributes(pomDocument, context);
134142
hasIssues |= fixDuplicateDependencies(pomDocument, context);
135143
hasIssues |= fixDuplicatePlugins(pomDocument, context);
136144
hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context);
137145
hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context);
146+
hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context);
138147

139148
if (hasIssues) {
140149
context.success("Maven 4 compatibility issues fixed");
@@ -345,6 +354,151 @@ private boolean fixIncorrectParentRelativePaths(
345354
return false;
346355
}
347356

357+
private Set<String> collectAllDefinedProperties(Map<Path, Document> pomMap) {
358+
Set<String> properties = new HashSet<>();
359+
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
360+
collectPropertiesFromDom(entry.getValue(), properties);
361+
}
362+
return properties;
363+
}
364+
365+
private void collectPropertiesFromDom(Document document, Set<String> properties) {
366+
Element root = document.root();
367+
368+
root.childElement(PROPERTIES)
369+
.ifPresent(propsElement -> propsElement.childElements().forEach(child -> properties.add(child.name())));
370+
371+
root.childElement(PROFILES)
372+
.ifPresent(profiles -> profiles.childElements(PROFILE)
373+
.forEach(profile -> profile.childElement(PROPERTIES)
374+
.ifPresent(propsElement ->
375+
propsElement.childElements().forEach(child -> properties.add(child.name())))));
376+
}
377+
378+
/**
379+
* Fixes dependencies with undefined property expressions by commenting them out.
380+
*/
381+
private boolean fixUndefinedPropertyExpressions(
382+
Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) {
383+
Element root = pomDocument.root();
384+
385+
Stream<DependencyContainer> dependencyContainers = Stream.concat(
386+
Stream.of(
387+
new DependencyContainer(
388+
root.childElement(DEPENDENCIES).orElse(null), DEPENDENCIES),
389+
new DependencyContainer(
390+
root.childElement(DEPENDENCY_MANAGEMENT)
391+
.flatMap(dm -> dm.childElement(DEPENDENCIES))
392+
.orElse(null),
393+
DEPENDENCY_MANAGEMENT))
394+
.filter(container -> container.element != null),
395+
root.childElement(PROFILES).stream()
396+
.flatMap(profiles -> profiles.childElements(PROFILE))
397+
.flatMap(profile -> Stream.of(
398+
new DependencyContainer(
399+
profile.childElement(DEPENDENCIES)
400+
.orElse(null),
401+
"profile dependencies"),
402+
new DependencyContainer(
403+
profile.childElement(DEPENDENCY_MANAGEMENT)
404+
.flatMap(dm -> dm.childElement(DEPENDENCIES))
405+
.orElse(null),
406+
"profile dependencyManagement"))
407+
.filter(container -> container.element != null)));
408+
409+
return dependencyContainers
410+
.map(container -> fixUndefinedPropertyExpressionsInSection(
411+
container.element, allDefinedProperties, pomDocument, context, container.sectionName))
412+
.reduce(false, Boolean::logicalOr);
413+
}
414+
415+
/**
416+
* Fixes undefined property expressions in a specific dependencies section.
417+
*/
418+
private boolean fixUndefinedPropertyExpressionsInSection(
419+
Element dependenciesElement,
420+
Set<String> allDefinedProperties,
421+
Document pomDocument,
422+
UpgradeContext context,
423+
String sectionName) {
424+
boolean fixed = false;
425+
List<Element> dependencies =
426+
dependenciesElement.childElements(DEPENDENCY).toList();
427+
Editor editor = new Editor(pomDocument);
428+
429+
for (Element dependency : dependencies) {
430+
Set<String> undefinedProps = findUndefinedProperties(dependency, allDefinedProperties);
431+
if (!undefinedProps.isEmpty()) {
432+
String propLabel = undefinedProps.size() > 1 ? "properties" : "property";
433+
String propsStr = "'" + String.join("', '", undefinedProps) + "'";
434+
435+
Comment comment = editor.commentOutElement(dependency);
436+
String elementXml = comment.content().trim();
437+
comment.content(
438+
" mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " ");
439+
440+
context.detail("Fixed: Commented out dependency with undefined " + propLabel + " " + propsStr + " in "
441+
+ sectionName);
442+
fixed = true;
443+
}
444+
}
445+
446+
return fixed;
447+
}
448+
449+
/**
450+
* Finds undefined property expressions in a dependency's coordinate fields.
451+
*/
452+
private Set<String> findUndefinedProperties(Element dependency, Set<String> allDefinedProperties) {
453+
Set<String> undefinedProperties = new HashSet<>();
454+
455+
String groupId = dependency.childText(MavenPomElements.Elements.GROUP_ID);
456+
String artifactId = dependency.childText(MavenPomElements.Elements.ARTIFACT_ID);
457+
String version = dependency.childText(MavenPomElements.Elements.VERSION);
458+
459+
collectUndefinedExpressions(groupId, allDefinedProperties, undefinedProperties);
460+
collectUndefinedExpressions(artifactId, allDefinedProperties, undefinedProperties);
461+
collectUndefinedExpressions(version, allDefinedProperties, undefinedProperties);
462+
463+
return undefinedProperties;
464+
}
465+
466+
private void collectUndefinedExpressions(String value, Set<String> allDefinedProperties, Set<String> result) {
467+
if (value == null) {
468+
return;
469+
}
470+
Matcher matcher = EXPRESSION_PATTERN.matcher(value);
471+
while (matcher.find()) {
472+
String propertyName = matcher.group(1);
473+
if (!isWellKnownProperty(propertyName) && !allDefinedProperties.contains(propertyName)) {
474+
result.add(propertyName);
475+
}
476+
}
477+
}
478+
479+
private static boolean isWellKnownProperty(String propertyName) {
480+
if (propertyName.startsWith("project.")
481+
|| propertyName.startsWith("pom.")
482+
|| propertyName.startsWith("env.")
483+
|| propertyName.startsWith("settings.")
484+
|| propertyName.startsWith("maven.")) {
485+
return true;
486+
}
487+
if (propertyName.startsWith("java.")
488+
|| propertyName.startsWith("os.")
489+
|| propertyName.startsWith("user.")
490+
|| propertyName.startsWith("file.")
491+
|| propertyName.startsWith("line.")
492+
|| propertyName.startsWith("path.")
493+
|| propertyName.startsWith("sun.")) {
494+
return true;
495+
}
496+
return "basedir".equals(propertyName)
497+
|| "revision".equals(propertyName)
498+
|| "sha1".equals(propertyName)
499+
|| "changelist".equals(propertyName);
500+
}
501+
348502
/**
349503
* Recursively finds all elements with a specific attribute value.
350504
*/

0 commit comments

Comments
 (0)