diff --git a/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java b/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java index ef8f53ad..598cad70 100644 --- a/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java +++ b/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java @@ -58,6 +58,8 @@ public class RemoveRedundantDependencies extends ScanningRecipe scope/configuration -> Set of transitive dependencies Map>> transitivesByProjectAndScope; + // Cache of each coordinate's own clean dependency closure, shared across all source files in the run + Map> closureCache; } @Value @@ -68,7 +70,7 @@ public static class TransitiveDependency { @Override public Accumulator getInitialValue(ExecutionContext ctx) { - return new Accumulator(new HashMap<>()); + return new Accumulator(new HashMap<>(), new HashMap<>()); } @Override @@ -145,24 +147,17 @@ private void resolveTransitivesFromPom( MavenPomDownloader downloader, ExecutionContext ctx, Set transitives) { + List effectiveRepos = withMavenCentral(repositories); try { - // Ensure we have Maven Central in the repositories - List effectiveRepos = new ArrayList<>(repositories); - if (effectiveRepos.stream().noneMatch(r -> r.getUri().contains("repo.maven.apache.org") || - r.getUri().contains("repo1.maven.org"))) { - effectiveRepos.add(MavenRepository.MAVEN_CENTRAL); - } - // Get the resolved dependencies for compile scope (which includes most transitives) - Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, effectiveRepos); - ResolvedPom resolvedPom = pom.resolve(emptyList(), downloader, effectiveRepos, ctx); + ResolvedPom resolvedPom = resolvePom(gav, effectiveRepos, downloader, ctx); ResolvedPom patchedPom = applyExclusions(resolvedPom, effectiveExclusions); List resolved = patchedPom.resolveDependencies(Scope.Compile, downloader, ctx); // Collect all dependencies (both direct and transitive of the parent) Set visited = new HashSet<>(); for (ResolvedDependency dep : resolved) { - collectAllDependencies(dep, transitives, visited); + collectAllDependencies(dep, transitives, visited, effectiveRepos, downloader, ctx); } } catch (MavenDownloadingException | MavenDownloadingExceptions e) { // If we can't download/resolve the POM, fall back to not detecting redundancies @@ -180,11 +175,65 @@ private ResolvedPom applyExclusions(ResolvedPom resolvedPom, List } private void collectAllDependencies(ResolvedDependency dep, Set transitives, - Set visited) { + Set visited, List repositories, + MavenPomDownloader downloader, ExecutionContext ctx) { if (visited.add(dep.getGav())) { - transitives.add(new TransitiveDependency(dep.getGav(), new HashSet<>(dep.getEffectiveExclusions()))); + transitives.add(new TransitiveDependency(dep.getGav(), + relevantExclusions(dep, repositories, downloader, ctx))); + for (ResolvedDependency transitive : dep.getDependencies()) { + collectAllDependencies(transitive, transitives, visited, repositories, downloader, ctx); + } + } + } + + // Effective exclusions are resolved in the parent's whole tree, so they pick up no-op exclusions + // from sibling branches; keep only those targeting this coordinate's own dependency closure. + private Set relevantExclusions(ResolvedDependency dep, List repositories, + MavenPomDownloader downloader, ExecutionContext ctx) { + Set exclusions = new HashSet<>(dep.getEffectiveExclusions()); + if (!exclusions.isEmpty()) { + exclusions.retainAll(dependencyClosure(dep.getGav(), repositories, downloader, ctx)); + } + return exclusions; + } + + private Set dependencyClosure(ResolvedGroupArtifactVersion gav, List repositories, + MavenPomDownloader downloader, ExecutionContext ctx) { + return acc.closureCache.computeIfAbsent(gav, g -> { + Set closure = new HashSet<>(); + try { + for (ResolvedDependency d : resolvePom(g, repositories, downloader, ctx) + .resolveDependencies(Scope.Compile, downloader, ctx)) { + collectClosure(d, closure); + } + } catch (MavenDownloadingException | MavenDownloadingExceptions e) { + // Best-effort: an unresolvable closure leaves the exclusions unfiltered + } + return closure; + }); + } + + private ResolvedPom resolvePom(ResolvedGroupArtifactVersion gav, List repositories, + MavenPomDownloader downloader, ExecutionContext ctx) + throws MavenDownloadingException, MavenDownloadingExceptions { + List repos = withMavenCentral(repositories); + Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, repos); + return pom.resolve(emptyList(), downloader, repos, ctx); + } + + private List withMavenCentral(List repositories) { + List effectiveRepos = new ArrayList<>(repositories); + if (effectiveRepos.stream().noneMatch(r -> r.getUri().contains("repo.maven.apache.org") || + r.getUri().contains("repo1.maven.org"))) { + effectiveRepos.add(MavenRepository.MAVEN_CENTRAL); + } + return effectiveRepos; + } + + private void collectClosure(ResolvedDependency dep, Set closure) { + if (closure.add(dep.getGav().asGroupArtifact())) { for (ResolvedDependency transitive : dep.getDependencies()) { - collectAllDependencies(transitive, transitives, visited); + collectClosure(transitive, closure); } } } diff --git a/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java b/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java index f163f7da..084e035f 100644 --- a/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java +++ b/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java @@ -495,6 +495,168 @@ void keepsDirectCompileTomcatEmbedCoreWhenProviderIsProvidedScoped() { ); } + @Test + void removesJakartaClientWhenTransitiveExclusionsAreEquivalent() { + // In the starter's tree the transitive jakarta.ws.rs-api gains a spurious exclusion of + // jakarta.activation-api (which it can never bring on its own); the direct declaration has none, + // so the two are effectively equivalent and the direct one is redundant. + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "org.springframework.boot", "spring-boot-starter-*")), + //language=xml + pomXml( + """ + + 4.0.0 + com.sample + sample + 1.0-SNAPSHOT + + org.springframework.boot + spring-boot-starter-parent + 3.2.3 + + + + + org.springframework.boot + spring-boot-starter-jersey + + + jakarta.ws.rs + jakarta.ws.rs-api + 3.1.0 + + + + """, + """ + + 4.0.0 + com.sample + sample + 1.0-SNAPSHOT + + org.springframework.boot + spring-boot-starter-parent + 3.2.3 + + + + + org.springframework.boot + spring-boot-starter-jersey + + + + """ + ) + ); + } + + @Test + void keepsJerseyClientWhenDirectExclusionsDifferFromTransitive() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "org.springframework.boot", "spring-boot-starter-*")), + //language=xml + pomXml( + """ + + 4.0.0 + com.sample + sample + 1.0-SNAPSHOT + + org.springframework.boot + spring-boot-starter-parent + 3.2.3 + + + + + org.springframework.boot + spring-boot-starter-jersey + + + org.glassfish.jersey.core + jersey-client + 3.1.5 + + + jakarta.inject + jakarta.inject-api + + + + + + """ + ) + ); + } + + @Test + void removesTomcatEmbedCoreWhenExclusionsMatchTransitive() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "org.springframework.boot", "spring-boot-starter-*")), + //language=xml + pomXml( + """ + + 4.0.0 + com.sample + sample + 1.0-SNAPSHOT + + org.springframework.boot + spring-boot-starter-parent + 3.2.3 + + + + + org.springframework.boot + spring-boot-starter-web + + + org.apache.tomcat.embed + tomcat-embed-core + + + org.apache.tomcat + tomcat-annotations-api + + + + + + """, + """ + + 4.0.0 + com.sample + sample + 1.0-SNAPSHOT + + org.springframework.boot + spring-boot-starter-parent + 3.2.3 + + + + + org.springframework.boot + spring-boot-starter-web + + + + """ + ) + ); + } + @Test void removeRedundantGradleDependency() { rewriteRun(