Skip to content

Commit 30e77ff

Browse files
Fix RemoveDependency.unlessUsing for multi-module Maven projects (#183)
* Fix `RemoveDependency.unlessUsing` for multi-module Maven projects `RemoveDependency` scans every source file and records whether `unlessUsing` matched against the file's `JavaProject` marker. In a multi-module Maven project the parent pom carries its own `JavaProject` distinct from the children, so the parent module's accumulator entry never reflects type usages found in child modules. The visitor would then strip the `<dependency>` from the parent pom even though child modules in the same reactor still relied on inheriting it. Now the scanner additionally records the `MavenResolutionResult.id -> JavaProject` mapping for every pom it visits. When the visitor is about to remove a `<dependency>` from a pom, it walks that pom's descendant modules via `MavenResolutionResult.getModules()` and preserves the declaration if any descendant's `JavaProject` shows the type in use. * Allow `RemoveDependency` to remove ancestor decl when descendants self-declare The descendant-aware preservation introduced in the previous commit was intentionally conservative: any descendant module whose Java sources used the `unlessUsing` type blocked removal of the dep from an ancestor pom. That over-preserves when a descendant module also declares the same dependency directly in its own `<dependencies>` — in that case the descendant is self-sufficient and the ancestor's declaration is dead weight. Refine the check: a descendant only blocks removal if it uses the type AND does not declare the dependency itself. The self-declaration check looks at the raw `requested` pom (as-written), not the resolved/inherited dependency list, so a child that inherits the dep from the ancestor we might modify still correctly blocks removal.
1 parent 5559325 commit 30e77ff

2 files changed

Lines changed: 398 additions & 8 deletions

File tree

src/main/java/org/openrewrite/java/dependencies/RemoveDependency.java

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,18 @@
2121
import org.openrewrite.*;
2222
import org.openrewrite.java.marker.JavaProject;
2323
import org.openrewrite.java.search.UsesType;
24+
import org.openrewrite.maven.tree.Dependency;
25+
import org.openrewrite.maven.tree.MavenResolutionResult;
2426

2527
import java.util.HashMap;
2628
import java.util.Map;
29+
import java.util.UUID;
30+
31+
import static org.openrewrite.internal.StringUtils.matchesGlob;
2732

2833
@EqualsAndHashCode(callSuper = false)
2934
@Value
30-
public class RemoveDependency extends ScanningRecipe<Map<JavaProject, Boolean>> {
35+
public class RemoveDependency extends ScanningRecipe<RemoveDependency.Accumulator> {
3136
@Option(displayName = "Group ID",
3237
description = "The first part of a dependency coordinate `com.google.guava:guava:VERSION`. This can be a glob expression.",
3338
example = "com.fasterxml.jackson*")
@@ -68,13 +73,18 @@ public class RemoveDependency extends ScanningRecipe<Map<JavaProject, Boolean>>
6873
String description = "For Gradle project, removes a single dependency from the dependencies section of the `build.gradle`.\n" +
6974
"For Maven project, removes a single dependency from the `<dependencies>` section of the pom.xml.";
7075

76+
public static class Accumulator {
77+
final Map<JavaProject, Boolean> projectToInUse = new HashMap<>();
78+
final Map<UUID, JavaProject> mavenIdToProject = new HashMap<>();
79+
}
80+
7181
@Override
72-
public Map<JavaProject, Boolean> getInitialValue(ExecutionContext ctx) {
73-
return new HashMap<>();
82+
public Accumulator getInitialValue(ExecutionContext ctx) {
83+
return new Accumulator();
7484
}
7585

7686
@Override
77-
public TreeVisitor<?, ExecutionContext> getScanner(Map<JavaProject, Boolean> projectToInUse) {
87+
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
7888
if (unlessUsing == null) {
7989
return TreeVisitor.noop();
8090
}
@@ -83,15 +93,19 @@ public TreeVisitor<?, ExecutionContext> getScanner(Map<JavaProject, Boolean> pro
8393
@Override
8494
public Tree preVisit(Tree tree, ExecutionContext ctx) {
8595
stopAfterPreVisit();
86-
tree.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject ->
87-
projectToInUse.compute(javaProject, (jp, foundSoFar) -> Boolean.TRUE.equals(foundSoFar) || tree != usesType.visit(tree, ctx)));
96+
tree.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> {
97+
acc.projectToInUse.compute(javaProject, (jp, foundSoFar) ->
98+
Boolean.TRUE.equals(foundSoFar) || tree != usesType.visit(tree, ctx));
99+
tree.getMarkers().findFirst(MavenResolutionResult.class).ifPresent(mrr ->
100+
acc.mavenIdToProject.put(mrr.getId(), javaProject));
101+
});
88102
return tree;
89103
}
90104
};
91105
}
92106

93107
@Override
94-
public TreeVisitor<?, ExecutionContext> getVisitor(Map<JavaProject, Boolean> projectToInUse) {
108+
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
95109
return new TreeVisitor<Tree, ExecutionContext>() {
96110
final TreeVisitor<?, ExecutionContext> gradleRemoveDep = new org.openrewrite.gradle.RemoveDependency(groupId, artifactId, configuration).getVisitor();
97111
final TreeVisitor<?, ExecutionContext> mavenRemoveDep = new org.openrewrite.maven.RemoveDependency(groupId, artifactId, scope).getVisitor();
@@ -103,7 +117,11 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Map<JavaProject, Boolean> pro
103117
}
104118
if (unlessUsing != null) {
105119
JavaProject jp = tree.getMarkers().findFirst(JavaProject.class).orElse(null);
106-
if (jp == null || Boolean.TRUE.equals(projectToInUse.get(jp))) {
120+
if (jp == null || Boolean.TRUE.equals(acc.projectToInUse.get(jp))) {
121+
return tree;
122+
}
123+
MavenResolutionResult mrr = tree.getMarkers().findFirst(MavenResolutionResult.class).orElse(null);
124+
if (mrr != null && anyDescendantPreservesDependency(mrr, acc)) {
107125
return tree;
108126
}
109127
}
@@ -118,4 +136,35 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Map<JavaProject, Boolean> pro
118136
}
119137
};
120138
}
139+
140+
/**
141+
* Keep this pom's `<dependency>` declaration if any descendant module in the reactor uses the
142+
* `unlessUsing` type AND does not also declare the dependency itself. A descendant that
143+
* self-declares the dependency in its own pom is unaffected by removing it from an ancestor,
144+
* so it should not block removal.
145+
*/
146+
private boolean anyDescendantPreservesDependency(MavenResolutionResult mrr, Accumulator acc) {
147+
for (MavenResolutionResult child : mrr.getModules()) {
148+
JavaProject childJp = acc.mavenIdToProject.get(child.getId());
149+
if (childJp != null && Boolean.TRUE.equals(acc.projectToInUse.get(childJp)) && !declaresDependency(child)) {
150+
return true;
151+
}
152+
if (anyDescendantPreservesDependency(child, acc)) {
153+
return true;
154+
}
155+
}
156+
return false;
157+
}
158+
159+
private boolean declaresDependency(MavenResolutionResult mrr) {
160+
// Look at the raw `requested` pom — the as-written `<dependencies>` of this module only,
161+
// excluding inheritance from a parent pom. A module that inherits the dep from the parent
162+
// we may be modifying should still block removal.
163+
for (Dependency d : mrr.getPom().getRequested().getDependencies()) {
164+
if (matchesGlob(d.getGroupId(), groupId) && matchesGlob(d.getArtifactId(), artifactId)) {
165+
return true;
166+
}
167+
}
168+
return false;
169+
}
121170
}

0 commit comments

Comments
 (0)