Skip to content

Commit 22fd6f5

Browse files
MBoegerstimtebeek
MBoegers
and
timtebeek
authored
Skip redundant property override in child when parent declares same dependency (#6955)
* Skip redundant property override in child POM when local parent also declares dependency When UpgradeDependencyVersion processes a managed dependency whose version is a property, and a local parent POM also declares the same dependency, the parent will handle the property change. The child should skip it to avoid a redundant property override. Fixes #6945 * rollback formatter changes to reduce PR noise * Only skip child when parent declares dependency without explicit version Addresses review feedback: if the local parent declares the same dependency with an explicit version, it will update the version tag directly and won't handle the property change. The child must still add the property override in that case. --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent f4e766e commit 22fd6f5

2 files changed

Lines changed: 101 additions & 6 deletions

File tree

‎rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.openrewrite.semver.VersionComparator;
3030
import org.openrewrite.xml.AddToTagVisitor;
3131
import org.openrewrite.xml.ChangeTagValueVisitor;
32-
import org.openrewrite.xml.XPathMatcher;
3332
import org.openrewrite.xml.tree.Xml;
3433

3534
import java.nio.file.Path;
@@ -315,6 +314,30 @@ private boolean isManagedByLocalParent(String groupId, String artifactId) {
315314
return false;
316315
}
317316

317+
/**
318+
* Check if a local parent POM (in the same repository) also declares a dependency
319+
* on the same groupId:artifactId without an explicit version. If so, the parent
320+
* will also go through the managed-dependency property path and handle the property
321+
* change — the child can skip it.
322+
*/
323+
private boolean isDeclaredByLocalParent(String groupId, String artifactId) {
324+
MavenResolutionResult current = getResolutionResult().getParent();
325+
while (current != null) {
326+
ResolvedPom parentPom = current.getPom();
327+
if (!accumulator.projectArtifacts.contains(new GroupArtifact(parentPom.getGroupId(), parentPom.getArtifactId()))) {
328+
break; // Reached a non-local (remote) parent
329+
}
330+
for (Dependency dep : parentPom.getRequested().getDependencies()) {
331+
if (groupId.equals(dep.getGroupId()) && artifactId.equals(dep.getArtifactId())
332+
&& dep.getVersion() == null) {
333+
return true;
334+
}
335+
}
336+
current = current.getParent();
337+
}
338+
return false;
339+
}
340+
318341
private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenDownloadingException {
319342
ResolvedDependency d = findDependency(t);
320343
if (d != null && d.getRepository() != null) {
@@ -330,9 +353,12 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD
330353
// if a managed dependency is expressed as a property, change the property value
331354
// do this only when a requested bom is absent, otherwise changing property has no effect
332355
if (dm != null && isProperty(dm.getRequested().getVersion()) && dm.getRequestedBom() == null) {
333-
doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2,
334-
dm.getRequested().getVersion().length() - 1),
335-
newerVersion, overrideManagedVersion, false).getVisitor());
356+
// if a local parent also declares this dependency, it will handle the property change
357+
if (!isDeclaredByLocalParent(d.getGroupId(), d.getArtifactId())) {
358+
doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2,
359+
dm.getRequested().getVersion().length() - 1),
360+
newerVersion, overrideManagedVersion, false).getVisitor());
361+
}
336362
} else if (dm != null && dm.getBomGav() == null) {
337363
// if the version is managed directly (not from a BOM) and comes from a local parent POM
338364
// (in the same repository), don't add an explicit version
@@ -489,7 +515,6 @@ private Xml.Document attemptBomUpgrade(Xml.Document document, ResolvedManagedDep
489515

490516
private List<String> getAvailableBomVersions(String groupId, String artifactId, String currentVersion, ExecutionContext ctx)
491517
throws MavenDownloadingException {
492-
//noinspection SpellCheckingInspection
493518
MavenExecutionContextView mctx = MavenExecutionContextView.view(ctx);
494519
MavenSettings settings = mctx.effectiveSettings(getResolutionResult());
495520
MavenPomDownloader downloader = new MavenPomDownloader(
@@ -520,7 +545,6 @@ private List<String> getAvailableBomVersions(String groupId, String artifactId,
520545
String dependencyGroupId,
521546
String dependencyArtifactId,
522547
ExecutionContext ctx) throws MavenDownloadingException {
523-
//noinspection SpellCheckingInspection
524548
MavenExecutionContextView mctx = MavenExecutionContextView.view(ctx);
525549
MavenSettings settings = mctx.effectiveSettings(getResolutionResult());
526550
MavenPomDownloader downloader = new MavenPomDownloader(

‎rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,6 +1290,77 @@ void upgradeVersionDefinedViaNestedExplicitPropertyInRemoteParent() {
12901290
);
12911291
}
12921292

1293+
@Issue("https://github.com/openrewrite/rewrite/issues/6945")
1294+
@Test
1295+
void upgradeVersionDefinedViaImplicitPropertyInRemoteParentOfLocalParent_withoutRelativePath() {
1296+
rewriteRun(
1297+
spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)),
1298+
pomXml(
1299+
"""
1300+
<project>
1301+
<parent>
1302+
<groupId>org.springframework.boot</groupId>
1303+
<artifactId>spring-boot-dependencies</artifactId>
1304+
<version>3.3.0</version>
1305+
</parent>
1306+
<groupId>com.mycompany</groupId>
1307+
<artifactId>my-parent</artifactId>
1308+
<version>1</version>
1309+
<dependencies>
1310+
<dependency>
1311+
<groupId>org.flywaydb</groupId>
1312+
<artifactId>flyway-core</artifactId>
1313+
</dependency>
1314+
</dependencies>
1315+
</project>
1316+
""",
1317+
"""
1318+
<project>
1319+
<parent>
1320+
<groupId>org.springframework.boot</groupId>
1321+
<artifactId>spring-boot-dependencies</artifactId>
1322+
<version>3.3.0</version>
1323+
</parent>
1324+
<groupId>com.mycompany</groupId>
1325+
<artifactId>my-parent</artifactId>
1326+
<version>1</version>
1327+
<properties>
1328+
<flyway.version>10.15.0</flyway.version>
1329+
</properties>
1330+
<dependencies>
1331+
<dependency>
1332+
<groupId>org.flywaydb</groupId>
1333+
<artifactId>flyway-core</artifactId>
1334+
</dependency>
1335+
</dependencies>
1336+
</project>
1337+
"""
1338+
),
1339+
mavenProject("my-child",
1340+
pomXml(
1341+
"""
1342+
<project>
1343+
<parent>
1344+
<groupId>com.mycompany</groupId>
1345+
<artifactId>my-parent</artifactId>
1346+
<version>1</version>
1347+
</parent>
1348+
<groupId>com.mycompany</groupId>
1349+
<artifactId>my-child</artifactId>
1350+
<version>1</version>
1351+
<dependencies>
1352+
<dependency>
1353+
<groupId>org.flywaydb</groupId>
1354+
<artifactId>flyway-core</artifactId>
1355+
</dependency>
1356+
</dependencies>
1357+
</project>
1358+
"""
1359+
)
1360+
)
1361+
);
1362+
}
1363+
12931364
@Test
12941365
void upgradeBomImport() {
12951366
rewriteRun(

0 commit comments

Comments
 (0)
⚡