Skip to content

Commit eeacc5e

Browse files
committed
Revert "ChangeDependencyGroupIdAndArtifactId does not update properties in parent pom (#5815)"
This reverts commit faf51f5.
1 parent cba3447 commit eeacc5e

5 files changed

Lines changed: 165 additions & 340 deletions

File tree

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

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

31-
import java.nio.file.Path;
3231
import java.util.*;
3332

3433
import static java.util.Collections.max;
@@ -38,7 +37,7 @@
3837

3938
@Value
4039
@EqualsAndHashCode(callSuper = false)
41-
public class ChangeDependencyGroupIdAndArtifactId extends ScanningRecipe<ChangeDependencyGroupIdAndArtifactId.Accumulator> {
40+
public class ChangeDependencyGroupIdAndArtifactId extends Recipe {
4241
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
4342

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

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

157154
@Override
158155
public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
159156
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+
}
160165
return super.visitDocument(document, ctx);
161166
}
162167

@@ -165,59 +170,58 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
165170
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
166171
boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId);
167172
if (isOldDependencyTag && isNewDependencyPresent) {
168-
acc.isNewDependencyPresent = true;
173+
doAfterVisit(new RemoveContentVisitor<>(tag, true, true));
174+
maybeUpdateModel();
169175
return t;
170176
}
171177
if (isOldDependencyTag || isPluginDependencyTag(oldGroupId, oldArtifactId)) {
172-
Optional<String> groupIdFromTag = t.getChildValue("groupId");
173-
Optional<String> artifactIdFromTag = t.getChildValue("artifactId");
174-
if (!groupIdFromTag.isPresent() || !artifactIdFromTag.isPresent()) {
175-
return t;
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);
176183
}
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;
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);
190189
}
191-
190+
String currentVersion = t.getChildValue("version").orElse(null);
192191
if (newVersion != null) {
193192
try {
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);
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);
197196
Optional<Xml.Tag> versionTag = t.getChild("version");
198197

199198
boolean configuredToOverrideManageVersion = overrideManagedVersion != null && overrideManagedVersion; // False by default
200199
boolean configuredToChangeManagedDependency = changeManagedDependency == null || changeManagedDependency; // True by default
201200

201+
boolean versionTagPresent = versionTag.isPresent();
202202
boolean oldDependencyManaged = isDependencyManaged(scope, oldGroupId, oldArtifactId);
203203
boolean newDependencyManaged = isDependencyManaged(scope, groupId, artifactId);
204-
if (versionTag.isPresent()) {
204+
if (versionTagPresent) {
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-
acc.removeVersionTag = true;
207+
t = (Xml.Tag) new RemoveContentVisitor<>(versionTag.get(), false, true).visit(t, ctx);
208208
} else {
209209
// Otherwise, change the version to the new value.
210-
storeParentPomProperty(currentVersion.orElse(null), resolvedNewVersion);
211-
acc.changeVersion = resolvedNewVersion;
210+
t = changeChildTagValue(t, "version", resolvedNewVersion, ctx);
212211
}
213212
} else if (configuredToOverrideManageVersion || !newDependencyManaged) {
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;
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());
216217
}
217218
} catch (MavenDownloadingException e) {
218219
return e.warn(tag);
219220
}
220221
}
222+
if (t != tag) {
223+
maybeUpdateModel();
224+
}
221225
}
222226

223227
//noinspection ConstantConditions
@@ -245,122 +249,23 @@ private boolean isDependencyManaged(Scope scope, String groupId, String artifact
245249
}
246250

247251
@SuppressWarnings("ConstantConditions")
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-
}
252+
private String resolveSemverVersion(ExecutionContext ctx, String groupId, String artifactId, @Nullable String currentVersion) throws MavenDownloadingException {
253+
if (versionComparator == null) {
254+
return newVersion;
255255
}
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());
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);
303263
}
304264
}
305-
return t;
306-
}
307-
308-
boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId);
309-
if (isOldDependencyTag && acc.isNewDependencyPresent) {
310-
doAfterVisit(new RemoveContentVisitor<>(t, true, true));
311-
maybeUpdateModel();
312-
return t;
313-
}
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-
}
321265

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-
}
334266
}
335-
336-
return t;
267+
return availableVersions.isEmpty() ? newVersion : max(availableVersions, versionComparator);
337268
}
338269
};
339270
}
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-
}
366271
}

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

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

18-
import org.jetbrains.annotations.Contract;
1918
import org.jspecify.annotations.Nullable;
2019
import org.openrewrite.ExecutionContext;
2120
import org.openrewrite.SourceFile;
@@ -282,29 +281,19 @@ private boolean isTag(String name) {
282281
return getCursor().getValue() instanceof Xml.Tag && name.equals(getCursor().<Xml.Tag>getValue().getName());
283282
}
284283

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-
*/
299284
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) {
300289
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
301290
if (childTag.isPresent()) {
302291
String oldValue = childTag.get().getValue().orElse(null);
303292
if (newValue != null && !newValue.equals(oldValue)) {
304293
if (isProperty(oldValue)) {
305294
String propertyName = oldValue.substring(2, oldValue.length() - 1);
306295
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
307-
doAfterVisit((TreeVisitor<?, P>) new ChangePropertyValue(propertyName, newValue, false, false).getVisitor());
296+
doAfterVisit((TreeVisitor<?, P>) new ChangePropertyValue(propertyName, newValue, addPropertyIfMissing, false).getVisitor());
308297
}
309298
} else {
310299
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, p);
@@ -314,7 +303,6 @@ protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullabl
314303
return tag;
315304
}
316305

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

0 commit comments

Comments
 (0)