From f513d78eb65fe16ab3cb3c4f500a3a82311d6c05 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 30 Jul 2026 19:16:00 +0200 Subject: [PATCH] Support wildcards matching removal candidates in `RemoveRedundantDependencies` The visitor guarded every removal candidate with a check that it did not match the recipe's `groupId`/`artifactId` globs. The intent was to avoid deleting the provider dependency itself, but the effect was to suppress removal of anything the glob matched. The wider the glob, the more it suppressed: `*`/`*` matched every declaration and made the recipe a complete no-op, and `spring-boot-starter-*` missed `spring-boot-starter-json` even though `spring-boot-starter-web` provides it. Record which direct dependency contributed each transitive on `TransitiveDependency.providedBy`, and skip only genuine self-provision rather than every glob match. --- .../RemoveRedundantDependencies.java | 29 ++- .../RemoveRedundantDependenciesTest.java | 241 ++++++++++++++++++ 2 files changed, 258 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java b/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java index 1bd75af8..43e936f2 100644 --- a/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java +++ b/src/main/java/org/openrewrite/java/dependencies/RemoveRedundantDependencies.java @@ -64,6 +64,10 @@ public static class Accumulator { public static class TransitiveDependency { ResolvedGroupArtifactVersion gav; Set exclusions; + // The direct dependency whose closure supplied this entry, so that a dependency is never + // considered redundant on the strength of its own transitives. Without this a glob such as + // `*` would match every declaration and, in the absence of any other guard, delete them all. + GroupArtifact providedBy; } @Override @@ -146,6 +150,7 @@ private void resolveTransitivesFromPom( ExecutionContext ctx, Set transitives) { List repos = withMavenCentral(repositories); + GroupArtifact providedBy = new GroupArtifact(gav.getGroupId(), gav.getArtifactId()); try { // Get the resolved dependencies for compile scope (which includes most transitives) Pom pom = downloader.download(gav.asGroupArtifactVersion(), null, null, repos); @@ -155,7 +160,7 @@ private void resolveTransitivesFromPom( // Collect all dependencies (both direct and transitive of the parent) Set visited = new HashSet<>(); for (ResolvedDependency dep : patchedPom.resolveDependencies(Scope.Compile, downloader, ctx)) { - collectAllDependencies(dep, transitives, visited); + collectAllDependencies(dep, transitives, visited, providedBy); } } catch (MavenDownloadingException | MavenDownloadingExceptions e) { // If we can't download/resolve the POM, fall back to not detecting redundancies @@ -173,11 +178,11 @@ private ResolvedPom applyExclusions(ResolvedPom resolvedPom, List } private void collectAllDependencies(ResolvedDependency dep, Set transitives, - Set visited) { + Set visited, GroupArtifact providedBy) { if (visited.add(dep.getGav())) { - transitives.add(new TransitiveDependency(dep.getGav(), declaredExclusions(dep))); + transitives.add(new TransitiveDependency(dep.getGav(), declaredExclusions(dep), providedBy)); for (ResolvedDependency transitive : dep.getDependencies()) { - collectAllDependencies(transitive, transitives, visited); + collectAllDependencies(transitive, transitives, visited, providedBy); } } } @@ -239,7 +244,6 @@ public TreeVisitor getVisitor(Accumulator acc) { 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 @@ -264,7 +268,6 @@ public TreeVisitor getVisitor(Accumulator acc) { for (List 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 transitives = scopeToTransitives.getOrDefault( @@ -282,11 +285,6 @@ public TreeVisitor getVisitor(Accumulator acc) { return result; } - private boolean doesNotMatchArguments(ResolvedDependency dep) { - return !StringUtils.matchesGlob(dep.getGroupId(), groupId) || - !StringUtils.matchesGlob(dep.getArtifactId(), artifactId); - } - private boolean isRedundant(ResolvedDependency dep, Set transitives) { Set depExclusions = declaredExclusions(dep); for (TransitiveDependency transitive : transitives) { @@ -294,13 +292,20 @@ private boolean isRedundant(ResolvedDependency dep, Set tr if (dep.getGroupId().equals(gav.getGroupId()) && dep.getArtifactId().equals(gav.getArtifactId()) && dep.getVersion().equals(gav.getVersion()) && - depExclusions.equals(transitive.getExclusions())) { + depExclusions.equals(transitive.getExclusions()) && + !isProvidedByItself(dep, transitive)) { return true; } } return false; } + private boolean isProvidedByItself(ResolvedDependency dep, TransitiveDependency transitive) { + GroupArtifact providedBy = transitive.getProvidedBy(); + return dep.getGroupId().equals(providedBy.getGroupId()) && + dep.getArtifactId().equals(providedBy.getArtifactId()); + } + private Set getCompatibleGradleTransitives( Map> scopeToTransitives, String targetScope) { diff --git a/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java b/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java index cdcdb3ff..787c55ee 100644 --- a/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java +++ b/src/test/java/org/openrewrite/java/dependencies/RemoveRedundantDependenciesTest.java @@ -780,4 +780,245 @@ void removeRedundantGradleDependency() { ) ); } + + @Test + void globGroupIdMatchesProvider() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "com.fasterxml.jackson.*", "jackson-databind")), + //language=xml + pomXml(JACKSON_BEFORE, JACKSON_AFTER) + ); + } + + @Test + void singleCharacterWildcardMatchesProvider() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "com.fasterxml.jackson.cor?", "jackson-databin?")), + //language=xml + pomXml(JACKSON_BEFORE, JACKSON_AFTER) + ); + } + + @Test + void wildcardArtifactIdRemovesDependenciesProvidedBySibling() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies( + "com.fasterxml.jackson.core", "*")), + //language=xml + pomXml(JACKSON_BEFORE, JACKSON_AFTER) + ); + } + + @Test + void matchAllWildcardsRemoveEveryTransitivelyProvidedDependency() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies("*", "*")), + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + com.fasterxml.jackson.core + jackson-databind + 2.17.0 + + + com.fasterxml.jackson.core + jackson-core + 2.17.0 + + + com.fasterxml.jackson.core + jackson-annotations + 2.17.0 + + + + """, + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + com.fasterxml.jackson.core + jackson-databind + 2.17.0 + + + + """ + ) + ); + } + + @Test + void matchAllWildcardsKeepDependenciesNobodyElseProvides() { + rewriteRun( + spec -> spec.recipe(new RemoveRedundantDependencies("*", "*")), + //language=xml + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + com.fasterxml.jackson.core + jackson-databind + 2.17.0 + + + org.apache.commons + commons-lang3 + 3.14.0 + + + + """ + ) + ); + } + + @Test + void removesStarterMatchedByTheSameGlobAsItsProvider() { + // spring-boot-starter-web transitively provides spring-boot-starter-json. Both match the + // `spring-boot-starter-*` glob, so the redundant one must still be removed. + 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.springframework.boot + spring-boot-starter-json + + + + """, + """ + + 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 removeRedundantGradleDependencyWithWildcardArtifactId() { + rewriteRun( + spec -> spec.beforeRecipe(withToolingApi()) + .recipe(new RemoveRedundantDependencies( + "com.fasterxml.jackson.core", "*")), + mavenProject("my-app", + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.0' + implementation 'com.fasterxml.jackson.core:jackson-core:2.17.0' + } + """, + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.0' + } + """ + ) + ) + ); + } + + private static final String JACKSON_BEFORE = """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + com.fasterxml.jackson.core + jackson-databind + 2.17.0 + + + com.fasterxml.jackson.core + jackson-core + 2.17.0 + + + + """; + + private static final String JACKSON_AFTER = """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + com.fasterxml.jackson.core + jackson-databind + 2.17.0 + + + + """; }