Skip to content

Keep direct dependencies whose scope or exclusions differ from the transitive one #188

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

Merged
timtebeek merged 4 commits into from
Jul 10, 2026
Merged

Keep direct dependencies whose scope or exclusions differ from the transitive one #188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3257855
Keep direct dependencies whose scope or exclusions differ from the tr…
timtebeek Jul 10, 2026
a274e06
Simplify test POMs: drop XML declaration and project attributes
timtebeek Jul 10, 2026
6890da4
Extract handleGradle/handleMaven and trim comments
timtebeek Jul 10, 2026
2ea65bb
Rename getCompatibleTransitives to getCompatibleGradleTransitives
timtebeek Jul 10, 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
198 changes: 100 additions & 98 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 @@ -50,12 +50,20 @@ public class RemoveRedundantDependencies extends ScanningRecipe<RemoveRedundantD

String description = "Remove explicit dependencies that are already provided transitively by a specified dependency. " +
"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.";
"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.";

@Value
public static class Accumulator {
// Map from project identifier -> scope/configuration -> Set of transitive GAVs
Map<String, Map<String, Set<ResolvedGroupArtifactVersion>>> transitivesByProjectAndScope;
// Map from project identifier -> scope/configuration -> Set of transitive dependencies
Map<String, Map<String, Set<TransitiveDependency>>> transitivesByProjectAndScope;
}

@Value
public static class TransitiveDependency {
ResolvedGroupArtifactVersion gav;
Set<GroupArtifact> exclusions;
}

@Override
Expand All @@ -82,7 +90,7 @@ public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
StringUtils.matchesGlob(dep.getGroupId(), groupId) &&
StringUtils.matchesGlob(dep.getArtifactId(), artifactId)) {
// This is a matching parent dependency, resolve its transitives independently
Set<ResolvedGroupArtifactVersion> transitives = acc.transitivesByProjectAndScope
Set<TransitiveDependency> transitives = acc.transitivesByProjectAndScope
.computeIfAbsent(projectId, k -> new HashMap<>())
.computeIfAbsent(conf.getName(), k -> new HashSet<>());
resolveTransitivesFromPom(
Expand All @@ -101,14 +109,18 @@ public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
String projectId = maven.getPom().getGroupId() + ":" + maven.getPom().getArtifactId();
MavenPomDownloader downloader = new MavenPomDownloader(ctx);

for (Map.Entry<Scope, List<ResolvedDependency>> entry : maven.getDependencies().entrySet()) {
Scope depScope = entry.getKey();
for (ResolvedDependency dep : entry.getValue()) {
// A direct dependency appears under every scope bucket it is visible in, so process
// each matching parent once, keyed by its own effective (declared) scope.
Set<String> processed = new HashSet<>();
for (List<ResolvedDependency> deps : maven.getDependencies().values()) {
for (ResolvedDependency dep : deps) {
if (dep.isDirect() &&
StringUtils.matchesGlob(dep.getGroupId(), groupId) &&
StringUtils.matchesGlob(dep.getArtifactId(), artifactId)) {
StringUtils.matchesGlob(dep.getArtifactId(), artifactId) &&
processed.add(dep.getGroupId() + ":" + dep.getArtifactId())) {
// This is a matching parent dependency, resolve its transitives independently
Set<ResolvedGroupArtifactVersion> transitives = acc.transitivesByProjectAndScope
Scope depScope = Scope.fromName(dep.getRequested().getScope());
Set<TransitiveDependency> transitives = acc.transitivesByProjectAndScope
.computeIfAbsent(projectId, k -> new HashMap<>())
.computeIfAbsent(depScope.name().toLowerCase(), k -> new HashSet<>());
resolveTransitivesFromPom(
Expand All @@ -132,7 +144,7 @@ private void resolveTransitivesFromPom(
List<MavenRepository> repositories,
MavenPomDownloader downloader,
ExecutionContext ctx,
Set<ResolvedGroupArtifactVersion> transitives) {
Set<TransitiveDependency> transitives) {
try {
// Ensure we have Maven Central in the repositories
List<MavenRepository> effectiveRepos = new ArrayList<>(repositories);
Expand All @@ -148,8 +160,9 @@ private void resolveTransitivesFromPom(
List<ResolvedDependency> resolved = patchedPom.resolveDependencies(Scope.Compile, downloader, ctx);

// Collect all dependencies (both direct and transitive of the parent)
Set<ResolvedGroupArtifactVersion> visited = new HashSet<>();
for (ResolvedDependency dep : resolved) {
collectAllDependencies(dep, transitives);
collectAllDependencies(dep, transitives, visited);
}
} catch (MavenDownloadingException | MavenDownloadingExceptions e) {
// If we can't download/resolve the POM, fall back to not detecting redundancies
Expand All @@ -166,10 +179,12 @@ private ResolvedPom applyExclusions(ResolvedPom resolvedPom, List<GroupArtifact>
return patchedPom;
}

private void collectAllDependencies(ResolvedDependency dep, Set<ResolvedGroupArtifactVersion> transitives) {
if (transitives.add(dep.getGav())) {
private void collectAllDependencies(ResolvedDependency dep, Set<TransitiveDependency> transitives,
Set<ResolvedGroupArtifactVersion> visited) {
if (visited.add(dep.getGav())) {
transitives.add(new TransitiveDependency(dep.getGav(), new HashSet<>(dep.getEffectiveExclusions())));
for (ResolvedDependency transitive : dep.getDependencies()) {
collectAllDependencies(transitive, transitives);
collectAllDependencies(transitive, transitives, visited);
}
}
}
Expand All @@ -185,112 +200,112 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
return tree;
}

SourceFile sf = (SourceFile) tree;
Tree result = sf;

// Handle Gradle
Optional<GradleProject> gradleOpt = sf.getMarkers().findFirst(GradleProject.class);
Optional<GradleProject> gradleOpt = tree.getMarkers().findFirst(GradleProject.class);
if (gradleOpt.isPresent()) {
GradleProject gradle = gradleOpt.get();
String projectId = gradle.getGroup() + ":" + gradle.getName();
Map<String, Set<ResolvedGroupArtifactVersion>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());

for (GradleDependencyConfiguration conf : gradle.getConfigurations()) {
Set<ResolvedGroupArtifactVersion> transitives = getCompatibleTransitives(
scopeToTransitives, conf.getName(), true);
if (transitives.isEmpty()) {
continue;
}

for (ResolvedDependency dep : conf.getResolved()) {
if (dep.isDirect() &&
doesNotMatchArguments(dep) &&
isInTransitives(dep, transitives)) {
// This direct dependency is transitively provided, remove it
// Don't specify configuration - Gradle's resolved config names differ from declaration names
result = new RemoveDependency(
dep.getGroupId(), dep.getArtifactId(), null, null, null)
.getVisitor().visit(result, ctx);
}
}
}
return result;
return handleGradle(ctx, gradle, tree);
}

// Handle Maven
Optional<MavenResolutionResult> mavenOpt = sf.getMarkers().findFirst(MavenResolutionResult.class);
Optional<MavenResolutionResult> mavenOpt = tree.getMarkers().findFirst(MavenResolutionResult.class);
if (mavenOpt.isPresent()) {
MavenResolutionResult maven = mavenOpt.get();
String projectId = maven.getPom().getGroupId() + ":" + maven.getPom().getArtifactId();
Map<String, Set<ResolvedGroupArtifactVersion>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());

for (Map.Entry<Scope, List<ResolvedDependency>> entry : maven.getDependencies().entrySet()) {
String scope = entry.getKey().name().toLowerCase();
Set<ResolvedGroupArtifactVersion> transitives = getCompatibleTransitives(
scopeToTransitives, scope, false);
if (transitives.isEmpty()) {
continue;
return handleMaven(ctx, maven, tree);
}

return tree;
}

private @Nullable Tree handleGradle(ExecutionContext ctx, GradleProject gradle, Tree result) {
String projectId = gradle.getGroup() + ":" + gradle.getName();
Map<String, Set<TransitiveDependency>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());

for (GradleDependencyConfiguration conf : gradle.getConfigurations()) {
Set<TransitiveDependency> transitives = getCompatibleGradleTransitives(
scopeToTransitives, conf.getName());
if (transitives.isEmpty()) {
continue;
}

for (ResolvedDependency dep : conf.getResolved()) {
if (dep.isDirect() &&
doesNotMatchArguments(dep) &&
isRedundant(dep, transitives)) {
// This direct dependency is transitively provided, remove it
// Don't specify configuration - Gradle's resolved config names differ from declaration names
result = new RemoveDependency(
dep.getGroupId(), dep.getArtifactId(), null, null, null)
.getVisitor().visit(result, ctx);
}
}
}
return result;
}

for (ResolvedDependency dep : entry.getValue()) {
if (dep.isDirect() &&
doesNotMatchArguments(dep) &&
isInTransitives(dep, transitives)) {
// This direct dependency is transitively provided, remove it
private @Nullable Tree handleMaven(ExecutionContext ctx, MavenResolutionResult maven, Tree result) {
String projectId = maven.getPom().getGroupId() + ":" + maven.getPom().getArtifactId();
Map<String, Set<TransitiveDependency>> scopeToTransitives =
acc.transitivesByProjectAndScope.getOrDefault(projectId, emptyMap());

// 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
// mark a narrower direct declaration as redundant.
Set<String> processed = new HashSet<>();
for (List<ResolvedDependency> deps : maven.getDependencies().values()) {
for (ResolvedDependency dep : deps) {
if (dep.isDirect() &&
doesNotMatchArguments(dep) &&
processed.add(dep.getGroupId() + ":" + dep.getArtifactId())) {
Scope depScope = Scope.fromName(dep.getRequested().getScope());
Set<TransitiveDependency> transitives = scopeToTransitives.getOrDefault(
depScope.name().toLowerCase(), emptySet());
if (isRedundant(dep, transitives)) {
// This direct dependency is transitively provided at the same scope and
// with the same exclusions, remove it.
result = new RemoveDependency(
dep.getGroupId(), dep.getArtifactId(), null, null, scope)
dep.getGroupId(), dep.getArtifactId(), null, null, depScope.name().toLowerCase())
.getVisitor().visit(result, ctx);
}
}
}
return result;
}

return tree;
return result;
}

private boolean doesNotMatchArguments(ResolvedDependency dep) {
return !StringUtils.matchesGlob(dep.getGroupId(), groupId) ||
!StringUtils.matchesGlob(dep.getArtifactId(), artifactId);
}

private boolean isInTransitives(ResolvedDependency dep, Set<ResolvedGroupArtifactVersion> transitives) {
// Check if this dependency's GAV matches any transitive
// We match on groupId:artifactId:version exactly
for (ResolvedGroupArtifactVersion transitive : transitives) {
if (dep.getGroupId().equals(transitive.getGroupId()) &&
dep.getArtifactId().equals(transitive.getArtifactId()) &&
dep.getVersion().equals(transitive.getVersion())) {
private boolean isRedundant(ResolvedDependency dep, Set<TransitiveDependency> transitives) {
Set<GroupArtifact> depExclusions = new HashSet<>(dep.getEffectiveExclusions());
for (TransitiveDependency transitive : transitives) {
ResolvedGroupArtifactVersion gav = transitive.getGav();
if (dep.getGroupId().equals(gav.getGroupId()) &&
dep.getArtifactId().equals(gav.getArtifactId()) &&
dep.getVersion().equals(gav.getVersion()) &&
depExclusions.equals(transitive.getExclusions())) {
return true;
}
}
return false;
}

/**
* Get transitives from this scope/configuration and any broader ones.
*/
private Set<ResolvedGroupArtifactVersion> getCompatibleTransitives(
Map<String, Set<ResolvedGroupArtifactVersion>> scopeToTransitives,
String targetScope,
boolean isGradle) {
private Set<TransitiveDependency> getCompatibleGradleTransitives(
Map<String, Set<TransitiveDependency>> scopeToTransitives,
String targetScope) {

Set<ResolvedGroupArtifactVersion> result = new HashSet<>();
Set<TransitiveDependency> result = new HashSet<>();

// Always include transitives from the same scope
Set<ResolvedGroupArtifactVersion> sameScope = scopeToTransitives.get(targetScope);
Set<TransitiveDependency> sameScope = scopeToTransitives.get(targetScope);
if (sameScope != null) {
result.addAll(sameScope);
}

// Include transitives from broader scopes
List<String> broaderScopes = isGradle ?
getBroaderGradleScopes(targetScope) :
getBroaderMavenScopes(targetScope);
for (String broader : broaderScopes) {
Set<ResolvedGroupArtifactVersion> broaderTransitives = scopeToTransitives.get(broader);
for (String broader : getBroaderGradleScopes(targetScope)) {
Set<TransitiveDependency> broaderTransitives = scopeToTransitives.get(broader);
Comment on lines -289 to +308

@steve-aom-elliott steve-aom-elliott Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we end up dropping the differing scopes check for Maven and now only use the broader gradle scopes? I may have missed something here.

@timtebeek timtebeek Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question — the short version is that Maven still checks scope, it just does it differently, so the widening helper is no longer needed there.

Maven (handleMaven): each direct dependency is now evaluated against transitives collected under its own declared scope, matched exactly:

Scope depScope = Scope.fromName(dep.getRequested().getScope());
Set<TransitiveDependency> transitives = scopeToTransitives.getOrDefault(depScope.name().toLowerCase(), emptySet());
if (isRedundant(dep, transitives)) { ... }

The scan phase keys the parent's transitives by the parent's own declared scope as well, so it's an apples-to-apples comparison. getBroaderMavenScopes only existed because the old code iterated maven.getDependencies() resolution buckets — where a compile dep also shows up in the runtime/test buckets — and keyed transitives by bucket; that mismatch is what needed reconciling. Keying by declared scope removes the need.

This is also a deliberate behavior change: we no longer treat a broader-scoped transitive as covering a narrower-scoped direct declaration, since that changes the effective classpath (e.g. dropping a direct runtime dep because a compile transitive provides it). It's locked in by keepsRuntimeTomcatEmbedCoreWhenTransitiveIsCompileScoped and keepsDirectCompileTomcatEmbedCoreWhenProviderIsProvidedScoped.

Gradle (handleGradle): still uses getCompatibleTransitives/getBroaderGradleScopes because there transitives are keyed by configuration name, and Gradle configurations genuinely inherit (testImplementation → implementation → api). A dependency in one configuration legitimately sees transitives from the configurations it extends, so we still walk those. Maven's flat declared-scope model has no such inheritance, so exact-scope matching is sufficient.

Happy to add a code comment making the Maven-vs-Gradle asymmetry explicit if that would help.

@steve-aom-elliott steve-aom-elliott Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just a method rename to getCompatibleGradleTransitives or something like that. I guess I missed that it was no longer called by the handleMaven that was extracted.

if (broaderTransitives != null) {
result.addAll(broaderTransitives);
}
Expand All @@ -313,19 +328,6 @@ private List<String> getBroaderGradleScopes(String scope) {
return emptyList();
}
}

private List<String> getBroaderMavenScopes(String scope) {
switch (scope.toLowerCase()) {
case "runtime":
return singletonList("compile");
case "provided":
return Arrays.asList("compile", "runtime");
case "test":
return Arrays.asList("compile", "runtime", "provided");
default:
return emptyList();
}
}
};
}
}
Loading
Loading