Skip to content

Commit fa0b361

Browse files
Jenson3210timtebeekgithub-actions[bot]
authored
Match upstream Maven recipe changes to ChangeDependency (#151)
* Fix ChangeDependencyTest.changeMavenDependency() test failure The upstream Maven recipe was converted to a ScanningRecipe, which requires updating ChangeDependency to also be a ScanningRecipe to properly invoke the scanning phase. This change delegates to the Maven recipe's scanning implementation to fix the failing test. * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * rename method * Adapt RelocatedDependencyCheck to use ChangeDependency as ScanningRecipe Extract a helper method to properly invoke ChangeDependency's scanning phase only for the current file, maintaining the existing behavior while accommodating the recipe's conversion to ScanningRecipe. * Extract a method for Gradle too; check Maven acceptable first (cheaper) --------- Co-authored-by: Tim te Beek <tim@moderne.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent eaaf618 commit fa0b361

2 files changed

Lines changed: 52 additions & 25 deletions

File tree

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

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@
1818
import lombok.AllArgsConstructor;
1919
import lombok.EqualsAndHashCode;
2020
import lombok.Getter;
21+
import lombok.Value;
2122
import org.jspecify.annotations.Nullable;
2223
import org.openrewrite.*;
24+
import org.openrewrite.maven.ChangeDependencyGroupIdAndArtifactId;
2325

2426
@AllArgsConstructor
2527
@EqualsAndHashCode(callSuper = false)
2628
@Getter
27-
public class ChangeDependency extends Recipe {
29+
public class ChangeDependency extends ScanningRecipe<ChangeDependency.Accumulator> {
2830
// Gradle and Maven shared parameters
2931
@Option(displayName = "Old group ID",
3032
description = "The old group ID to replace. The group ID is the first part of a dependency coordinate 'com.google.guava:guava:VERSION'. Supports glob expressions.",
@@ -90,25 +92,30 @@ public String getDescription() {
9092
@Override
9193
public Validated<Object> validate(ExecutionContext ctx) {
9294
return super.validate(ctx)
93-
.and(((Recipe) new org.openrewrite.gradle.ChangeDependency(
94-
oldGroupId, oldArtifactId, newGroupId, newArtifactId, newVersion, versionPattern, overrideManagedVersion, changeManagedDependency)).validate())
95-
.and(((Recipe) new org.openrewrite.maven.ChangeDependencyGroupIdAndArtifactId(
96-
oldGroupId, oldArtifactId, newGroupId, newArtifactId, newVersion, versionPattern, overrideManagedVersion, changeManagedDependency)).validate());
95+
.and(getChangeMavenDependency().validate())
96+
.and(getChangeGradleDependency().validate());
97+
}
98+
99+
@Value
100+
public static class Accumulator {
101+
ChangeDependencyGroupIdAndArtifactId.Accumulator mavenAccumulator;
102+
}
103+
104+
@Override
105+
public Accumulator getInitialValue(ExecutionContext ctx) {
106+
return new Accumulator(getChangeMavenDependency().getInitialValue(ctx));
107+
}
108+
109+
@Override
110+
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
111+
return getChangeMavenDependency().getScanner(acc.getMavenAccumulator());
97112
}
98113

99114
@Override
100-
public TreeVisitor<?, ExecutionContext> getVisitor() {
115+
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
101116
return new TreeVisitor<Tree, ExecutionContext>() {
102-
final TreeVisitor<?, ExecutionContext> mavenVisitor = new org.openrewrite.maven.ChangeDependencyGroupIdAndArtifactId(
103-
oldGroupId, oldArtifactId,
104-
newGroupId, newArtifactId,
105-
newVersion, versionPattern,
106-
overrideManagedVersion, changeManagedDependency).getVisitor();
107-
final TreeVisitor<?, ExecutionContext> gradleVisitor = new org.openrewrite.gradle.ChangeDependency(
108-
oldGroupId, oldArtifactId,
109-
newGroupId, newArtifactId,
110-
newVersion, versionPattern,
111-
overrideManagedVersion).getVisitor();
117+
final TreeVisitor<?, ExecutionContext> mavenVisitor = getChangeMavenDependency().getVisitor(acc.getMavenAccumulator());
118+
final TreeVisitor<?, ExecutionContext> gradleVisitor = getChangeGradleDependency().getVisitor();
112119

113120
@Override
114121
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
@@ -121,13 +128,29 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
121128
return tree;
122129
}
123130
SourceFile s = (SourceFile) tree;
124-
if (gradleVisitor.isAcceptable(s, ctx)) {
125-
s = (SourceFile) gradleVisitor.visitNonNull(s, ctx);
126-
} else if (mavenVisitor.isAcceptable(s, ctx)) {
131+
if (mavenVisitor.isAcceptable(s, ctx)) {
127132
s = (SourceFile) mavenVisitor.visitNonNull(s, ctx);
133+
} else if (gradleVisitor.isAcceptable(s, ctx)) {
134+
s = (SourceFile) gradleVisitor.visitNonNull(s, ctx);
128135
}
129136
return s;
130137
}
131138
};
132139
}
140+
141+
private org.openrewrite.gradle.ChangeDependency getChangeGradleDependency() {
142+
return new org.openrewrite.gradle.ChangeDependency(
143+
oldGroupId, oldArtifactId,
144+
newGroupId, newArtifactId,
145+
newVersion, versionPattern,
146+
overrideManagedVersion, changeManagedDependency);
147+
}
148+
149+
private ChangeDependencyGroupIdAndArtifactId getChangeMavenDependency() {
150+
return new ChangeDependencyGroupIdAndArtifactId(
151+
oldGroupId, oldArtifactId,
152+
newGroupId, newArtifactId,
153+
newVersion, versionPattern,
154+
overrideManagedVersion, changeManagedDependency);
155+
}
133156
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,7 @@ private <T extends Tree> T maybeUpdate(T tree, String groupId, @Nullable String
221221
if (Boolean.TRUE.equals(changeDependencies) && artifactId != null) {
222222
String newGroupId = relocation.getTo().getGroupId();
223223
String newArtifactId = Optional.ofNullable(relocation.getTo().getArtifactId()).orElse(artifactId);
224-
doAfterVisit(new ChangeDependency(
225-
groupId, artifactId, newGroupId, newArtifactId,
226-
"latest.release", null, null, null).getVisitor());
224+
doAfterVisit(changeDependency(this, groupId, artifactId, newGroupId, newArtifactId, ctx));
227225
} else {
228226
return getSearchResultFound(tree, relocation);
229227
}
@@ -265,9 +263,7 @@ private <T extends Tree> T maybeUpdate(T tree, String groupId, @Nullable String
265263
if (Boolean.TRUE.equals(changeDependencies) && artifactId != null) {
266264
String newGroupId = relocation.getTo().getGroupId();
267265
String newArtifactId = Optional.ofNullable(relocation.getTo().getArtifactId()).orElse(artifactId);
268-
doAfterVisit(new ChangeDependency(
269-
groupId, artifactId, newGroupId, newArtifactId,
270-
"latest.release", null, null, null).getVisitor());
266+
doAfterVisit(changeDependency(this, groupId, artifactId, newGroupId, newArtifactId, ctx));
271267
} else {
272268
return getSearchResultFound(tree, relocation);
273269
}
@@ -304,4 +300,12 @@ private <T extends Tree> T getSearchResultFound(T tree, Relocation relocation) {
304300
}
305301
};
306302
}
303+
304+
private static TreeVisitor<?, ExecutionContext> changeDependency(TreeVisitor<?, ExecutionContext> visitor, String oldGroupId, String oldArtifactId, String newGroupId, String newArtifactId, ExecutionContext ctx) {
305+
//call scanner only for the current file as we need the recipe to behave like a non-scanning recipe for now. Later we can change this to support parent poms etc...
306+
ChangeDependency changeDependency = new ChangeDependency(oldGroupId, oldArtifactId, newGroupId, newArtifactId, "latest.release", null, null, null);
307+
ChangeDependency.Accumulator acc = changeDependency.getInitialValue(ctx);
308+
changeDependency.getScanner(acc).visit(visitor.getCursor().firstEnclosingOrThrow(SourceFile.class), ctx);
309+
return changeDependency.getVisitor(acc);
310+
}
307311
}

0 commit comments

Comments
 (0)