Skip to content

Commit fd468a6

Browse files
sambsnydsambsnyd
committed
Remove more redundant dependencies by simulating dependency resolution.
Gradle has a "newest wins" conflict resolution policy, and our GradleProject marker is based on the results of that conflict resolution. So we can't look at it alone to determine what the results would be like were a different set of dependencies requested.
1 parent e44e4f1 commit fd468a6

File tree

2 files changed

+82
-25
lines changed

2 files changed

+82
-25
lines changed

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveRedundantDependencyVersions.java‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
import org.openrewrite.internal.StringUtils;
3232
import org.openrewrite.java.JavaIsoVisitor;
3333
import org.openrewrite.java.tree.*;
34+
import org.openrewrite.marker.Markup;
3435
import org.openrewrite.maven.MavenDownloadingException;
36+
import org.openrewrite.maven.MavenDownloadingExceptions;
3537
import org.openrewrite.maven.internal.MavenPomDownloader;
3638
import org.openrewrite.maven.tree.*;
3739
import org.openrewrite.semver.ExactVersion;
@@ -275,8 +277,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
275277
} catch (Exception ignore) {
276278
}
277279

278-
if (shouldRemoveRedundantDependency(dependency, m.getSimpleName())) {
279-
return null;
280+
try {
281+
if (shouldRemoveRedundantDependency(dependency, m.getSimpleName(), gp.getMavenRepositories(), ctx)) {
282+
return null;
283+
}
284+
} catch (MavenDownloadingException e) {
285+
return Markup.error(m, e);
280286
}
281287
}
282288
return m;
@@ -295,23 +301,12 @@ private boolean isDependencyManagmentMethod(String methodName) {
295301
return DEPENDENCY_MANAGEMENT_METHODS.contains(methodName);
296302
}
297303

298-
private boolean shouldRemoveRedundantDependency(Dependency dependency, String configurationName) {
299-
if ((groupPattern != null && !matchesGlob(dependency.getGroupId(), groupPattern)) ||
300-
(artifactPattern != null && !matchesGlob(dependency.getArtifactId(), artifactPattern))) {
304+
private boolean shouldRemoveRedundantDependency(@Nullable Dependency dependency, String configurationName, List<MavenRepository> repositories, ExecutionContext ctx) throws MavenDownloadingException {
305+
if (dependency == null || ((groupPattern != null && !matchesGlob(dependency.getGroupId(), groupPattern))
306+
|| (artifactPattern != null && !matchesGlob(dependency.getArtifactId(), artifactPattern)))) {
301307
return false;
302308
}
303309

304-
String dependencyManagedVersion = null;
305-
if (platforms.get(configurationName) != null && !platforms.get(configurationName).isEmpty()) {
306-
dependencyManagedVersion = platforms.get(configurationName)
307-
.stream()
308-
.map(e -> e.getManagedDependency(dependency.getGroupId(), dependency.getArtifactId(), null, null))
309-
.filter(Objects::nonNull)
310-
.map(ResolvedManagedDependency::getVersion)
311-
.max(VERSION_COMPARATOR)
312-
.orElse(null);
313-
}
314-
315310
for (Map.Entry<String, List<ResolvedDependency>> entry : directDependencies.entrySet()) {
316311
for (ResolvedDependency d : entry.getValue()) {
317312
// ignore self
@@ -323,18 +318,39 @@ private boolean shouldRemoveRedundantDependency(Dependency dependency, String co
323318
if (d.getDependencies() == null) {
324319
continue;
325320
}
326-
327-
ResolvedDependency resolvedDependency = d.findDependency(dependency.getGroupId(), dependency.getArtifactId());
328-
if (resolvedDependency != null && (dependency.getVersion() == null ||
329-
(dependencyManagedVersion == null && matchesConfiguration(configurationName, entry.getKey()) && dependency.getVersion().equals(resolvedDependency.getVersion())) ||
330-
(dependencyManagedVersion != null && VERSION_COMPARATOR.compare(dependency.getVersion(), dependencyManagedVersion) <= 0))) {
321+
if (matchesConfiguration(configurationName, entry.getKey())
322+
&& d.findDependency(dependency.getGroupId(), dependency.getArtifactId()) != null
323+
&& dependsOnNewerVersion(dependency.getGav(), d.getGav().asGroupArtifactVersion(), repositories, ctx)) {
331324
return true;
332325
}
333326
}
334327
}
335328
return false;
336329
}
337330

331+
private boolean dependsOnNewerVersion(GroupArtifactVersion searchGav, GroupArtifactVersion toSearch, List<MavenRepository> repositories, ExecutionContext ctx) throws MavenDownloadingException {
332+
if (toSearch.getVersion() == null) {
333+
return false;
334+
}
335+
if (searchGav.asGroupArtifact().equals(toSearch.asGroupArtifact())) {
336+
return searchGav.getVersion() == null || matchesComparator(toSearch.getVersion(), searchGav.getVersion());
337+
}
338+
MavenPomDownloader mpd = new MavenPomDownloader(ctx);
339+
try {
340+
List<ResolvedDependency> resolved = mpd.download(toSearch, null, null, repositories)
341+
.resolve(emptyList(), mpd, repositories, ctx)
342+
.resolveDependencies(Scope.Runtime, mpd, ctx);
343+
for (ResolvedDependency r : resolved) {
344+
if (Objects.equals(searchGav.getGroupId(), r.getGroupId()) && Objects.equals(searchGav.getArtifactId(), r.getArtifactId())) {
345+
return searchGav.getVersion() == null || matchesComparator(r.getVersion(), searchGav.getVersion());
346+
}
347+
}
348+
} catch (MavenDownloadingExceptions e) {
349+
throw e.getExceptions().get(0);
350+
}
351+
return false;
352+
}
353+
338354
boolean matchesConfiguration(String configA, String configB) {
339355
if (configA.equals("runtimeOnly") && configB.equals("implementation")) {
340356
return true;

‎rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveRedundantDependencyVersionsTest.java‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.openrewrite.gradle.Assertions.buildGradleKts;
2525
import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi;
2626

27+
@SuppressWarnings("GroovyAssignabilityCheck")
2728
class RemoveRedundantDependencyVersionsTest implements RewriteTest {
2829
@Override
2930
public void defaults(RecipeSpec spec) {
@@ -312,6 +313,7 @@ void keepStrictConstraint() {
312313
@Test
313314
void removeDirectDependencyWithLowerVersionNumberIfDependencyIsLoadedTransitivelyWithHigherVersionNumber() {
314315
rewriteRun(
316+
spec -> spec.recipe(new RemoveRedundantDependencyVersions(null, null, RemoveRedundantDependencyVersions.Comparator.GTE)),
315317
buildGradle(
316318
"""
317319
plugins {
@@ -401,10 +403,10 @@ void keepDirectDependencyWithHigherVersionNumberIfDependencyIsLoadedTransitively
401403
}
402404
403405
dependencies {
404-
// BOM `spring-boot-dependencies:3.3.3` describes `spring-webmvc:6.1.12`
406+
// Without explicit spring-webmvc:6.2.8 gradle would resolve 6.2.7
405407
implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3"))
406408
implementation("org.springframework.boot:spring-boot-starter-web-services:3.3.3")
407-
implementation("org.springframework:spring-webmvc:6.1.13")
409+
implementation("org.springframework:spring-webmvc:6.2.8")
408410
}
409411
""",
410412
"""
@@ -417,10 +419,10 @@ void keepDirectDependencyWithHigherVersionNumberIfDependencyIsLoadedTransitively
417419
}
418420
419421
dependencies {
420-
// BOM `spring-boot-dependencies:3.3.3` describes `spring-webmvc:6.1.12`
422+
// Without explicit spring-webmvc:6.2.8 gradle would resolve 6.2.7
421423
implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3"))
422424
implementation("org.springframework.boot:spring-boot-starter-web-services")
423-
implementation("org.springframework:spring-webmvc:6.1.13")
425+
implementation("org.springframework:spring-webmvc:6.2.8")
424426
}
425427
"""
426428
)
@@ -430,6 +432,7 @@ void keepDirectDependencyWithHigherVersionNumberIfDependencyIsLoadedTransitively
430432
@Test
431433
void handleSeveralPlatformDependencies() {
432434
rewriteRun(
435+
spec -> spec.recipe(new RemoveRedundantDependencyVersions(null, null, RemoveRedundantDependencyVersions.Comparator.GTE)),
433436
buildGradle(
434437
"""
435438
plugins {
@@ -732,4 +735,42 @@ void removeRepeatedDependency() {
732735
)
733736
);
734737
}
738+
739+
@Test
740+
void webfluxGTE() {
741+
rewriteRun(
742+
spec -> spec.recipe(new RemoveRedundantDependencyVersions(null, null, RemoveRedundantDependencyVersions.Comparator.GTE)),
743+
buildGradle(
744+
"""
745+
plugins {
746+
id "java"
747+
}
748+
749+
repositories {
750+
mavenCentral()
751+
}
752+
753+
dependencies {
754+
// spring-cloud-starter-gateway-server-webflux:4.30 depends on reactor-netty-http:1.2.6
755+
implementation("org.springframework.cloud:spring-cloud-starter-gateway-server-webflux:4.3.0")
756+
implementation("io.projectreactor.netty:reactor-netty-http:1.1.13")
757+
}
758+
""",
759+
"""
760+
plugins {
761+
id "java"
762+
}
763+
764+
repositories {
765+
mavenCentral()
766+
}
767+
768+
dependencies {
769+
// spring-cloud-starter-gateway-server-webflux:4.30 depends on reactor-netty-http:1.2.6
770+
implementation("org.springframework.cloud:spring-cloud-starter-gateway-server-webflux:4.3.0")
771+
}
772+
"""
773+
)
774+
);
775+
}
735776
}

0 commit comments

Comments
 (0)