Replaced early exit logic for versions using property values with pro…

…perty resolution, allowing dependencies/plugins using redundant property versions to correctly be removed. (#4236)
openrewrite · Jun 7, 2024 · 28e7b12 · 28e7b12
1 parent da30d7e
commit 28e7b12
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 5 deletions.
diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveRedundantDependencyVersions.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveRedundantDependencyVersions.java
@@ -229,7 +229,7 @@ private boolean matchesArtifact(Plugin p) {
              */
             private boolean matchesVersion(ResolvedManagedDependency d, ExecutionContext ctx) {
                 MavenResolutionResult mrr = getResolutionResult();
-                if (d.getRequested().getVersion() == null || d.getRequested().getVersion().contains("${") || mrr.getPom().getRequested().getParent() == null) {
+                if (d.getRequested().getVersion() == null || mrr.getPom().getRequested().getParent() == null) {
                     return false;
                 }
                 try {
@@ -255,7 +255,7 @@ private boolean matchesVersion(ResolvedManagedDependency d, ExecutionContext ctx
             }
 
             private boolean matchesVersion(ResolvedDependency d) {
-                if (d.getRequested().getVersion() == null || d.getRequested().getVersion().contains("${")) {
+                if (d.getRequested().getVersion() == null) {
                     return false;
                 }
                 String managedVersion = getResolutionResult().getPom().getManagedVersion(d.getGroupId(),
@@ -264,7 +264,7 @@ private boolean matchesVersion(ResolvedDependency d) {
             }
 
             private boolean matchesVersion(Plugin p) {
-                if (p.getVersion() == null || p.getVersion().contains("${")) {
+                if (p.getVersion() == null) {
                     return false;
                 }
                 String managedVersion = getManagedPluginVersion(getResolutionResult().getPom(), p.getGroupId(), p.getArtifactId());
@@ -279,7 +279,8 @@ private boolean matchesComparator(@Nullable String managedVersion, String reques
                     return true;
                 }
                 int comparison = new LatestIntegration(null)
-                        .compare(null, managedVersion, requestedVersion);
+                        .compare(null, managedVersion,
+                                Objects.requireNonNull(getResolutionResult().getPom().getValue(requestedVersion)));
                 if (comparison < 0) {
                     return comparator.equals(Comparator.LT) || comparator.equals(Comparator.LTE);
                 } else if (comparison > 0) {

diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveRedundantDependencyVersionsTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveRedundantDependencyVersionsTest.java
@@ -1293,7 +1293,7 @@ void allowUpgradeOfManagedVersion() {
     }
 
     @Test
-    void propertySubstitution() {
+    void pluginPropertySubstitution() {
         rewriteRun(
           spec -> spec.recipe(new RemoveRedundantDependencyVersions(null, null, RemoveRedundantDependencyVersions.Comparator.GTE, null)),
           pomXml(
@@ -1321,6 +1321,84 @@ void propertySubstitution() {
                     </plugins>
                 </build>
             </project>
+            """, """
+            <project>
+                <parent>
+                    <groupId>org.springframework.boot</groupId>
+                    <artifactId>spring-boot-starter-parent</artifactId>
+                    <version>2.3.0.RELEASE</version>
+                    <relativePath/>
+                </parent>
+            
+                <groupId>com.example</groupId>
+                <artifactId>test</artifactId>
+                <version>1.0.0-SNAPSHOT</version>
+            
+                <modelVersion>4.0.0</modelVersion>
+                <build>
+                    <plugins>
+                        <plugin>
+                            <artifactId>kotlin-maven-plugin</artifactId>
+                            <groupId>org.jetbrains.kotlin</groupId>
+                        </plugin>
+                    </plugins>
+                </build>
+            </project>
+            """
+          )
+        );
+    }
+
+    @Test
+    void dependencyPropertySubstitution() {
+        rewriteRun(
+          spec -> spec.recipe(new RemoveRedundantDependencyVersions(null, null, RemoveRedundantDependencyVersions.Comparator.GTE, null)),
+          pomXml(
+            """
+            <project>
+                <parent>
+                    <groupId>org.springframework.boot</groupId>
+                    <artifactId>spring-boot-starter-parent</artifactId>
+                    <version>2.5.4</version>
+                    <relativePath/>
+                </parent>
+            
+                <groupId>com.example</groupId>
+                <artifactId>test</artifactId>
+                <version>1.0.0-SNAPSHOT</version>
+            
+                <modelVersion>4.0.0</modelVersion>
+                
+                <dependencies>
+                    <dependency>
+                        <groupId>org.springframework</groupId>
+                        <artifactId>spring-web</artifactId>
+                        <version>${spring-framework.version}</version>
+                    </dependency>
+                </dependencies>
+            </project>
+            """, """
+            <project>
+                <parent>
+                    <groupId>org.springframework.boot</groupId>
+                    <artifactId>spring-boot-starter-parent</artifactId>
+                    <version>2.5.4</version>
+                    <relativePath/>
+                </parent>
+            
+                <groupId>com.example</groupId>
+                <artifactId>test</artifactId>
+                <version>1.0.0-SNAPSHOT</version>
+            
+                <modelVersion>4.0.0</modelVersion>
+                
+                <dependencies>
+                    <dependency>
+                        <groupId>org.springframework</groupId>
+                        <artifactId>spring-web</artifactId>
+                    </dependency>
+                </dependencies>
+            </project>
             """
           )
         );