Skip to content

Commit cbe16ac

Browse files
timtebeektimtebeek
authored
Restore generalized property handling (#5517)
* Revert "Revert generalized maven recipe property handling (#5516)" This reverts commit ae563f8. * Restore additional version properties
1 parent 5158023 commit cbe16ac

File tree

6 files changed

+76
-88
lines changed

6 files changed

+76
-88
lines changed

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.openrewrite.semver.Semver;
2626
import org.openrewrite.semver.VersionComparator;
2727
import org.openrewrite.xml.AddToTagVisitor;
28-
import org.openrewrite.xml.ChangeTagValueVisitor;
2928
import org.openrewrite.xml.RemoveContentVisitor;
3029
import org.openrewrite.xml.tree.Xml;
3130

@@ -178,13 +177,13 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
178177
if (isOldDependencyTag || isPluginDependencyTag(oldGroupId, oldArtifactId)) {
179178
String groupId = newGroupId;
180179
if (groupId != null) {
181-
t = changeChildTagValue2(t, "groupId", groupId, ctx);
180+
t = changeChildTagValue(t, "groupId", groupId, ctx);
182181
} else {
183182
groupId = t.getChildValue("groupId").orElseThrow(NoSuchElementException::new);
184183
}
185184
String artifactId = newArtifactId;
186185
if (artifactId != null) {
187-
t = changeChildTagValue2(t, "artifactId", artifactId, ctx);
186+
t = changeChildTagValue(t, "artifactId", artifactId, ctx);
188187
} else {
189188
artifactId = t.getChildValue("artifactId").orElseThrow(NoSuchElementException::new);
190189
}
@@ -208,7 +207,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
208207
t = (Xml.Tag) new RemoveContentVisitor<>(versionTag.get(), false, true).visit(t, ctx);
209208
} else {
210209
// Otherwise, change the version to the new value.
211-
t = changeChildTagValue2(t, "version", resolvedNewVersion, ctx);
210+
t = changeChildTagValue(t, "version", resolvedNewVersion, ctx);
212211
}
213212
} else if (configuredToOverrideManageVersion || !newDependencyManaged) {
214213
//If the version is not present, add the version if we are explicitly overriding a managed version or if no managed version exists.
@@ -239,17 +238,7 @@ private boolean checkIfNewDependencyPresents(@Nullable String groupId, @Nullable
239238
.anyMatch(rd -> (version == null) || version.equals(rd.getVersion()));
240239
}
241240

242-
@Deprecated // Use changeChildTagValue from MavenVisitor instead: https://github.com/openrewrite/rewrite/pull/5516
243-
private Xml.Tag changeChildTagValue2(Xml.Tag tag, String childTagName, String newValue, ExecutionContext ctx) {
244-
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
245-
if (childTag.isPresent() && !newValue.equals(childTag.get().getValue().orElse(null))) {
246-
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, ctx);
247-
}
248-
return tag;
249-
}
250-
251241
private boolean isDependencyManaged(Scope scope, String groupId, String artifactId) {
252-
253242
MavenResolutionResult result = getResolutionResult();
254243
for (ResolvedManagedDependency managedDependency : result.getPom().getDependencyManagement()) {
255244
if (groupId.equals(managedDependency.getGroupId()) && artifactId.equals(managedDependency.getArtifactId())) {

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.openrewrite.maven.tree.ResolvedPom;
2727
import org.openrewrite.semver.Semver;
2828
import org.openrewrite.semver.VersionComparator;
29-
import org.openrewrite.xml.ChangeTagValueVisitor;
3029
import org.openrewrite.xml.RemoveContentVisitor;
3130
import org.openrewrite.xml.tree.Xml;
3231

@@ -41,11 +40,6 @@ public class ChangeManagedDependencyGroupIdAndArtifactId extends Recipe {
4140
@EqualsAndHashCode.Exclude
4241
MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
4342

44-
// there are several implicitly defined version properties that we should never attempt to update
45-
private static final Set<String> implicitlyDefinedVersionProperties = new HashSet<>(Arrays.asList(
46-
"${version}", "${project.version}", "${pom.version}", "${project.parent.version}"
47-
));
48-
4943
@Option(displayName = "Old groupId",
5044
description = "The old groupId to replace. The groupId is the first part of a managed dependency coordinate `com.google.guava:guava:VERSION`.",
5145
example = "org.openrewrite.recipe")
@@ -146,24 +140,17 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
146140

147141
@Override
148142
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
149-
150143
Xml.Tag t = super.visitTag(tag, ctx);
151144
if (isManagedDependencyTag(oldGroupId, oldArtifactId)) {
152-
Optional<Xml.Tag> groupIdTag = t.getChild("groupId");
153-
boolean changed = false;
154-
if (groupIdTag.isPresent() && !newGroupId.equals(groupIdTag.get().getValue().orElse(null))) {
155-
doAfterVisit(new ChangeTagValueVisitor<>(groupIdTag.get(), newGroupId));
156-
changed = true;
145+
if (t.getChild("groupId").isPresent()) {
146+
t = changeChildTagValue(t, "groupId", newGroupId, ctx);
157147
}
158-
Optional<Xml.Tag> artifactIdTag = t.getChild("artifactId");
159-
if (artifactIdTag.isPresent() && !newArtifactId.equals(artifactIdTag.get().getValue().orElse(null))) {
160-
doAfterVisit(new ChangeTagValueVisitor<>(artifactIdTag.get(), newArtifactId));
161-
changed = true;
148+
if (t.getChild("artifactId").isPresent()) {
149+
t = changeChildTagValue(t, "artifactId", newArtifactId, ctx);
162150
}
163151
if (newVersion != null) {
164152
try {
165153
Optional<Xml.Tag> versionTag = t.getChild("version");
166-
167154
if (versionTag.isPresent()) {
168155
String resolvedArtifactId = newArtifactId;
169156
if (resolvedArtifactId.contains("${")) {
@@ -172,14 +159,13 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
172159
resolvedArtifactId = ResolvedPom.placeholderHelper.replacePlaceholders(newArtifactId, properties::get);
173160
}
174161
String resolvedNewVersion = resolveSemverVersion(ctx, newGroupId, resolvedArtifactId, getResolutionResult().getPom().getValue(versionTag.get().getValue().orElse(null)));
175-
t = (Xml.Tag) new ChangeTagValueVisitor<>(versionTag.get(), resolvedNewVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
162+
t = changeChildTagValue(t, "version", resolvedNewVersion, ctx);
176163
}
177-
changed = true;
178164
} catch (MavenDownloadingException e) {
179165
return e.warn(t);
180166
}
181167
}
182-
if (changed) {
168+
if (t != tag) {
183169
maybeUpdateModel();
184170
doAfterVisit(new RemoveRedundantDependencyVersions(null, null, null, null).getVisitor());
185171
if (isNewDependencyPresent) {

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@
2323
import org.openrewrite.Recipe;
2424
import org.openrewrite.TreeVisitor;
2525
import org.openrewrite.maven.table.MavenMetadataFailures;
26-
import org.openrewrite.xml.ChangeTagValueVisitor;
2726
import org.openrewrite.xml.tree.Xml;
2827

29-
import java.util.Optional;
30-
3128
@Value
3229
@EqualsAndHashCode(callSuper = false)
3330
public class ChangePluginGroupIdAndArtifactId extends Recipe {
@@ -90,13 +87,13 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
9087
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
9188
if (isPluginTag(oldGroupId, oldArtifactId)) {
9289
if (newGroupId != null) {
93-
t = changeChildTagValue2(t, "groupId", newGroupId, ctx);
90+
t = changeChildTagValue(t, "groupId", newGroupId, ctx);
9491
}
9592
if (newArtifactId != null) {
96-
t = changeChildTagValue2(t, "artifactId", newArtifactId, ctx);
93+
t = changeChildTagValue(t, "artifactId", newArtifactId, ctx);
9794
}
9895
if (newVersion != null) {
99-
t = changeChildTagValue2(t, "version", newVersion, ctx);
96+
t = changeChildTagValue(t, "version", newVersion, ctx);
10097
}
10198
if (t != tag) {
10299
maybeUpdateModel();
@@ -105,14 +102,6 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
105102
//noinspection ConstantConditions
106103
return t;
107104
}
108-
109-
private Xml.Tag changeChildTagValue2(Xml.Tag tag, String childTagName, String newValue, ExecutionContext ctx) {
110-
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
111-
if (childTag.isPresent() && !newValue.equals(childTag.get().getValue().orElse(null))) {
112-
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, ctx);
113-
}
114-
return tag;
115-
}
116105
};
117106
}
118107
}

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

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,6 @@ public class UpgradeDependencyVersion extends ScanningRecipe<UpgradeDependencyVe
5252
@EqualsAndHashCode.Exclude
5353
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
5454

55-
// there are several implicitly defined version properties that we should never attempt to update
56-
private static final Collection<String> implicitlyDefinedVersionProperties = Arrays.asList(
57-
"${version}", "${project.version}", "${pom.version}", "${project.parent.version}"
58-
);
59-
6055
@Option(displayName = "Group",
6156
description = "The first part of a dependency coordinate `com.google.guava:guava:VERSION`. This can be a glob expression.",
6257
example = "com.fasterxml.jackson*")
@@ -156,8 +151,7 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) {
156151
Optional<Xml.Tag> version = tag.getChild("version");
157152
if (version.isPresent()) {
158153
String requestedVersion = d.getRequested().getVersion();
159-
if (requestedVersion != null && requestedVersion.startsWith("${") &&
160-
!implicitlyDefinedVersionProperties.contains(requestedVersion)) {
154+
if (isProperty(requestedVersion)) {
161155
String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1);
162156
if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
163157
storeParentPomProperty(getResolutionResult().getParent(), propertyName, newerVersion);
@@ -180,8 +174,7 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) {
180174
* @param propertyName the name of the property to update, if found in any the parent pom source file
181175
* @param newerVersion the resolved newer version that any matching parent pom property should be updated to
182176
*/
183-
private void storeParentPomProperty(
184-
@Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName, String newerVersion) {
177+
private void storeParentPomProperty(@Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName, String newerVersion) {
185178
if (currentMavenResolutionResult == null) {
186179
return; // No parent contained the property; might then be in the same source file, or an import BOM
187180
}
@@ -270,10 +263,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
270263
}
271264

272265
private void retainVersions() {
273-
for (Recipe retainVersionRecipe : RetainVersions.plan(this, retainVersions == null ?
274-
emptyList() : retainVersions)) {
275-
doAfterVisit(retainVersionRecipe.getVisitor());
276-
}
266+
RetainVersions.plan(this, retainVersions == null ? emptyList() : retainVersions)
267+
.forEach(it -> doAfterVisit(it.getVisitor()));
277268
}
278269
}
279270

@@ -284,26 +275,13 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD
284275
// as a source file, attempt to find a new version.
285276
String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx);
286277
if (newerVersion != null) {
287-
Optional<Xml.Tag> version = t.getChild("version");
288-
if (version.isPresent()) {
289-
String requestedVersion = d.getRequested().getVersion();
290-
if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) {
291-
String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1);
292-
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
293-
doAfterVisit(new ChangePropertyValue(propertyName, newerVersion, overrideManagedVersion, false).getVisitor());
294-
}
295-
} else {
296-
t = (Xml.Tag) new ChangeTagValueVisitor<>(version.get(), newerVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
297-
}
278+
if (t.getChild("version").isPresent()) {
279+
t = changeChildTagValue(t, "version", newerVersion, overrideManagedVersion, ctx);
298280
} else if (Boolean.TRUE.equals(overrideManagedVersion)) {
299281
ResolvedManagedDependency dm = findManagedDependency(t);
300282
// if a managed dependency is expressed as a property, change the property value
301283
// do this only when a requested bom is absent, otherwise changing property has no effect
302-
if (dm != null &&
303-
dm.getRequested().getVersion() != null &&
304-
dm.getRequested().getVersion().startsWith("${") &&
305-
!implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion()) &&
306-
dm.getRequestedBom() == null) {
284+
if (dm != null && isProperty(dm.getRequested().getVersion()) && dm.getRequestedBom() == null) {
307285
doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2,
308286
dm.getRequested().getVersion().length() - 1),
309287
newerVersion, overrideManagedVersion, false).getVisitor());
@@ -359,20 +337,14 @@ private Xml.Tag upgradePluginDependency(ExecutionContext ctx, Xml.Tag t) throws
359337
if (groupId != null && artifactId != null && version != null) {
360338
String newerVersion = findNewerVersion(groupId, artifactId, resolveVersion(version), ctx);
361339
if (newerVersion != null) {
362-
if (version.startsWith("${") && !implicitlyDefinedVersionProperties.contains(version)) {
363-
doAfterVisit(new ChangePropertyValue(version.substring(2, version.length() - 1), newerVersion, overrideManagedVersion, false).getVisitor());
364-
} else {
365-
Optional<Xml.Tag> versionTag = t.getChild("version");
366-
assert versionTag.isPresent();
367-
t = (Xml.Tag) new ChangeTagValueVisitor<>(versionTag.get(), newerVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
368-
}
340+
t = changeChildTagValue(t, "version", newerVersion, overrideManagedVersion, ctx);
369341
}
370342
}
371343
return t;
372344
}
373345

374346
private String resolveVersion(String version) {
375-
if (version.startsWith("${") && !implicitlyDefinedVersionProperties.contains(version)) {
347+
if (isProperty(version)) {
376348
Map<String, String> properties = getResolutionResult().getPom().getProperties();
377349
String property = version.substring(2, version.length() - 1);
378350
return properties.getOrDefault(property, version);
@@ -384,7 +356,7 @@ private String resolveVersion(String version) {
384356
String newerVersion = findNewerVersion(groupId, artifactId, version2, ctx);
385357
if (newerVersion == null) {
386358
return null;
387-
} else if (requestedVersion != null && requestedVersion.startsWith("${")) {
359+
} else if (isProperty(requestedVersion)) {
388360
//noinspection unchecked
389361
return (TreeVisitor<Xml, ExecutionContext>) new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)
390362
.getVisitor();

‎rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,4 +1624,56 @@ void changePluginDependencyGroupIdAndArtifactId() {
16241624
)
16251625
);
16261626
}
1627+
1628+
@Test
1629+
void changeDependencyGroupIdAndArtifactIdWithVersionProperty() {
1630+
rewriteRun(
1631+
spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId(
1632+
"javax.activation",
1633+
"javax.activation-api",
1634+
"jakarta.activation",
1635+
"jakarta.activation-api",
1636+
"1.2.x",
1637+
null
1638+
)),
1639+
pomXml(
1640+
"""
1641+
<project>
1642+
<modelVersion>4.0.0</modelVersion>
1643+
<groupId>com.mycompany.app</groupId>
1644+
<artifactId>my-app</artifactId>
1645+
<version>1</version>
1646+
<properties>
1647+
<activation.api>1.2.0</activation.api>
1648+
</properties>
1649+
<dependencies>
1650+
<dependency>
1651+
<groupId>javax.activation</groupId>
1652+
<artifactId>javax.activation-api</artifactId>
1653+
<version>${activation.api}</version>
1654+
</dependency>
1655+
</dependencies>
1656+
</project>
1657+
""",
1658+
"""
1659+
<project>
1660+
<modelVersion>4.0.0</modelVersion>
1661+
<groupId>com.mycompany.app</groupId>
1662+
<artifactId>my-app</artifactId>
1663+
<version>1</version>
1664+
<properties>
1665+
<activation.api>1.2.2</activation.api>
1666+
</properties>
1667+
<dependencies>
1668+
<dependency>
1669+
<groupId>jakarta.activation</groupId>
1670+
<artifactId>jakarta.activation-api</artifactId>
1671+
<version>${activation.api}</version>
1672+
</dependency>
1673+
</dependencies>
1674+
</project>
1675+
"""
1676+
)
1677+
);
1678+
}
16271679
}

0 commit comments

Comments
 (0)