Skip to content

Commit 9fe1fb5

Browse files
Jenson3210Jenson3210
authored
Mark parent tag in DependencyInsight when dependency is inherited (#6693)
* Add failing test for DependencyInsight parent tag marking When a dependency is inherited from an external parent POM, the <parent> tag should be marked by DependencyInsight. Currently only <dependency> tags are marked. Reproducer for moderneinc/customer-requests#1803 * Mark parent tag in DependencyInsight when dependency is inherited When a dependency is inherited from a parent POM (rather than declared directly in the current POM), DependencyInsight now marks the <parent> tag with the matching dependencies. This fixes the issue where preconditions like IsLikelySpringFramework would fail when Spring dependencies were only present through parent inheritance. Fixes moderneinc/customer-requests#1803 * Add test: parent tag not marked when child declares dependency Verifies that when a child POM declares its own dependency (even with version from parent's dependencyManagement), only the <dependency> tag is marked, not the <parent> tag. * Fix parent marking when target is reachable via multiple paths A target dependency might be reachable through multiple "direct" dependencies in Maven's resolution (e.g., spring-core via both spring-boot-starter-web and spring-web). The parent tag should only be marked if the target is NOT reachable through any declared dependency. Changed the algorithm to: 1. Collect all targets and track which are "covered" by declared deps 2. Only mark parent with targets that aren't covered Added test for transitive dependency coming from child's declared dependency. * Remove unnecessary comments from tests
1 parent 8315c06 commit 9fe1fb5

2 files changed

Lines changed: 263 additions & 0 deletions

File tree

‎rewrite-maven/src/main/java/org/openrewrite/maven/search/DependencyInsight.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ private static class MarkIndividualDependency extends MavenIsoVisitor<ExecutionC
200200
@Override
201201
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
202202
Xml.Tag t = super.visitTag(tag, ctx);
203+
204+
if (isParentTag()) {
205+
return markParentTag(t, ctx);
206+
}
207+
203208
if (!isDependencyTag()) {
204209
return t;
205210
}
@@ -237,5 +242,52 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
237242
}
238243
return t;
239244
}
245+
246+
private Xml.Tag markParentTag(Xml.Tag t, ExecutionContext ctx) {
247+
// Collect all target dependencies and track which are covered by declared dependencies.
248+
// A target is "covered" if it's reachable through ANY declared dependency.
249+
// Only mark parent with targets that aren't covered by any declared dependency.
250+
Set<String> coveredTargets = new HashSet<>();
251+
Set<String> allTargets = new HashSet<>();
252+
253+
for (Map.Entry<Scope, Set<GroupArtifactVersion>> entry : scopeToDirectDependency.entrySet()) {
254+
for (GroupArtifactVersion directGav : entry.getValue()) {
255+
Set<GroupArtifactVersion> targets = directDependencyToTargetDependency.get(directGav);
256+
if (targets != null) {
257+
boolean isDeclared = isDeclaredInCurrentPom(directGav);
258+
for (GroupArtifactVersion target : targets) {
259+
if (!Boolean.TRUE.equals(onlyDirect) || directGav.equals(target)) {
260+
String targetStr = target.getGroupId() + ":" + target.getArtifactId() + ":" + target.getVersion();
261+
allTargets.add(targetStr);
262+
if (isDeclared) {
263+
coveredTargets.add(targetStr);
264+
}
265+
}
266+
}
267+
}
268+
}
269+
}
270+
271+
// Only mark with targets that aren't covered by declared dependencies
272+
Set<String> inheritedTargets = new TreeSet<>(allTargets);
273+
inheritedTargets.removeAll(coveredTargets);
274+
275+
if (!inheritedTargets.isEmpty()) {
276+
return SearchResult.found(t, String.join(",", inheritedTargets));
277+
}
278+
return t;
279+
}
280+
281+
private boolean isDeclaredInCurrentPom(GroupArtifactVersion gav) {
282+
ResolvedPom pom = getResolutionResult().getPom();
283+
for (Dependency dep : pom.getRequested().getDependencies()) {
284+
String groupId = pom.getValue(dep.getGroupId());
285+
String artifactId = pom.getValue(dep.getArtifactId());
286+
if (gav.getGroupId().equals(groupId) && gav.getArtifactId().equals(artifactId)) {
287+
return true;
288+
}
289+
}
290+
return false;
291+
}
240292
}
241293
}

‎rewrite-maven/src/test/java/org/openrewrite/maven/search/DependencyInsightTest.java‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.openrewrite.test.RewriteTest;
2727

2828
import static org.assertj.core.api.Assertions.assertThat;
29+
import static org.openrewrite.java.Assertions.mavenProject;
2930
import static org.openrewrite.maven.Assertions.pomXml;
3031

3132
class DependencyInsightTest implements RewriteTest {
@@ -311,6 +312,216 @@ void findTwoDependenciesAndTheirDataTableRows() {
311312
);
312313
}
313314

315+
@Issue("https://github.com/moderneinc/customer-requests/issues/1803")
316+
@Test
317+
void findDependencyDeclaredInProjectParent() {
318+
rewriteRun(
319+
spec -> spec.recipe(new DependencyInsight("com.google.guava", "guava", null, null, null)),
320+
mavenProject("parent",
321+
pomXml(
322+
"""
323+
<project>
324+
<groupId>org.sample</groupId>
325+
<artifactId>parent</artifactId>
326+
<version>1.0.0</version>
327+
<packaging>pom</packaging>
328+
<modules>
329+
<module>child</module>
330+
</modules>
331+
<dependencies>
332+
<dependency>
333+
<groupId>com.google.guava</groupId>
334+
<artifactId>guava</artifactId>
335+
<version>29.0-jre</version>
336+
</dependency>
337+
</dependencies>
338+
</project>
339+
""",
340+
"""
341+
<project>
342+
<groupId>org.sample</groupId>
343+
<artifactId>parent</artifactId>
344+
<version>1.0.0</version>
345+
<packaging>pom</packaging>
346+
<modules>
347+
<module>child</module>
348+
</modules>
349+
<dependencies>
350+
<!--~~(com.google.guava:guava:29.0-jre)~~>--><dependency>
351+
<groupId>com.google.guava</groupId>
352+
<artifactId>guava</artifactId>
353+
<version>29.0-jre</version>
354+
</dependency>
355+
</dependencies>
356+
</project>
357+
"""
358+
),
359+
mavenProject("child",
360+
pomXml(
361+
"""
362+
<project>
363+
<parent>
364+
<groupId>org.sample</groupId>
365+
<artifactId>parent</artifactId>
366+
<version>1.0.0</version>
367+
<relativePath>../</relativePath>
368+
</parent>
369+
<artifactId>child</artifactId>
370+
</project>
371+
""",
372+
"""
373+
<project>
374+
<!--~~(com.google.guava:guava:29.0-jre)~~>--><parent>
375+
<groupId>org.sample</groupId>
376+
<artifactId>parent</artifactId>
377+
<version>1.0.0</version>
378+
<relativePath>../</relativePath>
379+
</parent>
380+
<artifactId>child</artifactId>
381+
</project>
382+
""",
383+
spec -> spec.path("child/pom.xml")
384+
)
385+
)
386+
)
387+
);
388+
}
389+
390+
@Issue("https://github.com/moderneinc/customer-requests/issues/1803")
391+
@Test
392+
void doNotMarkParentTagWhenDependencyDeclaredInChild() {
393+
rewriteRun(
394+
spec -> spec.recipe(new DependencyInsight("com.google.guava", "guava", null, null, null)),
395+
mavenProject("parent",
396+
pomXml(
397+
"""
398+
<project>
399+
<groupId>org.sample</groupId>
400+
<artifactId>parent</artifactId>
401+
<version>1.0.0</version>
402+
<packaging>pom</packaging>
403+
<modules>
404+
<module>child</module>
405+
</modules>
406+
<dependencyManagement>
407+
<dependencies>
408+
<dependency>
409+
<groupId>com.google.guava</groupId>
410+
<artifactId>guava</artifactId>
411+
<version>29.0-jre</version>
412+
</dependency>
413+
</dependencies>
414+
</dependencyManagement>
415+
</project>
416+
"""
417+
),
418+
mavenProject("child",
419+
pomXml(
420+
"""
421+
<project>
422+
<parent>
423+
<groupId>org.sample</groupId>
424+
<artifactId>parent</artifactId>
425+
<version>1.0.0</version>
426+
<relativePath>../</relativePath>
427+
</parent>
428+
<artifactId>child</artifactId>
429+
<dependencies>
430+
<dependency>
431+
<groupId>com.google.guava</groupId>
432+
<artifactId>guava</artifactId>
433+
</dependency>
434+
</dependencies>
435+
</project>
436+
""",
437+
"""
438+
<project>
439+
<parent>
440+
<groupId>org.sample</groupId>
441+
<artifactId>parent</artifactId>
442+
<version>1.0.0</version>
443+
<relativePath>../</relativePath>
444+
</parent>
445+
<artifactId>child</artifactId>
446+
<dependencies>
447+
<!--~~(com.google.guava:guava:29.0-jre)~~>--><dependency>
448+
<groupId>com.google.guava</groupId>
449+
<artifactId>guava</artifactId>
450+
</dependency>
451+
</dependencies>
452+
</project>
453+
""",
454+
spec -> spec.path("child/pom.xml")
455+
)
456+
)
457+
)
458+
);
459+
}
460+
461+
@Issue("https://github.com/moderneinc/customer-requests/issues/1803")
462+
@Test
463+
void doNotMarkParentTagWhenTransitiveDependencyComesFromChildDeclaredDependency() {
464+
rewriteRun(
465+
spec -> spec.recipe(new DependencyInsight("org.springframework", "spring-core", null, null, null)),
466+
mavenProject("parent",
467+
pomXml(
468+
"""
469+
<project>
470+
<groupId>org.sample</groupId>
471+
<artifactId>parent</artifactId>
472+
<version>1.0.0</version>
473+
<packaging>pom</packaging>
474+
<modules>
475+
<module>child</module>
476+
</modules>
477+
</project>
478+
"""
479+
),
480+
mavenProject("child",
481+
pomXml(
482+
"""
483+
<project>
484+
<parent>
485+
<groupId>org.sample</groupId>
486+
<artifactId>parent</artifactId>
487+
<version>1.0.0</version>
488+
<relativePath>../</relativePath>
489+
</parent>
490+
<artifactId>child</artifactId>
491+
<dependencies>
492+
<dependency>
493+
<groupId>org.springframework.boot</groupId>
494+
<artifactId>spring-boot-starter-web</artifactId>
495+
<version>2.7.18</version>
496+
</dependency>
497+
</dependencies>
498+
</project>
499+
""",
500+
"""
501+
<project>
502+
<parent>
503+
<groupId>org.sample</groupId>
504+
<artifactId>parent</artifactId>
505+
<version>1.0.0</version>
506+
<relativePath>../</relativePath>
507+
</parent>
508+
<artifactId>child</artifactId>
509+
<dependencies>
510+
<!--~~(org.springframework:spring-core:5.3.31)~~>--><dependency>
511+
<groupId>org.springframework.boot</groupId>
512+
<artifactId>spring-boot-starter-web</artifactId>
513+
<version>2.7.18</version>
514+
</dependency>
515+
</dependencies>
516+
</project>
517+
""",
518+
spec -> spec.path("child/pom.xml")
519+
)
520+
)
521+
)
522+
);
523+
}
524+
314525
@Test
315526
@Disabled("Test is logically correct, but the MavenResolutionResult's dependency graph is not")
316527
void jacksonIsFoundInternally() {

0 commit comments

Comments
 (0)
⚡