Skip to content

Commit

Permalink
Ensure that maven compiler configuration is set after a java version …
Browse files Browse the repository at this point in the history 
…upgrade.

Before these changes if no property or maven compiler plugin configured java version no change would be made.

Fixes #514
  • Loading branch information
@sambsnyd
sambsnyd committed Jul 29, 2024
1 parent bc0d2eb commit 8d15935
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 236 deletions.
68 changes: 55 additions & 13 deletions src/main/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.openrewrite.TreeVisitor;
import org.openrewrite.maven.AddProperty;
import org.openrewrite.maven.MavenIsoVisitor;
import org.openrewrite.maven.tree.MavenResolutionResult;
import org.openrewrite.xml.XPathMatcher;
import org.openrewrite.xml.tree.Xml;

Expand All @@ -48,6 +49,8 @@ public class UpdateMavenProjectPropertyJavaVersion extends Recipe {
.map(property -> "/project/properties/" + property)
.map(XPathMatcher::new).collect(Collectors.toList());

private static final XPathMatcher PLUGINS_MATCHER = new XPathMatcher("/project/build//plugins");

@Option(displayName = "Java version",
description = "The Java version to upgrade to.",
example = "11")
Expand All @@ -70,13 +73,14 @@ public String getDescription() {
" * `maven.compiler.target`\n" +
" * `maven.compiler.release`\n" +
" * `release.version`\n\n" +
" These project properties are not added if they are not currently set, but only updated in place.";
"If none of these properties are in use and the maven compiler plugin is not otherwise configured adds the `maven.compiler.release` property.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new MavenIsoVisitor<ExecutionContext>() {
final Set<String> propertiesExplicitlyReferenced = new HashSet<>();
boolean compilerPluginConfiguredExplicitly;

@Override
public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
Expand All @@ -87,34 +91,52 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
Optional<String> pathToLocalParent = d.getRoot().getChild("parent")
.flatMap(parent -> parent.getChild("relativePath"))
.flatMap(Xml.Tag::getValue);
if(pathToLocalParent.isPresent()) {
if (pathToLocalParent.isPresent()) {
return d;
}

// Otherwise override remote parent's properties locally
Map<String, String> currentProperties = getResolutionResult().getPom().getRequested().getProperties();
MavenResolutionResult mrr = getResolutionResult();
Map<String, String> currentProperties = mrr.getPom().getRequested().getProperties();
for (String property : JAVA_VERSION_PROPERTIES) {
if(currentProperties.containsKey(property) || !propertiesExplicitlyReferenced.contains(property)) {
if (currentProperties.containsKey(property) || !propertiesExplicitlyReferenced.contains(property)) {
continue;
}
d = (Xml.Document) new AddProperty(property, String.valueOf(version), null, false)
.getVisitor()
.visitNonNull(d, ctx);
}

// When none of the relevant properties are explicitly configured Maven defaults to Java 8
// The release option was added in 9
// If no properties have yet been updated then set release explicitly
if (version >= 9 &&
!compilerPluginConfiguredExplicitly &&
currentProperties.keySet()
.stream()
.noneMatch(JAVA_VERSION_PROPERTIES::contains)) {
d = (Xml.Document) new AddProperty("maven.compiler.release", String.valueOf(version), null, false)
.getVisitor()
.visitNonNull(d, ctx);
HashMap<String, String> updatedProps = new HashMap<>(currentProperties);
updatedProps.put("maven.compiler.release", version.toString());
mrr = mrr.withPom(mrr.getPom().withRequested(mrr.getPom().getRequested().withProperties(updatedProps)));

d = d.withMarkers(d.getMarkers().setByType(mrr));
}
return d;
}

@Override
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
tag = super.visitTag(tag, ctx);
Optional<String> s = tag.getValue()
Xml.Tag t = super.visitTag(tag, ctx);
Optional<String> s = t.getValue()
.map(it -> it.replace("${", "").replace("}", "").trim())
.filter(JAVA_VERSION_PROPERTIES::contains);
if(s.isPresent()) {
if (s.isPresent()) {
propertiesExplicitlyReferenced.add(s.get());
} else if (JAVA_VERSION_XPATH_MATCHERS.stream().anyMatch(matcher -> matcher.matches(getCursor()))) {
Optional<Float> maybeVersion = tag.getValue().flatMap(
Optional<Float> maybeVersion = t.getValue().flatMap(
value -> {
try {
return Optional.of(Float.parseFloat(value));
Expand All @@ -125,16 +147,36 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
);

if (!maybeVersion.isPresent()) {
return tag;
return t;
}
float currentVersion = maybeVersion.get();
if (currentVersion >= version) {
return tag;
return t;
}
return t.withValue(String.valueOf(version));
} else if (PLUGINS_MATCHER.matches(getCursor())) {
Optional<Xml.Tag> maybeCompilerPlugin = t.getChildren().stream()
.filter(plugin ->
"plugin".equals(plugin.getName()) &&
"org.apache.maven.plugins".equals(plugin.getChildValue("groupId").orElse("org.apache.maven.plugins")) &&
"maven-compiler-plugin".equals(plugin.getChildValue("artifactId").orElse(null)))
.findAny();
Optional<Xml.Tag> maybeCompilerPluginConfig = maybeCompilerPlugin
.flatMap(it -> it.getChild("configuration"));
if (!maybeCompilerPluginConfig.isPresent()) {
return t;
}
Xml.Tag compilerPluginConfig = maybeCompilerPluginConfig.get();
Optional<String> source = compilerPluginConfig.getChildValue("source");
Optional<String> target = compilerPluginConfig.getChildValue("target");
Optional<String> release = compilerPluginConfig.getChildValue("release");
if (source.isPresent()
|| target.isPresent()
|| release.isPresent()) {
compilerPluginConfiguredExplicitly = true;
}
return tag.withValue(String.valueOf(version));
}

return tag;
return t;
}
};
}
Expand Down
8 changes: 1 addition & 7 deletions ...n/java/org/openrewrite/java/migrate/maven/UseMavenCompilerPluginReleaseConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class UseMavenCompilerPluginReleaseConfiguration extends Recipe {
private static final XPathMatcher PLUGINS_MATCHER = new XPathMatcher("/project/build//plugins");

@Option(
displayName = "Release Version",
displayName = "Release version",
description = "The new value for the release configuration. This recipe prefers ${java.version} if defined.",
example = "11"
)
Expand Down Expand Up @@ -86,12 +86,6 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
}
Xml.Tag updated = filterTagChildren(t, compilerPluginConfig,
child -> !("source".equals(child.getName()) || "target".equals(child.getName())));
if (updated != t
&& source.map("${maven.compiler.source}"::equals).orElse(true)
&& target.map("${maven.compiler.target}"::equals).orElse(true)) {
return filterTagChildren(updated, maybeCompilerPlugin.get(),
child -> !("configuration".equals(child.getName()) && child.getChildren().isEmpty()));
}
String releaseVersionValue = hasJavaVersionProperty(getCursor().firstEnclosingOrThrow(Xml.Document.class))
? "${java.version}" : releaseVersion.toString();
updated = addOrUpdateChild(updated, compilerPluginConfig,
Expand Down
Loading

1 comment on commit 8d15935

@pzygielo
Copy link

@pzygielo pzygielo commented on 8d15935 Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this (using UpgradeToJava17 recipe) adds

<maven.compiler.release>17</maven.compiler.release>

to every child regardless of the same being set in parent already...

I'll try to extract standalone reproducer to check this.

Please sign in to comment.