From b0b325d40bd1adf18496b8b862f9fb97dcf91c47 Mon Sep 17 00:00:00 2001 From: Hoan Nguyen Date: Fri, 24 May 2024 15:33:34 -0700 Subject: [PATCH 01/16] Improve UpgradeDependencyVersion to update dependency version whose value is defined via a property from the parent POM. --- .../maven/UpgradeDependencyVersion.java | 157 +++++++++++++----- .../maven/UpgradeDependencyVersionTest.java | 53 ++++++ 2 files changed, 172 insertions(+), 38 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 21d584c5d5a..a63d47dfc25 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -30,6 +30,7 @@ import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; +import java.nio.file.Path; import java.util.*; import static java.util.Collections.emptyList; @@ -48,7 +49,7 @@ */ @Value @EqualsAndHashCode(callSuper = false) -public class UpgradeDependencyVersion extends ScanningRecipe> { +public class UpgradeDependencyVersion extends ScanningRecipe { @EqualsAndHashCode.Exclude transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this); @@ -125,35 +126,81 @@ public String getDescription() { } @Override - public Set getInitialValue(ExecutionContext ctx) { - return new HashSet<>(); + public Accumulator getInitialValue(ExecutionContext ctx) { + return new Accumulator(); } @Override - public TreeVisitor getScanner(Set projectArtifacts) { + public TreeVisitor getScanner(Accumulator accumulator) { return new MavenIsoVisitor() { @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { ResolvedPom pom = getResolutionResult().getPom(); - projectArtifacts.add(new GroupArtifact(pom.getGroupId(), pom.getArtifactId())); - return document; + accumulator.projectArtifacts.add(new GroupArtifact(pom.getGroupId(), pom.getArtifactId())); + return super.visitDocument(document, ctx); + } + + @Override + public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext executionContext) { + ResolvedDependency d = findDependency(tag); + if (d != null && d.getRepository() != null) { + // if the resolved dependency exists AND it does not represent an artifact that was parsed + // as a source file, attempt to find a new version. + try { + String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), executionContext); + if (newerVersion != null) { + Optional version = tag.getChild("version"); + if (version.isPresent()) { + String requestedVersion = d.getRequested().getVersion(); + if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) { + String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); + if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { + getPomDeclaringProperty(getResolutionResult(), propertyName) + .ifPresent(pom -> accumulator.pomProperties.add( + new PomProperty(requireNonNull(pom.getSourcePath()), propertyName))); + } + } + } + } + } catch (MavenDownloadingException e) { + return e.warn(tag); + } + } + return super.visitTag(tag, executionContext); + } + + @Nullable + private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException { + return UpgradeDependencyVersion.this.findNewerVersion(version, ctx, () -> downloadMetadata(groupId, artifactId, ctx)); } }; } @Override - public TreeVisitor getVisitor(Set projectArtifacts) { - VersionComparator versionComparator = Semver.validate(newVersion, versionPattern).getValue(); - assert versionComparator != null; - + public TreeVisitor getVisitor(Accumulator accumulator) { return new MavenIsoVisitor() { private final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); + @Override + public Xml.Document visitDocument(final Xml.Document document, final ExecutionContext executionContext) { + return super.visitDocument(document, executionContext); + } + @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); try { - if (isDependencyTag(groupId, artifactId)) { + if (isPropertyTag()) { + accumulator.pomProperties.stream() + .filter(pomProperty -> pomProperty.pomFilePath.equals(getResolutionResult().getPom().getRequested().getSourcePath())) + .map(PomProperty::getProperty) + .filter(propertyName -> propertyName.equals(tag.getName()) + && !tag.getValue().map(newVersion::equals).orElse(false)) + .forEach(propertyName -> { + doAfterVisit(new ChangeTagValueVisitor<>(tag, newVersion)); + maybeUpdateModel(); + }); + } else if (isDependencyTag(groupId, artifactId)) { t = upgradeDependency(ctx, t); } else if (isManagedDependencyTag(groupId, artifactId)) { if (isManagedDependencyImportTag(groupId, artifactId)) { @@ -221,7 +268,10 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD if (version.isPresent()) { String requestedVersion = d.getRequested().getVersion(); if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) { - doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false).getVisitor()); + String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); + if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { + doAfterVisit(new ChangePropertyValue(propertyName, newerVersion, overrideManagedVersion, false).getVisitor()); + } } else { t = (Xml.Tag) new ChangeTagValueVisitor<>(version.get(), newerVersion).visitNonNull(t, 0, getCursor().getParentOrThrow()); } @@ -258,7 +308,7 @@ private TreeVisitor upgradeManagedDependency(Xml.Tag tag, String artifactId = managedDependency.getArtifactId(); String version = managedDependency.getVersion(); if (version != null && - !projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && + !accumulator.projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && matchesGlob(groupId, UpgradeDependencyVersion.this.groupId) && matchesGlob(artifactId, UpgradeDependencyVersion.this.artifactId)) { return upgradeVersion(ctx, t, managedDependency.getRequested().getVersion(), groupId, artifactId, version); @@ -268,7 +318,7 @@ private TreeVisitor upgradeManagedDependency(Xml.Tag tag, if (dm.getBomGav() != null) { String group = getResolutionResult().getPom().getValue(tag.getChildValue("groupId").orElse(getResolutionResult().getPom().getGroupId())); String artifactId = getResolutionResult().getPom().getValue(tag.getChildValue("artifactId").orElse("")); - if (!projectArtifacts.contains(new GroupArtifact(group, artifactId))) { + if (!accumulator.projectArtifacts.contains(new GroupArtifact(group, artifactId))) { ResolvedGroupArtifactVersion bom = dm.getBomGav(); if (Objects.equals(group, bom.getGroupId()) && Objects.equals(artifactId, bom.getArtifactId())) { @@ -329,33 +379,64 @@ public TreeVisitor upgradeVersion(ExecutionContext ctx, X @Nullable private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException { - String finalVersion = !Semver.isVersion(version) ? "0.0.0" : version; + return UpgradeDependencyVersion.this.findNewerVersion(version, ctx, () -> downloadMetadata(groupId, artifactId, ctx)); + } + }; + } - // in the case of "latest.patch", a new version can only be derived if the - // current version is a semantic version - if (versionComparator instanceof LatestPatch && !versionComparator.isValid(finalVersion, finalVersion)) { - return null; - } + @Nullable + private String findNewerVersion(String version, ExecutionContext ctx, MavenMetadataFailures.MavenMetadataDownloader download) throws MavenDownloadingException { + VersionComparator versionComparator = requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); - try { - MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx)); - List versions = new ArrayList<>(); - for (String v : mavenMetadata.getVersioning().getVersions()) { - if (versionComparator.isValid(finalVersion, v)) { - versions.add(v); - } - } - // handle upgrades from non semver versions like "org.springframework.cloud:spring-cloud-dependencies:Camden.SR5" - if (!Semver.isVersion(finalVersion) && !versions.isEmpty()) { - versions.sort(versionComparator); - return versions.get(versions.size() - 1); - } - return versionComparator.upgrade(finalVersion, versions).orElse(null); - } catch (IllegalStateException e) { - // this can happen when we encounter exotic versions - return null; + String finalVersion = !Semver.isVersion(version) ? "0.0.0" : version; + + // in the case of "latest.patch", a new version can only be derived if the + // current version is a semantic version + if (versionComparator instanceof LatestPatch && !versionComparator.isValid(finalVersion, finalVersion)) { + return null; + } + + try { + MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, download); + List versions = new ArrayList<>(); + for (String v : mavenMetadata.getVersioning().getVersions()) { + if (versionComparator.isValid(finalVersion, v)) { + versions.add(v); } } - }; + // handle upgrades from non semver versions like "org.springframework.cloud:spring-cloud-dependencies:Camden.SR5" + if (!Semver.isVersion(finalVersion) && !versions.isEmpty()) { + versions.sort(versionComparator); + return versions.get(versions.size() - 1); + } + return versionComparator.upgrade(finalVersion, versions).orElse(null); + } catch (IllegalStateException e) { + // this can happen when we encounter exotic versions + return null; + } + } + + private static Optional getPomDeclaringProperty( + @Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName) { + if (currentMavenResolutionResult == null) { + return Optional.empty(); + } + Pom pom = currentMavenResolutionResult.getPom().getRequested(); + if (pom.getProperties().containsKey(propertyName)) { + return Optional.of(pom); + } + return getPomDeclaringProperty(currentMavenResolutionResult.getParent(), propertyName); + } + + @Value + public static class Accumulator { + Set projectArtifacts = new HashSet<>(); + Set pomProperties = new HashSet<>(); + } + + @Value + public static class PomProperty { + Path pomFilePath; + String property; } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 88eff79aa10..31835aa4cd2 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -796,6 +796,59 @@ void upgradeDependencyOnlyTargetsSpecificDependencyProperty() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/4193") + @Test + void upgradeVersionDefinedViaPropertyInLocalParent() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("com.google.guava", "guava", "14.0", "", false, null)), + pomXml( + """ + + com.mycompany + my-parent + 1 + + 13.0 + + + """, + """ + + com.mycompany + my-parent + 1 + + 14.0 + + + """ + ), + mavenProject("my-child", + pomXml( + """ + + + com.mycompany + my-parent + 1 + + com.mycompany + my-child + 1 + + + com.google.guava + guava + ${guava.version} + + + + """ + ) + ) + ); + } + @Test void upgradeBomImport() { rewriteRun( From 896f1b29ed8ecbfff962c6ca5e46905ae8e2872b Mon Sep 17 00:00:00 2001 From: Hoan Nguyen Date: Fri, 24 May 2024 16:50:05 -0700 Subject: [PATCH 02/16] Make `VersionComparator versionComparator` field to reduce the number of instantiations. --- .../maven/UpgradeDependencyVersion.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index a63d47dfc25..1f9b6d8df96 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -133,6 +133,9 @@ public Accumulator getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(Accumulator accumulator) { return new MavenIsoVisitor() { + private final VersionComparator versionComparator = + requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); + @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { ResolvedPom pom = getResolutionResult().getPom(); @@ -170,8 +173,10 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext executionConte } @Nullable - private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException { - return UpgradeDependencyVersion.this.findNewerVersion(version, ctx, () -> downloadMetadata(groupId, artifactId, ctx)); + private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) + throws MavenDownloadingException { + return UpgradeDependencyVersion.this.findNewerVersion( + version, ctx, () -> downloadMetadata(groupId, artifactId, ctx), versionComparator); } }; } @@ -180,6 +185,8 @@ private String findNewerVersion(String groupId, String artifactId, String versio public TreeVisitor getVisitor(Accumulator accumulator) { return new MavenIsoVisitor() { private final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); + private final VersionComparator versionComparator = + requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); @Override public Xml.Document visitDocument(final Xml.Document document, final ExecutionContext executionContext) { @@ -378,16 +385,18 @@ public TreeVisitor upgradeVersion(ExecutionContext ctx, X } @Nullable - private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException { - return UpgradeDependencyVersion.this.findNewerVersion(version, ctx, () -> downloadMetadata(groupId, artifactId, ctx)); + private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) + throws MavenDownloadingException { + return UpgradeDependencyVersion.this.findNewerVersion( + version, ctx, () -> downloadMetadata(groupId, artifactId, ctx), versionComparator); } }; } @Nullable - private String findNewerVersion(String version, ExecutionContext ctx, MavenMetadataFailures.MavenMetadataDownloader download) throws MavenDownloadingException { - VersionComparator versionComparator = requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); - + private String findNewerVersion( + String version, ExecutionContext ctx, MavenMetadataFailures.MavenMetadataDownloader download, + VersionComparator versionComparator) throws MavenDownloadingException { String finalVersion = !Semver.isVersion(version) ? "0.0.0" : version; // in the case of "latest.patch", a new version can only be derived if the From 3b387bc9ef920c92b1499bacf9454c0e84dfbd3b Mon Sep 17 00:00:00 2001 From: Hoan Nguyen Date: Sat, 25 May 2024 13:46:17 -0700 Subject: [PATCH 03/16] Remove debugging code. --- .../java/org/openrewrite/maven/UpgradeDependencyVersion.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 1f9b6d8df96..28513854c65 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -188,11 +188,6 @@ public TreeVisitor getVisitor(Accumulator accumulator) { private final VersionComparator versionComparator = requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); - @Override - public Xml.Document visitDocument(final Xml.Document document, final ExecutionContext executionContext) { - return super.visitDocument(document, executionContext); - } - @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); From 89bb8d5471ef9831cb78f58350e3a8b9d822caad Mon Sep 17 00:00:00 2001 From: Hoan Nguyen Date: Mon, 27 May 2024 09:33:49 -0700 Subject: [PATCH 04/16] Avoid using Java stream. --- .../maven/UpgradeDependencyVersion.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 28513854c65..99f380c0d7e 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -193,15 +193,16 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); try { if (isPropertyTag()) { - accumulator.pomProperties.stream() - .filter(pomProperty -> pomProperty.pomFilePath.equals(getResolutionResult().getPom().getRequested().getSourcePath())) - .map(PomProperty::getProperty) - .filter(propertyName -> propertyName.equals(tag.getName()) - && !tag.getValue().map(newVersion::equals).orElse(false)) - .forEach(propertyName -> { + if (!tag.getValue().map(newVersion::equals).orElse(false)) { + Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath(); + for (PomProperty pomProperty : accumulator.pomProperties) { + if (pomProperty.pomFilePath.equals(pomSourcePath) + && pomProperty.property.equals(tag.getName())) { doAfterVisit(new ChangeTagValueVisitor<>(tag, newVersion)); maybeUpdateModel(); - }); + } + } + } } else if (isDependencyTag(groupId, artifactId)) { t = upgradeDependency(ctx, t); } else if (isManagedDependencyTag(groupId, artifactId)) { From 629a6eebf378e8c38b79c1d0c2000732a60398c1 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Jun 2024 15:44:59 +0200 Subject: [PATCH 05/16] Add tests to verify behavior with remote parent property --- .../maven/UpgradeDependencyVersionTest.java | 182 ++++++++++++++---- 1 file changed, 142 insertions(+), 40 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 31835aa4cd2..206e1ba3a6d 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -16,6 +16,7 @@ package org.openrewrite.maven; import com.google.common.collect.Lists; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -37,7 +38,6 @@ class UpgradeDependencyVersionTest implements RewriteTest { - @DocumentExample @Test void doNotOverrideImplicitProperty() { rewriteRun( @@ -133,6 +133,7 @@ void doNotOverrideImplicitProperty() { ); } + @DocumentExample @Test void updateManagedDependencyVersion() { rewriteRun( @@ -576,11 +577,11 @@ void upgradePluginDependenciesOnProperty() { com.mycompany.app my-app 1 - + 4.33.0 - + @@ -604,11 +605,11 @@ void upgradePluginDependenciesOnProperty() { com.mycompany.app my-app 1 - + 4.33.2 - + @@ -849,6 +850,107 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/4193") + @Test + void upgradeVersionDefinedViaImplicitPropertyInRemoteParent() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)), + pomXml( + """ + + + org.springframework.boot + spring-boot-dependencies + 3.3.0 + + com.mycompany + my-child + 1 + + + org.flywaydb + flyway-core + + + + """, + """ + + + org.springframework.boot + spring-boot-dependencies + 3.3.0 + + com.mycompany + my-child + 1 + + 10.15.0 + + + + org.flywaydb + flyway-core + + + + """ + ) + ); + } + + @Disabled("Anti-pattern not yet supported") + @Issue("https://github.com/openrewrite/rewrite/issues/4193") + @Test + void upgradeVersionDefinedViaExplicitPropertyInRemoteParent() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)), + pomXml( + """ + + + org.springframework.boot + spring-boot-dependencies + 3.3.0 + + com.mycompany + my-child + 1 + + + org.flywaydb + flyway-core + ${flyway.version} + + + + """, + """ + + + org.springframework.boot + spring-boot-dependencies + 3.3.0 + + com.mycompany + my-child + 1 + + 10.15.0 + + + + org.flywaydb + flyway-core + ${flyway.version} + + + + """ + ) + ); + } + @Test void upgradeBomImport() { rewriteRun( @@ -1080,7 +1182,7 @@ void dependencyManagementResolvedFromProperty() { } @ParameterizedTest - @ValueSource(strings = { "3.0.12.RELEASE", "=3.0.12.RELEASE" }) + @ValueSource(strings = {"3.0.12.RELEASE", "=3.0.12.RELEASE"}) @Issue("https://github.com/openrewrite/rewrite/issues/4166") void upgradeToExactVersion(String version) { rewriteRun( @@ -1255,11 +1357,11 @@ void removesRedundantExplicitVersionsMatchingOldImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1271,7 +1373,7 @@ void removesRedundantExplicitVersionsMatchingOldImport() { - + org.junit.jupiter @@ -1284,11 +1386,11 @@ void removesRedundantExplicitVersionsMatchingOldImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1300,7 +1402,7 @@ void removesRedundantExplicitVersionsMatchingOldImport() { - + org.junit.jupiter @@ -1321,11 +1423,11 @@ void removesRedundantExplicitVersionsMatchingNewImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1337,7 +1439,7 @@ void removesRedundantExplicitVersionsMatchingNewImport() { - + org.junit.jupiter @@ -1350,11 +1452,11 @@ void removesRedundantExplicitVersionsMatchingNewImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1366,7 +1468,7 @@ void removesRedundantExplicitVersionsMatchingNewImport() { - + org.junit.jupiter @@ -1387,11 +1489,11 @@ void keepsRedundantExplicitVersionsNotMatchingOldOrNewImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1403,7 +1505,7 @@ void keepsRedundantExplicitVersionsNotMatchingOldOrNewImport() { - + org.junit.jupiter @@ -1416,11 +1518,11 @@ void keepsRedundantExplicitVersionsNotMatchingOldOrNewImport() { """ 4.0.0 - + com.mycompany.app my-app 1 - + @@ -1432,7 +1534,7 @@ void keepsRedundantExplicitVersionsNotMatchingOldOrNewImport() { - + org.junit.jupiter @@ -1461,7 +1563,7 @@ void dependencyWithExplicitVersionRemovedFromDepMgmt() { org.sample sample 1.0.0 - + @@ -1473,7 +1575,7 @@ void dependencyWithExplicitVersionRemovedFromDepMgmt() { - + com.jcraft @@ -1490,7 +1592,7 @@ void dependencyWithExplicitVersionRemovedFromDepMgmt() { org.sample sample 1.0.0 - + @@ -1502,7 +1604,7 @@ void dependencyWithExplicitVersionRemovedFromDepMgmt() { - + com.jcraft @@ -1527,7 +1629,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmt() { org.sample sample 1.0.0 - + @@ -1539,7 +1641,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmt() { - + com.jcraft @@ -1555,7 +1657,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmt() { org.sample sample 1.0.0 - + @@ -1567,7 +1669,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmt() { - + com.jcraft @@ -1592,7 +1694,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmtRetainSpecificVersion() { org.sample sample 1.0.0 - + @@ -1604,7 +1706,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmtRetainSpecificVersion() { - + com.jcraft @@ -1620,7 +1722,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmtRetainSpecificVersion() { org.sample sample 1.0.0 - + @@ -1632,7 +1734,7 @@ void dependencyWithoutExplicitVersionRemovedFromDepMgmtRetainSpecificVersion() { - + com.jcraft @@ -1660,7 +1762,7 @@ void multipleRetainVersions() { org.sample sample 1.0.0 - + @@ -1672,7 +1774,7 @@ void multipleRetainVersions() { - + com.jcraft @@ -1692,7 +1794,7 @@ void multipleRetainVersions() { org.sample sample 1.0.0 - + @@ -1704,7 +1806,7 @@ void multipleRetainVersions() { - + com.jcraft From bf2043ef630f69fb4f80b0471e1ed8492556d43a Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Jun 2024 23:02:51 +0200 Subject: [PATCH 06/16] Rename variable as suggested --- .../org/openrewrite/maven/UpgradeDependencyVersion.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 3ccf1563b96..690f65c521f 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -144,13 +144,13 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { } @Override - public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext executionContext) { + public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { ResolvedDependency d = findDependency(tag); if (d != null && d.getRepository() != null) { // if the resolved dependency exists AND it does not represent an artifact that was parsed // as a source file, attempt to find a new version. try { - String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), executionContext); + String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx); if (newerVersion != null) { Optional version = tag.getChild("version"); if (version.isPresent()) { @@ -169,7 +169,7 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext executionConte return e.warn(tag); } } - return super.visitTag(tag, executionContext); + return super.visitTag(tag, ctx); } @Nullable From be80727a8707dcefb5495597e5b832a521161430 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Jun 2024 23:40:02 +0200 Subject: [PATCH 07/16] Inline single use of private method delegate --- .../maven/UpgradeDependencyVersion.java | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 690f65c521f..2b4de62dd22 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -150,7 +150,8 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { // if the resolved dependency exists AND it does not represent an artifact that was parsed // as a source file, attempt to find a new version. try { - String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx); + String newerVersion = findNewerVersion(d.getVersion(), + () -> downloadMetadata(d.getGroupId(), d.getArtifactId(), ctx), versionComparator, ctx); if (newerVersion != null) { Optional version = tag.getChild("version"); if (version.isPresent()) { @@ -172,12 +173,6 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { return super.visitTag(tag, ctx); } - @Nullable - private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) - throws MavenDownloadingException { - return UpgradeDependencyVersion.this.findNewerVersion( - version, ctx, () -> downloadMetadata(groupId, artifactId, ctx), versionComparator); - } }; } @@ -196,8 +191,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (!tag.getValue().map(newVersion::equals).orElse(false)) { Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath(); for (PomProperty pomProperty : accumulator.pomProperties) { - if (pomProperty.pomFilePath.equals(pomSourcePath) - && pomProperty.property.equals(tag.getName())) { + if (pomProperty.pomFilePath.equals(pomSourcePath) && + pomProperty.property.equals(tag.getName())) { doAfterVisit(new ChangeTagValueVisitor<>(tag, newVersion)); maybeUpdateModel(); } @@ -383,15 +378,14 @@ public TreeVisitor upgradeVersion(ExecutionContext ctx, X private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException { return UpgradeDependencyVersion.this.findNewerVersion( - version, ctx, () -> downloadMetadata(groupId, artifactId, ctx), versionComparator); + version, () -> downloadMetadata(groupId, artifactId, ctx), versionComparator, ctx); } }; } @Nullable private String findNewerVersion( - String version, ExecutionContext ctx, MavenMetadataFailures.MavenMetadataDownloader download, - VersionComparator versionComparator) throws MavenDownloadingException { + String version, MavenMetadataFailures.MavenMetadataDownloader download, VersionComparator versionComparator, ExecutionContext ctx) throws MavenDownloadingException { String finalVersion = !Semver.isVersion(version) ? "0.0.0" : version; // in the case of "latest.patch", a new version can only be derived if the From f59d8fb3950646785d85650e5d40cc1a9c11fe5f Mon Sep 17 00:00:00 2001 From: Hoan Nguyen Date: Thu, 20 Jun 2024 22:31:03 -0700 Subject: [PATCH 08/16] Filter out `null` value instead of throwing exception. Store resolved newer version and use it to update the property instead of using the version in the argument which could contain a dependency version selector pattern. --- .../openrewrite/maven/UpgradeDependencyVersion.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 2b4de62dd22..193f28ad475 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -160,8 +160,10 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { getPomDeclaringProperty(getResolutionResult(), propertyName) - .ifPresent(pom -> accumulator.pomProperties.add( - new PomProperty(requireNonNull(pom.getSourcePath()), propertyName))); + .map(Pom::getSourcePath) + .filter(Objects::nonNull) + .ifPresent(pomSourcePath -> accumulator.pomProperties.add( + new PomProperty(pomSourcePath, propertyName, newerVersion))); } } } @@ -192,8 +194,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath(); for (PomProperty pomProperty : accumulator.pomProperties) { if (pomProperty.pomFilePath.equals(pomSourcePath) && - pomProperty.property.equals(tag.getName())) { - doAfterVisit(new ChangeTagValueVisitor<>(tag, newVersion)); + pomProperty.propertyName.equals(tag.getName())) { + doAfterVisit(new ChangeTagValueVisitor<>(tag, pomProperty.propertyValue)); maybeUpdateModel(); } } @@ -435,6 +437,7 @@ public static class Accumulator { @Value public static class PomProperty { Path pomFilePath; - String property; + String propertyName; + String propertyValue; } } From 6b87c17092fb90c84bde5f785cc98a4cb533a6bf Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 23 Jun 2024 14:15:23 +0200 Subject: [PATCH 09/16] Update test to show we support version patterns too --- .../org/openrewrite/maven/UpgradeDependencyVersionTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 206e1ba3a6d..4e9b7ade62a 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -854,7 +854,7 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { @Test void upgradeVersionDefinedViaImplicitPropertyInRemoteParent() { rewriteRun( - spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)), + spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.x", "", true, null)), pomXml( """ @@ -904,7 +904,7 @@ void upgradeVersionDefinedViaImplicitPropertyInRemoteParent() { @Test void upgradeVersionDefinedViaExplicitPropertyInRemoteParent() { rewriteRun( - spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)), + spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.x", "", true, null)), pomXml( """ From 462061965323802f5f15ba0a9b94ac03ed4adb1b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 14:25:22 +0200 Subject: [PATCH 10/16] Limit property accumulation to matching dependencies --- .../src/main/java/org/openrewrite/maven/MavenVisitor.java | 3 ++- .../org/openrewrite/maven/UpgradeDependencyVersion.java | 6 ++++-- .../openrewrite/maven/UpgradeDependencyVersionTest.java | 8 ++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java index 89524e3494f..a98a07dae9f 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java @@ -266,7 +266,8 @@ public ResolvedDependency findDependency(Xml.Tag tag) { @Nullable public ResolvedManagedDependency findManagedDependency(Xml.Tag tag) { - String groupId = getResolutionResult().getPom().getValue(tag.getChildValue("groupId").orElse(getResolutionResult().getPom().getGroupId())); + String groupId = getResolutionResult().getPom().getValue(tag.getChildValue("groupId") + .orElse(getResolutionResult().getPom().getGroupId())); String artifactId = getResolutionResult().getPom().getValue(tag.getChildValue("artifactId").orElse("")); String classifier = getResolutionResult().getPom().getValue(tag.getChildValue("classifier").orElse(null)); if (groupId != null && artifactId != null) { diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 193f28ad475..9b2f764ed04 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -145,10 +145,12 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { @Override public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { - ResolvedDependency d = findDependency(tag); - if (d != null && d.getRepository() != null) { + if (isDependencyTag(groupId, artifactId) || + isManagedDependencyTag(groupId, artifactId) || + isPluginDependencyTag(groupId, artifactId)) { // if the resolved dependency exists AND it does not represent an artifact that was parsed // as a source file, attempt to find a new version. + ResolvedDependency d = findDependency(tag); try { String newerVersion = findNewerVersion(d.getVersion(), () -> downloadMetadata(d.getGroupId(), d.getArtifactId(), ctx), versionComparator, ctx); diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 4e9b7ade62a..145c21fad5a 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -810,6 +810,7 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { 1 13.0 + 4.13.2 """, @@ -820,6 +821,7 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { 1 14.0 + 4.13.2 """ @@ -842,6 +844,12 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { guava ${guava.version} + + junit + junit + ${junit.version} + test + """ From d645dc424a3011225c9fd12b79651bed728d1d96 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 15:10:33 +0200 Subject: [PATCH 11/16] Restore check that ResolvedDependency.getRepository() != null --- .../maven/UpgradeDependencyVersion.java | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 9b2f764ed04..707b3daa251 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -151,27 +151,29 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { // if the resolved dependency exists AND it does not represent an artifact that was parsed // as a source file, attempt to find a new version. ResolvedDependency d = findDependency(tag); - try { - String newerVersion = findNewerVersion(d.getVersion(), - () -> downloadMetadata(d.getGroupId(), d.getArtifactId(), ctx), versionComparator, ctx); - if (newerVersion != null) { - Optional version = tag.getChild("version"); - if (version.isPresent()) { - String requestedVersion = d.getRequested().getVersion(); - if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) { - String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); - if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { - getPomDeclaringProperty(getResolutionResult(), propertyName) - .map(Pom::getSourcePath) - .filter(Objects::nonNull) - .ifPresent(pomSourcePath -> accumulator.pomProperties.add( - new PomProperty(pomSourcePath, propertyName, newerVersion))); + if (d != null && d.getRepository() != null) { + try { + String newerVersion = findNewerVersion(d.getVersion(), + () -> downloadMetadata(d.getGroupId(), d.getArtifactId(), ctx), versionComparator, ctx); + if (newerVersion != null) { + Optional version = tag.getChild("version"); + if (version.isPresent()) { + String requestedVersion = d.getRequested().getVersion(); + if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) { + String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); + if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { + getPomDeclaringProperty(getResolutionResult(), propertyName) + .map(Pom::getSourcePath) + .filter(Objects::nonNull) + .ifPresent(pomSourcePath -> accumulator.pomProperties.add( + new PomProperty(pomSourcePath, propertyName, newerVersion))); + } } } } + } catch (MavenDownloadingException e) { + return e.warn(tag); } - } catch (MavenDownloadingException e) { - return e.warn(tag); } } return super.visitTag(tag, ctx); From 6cba60b0334beedbd298e8196ce3f14bc073f2f9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 15:52:40 +0200 Subject: [PATCH 12/16] Do not look at managed or plugin dependency version properties yet --- .../org/openrewrite/maven/UpgradeDependencyVersion.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 707b3daa251..a233371797e 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -145,13 +145,11 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { @Override public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { - if (isDependencyTag(groupId, artifactId) || - isManagedDependencyTag(groupId, artifactId) || - isPluginDependencyTag(groupId, artifactId)) { - // if the resolved dependency exists AND it does not represent an artifact that was parsed - // as a source file, attempt to find a new version. + if (isDependencyTag(groupId, artifactId)) { ResolvedDependency d = findDependency(tag); if (d != null && d.getRepository() != null) { + // if the resolved dependency exists AND it does not represent an artifact that was parsed + // as a source file, attempt to find a new version. try { String newerVersion = findNewerVersion(d.getVersion(), () -> downloadMetadata(d.getGroupId(), d.getArtifactId(), ctx), versionComparator, ctx); From d8807be222d0e73ba8b5c7bc323e894b02fa55da Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 15:53:06 +0200 Subject: [PATCH 13/16] Compare actual new version, not version pattern --- .../openrewrite/maven/UpgradeDependencyVersion.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index a233371797e..6dcfaf8b1ef 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -192,11 +192,12 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); try { if (isPropertyTag()) { - if (!tag.getValue().map(newVersion::equals).orElse(false)) { - Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath(); - for (PomProperty pomProperty : accumulator.pomProperties) { - if (pomProperty.pomFilePath.equals(pomSourcePath) && - pomProperty.propertyName.equals(tag.getName())) { + Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath(); + for (PomProperty pomProperty : accumulator.pomProperties) { + if (pomProperty.pomFilePath.equals(pomSourcePath) && + pomProperty.propertyName.equals(tag.getName())) { + Optional value = tag.getValue(); + if (!value.isPresent() || !value.get().equals(pomProperty.propertyValue)) { doAfterVisit(new ChangeTagValueVisitor<>(tag, pomProperty.propertyValue)); maybeUpdateModel(); } From d9b0e979fbf21138d77ac0e76647eb7da76b2ed3 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 16:51:18 +0200 Subject: [PATCH 14/16] Break after finding the matching property --- .../java/org/openrewrite/maven/UpgradeDependencyVersion.java | 1 + 1 file changed, 1 insertion(+) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 6dcfaf8b1ef..f202d77fa49 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -201,6 +201,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { doAfterVisit(new ChangeTagValueVisitor<>(tag, pomProperty.propertyValue)); maybeUpdateModel(); } + break; } } } else if (isDependencyTag(groupId, artifactId)) { From c0b2d1566e3756621544936de0447cfa67de03b4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 17:50:57 +0200 Subject: [PATCH 15/16] Pull up `getPomDeclaringProperty` closer to usage --- .../maven/UpgradeDependencyVersion.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index f202d77fa49..ab5055e0ab6 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -177,6 +177,17 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { return super.visitTag(tag, ctx); } + private Optional getPomDeclaringProperty( + @Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName) { + if (currentMavenResolutionResult == null) { + return Optional.empty(); + } + Pom pom = currentMavenResolutionResult.getPom().getRequested(); + if (pom.getProperties().containsKey(propertyName)) { + return Optional.of(pom); + } + return getPomDeclaringProperty(currentMavenResolutionResult.getParent(), propertyName); + } }; } @@ -420,18 +431,6 @@ private String findNewerVersion( } } - private static Optional getPomDeclaringProperty( - @Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName) { - if (currentMavenResolutionResult == null) { - return Optional.empty(); - } - Pom pom = currentMavenResolutionResult.getPom().getRequested(); - if (pom.getProperties().containsKey(propertyName)) { - return Optional.of(pom); - } - return getPomDeclaringProperty(currentMavenResolutionResult.getParent(), propertyName); - } - @Value public static class Accumulator { Set projectArtifacts = new HashSet<>(); From e0bd56922156252ce36abb1dfd9f4f36a8ad5d88 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Jun 2024 18:11:50 +0200 Subject: [PATCH 16/16] Inline & document parent pom handling into `storeParentPomProperty` --- .../maven/UpgradeDependencyVersion.java | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index ab5055e0ab6..6c99dc6f11a 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -157,14 +157,11 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { Optional version = tag.getChild("version"); if (version.isPresent()) { String requestedVersion = d.getRequested().getVersion(); - if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) { + if (requestedVersion != null && requestedVersion.startsWith("${") && + !implicitlyDefinedVersionProperties.contains(requestedVersion)) { String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1); if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) { - getPomDeclaringProperty(getResolutionResult(), propertyName) - .map(Pom::getSourcePath) - .filter(Objects::nonNull) - .ifPresent(pomSourcePath -> accumulator.pomProperties.add( - new PomProperty(pomSourcePath, propertyName, newerVersion))); + storeParentPomProperty(getResolutionResult().getParent(), propertyName, newerVersion); } } } @@ -177,16 +174,27 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) { return super.visitTag(tag, ctx); } - private Optional getPomDeclaringProperty( - @Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName) { + /** + * Recursively look for a parent POM that's still part of the sources, which contains the version property. + * If found, store the property in the accumulator, such that we can update that source file later. + * @param currentMavenResolutionResult the current Maven resolution result parent to search for the property + * @param propertyName the name of the property to update, if found in any the parent pom source file + * @param newerVersion the resolved newer version that any matching parent pom property should be updated to + */ + private void storeParentPomProperty( + @Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName, String newerVersion) { if (currentMavenResolutionResult == null) { - return Optional.empty(); + return; // No parent contained the property; might then be in the same source file, or an import BOM } Pom pom = currentMavenResolutionResult.getPom().getRequested(); + if (pom.getSourcePath() == null) { + return; // Not a source file, so nothing to update + } if (pom.getProperties().containsKey(propertyName)) { - return Optional.of(pom); + accumulator.pomProperties.add(new PomProperty(pom.getSourcePath(), propertyName, newerVersion)); + return; // Property found, so no further searching is needed } - return getPomDeclaringProperty(currentMavenResolutionResult.getParent(), propertyName); + storeParentPomProperty(currentMavenResolutionResult.getParent(), propertyName, newerVersion); } }; }