Skip to content

Commit faf51f5

Browse files
steve-aom-elliottjevanlingengithub-actions[bot]timtebeek
authored
ChangeDependencyGroupIdAndArtifactId does not update properties in parent pom (openrewrite#5815)
* Adding example showcasing problem * Impl * Apply suggestion from @github-actions[bot] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Polish * Polish * Polish * Polish * Polish * Polish * Fix retain versions for UpgradeDependencyVersion * Polish * Polish * Polish * Polish * Polish * Minor polish --------- Co-authored-by: lingenj <jacob.van.lingen@moderne.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 038f49f commit faf51f5

5 files changed

Lines changed: 340 additions & 165 deletions

File tree

‎rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java‎

Lines changed: 147 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.openrewrite.xml.RemoveContentVisitor;
2929
import org.openrewrite.xml.tree.Xml;
3030

31+
import java.nio.file.Path;
3132
import java.util.*;
3233

3334
import static java.util.Collections.max;
@@ -37,7 +38,7 @@
3738

3839
@Value
3940
@EqualsAndHashCode(callSuper = false)
40-
public class ChangeDependencyGroupIdAndArtifactId extends Recipe {
41+
public class ChangeDependencyGroupIdAndArtifactId extends ScanningRecipe<ChangeDependencyGroupIdAndArtifactId.Accumulator> {
4142
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
4243

4344
@Option(displayName = "Old groupId",
@@ -143,25 +144,19 @@ public Validated<Object> validate() {
143144
}
144145

145146
@Override
146-
public TreeVisitor<?, ExecutionContext> getVisitor() {
147+
public Accumulator getInitialValue(ExecutionContext ctx) {
148+
return new Accumulator();
149+
}
150+
151+
@Override
152+
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
147153
return new MavenVisitor<ExecutionContext>() {
148-
@Nullable
149-
final VersionComparator versionComparator = newVersion != null ? Semver.validate(newVersion, versionPattern).getValue() : null;
150-
@Nullable
151-
private Collection<String> availableVersions;
154+
final @Nullable VersionComparator versionComparator = newVersion != null ? Semver.validate(newVersion, versionPattern).getValue() : null;
152155
private boolean isNewDependencyPresent;
153156

154157
@Override
155158
public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
156159
isNewDependencyPresent = checkIfNewDependencyPresents(newGroupId, newArtifactId, newVersion);
157-
// Any managed dependency change is unlikely to use the same version, so only update selectively.
158-
if ((changeManagedDependency == null || changeManagedDependency) && newVersion != null || versionPattern != null) {
159-
doAfterVisit(new ChangeManagedDependencyGroupIdAndArtifactId(
160-
oldGroupId, oldArtifactId,
161-
Optional.ofNullable(newGroupId).orElse(oldGroupId),
162-
Optional.ofNullable(newArtifactId).orElse(oldArtifactId),
163-
newVersion, versionPattern).getVisitor());
164-
}
165160
return super.visitDocument(document, ctx);
166161
}
167162

@@ -170,58 +165,59 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
170165
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
171166
boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId);
172167
if (isOldDependencyTag && isNewDependencyPresent) {
173-
doAfterVisit(new RemoveContentVisitor<>(tag, true, true));
174-
maybeUpdateModel();
168+
acc.isNewDependencyPresent = true;
175169
return t;
176170
}
177171
if (isOldDependencyTag || isPluginDependencyTag(oldGroupId, oldArtifactId)) {
178-
String groupId = newGroupId;
179-
if (groupId != null) {
180-
t = changeChildTagValue(t, "groupId", groupId, ctx);
181-
} else {
182-
groupId = t.getChildValue("groupId").orElseThrow(NoSuchElementException::new);
172+
Optional<String> groupIdFromTag = t.getChildValue("groupId");
173+
Optional<String> artifactIdFromTag = t.getChildValue("artifactId");
174+
if (!groupIdFromTag.isPresent() || !artifactIdFromTag.isPresent()) {
175+
return t;
183176
}
184-
String artifactId = newArtifactId;
185-
if (artifactId != null) {
186-
t = changeChildTagValue(t, "artifactId", artifactId, ctx);
187-
} else {
188-
artifactId = t.getChildValue("artifactId").orElseThrow(NoSuchElementException::new);
177+
178+
String groupId = groupIdFromTag.get();
179+
if (newGroupId != null) {
180+
storeParentPomProperty(groupId, newGroupId);
181+
groupId = newGroupId;
182+
acc.changeGroupId = groupId;
183+
}
184+
185+
String artifactId = artifactIdFromTag.get();
186+
if (newArtifactId != null) {
187+
storeParentPomProperty(artifactId, newArtifactId);
188+
artifactId = newArtifactId;
189+
acc.changeArtifactId = artifactId;
189190
}
190-
String currentVersion = t.getChildValue("version").orElse(null);
191+
191192
if (newVersion != null) {
192193
try {
193-
String resolvedNewVersion = resolveSemverVersion(ctx, groupId, artifactId, currentVersion);
194-
Optional<Xml.Tag> scopeTag = t.getChild("scope");
195-
Scope scope = scopeTag.map(xml -> Scope.fromName(xml.getValue().orElse("compile"))).orElse(Scope.Compile);
194+
Optional<String> currentVersion = t.getChildValue("version");
195+
String resolvedNewVersion = resolveSemverVersion(groupId, artifactId, currentVersion.orElse(newVersion), ctx);
196+
Scope scope = t.getChild("scope").map(xml -> Scope.fromName(xml.getValue().orElse(null))).orElse(Scope.Compile);
196197
Optional<Xml.Tag> versionTag = t.getChild("version");
197198

198199
boolean configuredToOverrideManageVersion = overrideManagedVersion != null && overrideManagedVersion; // False by default
199200
boolean configuredToChangeManagedDependency = changeManagedDependency == null || changeManagedDependency; // True by default
200201

201-
boolean versionTagPresent = versionTag.isPresent();
202202
boolean oldDependencyManaged = isDependencyManaged(scope, oldGroupId, oldArtifactId);
203203
boolean newDependencyManaged = isDependencyManaged(scope, groupId, artifactId);
204-
if (versionTagPresent) {
204+
if (versionTag.isPresent()) {
205205
// If the previous dependency had a version but the new artifact is managed, removed the version tag.
206206
if (!configuredToOverrideManageVersion && newDependencyManaged || (oldDependencyManaged && configuredToChangeManagedDependency)) {
207-
t = (Xml.Tag) new RemoveContentVisitor<>(versionTag.get(), false, true).visit(t, ctx);
207+
acc.removeVersionTag = true;
208208
} else {
209209
// Otherwise, change the version to the new value.
210-
t = changeChildTagValue(t, "version", resolvedNewVersion, ctx);
210+
storeParentPomProperty(currentVersion.orElse(null), resolvedNewVersion);
211+
acc.changeVersion = resolvedNewVersion;
211212
}
212213
} else if (configuredToOverrideManageVersion || !newDependencyManaged) {
213-
//If the version is not present, add the version if we are explicitly overriding a managed version or if no managed version exists.
214-
Xml.Tag newVersionTag = Xml.Tag.build("<version>" + resolvedNewVersion + "</version>");
215-
//noinspection ConstantConditions
216-
t = (Xml.Tag) new AddToTagVisitor<ExecutionContext>(t, newVersionTag, new MavenTagInsertionComparator(t.getChildren())).visitNonNull(t, ctx, getCursor().getParent());
214+
// If the version is not present, add the version if we are explicitly overriding a managed version or if no managed version exists.
215+
acc.createVersion = resolvedNewVersion;
217216
}
218217
} catch (MavenDownloadingException e) {
219218
return e.warn(tag);
220219
}
221220
}
222-
if (t != tag) {
223-
maybeUpdateModel();
224-
}
225221
}
226222

227223
//noinspection ConstantConditions
@@ -249,23 +245,122 @@ private boolean isDependencyManaged(Scope scope, String groupId, String artifact
249245
}
250246

251247
@SuppressWarnings("ConstantConditions")
252-
private String resolveSemverVersion(ExecutionContext ctx, String groupId, String artifactId, @Nullable String currentVersion) throws MavenDownloadingException {
253-
if (versionComparator == null) {
254-
return newVersion;
248+
private String resolveSemverVersion(String groupId, String artifactId, String version, ExecutionContext ctx) throws MavenDownloadingException {
249+
List<String> availableVersions = new ArrayList<>();
250+
MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
251+
for (String v : mavenMetadata.getVersioning().getVersions()) {
252+
if (versionComparator.isValid(version, v)) {
253+
availableVersions.add(v);
254+
}
255255
}
256-
String finalCurrentVersion = currentVersion != null ? currentVersion : newVersion;
257-
if (availableVersions == null) {
258-
availableVersions = new ArrayList<>();
259-
MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
260-
for (String v : mavenMetadata.getVersioning().getVersions()) {
261-
if (versionComparator.isValid(finalCurrentVersion, v)) {
262-
availableVersions.add(v);
256+
return availableVersions.isEmpty() ? newVersion : max(availableVersions, versionComparator);
257+
}
258+
259+
private void storeParentPomProperty(@Nullable String currentValue, String newValue) {
260+
if (isProperty(currentValue)) {
261+
String name = currentValue.substring(2, currentValue.length() - 1).trim();
262+
if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(name)) {
263+
storeParentPomProperty(getResolutionResult(), name, newValue);
264+
}
265+
}
266+
}
267+
268+
private void storeParentPomProperty(MavenResolutionResult resolutionResult, String name, String value) {
269+
Pom pom = resolutionResult.getPom().getRequested();
270+
if (pom.getSourcePath() != null && pom.getProperties().containsKey(name)) {
271+
acc.pomProperties.add(new PomProperty(pom.getSourcePath(), name, value));
272+
} else if (resolutionResult.getParent() != null) {
273+
storeParentPomProperty(resolutionResult.getParent(), name, value);
274+
}
275+
}
276+
};
277+
}
278+
279+
@Override
280+
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
281+
return new MavenVisitor<ExecutionContext>() {
282+
@Override
283+
public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
284+
// Any managed dependency change is unlikely to use the same version, so only update selectively.
285+
if ((changeManagedDependency == null || changeManagedDependency) && newVersion != null || versionPattern != null) {
286+
doAfterVisit(new ChangeManagedDependencyGroupIdAndArtifactId(
287+
oldGroupId, oldArtifactId,
288+
Optional.ofNullable(newGroupId).orElse(oldGroupId),
289+
Optional.ofNullable(newArtifactId).orElse(oldArtifactId),
290+
newVersion, versionPattern).getVisitor());
291+
}
292+
return super.visitDocument(document, ctx);
293+
}
294+
295+
@Override
296+
public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
297+
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
298+
if (isPropertyTag()) {
299+
Path sourcePath = getResolutionResult().getPom().getRequested().getSourcePath();
300+
for (PomProperty prop : acc.pomProperties) {
301+
if (prop.filePath.equals(sourcePath)) {
302+
doAfterVisit(new ChangePropertyValue(prop.name, prop.value, false, false).getVisitor());
263303
}
264304
}
305+
return t;
306+
}
265307

308+
boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId);
309+
if (isOldDependencyTag && acc.isNewDependencyPresent) {
310+
doAfterVisit(new RemoveContentVisitor<>(t, true, true));
311+
maybeUpdateModel();
312+
return t;
266313
}
267-
return availableVersions.isEmpty() ? newVersion : max(availableVersions, versionComparator);
314+
if (isOldDependencyTag || isPluginDependencyTag(oldGroupId, oldArtifactId)) {
315+
if (acc.changeGroupId != null) {
316+
t = changeChildTagValue(t, "groupId", newGroupId, ctx);
317+
}
318+
if (acc.changeArtifactId != null) {
319+
t = changeChildTagValue(t, "artifactId", newArtifactId, ctx);
320+
}
321+
322+
if (acc.createVersion != null) {
323+
Xml.Tag newVersionTag = Xml.Tag.build("<version>" + acc.createVersion + "</version>");
324+
t = (Xml.Tag) new AddToTagVisitor<ExecutionContext>(t, newVersionTag, new MavenTagInsertionComparator(t.getChildren())).visitNonNull(t, ctx, getCursor().getParent());
325+
} else if (acc.changeVersion != null) {
326+
t = changeChildTagValue(t, "version", acc.changeVersion, ctx);
327+
} else if (acc.removeVersionTag) {
328+
t = (Xml.Tag) new RemoveContentVisitor<>(t.getChild("version").get(), false, true).visitNonNull(t, ctx);
329+
}
330+
331+
if (t != tag) {
332+
maybeUpdateModel();
333+
}
334+
}
335+
336+
return t;
268337
}
269338
};
270339
}
340+
341+
public static class Accumulator {
342+
Set<PomProperty> pomProperties = new HashSet<>();
343+
boolean isNewDependencyPresent;
344+
345+
@Nullable
346+
String changeGroupId;
347+
348+
@Nullable
349+
String changeArtifactId;
350+
351+
boolean removeVersionTag;
352+
353+
@Nullable
354+
String createVersion;
355+
356+
@Nullable
357+
String changeVersion;
358+
}
359+
360+
@Value
361+
public static class PomProperty {
362+
Path filePath;
363+
String name;
364+
String value;
365+
}
271366
}

‎rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.openrewrite.maven;
1717

18+
import org.jetbrains.annotations.Contract;
1819
import org.jspecify.annotations.Nullable;
1920
import org.openrewrite.ExecutionContext;
2021
import org.openrewrite.SourceFile;
@@ -281,19 +282,29 @@ private boolean isTag(String name) {
281282
return getCursor().getValue() instanceof Xml.Tag && name.equals(getCursor().<Xml.Tag>getValue().getName());
282283
}
283284

285+
/**
286+
* Updates the value of a given child tag within an XML tag, if the new value differs from the current one.
287+
* <p>
288+
* If the current value of the tag is a property (e.g. {@code ${some.property}}), and the property is defined
289+
* in the same POM, the method updates the property instead of directly modifying the tag value.
290+
* This method does not update properties defined in a parent POM.
291+
*
292+
* @param tag The XML tag that contains the child tag.
293+
* @param childTagName The name of the child tag whose value should be changed.
294+
* @param newValue The new value to assign to the child tag. If {@code null} or equal to the current value,
295+
* no change is made.
296+
* @param p The visitor context.
297+
* @return The potentially updated tag.
298+
*/
284299
protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullable String newValue, P p) {
285-
return changeChildTagValue(tag, childTagName, newValue, false, p);
286-
}
287-
288-
protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullable String newValue, @Nullable Boolean addPropertyIfMissing, P p) {
289300
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
290301
if (childTag.isPresent()) {
291302
String oldValue = childTag.get().getValue().orElse(null);
292303
if (newValue != null && !newValue.equals(oldValue)) {
293304
if (isProperty(oldValue)) {
294305
String propertyName = oldValue.substring(2, oldValue.length() - 1);
295306
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
296-
doAfterVisit((TreeVisitor<?, P>) new ChangePropertyValue(propertyName, newValue, addPropertyIfMissing, false).getVisitor());
307+
doAfterVisit((TreeVisitor<?, P>) new ChangePropertyValue(propertyName, newValue, false, false).getVisitor());
297308
}
298309
} else {
299310
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, p);
@@ -303,6 +314,7 @@ protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullabl
303314
return tag;
304315
}
305316

317+
@Contract("null -> false")
306318
protected boolean isProperty(@Nullable String value) {
307319
return value != null && value.startsWith("${") && !IMPLICITLY_DEFINED_VERSION_PROPERTIES.contains(value);
308320
}

0 commit comments

Comments
 (0)
⚡