Skip to content

Ignore spurious transitive exclusions when detecting redundant dependencies #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
timtebeek wants to merge 3 commits into
base: main
Choose a base branch
from
+344 −17
Open

Ignore spurious transitive exclusions when detecting redundant dependencies #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e336a7
Ignore spurious transitive exclusions when detecting redundant depend…
timtebeek Jul 22, 2026
e5cc17a
Simplify redundant-dependency exclusion filtering
timtebeek Jul 22, 2026
2d49d4a
Compare declared exclusions, not effective ones, for redundant depend…
timtebeek Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 78 additions & 17 deletions src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ public class RemoveRedundantDependencies extends ScanningRecipe<RemoveRedundantD
"This recipe downloads and resolves the parent dependency's POM to determine its true transitive " +
"dependencies, allowing it to detect redundancies even when both dependencies are explicitly declared. " +
"A direct dependency is only removed when the transitive one provides it at the exact same scope and " +
"with the same exclusions, so that removing it does not change the effective classpath.";
"with the same effective exclusions, so that removing it does not change the effective classpath.";

@Value
public static class Accumulator {
// Map from project identifier -> scope/configuration -> Set of transitive dependencies
Map<String, Map<String, Set<TransitiveDependency>>> transitivesByProjectAndScope;
// Cache of each coordinate's own clean dependency closure, shared across all source files in the run
Map<ResolvedGroupArtifactVersion, Set<GroupArtifact>> closureCache;
}

@Value
Expand All @@ -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
Expand Down Expand Up @@ -145,14 +147,8 @@ private void resolveTransitivesFromPom(
MavenPomDownloader downloader,
ExecutionContext ctx,
Set<TransitiveDependency> transitives) {
List<MavenRepository> effectiveRepos = withMavenCentral(repositories);
try {
// Ensure we have Maven Central in the repositories
List<MavenRepository> 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);
Expand All @@ -162,7 +158,7 @@ private void resolveTransitivesFromPom(
// Collect all dependencies (both direct and transitive of the parent)
Set<ResolvedGroupArtifactVersion> 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
Expand All @@ -180,17 +176,79 @@ private ResolvedPom applyExclusions(ResolvedPom resolvedPom, List<GroupArtifact>
}

private void collectAllDependencies(ResolvedDependency dep, Set<TransitiveDependency> transitives,
Set<ResolvedGroupArtifactVersion> visited) {
Set<ResolvedGroupArtifactVersion> visited, List<MavenRepository> 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, acc.closureCache)));
for (ResolvedDependency transitive : dep.getDependencies()) {
collectAllDependencies(transitive, transitives, visited);
collectAllDependencies(transitive, transitives, visited, repositories, downloader, ctx);
}
}
}
};
}

// Compares a coordinate's declared (requested) exclusions rather than its effective ones: within a large
// tree the effective set is corrupted by dependency mediation (an artifact excluded here may already have
// been pruned by a shared node elsewhere, so it silently drops out of getEffectiveExclusions). The declared
// exclusions are then intersected with the coordinate's own clean closure, dropping no-op exclusions that
// target artifacts the coordinate could never bring on its own. Comparing this set on the direct and the
// transitively-provided declaration tells us whether removing the direct one changes the effective classpath.
private static Set<GroupArtifact> relevantExclusions(ResolvedDependency dep, List<MavenRepository> repositories,
MavenPomDownloader downloader, ExecutionContext ctx,
Map<ResolvedGroupArtifactVersion, Set<GroupArtifact>> closureCache) {
List<GroupArtifact> requested = dep.getRequested() == null ? null : dep.getRequested().getExclusions();
if (requested == null || requested.isEmpty()) {
return emptySet();
}
Set<GroupArtifact> exclusions = new HashSet<>(requested);
exclusions.retainAll(dependencyClosure(dep.getGav(), repositories, downloader, ctx, closureCache));
return exclusions;
}

private static Set<GroupArtifact> dependencyClosure(ResolvedGroupArtifactVersion gav, List<MavenRepository> repositories,
MavenPomDownloader downloader, ExecutionContext ctx,
Map<ResolvedGroupArtifactVersion, Set<GroupArtifact>> closureCache) {
return closureCache.computeIfAbsent(gav, g -> {
Set<GroupArtifact> 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 static ResolvedPom resolvePom(ResolvedGroupArtifactVersion gav, List<MavenRepository> repositories,
MavenPomDownloader downloader, ExecutionContext ctx)
throws MavenDownloadingException, MavenDownloadingExceptions {
List<MavenRepository> repos = withMavenCentral(repositories);
Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, repos);
return pom.resolve(emptyList(), downloader, repos, ctx);
}

private static List<MavenRepository> withMavenCentral(List<MavenRepository> repositories) {
List<MavenRepository> 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 static void collectClosure(ResolvedDependency dep, Set<GroupArtifact> closure) {
if (closure.add(dep.getGav().asGroupArtifact())) {
for (ResolvedDependency transitive : dep.getDependencies()) {
collectClosure(transitive, closure);
}
}
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
return new TreeVisitor<Tree, ExecutionContext>() {
Expand Down Expand Up @@ -219,6 +277,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
String projectId = gradle.getGroup() + ":" + gradle.getName();
Map<String, Set<TransitiveDependency>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());
MavenPomDownloader downloader = new MavenPomDownloader(ctx);

for (GradleDependencyConfiguration conf : gradle.getConfigurations()) {
Set<TransitiveDependency> transitives = getCompatibleGradleTransitives(
Expand All @@ -230,7 +289,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
for (ResolvedDependency dep : conf.getResolved()) {
if (dep.isDirect() &&
doesNotMatchArguments(dep) &&
isRedundant(dep, transitives)) {
isRedundant(dep, transitives, gradle.getMavenRepositories(), downloader, ctx)) {
// This direct dependency is transitively provided, remove it
// Don't specify configuration - Gradle's resolved config names differ from declaration names
result = new RemoveDependency(
Expand All @@ -246,6 +305,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
String projectId = maven.getPom().getGroupId() + ":" + maven.getPom().getArtifactId();
Map<String, Set<TransitiveDependency>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());
MavenPomDownloader downloader = new MavenPomDownloader(ctx);

// A direct dependency appears under every scope bucket it is visible in; evaluate each
// one once using its own effective scope so a wider transitive scope does not falsely
Expand All @@ -259,7 +319,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
Scope depScope = Scope.fromName(dep.getRequested().getScope());
Set<TransitiveDependency> transitives = scopeToTransitives.getOrDefault(
depScope.name().toLowerCase(), emptySet());
if (isRedundant(dep, transitives)) {
if (isRedundant(dep, transitives, maven.getPom().getRepositories(), downloader, ctx)) {
// This direct dependency is transitively provided at the same scope and
// with the same exclusions, remove it.
result = new RemoveDependency(
Expand All @@ -277,8 +337,9 @@ private boolean doesNotMatchArguments(ResolvedDependency dep) {
!StringUtils.matchesGlob(dep.getArtifactId(), artifactId);
}

private boolean isRedundant(ResolvedDependency dep, Set<TransitiveDependency> transitives) {
Set<GroupArtifact> depExclusions = new HashSet<>(dep.getEffectiveExclusions());
private boolean isRedundant(ResolvedDependency dep, Set<TransitiveDependency> transitives,
List<MavenRepository> repositories, MavenPomDownloader downloader, ExecutionContext ctx) {
Set<GroupArtifact> depExclusions = relevantExclusions(dep, repositories, downloader, ctx, acc.closureCache);
for (TransitiveDependency transitive : transitives) {
ResolvedGroupArtifactVersion gav = transitive.getGav();
if (dep.getGroupId().equals(gav.getGroupId()) &&
Expand Down
Loading
Loading