Skip to content

Commit a05505c

Browse files
timtebeekJenson3210
timtebeek
and
Jenson3210
authored
Trust resolved version over declared dep in dependency search (#181)
* Trust resolved version over declared dep in dependency search PR #179 extended ModuleHasDependency / RepositoryHasDependency to also match against requested/declared dependencies so they still match when resolution fails. This introduced false positives whenever a version range is supplied: the declared version string could satisfy the comparator even when the resolved version did not (e.g. a Gradle resolutionStrategy.force or platform alignment overriding the declared coordinate). - When a coordinate is present in the resolved dependencies, trust the resolved check and skip the declared-dependency fallback for that group:artifact. Only fall back for declared deps not already resolved. - Tighten versionMatches so a null declared version no longer auto-matches when a version constraint is supplied (same treatment as a ${...} property reference). Adds regression tests covering both rules in ModuleHasDependencyTest and RepositoryHasDependencyTest. * Apply suggestions from code review Co-authored-by: Jente Sondervorst <jentesondervorst@gmail.com> * Add Maven regression tests for BOM-managed dependency version range --------- Co-authored-by: Jente Sondervorst <jentesondervorst@gmail.com>
1 parent 64771c1 commit a05505c

4 files changed

Lines changed: 219 additions & 8 deletions

File tree

‎src/main/java/org/openrewrite/java/dependencies/search/ModuleHasDependency.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,17 @@ private boolean hasDependency(Tree tree) {
112112
if (mavenResult != null) {
113113
Scope requestedScope = scope == null ? null : Scope.fromName(scope);
114114
List<ResolvedDependency> dependencies = mavenResult.findDependencies(groupIdPattern, artifactIdPattern, requestedScope);
115+
Set<String> resolvedGAs = new HashSet<>();
115116
for (ResolvedDependency dependency : dependencies) {
117+
resolvedGAs.add(dependency.getGroupId() + ":" + dependency.getArtifactId());
116118
if (versionComparator == null || versionComparator.isValid(null, dependency.getVersion())) {
117119
return true;
118120
}
119121
}
120122
for (Dependency requested : mavenResult.getPom().getRequestedDependencies()) {
123+
if (resolvedGAs.contains(requested.getGroupId() + ":" + requested.getArtifactId())) {
124+
continue;
125+
}
121126
if (matchesRequested(requested, requestedScope, versionComparator)) {
122127
return true;
123128
}
@@ -127,16 +132,23 @@ private boolean hasDependency(Tree tree) {
127132

128133
GradleProject gp = tree.getMarkers().findFirst(GradleProject.class).orElse(null);
129134
if (gp != null) {
135+
Set<String> resolvedGAs = new HashSet<>();
130136
for (GradleDependencyConfiguration c : gp.getConfigurations()) {
131137
for (ResolvedDependency resolvedDependency : c.getDirectResolved()) {
132138
ResolvedDependency found = resolvedDependency.findDependency(groupIdPattern, artifactIdPattern);
133-
if (found != null && (versionComparator == null || versionComparator.isValid(null, found.getVersion()))) {
134-
return true;
139+
if (found != null) {
140+
resolvedGAs.add(found.getGroupId() + ":" + found.getArtifactId());
141+
if (versionComparator == null || versionComparator.isValid(null, found.getVersion())) {
142+
return true;
143+
}
135144
}
136145
}
137146
}
138147
for (GradleDependencyConfiguration c : gp.getConfigurations()) {
139148
for (Dependency requested : c.getRequested()) {
149+
if (resolvedGAs.contains(requested.getGroupId() + ":" + requested.getArtifactId())) {
150+
continue;
151+
}
140152
if (matchesRequested(requested, null, versionComparator)) {
141153
return true;
142154
}
@@ -166,10 +178,10 @@ private boolean matchesRequested(Dependency dep, @Nullable Scope requestedScope,
166178
}
167179

168180
private static boolean versionMatches(@Nullable String version, @Nullable VersionComparator cmp) {
169-
if (cmp == null || version == null) {
181+
if (cmp == null) {
170182
return true;
171183
}
172-
if (version.startsWith("${")) {
184+
if (version == null || version.startsWith("${")) {
173185
return false;
174186
}
175187
return cmp.isValid(null, version);

‎src/main/java/org/openrewrite/java/dependencies/search/RepositoryHasDependency.java‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
import org.openrewrite.semver.Semver;
3232
import org.openrewrite.semver.VersionComparator;
3333

34+
import java.util.HashSet;
3435
import java.util.List;
36+
import java.util.Set;
3537
import java.util.concurrent.atomic.AtomicBoolean;
3638

3739
@EqualsAndHashCode(callSuper = false)
@@ -107,12 +109,17 @@ private boolean hasDependency(Tree tree) {
107109
if (mavenResult != null) {
108110
Scope requestedScope = scope == null ? null : Scope.fromName(scope);
109111
List<ResolvedDependency> dependencies = mavenResult.findDependencies(groupIdPattern, artifactIdPattern, requestedScope);
112+
Set<String> resolvedGAs = new HashSet<>();
110113
for (ResolvedDependency dependency : dependencies) {
114+
resolvedGAs.add(dependency.getGroupId() + ":" + dependency.getArtifactId());
111115
if (versionComparator == null || versionComparator.isValid(null, dependency.getVersion())) {
112116
return true;
113117
}
114118
}
115119
for (Dependency requested : mavenResult.getPom().getRequestedDependencies()) {
120+
if (resolvedGAs.contains(requested.getGroupId() + ":" + requested.getArtifactId())) {
121+
continue;
122+
}
116123
if (matchesRequested(requested, requestedScope, versionComparator)) {
117124
return true;
118125
}
@@ -122,16 +129,23 @@ private boolean hasDependency(Tree tree) {
122129

123130
GradleProject gp = tree.getMarkers().findFirst(GradleProject.class).orElse(null);
124131
if (gp != null) {
132+
Set<String> resolvedGAs = new HashSet<>();
125133
for (GradleDependencyConfiguration c : gp.getConfigurations()) {
126134
for (ResolvedDependency resolvedDependency : c.getDirectResolved()) {
127135
ResolvedDependency found = resolvedDependency.findDependency(groupIdPattern, artifactIdPattern);
128-
if (found != null && (versionComparator == null || versionComparator.isValid(null, found.getVersion()))) {
129-
return true;
136+
if (found != null) {
137+
resolvedGAs.add(found.getGroupId() + ":" + found.getArtifactId());
138+
if (versionComparator == null || versionComparator.isValid(null, found.getVersion())) {
139+
return true;
140+
}
130141
}
131142
}
132143
}
133144
for (GradleDependencyConfiguration c : gp.getConfigurations()) {
134145
for (Dependency requested : c.getRequested()) {
146+
if (resolvedGAs.contains(requested.getGroupId() + ":" + requested.getArtifactId())) {
147+
continue;
148+
}
135149
if (matchesRequested(requested, null, versionComparator)) {
136150
return true;
137151
}
@@ -161,10 +175,10 @@ private boolean matchesRequested(Dependency dep, @Nullable Scope requestedScope,
161175
}
162176

163177
private static boolean versionMatches(@Nullable String version, @Nullable VersionComparator cmp) {
164-
if (cmp == null || version == null) {
178+
if (cmp == null) {
165179
return true;
166180
}
167-
if (version.startsWith("${")) {
181+
if (version == null || version.startsWith("${")) {
168182
return false;
169183
}
170184
return cmp.isValid(null, version);

‎src/test/java/org/openrewrite/java/dependencies/search/ModuleHasDependencyTest.java‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,100 @@ void gradleVersionRangeOnRequestedDoesNotMatchWhenOutOfRange() {
454454
)
455455
);
456456
}
457+
458+
@Language("groovy")
459+
private final static String GradleNoRepositoriesNoVersion = """
460+
plugins {
461+
id 'java-library'
462+
}
463+
dependencies {
464+
implementation 'org.springframework:spring-beans'
465+
}
466+
""";
467+
468+
@Test
469+
void gradleRequestedWithoutVersionAndConstraintDoesNotMatch() {
470+
// Force resolution failure (no repositories), so the requested fallback fires.
471+
rewriteRun(
472+
spec -> spec.recipe(new ModuleHasDependency(GroupId, ArtifactId, null, "[1.0,)", null)),
473+
mavenProject("project-gradle",
474+
buildGradle(GradleNoRepositoriesNoVersion),
475+
java(GradleJava)
476+
)
477+
);
478+
}
479+
}
480+
481+
@Nested
482+
class WhenResolvedVersionIsSourceOfTruth {
483+
484+
@Language("groovy")
485+
private final static String GradleForcedOutOfRange = """
486+
plugins {
487+
id 'java-library'
488+
}
489+
repositories {
490+
mavenCentral()
491+
}
492+
configurations.all {
493+
resolutionStrategy {
494+
force 'org.springframework:spring-beans:6.0.0'
495+
}
496+
}
497+
dependencies {
498+
implementation 'org.springframework:spring-beans:5.3.0'
499+
}
500+
""";
501+
502+
@Test
503+
void gradleVersionRangeDoesNotMatchDeclaredWhenResolvedVersionIsOutOfRange() {
504+
// The declared-dependency fallback must be skipped for an already-resolved coordinate (resolutionStrategy).
505+
rewriteRun(
506+
spec -> spec.recipe(new ModuleHasDependency(GroupId, ArtifactId, null, "[5.0,6.0)", null)),
507+
mavenProject("project-gradle",
508+
buildGradle(GradleForcedOutOfRange),
509+
java(GradleJava)
510+
)
511+
);
512+
}
513+
514+
@Language("xml")
515+
private final static String MavenBomManagedOutOfRange = """
516+
<project>
517+
<groupId>com.example</groupId>
518+
<artifactId>foo</artifactId>
519+
<version>1.0.0</version>
520+
<dependencyManagement>
521+
<dependencies>
522+
<dependency>
523+
<groupId>org.springframework.boot</groupId>
524+
<artifactId>spring-boot-dependencies</artifactId>
525+
<version>3.0.0</version>
526+
<type>pom</type>
527+
<scope>import</scope>
528+
</dependency>
529+
</dependencies>
530+
</dependencyManagement>
531+
<dependencies>
532+
<dependency>
533+
<groupId>org.springframework</groupId>
534+
<artifactId>spring-beans</artifactId>
535+
</dependency>
536+
</dependencies>
537+
</project>
538+
""";
539+
540+
@Test
541+
void mavenVersionRangeDoesNotMatchBomManagedDependencyWhenResolvedIsOutOfRange() {
542+
// Regression for rewrite-third-party#76: BOM-managed version (null on requested) must not match the range.
543+
rewriteRun(
544+
spec -> spec.recipe(new ModuleHasDependency(GroupId, ArtifactId, null, "[5.0,6.0)", null)),
545+
mavenProject("project-maven",
546+
pomXml(MavenBomManagedOutOfRange),
547+
java(MavenJava)
548+
)
549+
);
550+
}
457551
}
458552

459553
@Nested

‎src/test/java/org/openrewrite/java/dependencies/search/RepositoryHasDependencyTest.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
*/
1616
package org.openrewrite.java.dependencies.search;
1717

18+
import org.intellij.lang.annotations.Language;
1819
import org.junit.jupiter.api.Test;
1920
import org.openrewrite.DocumentExample;
2021
import org.openrewrite.test.RecipeSpec;
2122
import org.openrewrite.test.RewriteTest;
2223

24+
import static org.openrewrite.gradle.Assertions.buildGradle;
2325
import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi;
26+
import static org.openrewrite.java.Assertions.java;
2427
import static org.openrewrite.java.Assertions.mavenProject;
2528
import static org.openrewrite.maven.Assertions.pomXml;
2629

@@ -84,4 +87,92 @@ void usedAsDeclarativePrecondition() {
8487
)
8588
);
8689
}
90+
91+
@Language("java")
92+
private final static String GradleJava = """
93+
public class AGradle {}
94+
""";
95+
96+
@Test
97+
void gradleVersionRangeDoesNotMatchDeclaredWhenResolvedVersionIsOutOfRange() {
98+
// The declared-dependency fallback must be skipped for an already-resolved coordinate (resolutionStrategy).
99+
rewriteRun(
100+
spec -> spec.recipe(new RepositoryHasDependency("org.springframework", "spring-beans", null, "[5.0,6.0)")),
101+
mavenProject("project-gradle",
102+
//language=groovy
103+
buildGradle("""
104+
plugins {
105+
id 'java-library'
106+
}
107+
repositories {
108+
mavenCentral()
109+
}
110+
configurations.all {
111+
resolutionStrategy {
112+
force 'org.springframework:spring-beans:6.0.0'
113+
}
114+
}
115+
dependencies {
116+
implementation 'org.springframework:spring-beans:5.3.0'
117+
}
118+
"""),
119+
java(GradleJava)
120+
)
121+
);
122+
}
123+
124+
@Test
125+
void mavenVersionRangeDoesNotMatchBomManagedDependencyWhenResolvedIsOutOfRange() {
126+
// Regression for rewrite-third-party#76: BOM-managed version (null on requested) must not match the range.
127+
rewriteRun(
128+
spec -> spec.recipe(new RepositoryHasDependency("org.springframework", "spring-beans", null, "[5.0,6.0)")),
129+
mavenProject("project-maven",
130+
//language=xml
131+
pomXml("""
132+
<project>
133+
<groupId>com.example</groupId>
134+
<artifactId>foo</artifactId>
135+
<version>1.0.0</version>
136+
<dependencyManagement>
137+
<dependencies>
138+
<dependency>
139+
<groupId>org.springframework.boot</groupId>
140+
<artifactId>spring-boot-dependencies</artifactId>
141+
<version>3.0.0</version>
142+
<type>pom</type>
143+
<scope>import</scope>
144+
</dependency>
145+
</dependencies>
146+
</dependencyManagement>
147+
<dependencies>
148+
<dependency>
149+
<groupId>org.springframework</groupId>
150+
<artifactId>spring-beans</artifactId>
151+
</dependency>
152+
</dependencies>
153+
</project>
154+
""")
155+
)
156+
);
157+
}
158+
159+
@Test
160+
void gradleRequestedWithoutVersionAndConstraintDoesNotMatch() {
161+
// Force resolution failure (no repositories), so the requested fallback fires.
162+
rewriteRun(
163+
spec -> spec.recipe(new RepositoryHasDependency("org.springframework", "spring-beans", null, "[1.0,)")),
164+
mavenProject("project-gradle",
165+
//language=groovy
166+
buildGradle("""
167+
plugins {
168+
id 'java-library'
169+
}
170+
dependencies {
171+
implementation 'org.springframework:spring-beans'
172+
}
173+
"""),
174+
java(GradleJava)
175+
)
176+
);
177+
}
87178
}

0 commit comments

Comments
 (0)