diff --git a/src/main/java/org/openrewrite/java/dependencies/RemoveDependency.java b/src/main/java/org/openrewrite/java/dependencies/RemoveDependency.java index bc28a50e..ee67bf53 100644 --- a/src/main/java/org/openrewrite/java/dependencies/RemoveDependency.java +++ b/src/main/java/org/openrewrite/java/dependencies/RemoveDependency.java @@ -21,13 +21,18 @@ import org.openrewrite.*; import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.search.UsesType; +import org.openrewrite.maven.tree.Dependency; +import org.openrewrite.maven.tree.MavenResolutionResult; import java.util.HashMap; import java.util.Map; +import java.util.UUID; + +import static org.openrewrite.internal.StringUtils.matchesGlob; @EqualsAndHashCode(callSuper = false) @Value -public class RemoveDependency extends ScanningRecipe> { +public class RemoveDependency extends ScanningRecipe { @Option(displayName = "Group ID", description = "The first part of a dependency coordinate `com.google.guava:guava:VERSION`. This can be a glob expression.", example = "com.fasterxml.jackson*") @@ -68,13 +73,18 @@ public class RemoveDependency extends ScanningRecipe> String description = "For Gradle project, removes a single dependency from the dependencies section of the `build.gradle`.\n" + "For Maven project, removes a single dependency from the `` section of the pom.xml."; + public static class Accumulator { + final Map projectToInUse = new HashMap<>(); + final Map mavenIdToProject = new HashMap<>(); + } + @Override - public Map getInitialValue(ExecutionContext ctx) { - return new HashMap<>(); + public Accumulator getInitialValue(ExecutionContext ctx) { + return new Accumulator(); } @Override - public TreeVisitor getScanner(Map projectToInUse) { + public TreeVisitor getScanner(Accumulator acc) { if (unlessUsing == null) { return TreeVisitor.noop(); } @@ -83,15 +93,19 @@ public TreeVisitor getScanner(Map pro @Override public Tree preVisit(Tree tree, ExecutionContext ctx) { stopAfterPreVisit(); - tree.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> - projectToInUse.compute(javaProject, (jp, foundSoFar) -> Boolean.TRUE.equals(foundSoFar) || tree != usesType.visit(tree, ctx))); + tree.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> { + acc.projectToInUse.compute(javaProject, (jp, foundSoFar) -> + Boolean.TRUE.equals(foundSoFar) || tree != usesType.visit(tree, ctx)); + tree.getMarkers().findFirst(MavenResolutionResult.class).ifPresent(mrr -> + acc.mavenIdToProject.put(mrr.getId(), javaProject)); + }); return tree; } }; } @Override - public TreeVisitor getVisitor(Map projectToInUse) { + public TreeVisitor getVisitor(Accumulator acc) { return new TreeVisitor() { final TreeVisitor gradleRemoveDep = new org.openrewrite.gradle.RemoveDependency(groupId, artifactId, configuration).getVisitor(); final TreeVisitor mavenRemoveDep = new org.openrewrite.maven.RemoveDependency(groupId, artifactId, scope).getVisitor(); @@ -103,7 +117,11 @@ public TreeVisitor getVisitor(Map pro } if (unlessUsing != null) { JavaProject jp = tree.getMarkers().findFirst(JavaProject.class).orElse(null); - if (jp == null || Boolean.TRUE.equals(projectToInUse.get(jp))) { + if (jp == null || Boolean.TRUE.equals(acc.projectToInUse.get(jp))) { + return tree; + } + MavenResolutionResult mrr = tree.getMarkers().findFirst(MavenResolutionResult.class).orElse(null); + if (mrr != null && anyDescendantPreservesDependency(mrr, acc)) { return tree; } } @@ -118,4 +136,35 @@ public TreeVisitor getVisitor(Map pro } }; } + + /** + * Keep this pom's `` declaration if any descendant module in the reactor uses the + * `unlessUsing` type AND does not also declare the dependency itself. A descendant that + * self-declares the dependency in its own pom is unaffected by removing it from an ancestor, + * so it should not block removal. + */ + private boolean anyDescendantPreservesDependency(MavenResolutionResult mrr, Accumulator acc) { + for (MavenResolutionResult child : mrr.getModules()) { + JavaProject childJp = acc.mavenIdToProject.get(child.getId()); + if (childJp != null && Boolean.TRUE.equals(acc.projectToInUse.get(childJp)) && !declaresDependency(child)) { + return true; + } + if (anyDescendantPreservesDependency(child, acc)) { + return true; + } + } + return false; + } + + private boolean declaresDependency(MavenResolutionResult mrr) { + // Look at the raw `requested` pom — the as-written `` of this module only, + // excluding inheritance from a parent pom. A module that inherits the dep from the parent + // we may be modifying should still block removal. + for (Dependency d : mrr.getPom().getRequested().getDependencies()) { + if (matchesGlob(d.getGroupId(), groupId) && matchesGlob(d.getArtifactId(), artifactId)) { + return true; + } + } + return false; + } } diff --git a/src/test/java/org/openrewrite/java/dependencies/RemoveDependencyTest.java b/src/test/java/org/openrewrite/java/dependencies/RemoveDependencyTest.java index 5482189d..d4858119 100644 --- a/src/test/java/org/openrewrite/java/dependencies/RemoveDependencyTest.java +++ b/src/test/java/org/openrewrite/java/dependencies/RemoveDependencyTest.java @@ -177,6 +177,347 @@ class MyLoggingInterceptor { ); } + @Test + void doNotRemoveSpringRetryWhenRetryTemplateInUse() { + rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + package org.springframework.retry.support; + public class RetryTemplate { + } + """, + """ + package org.springframework.retry.policy; + public class SimpleRetryPolicy { + } + """, + """ + package org.springframework.retry.backoff; + public class ExponentialBackOffPolicy { + } + """ + )) + .recipe(new RemoveDependency("org.springframework.retry", "spring-retry", "org.springframework.retry..*", null, null)), + mavenProject("example", + //language=java + srcMainJava( + java( + """ + import org.springframework.retry.support.RetryTemplate; + import org.springframework.retry.policy.SimpleRetryPolicy; + import org.springframework.retry.backoff.ExponentialBackOffPolicy; + + class RetryConfig { + RetryTemplate retryTemplate() { + new SimpleRetryPolicy(); + new ExponentialBackOffPolicy(); + return new RetryTemplate(); + } + } + """ + ) + ), + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + org.springframework.retry + spring-retry + 2.0.0 + + + + """ + ) + ) + ); + } + + @Test + void doNotRemoveSpringRetryWhenOnlyImportsPresent() { + rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + package org.springframework.retry.support; + public class RetryTemplate { + } + """ + )) + .recipe(new RemoveDependency("org.springframework.retry", "spring-retry", "org.springframework.retry..*", null, null)), + mavenProject("example", + //language=java + srcMainJava( + java( + """ + import org.springframework.retry.support.RetryTemplate; + + class RetryConfig { + // imports but no body usage — should still keep the dep + } + """ + ) + ), + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + org.springframework.retry + spring-retry + 2.0.0 + + + + """ + ) + ) + ); + } + + @Test + void doNotRemoveSpringRetryInMultiModuleWhenChildUsesIt() { + rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + package org.springframework.retry.support; + public class RetryTemplate { + } + """ + )) + .recipe(new RemoveDependency("org.springframework.retry", "spring-retry", "org.springframework.retry..*", null, null)), + mavenProject("parent", + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + parent + 1 + pom + + + org.springframework.retry + spring-retry + 2.0.0 + + + + child + + + """ + ), + mavenProject("child", + //language=xml + pomXml( + """ + + 4.0.0 + + com.mycompany.app + parent + 1 + + child + + """ + ), + //language=java + srcMainJava( + java( + """ + import org.springframework.retry.support.RetryTemplate; + + class RetryConfig { + RetryTemplate retryTemplate() { + return new RetryTemplate(); + } + } + """ + ) + ) + ) + ) + ); + } + + @Test + void doNotRemoveSpringRetryWhenDepAndUsageInSameChildModule() { + rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + package org.springframework.retry.support; + public class RetryTemplate { + } + """ + )) + .recipe(new RemoveDependency("org.springframework.retry", "spring-retry", "org.springframework.retry..*", null, null)), + mavenProject("parent", + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + parent + 1 + pom + + child + + + """ + ), + mavenProject("child", + //language=xml + pomXml( + """ + + 4.0.0 + + com.mycompany.app + parent + 1 + + child + + + org.springframework.retry + spring-retry + 2.0.0 + + + + """ + ), + //language=java + srcMainJava( + java( + """ + import org.springframework.retry.support.RetryTemplate; + + class RetryConfig { + RetryTemplate retryTemplate() { + return new RetryTemplate(); + } + } + """ + ) + ) + ) + ) + ); + } + + @Test + void removeParentDependencyWhenChildSelfDeclaresAndUses() { + rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + package org.springframework.retry.support; + public class RetryTemplate { + } + """ + )) + .recipe(new RemoveDependency("org.springframework.retry", "spring-retry", "org.springframework.retry..*", null, null)), + mavenProject("parent", + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + parent + 1 + pom + + + org.springframework.retry + spring-retry + 2.0.0 + + + + child + + + """, + """ + + 4.0.0 + com.mycompany.app + parent + 1 + pom + + child + + + """ + ), + mavenProject("child", + //language=xml + pomXml( + """ + + 4.0.0 + + com.mycompany.app + parent + 1 + + child + + + org.springframework.retry + spring-retry + 2.0.0 + + + + """ + ), + //language=java + srcMainJava( + java( + """ + import org.springframework.retry.support.RetryTemplate; + + class RetryConfig { + RetryTemplate retryTemplate() { + return new RetryTemplate(); + } + } + """ + ) + ) + ) + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/11") @Test void doRemoveIfNotInUse() {