Skip to content

Commit e5cc17a

Browse files
committed
Simplify redundant-dependency exclusion filtering
Share the POM download/resolve path between resolveTransitivesFromPom and dependencyClosure via resolvePom/withMavenCentral helpers, and hoist the closure cache onto the Accumulator so it survives across source files in a multi-module build.
1 parent 6e336a7 commit e5cc17a

2 files changed

Lines changed: 30 additions & 24 deletions

File tree

src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public class RemoveRedundantDependencies extends ScanningRecipe<RemoveRedundantD
5858
public static class Accumulator {
5959
// Map from project identifier -> scope/configuration -> Set of transitive dependencies
6060
Map<String, Map<String, Set<TransitiveDependency>>> transitivesByProjectAndScope;
61+
// Cache of each coordinate's own clean dependency closure, shared across all source files in the run
62+
Map<ResolvedGroupArtifactVersion, Set<GroupArtifact>> closureCache;
6163
}
6264

6365
@Value
@@ -68,14 +70,12 @@ public static class TransitiveDependency {
6870

6971
@Override
7072
public Accumulator getInitialValue(ExecutionContext ctx) {
71-
return new Accumulator(new HashMap<>());
73+
return new Accumulator(new HashMap<>(), new HashMap<>());
7274
}
7375

7476
@Override
7577
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
7678
return new TreeVisitor<Tree, ExecutionContext>() {
77-
private final Map<ResolvedGroupArtifactVersion, Set<GroupArtifact>> closureCache = new HashMap<>();
78-
7979
@Override
8080
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
8181
if (tree == null) {
@@ -147,17 +147,10 @@ private void resolveTransitivesFromPom(
147147
MavenPomDownloader downloader,
148148
ExecutionContext ctx,
149149
Set<TransitiveDependency> transitives) {
150+
List<MavenRepository> effectiveRepos = withMavenCentral(repositories);
150151
try {
151-
// Ensure we have Maven Central in the repositories
152-
List<MavenRepository> effectiveRepos = new ArrayList<>(repositories);
153-
if (effectiveRepos.stream().noneMatch(r -> r.getUri().contains("repo.maven.apache.org") ||
154-
r.getUri().contains("repo1.maven.org"))) {
155-
effectiveRepos.add(MavenRepository.MAVEN_CENTRAL);
156-
}
157-
158152
// Get the resolved dependencies for compile scope (which includes most transitives)
159-
Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, effectiveRepos);
160-
ResolvedPom resolvedPom = pom.resolve(emptyList(), downloader, effectiveRepos, ctx);
153+
ResolvedPom resolvedPom = resolvePom(gav, effectiveRepos, downloader, ctx);
161154
ResolvedPom patchedPom = applyExclusions(resolvedPom, effectiveExclusions);
162155
List<ResolvedDependency> resolved = patchedPom.resolveDependencies(Scope.Compile, downloader, ctx);
163156

@@ -193,11 +186,8 @@ private void collectAllDependencies(ResolvedDependency dep, Set<TransitiveDepend
193186
}
194187
}
195188

196-
// A dependency's effective exclusions are resolved within the parent's whole tree, so they can
197-
// be polluted by exclusions that other branches declare against artifacts this coordinate could
198-
// never bring on its own (e.g. an optional dependency pruned elsewhere). Keep only the exclusions
199-
// that target something in the coordinate's own dependency closure, so the comparison against a
200-
// clean direct declaration is not thrown off by no-op exclusions.
189+
// Effective exclusions are resolved in the parent's whole tree, so they pick up no-op exclusions
190+
// from sibling branches; keep only those targeting this coordinate's own dependency closure.
201191
private Set<GroupArtifact> relevantExclusions(ResolvedDependency dep, List<MavenRepository> repositories,
202192
MavenPomDownloader downloader, ExecutionContext ctx) {
203193
Set<GroupArtifact> exclusions = new HashSet<>(dep.getEffectiveExclusions());
@@ -209,12 +199,11 @@ private Set<GroupArtifact> relevantExclusions(ResolvedDependency dep, List<Maven
209199

210200
private Set<GroupArtifact> dependencyClosure(ResolvedGroupArtifactVersion gav, List<MavenRepository> repositories,
211201
MavenPomDownloader downloader, ExecutionContext ctx) {
212-
return closureCache.computeIfAbsent(gav, g -> {
202+
return acc.closureCache.computeIfAbsent(gav, g -> {
213203
Set<GroupArtifact> closure = new HashSet<>();
214204
try {
215-
Pom pom = downloader.download(g.asGroupArtifactVersion(), null, null, repositories);
216-
ResolvedPom resolvedPom = pom.resolve(emptyList(), downloader, repositories, ctx);
217-
for (ResolvedDependency d : resolvedPom.resolveDependencies(Scope.Compile, downloader, ctx)) {
205+
for (ResolvedDependency d : resolvePom(g, repositories, downloader, ctx)
206+
.resolveDependencies(Scope.Compile, downloader, ctx)) {
218207
collectClosure(d, closure);
219208
}
220209
} catch (MavenDownloadingException | MavenDownloadingExceptions e) {
@@ -224,6 +213,23 @@ private Set<GroupArtifact> dependencyClosure(ResolvedGroupArtifactVersion gav, L
224213
});
225214
}
226215

216+
private ResolvedPom resolvePom(ResolvedGroupArtifactVersion gav, List<MavenRepository> repositories,
217+
MavenPomDownloader downloader, ExecutionContext ctx)
218+
throws MavenDownloadingException, MavenDownloadingExceptions {
219+
List<MavenRepository> repos = withMavenCentral(repositories);
220+
Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, repos);
221+
return pom.resolve(emptyList(), downloader, repos, ctx);
222+
}
223+
224+
private List<MavenRepository> withMavenCentral(List<MavenRepository> repositories) {
225+
List<MavenRepository> effectiveRepos = new ArrayList<>(repositories);
226+
if (effectiveRepos.stream().noneMatch(r -> r.getUri().contains("repo.maven.apache.org") ||
227+
r.getUri().contains("repo1.maven.org"))) {
228+
effectiveRepos.add(MavenRepository.MAVEN_CENTRAL);
229+
}
230+
return effectiveRepos;
231+
}
232+
227233
private void collectClosure(ResolvedDependency dep, Set<GroupArtifact> closure) {
228234
if (closure.add(dep.getGav().asGroupArtifact())) {
229235
for (ResolvedDependency transitive : dep.getDependencies()) {

src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -497,9 +497,9 @@ void keepsDirectCompileTomcatEmbedCoreWhenProviderIsProvidedScoped() {
497497

498498
@Test
499499
void removesJakartaClientWhenTransitiveExclusionsAreEquivalent() {
500-
// Resolved within the starter's whole tree, the transitive jakarta.ws.rs-api gains a spurious
501-
// effective exclusion of jakarta.activation-api (which it can never bring on its own), while the
502-
// direct declaration has none; the two are effectively equivalent so the direct one is redundant.
500+
// In the starter's tree the transitive jakarta.ws.rs-api gains a spurious exclusion of
501+
// jakarta.activation-api (which it can never bring on its own); the direct declaration has none,
502+
// so the two are effectively equivalent and the direct one is redundant.
503503
rewriteRun(
504504
spec -> spec.recipe(new RemoveRedundantDependencies(
505505
"org.springframework.boot", "spring-boot-starter-*")),

0 commit comments

Comments
 (0)