diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java index 49ff733f479..4ef22991ffd 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -17,22 +17,34 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Option; -import org.openrewrite.Recipe; +import org.openrewrite.ScanningRecipe; import org.openrewrite.TreeVisitor; -import org.jspecify.annotations.Nullable; import org.openrewrite.maven.tree.MavenResolutionResult; +import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion; +import org.openrewrite.xml.RemoveContentVisitor; import org.openrewrite.xml.tree.Xml; -import java.util.Optional; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static org.openrewrite.xml.AddOrUpdateChild.addOrUpdateChild; import static org.openrewrite.xml.FilterTagChildrenVisitor.filterTagChildren; @Value @EqualsAndHashCode(callSuper = false) -public class UseMavenCompilerPluginReleaseConfiguration extends Recipe { +public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe { + + private static final Pattern MAVEN_COMPILER_PROPERTY_PATTERN = + Pattern.compile("\\$\\{(maven\\.compiler\\.(?:source|target|testSource|testTarget))}"); + + private static final Set DEFAULT_MAVEN_COMPILER_PROPERTIES = new HashSet<>(Arrays.asList( + "${maven.compiler.source}", "${maven.compiler.target}", "${maven.compiler.release}", + "${maven.compiler.testSource}", "${maven.compiler.testTarget}", "${maven.compiler.testRelease}" + )); @Option( displayName = "Release version", @@ -44,70 +56,245 @@ public class UseMavenCompilerPluginReleaseConfiguration extends Recipe { String displayName = "Use Maven compiler plugin release configuration"; String description = "Replaces any explicit `source` or `target` configuration (if present) on the `maven-compiler-plugin` with " + - "`release`, and updates the `release` value if needed. Will not downgrade the Java version if the current version is higher."; + "`release`, and updates the `release` value if needed. When `testSource` or `testTarget` differ from the main " + + "version, introduces `testRelease`. Will not downgrade the Java version if the current version is higher. " + + "Also removes stale `maven.compiler.source`, `maven.compiler.target`, `maven.compiler.testSource`, and " + + "`maven.compiler.testTarget` properties that are no longer referenced."; + + public static class Accumulator { + // Only tracks usages from OUTSIDE the compiler plugin's source/target/testSource/testTarget tags, + // since those tags will be replaced by the visitor + Map> propertyUsages = new HashMap<>(); + } + + @Override + public Accumulator getInitialValue(ExecutionContext ctx) { + return new Accumulator(); + } + + @Override + public TreeVisitor getScanner(Accumulator acc) { + return new MavenIsoVisitor() { + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + // Skip visiting compiler plugin children entirely; the visitor will replace + // source/target/testSource/testTarget, so those references should not count as usages + if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + return tag; + } + + Xml.Tag t = super.visitTag(tag, ctx); + + // Track ${maven.compiler.*} property usages outside of + if (!isPropertyTag()) { + Optional value = t.getValue(); + if (value.isPresent() && value.get().contains("${maven.compiler.")) { + Matcher matcher = MAVEN_COMPILER_PROPERTY_PATTERN.matcher(value.get()); + while (matcher.find()) { + acc.propertyUsages.computeIfAbsent(matcher.group(1), k -> new HashSet<>()) + .add(getResolutionResult().getPom().getGav()); + } + } + } + + return t; + } + }; + } @Override - public TreeVisitor getVisitor() { + public TreeVisitor getVisitor(Accumulator acc) { return new MavenIsoVisitor() { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (!isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + + // Handle compiler plugin source/target → release replacement + if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + t = handleCompilerPlugin(t); + } + + return t; + } + + private Xml.Tag handleCompilerPlugin(Xml.Tag t) { + Optional maybeConfig = t.getChild("configuration"); + if (!maybeConfig.isPresent()) { + return t; + } + Xml.Tag config = maybeConfig.get(); + Optional source = config.getChildValue("source"); + Optional target = config.getChildValue("target"); + Optional release = config.getChildValue("release"); + Optional testSource = config.getChildValue("testSource"); + Optional testTarget = config.getChildValue("testTarget"); + Optional testRelease = config.getChildValue("testRelease"); + + boolean hasMainConfig = source.isPresent() || target.isPresent() || release.isPresent(); + boolean hasTestConfig = testSource.isPresent() || testTarget.isPresent() || testRelease.isPresent(); + + if (!hasMainConfig && !hasTestConfig) { return t; } - Optional maybeCompilerPluginConfig = t.getChild("configuration"); - if (!maybeCompilerPluginConfig.isPresent()) { + + // Determine whether to process main source/target → release + boolean processMain = hasMainConfig && + !versionNewerThanProposed(source) && + !versionNewerThanProposed(target) && + !versionNewerThanProposed(release); + + // Determine test handling: whether testRelease is needed after removing testSource/testTarget + boolean testNeedsOwnRelease = false; + @Nullable String testVersionValue = null; + if (hasTestConfig) { + testVersionValue = resolveVersion(testRelease, testSource, testTarget); + if (testVersionValue != null) { + if (DEFAULT_MAVEN_COMPILER_PROPERTIES.contains(testVersionValue)) { + testNeedsOwnRelease = false; + } else if (testVersionValue.startsWith("${")) { + testNeedsOwnRelease = true; + } else { + testNeedsOwnRelease = isHigherVersion(testVersionValue, releaseVersion.toString()); + } + } + } + + if (!processMain && !hasTestConfig) { return t; } - Xml.Tag compilerPluginConfig = maybeCompilerPluginConfig.get(); - Optional source = compilerPluginConfig.getChildValue("source"); - Optional target = compilerPluginConfig.getChildValue("target"); - Optional release = compilerPluginConfig.getChildValue("release"); - if (!source.isPresent() && !target.isPresent() && !release.isPresent()) { - return t; // Do not introduce a new tag if none of the values are present + + // Build the set of tags to remove from configuration + Set tagsToRemove = new HashSet<>(); + if (processMain) { + tagsToRemove.add("source"); + tagsToRemove.add("target"); } - if (currentNewerThanProposed(source) || - currentNewerThanProposed(target) || - currentNewerThanProposed(release)) { + if (hasTestConfig) { + tagsToRemove.add("testSource"); + tagsToRemove.add("testTarget"); + if (!testNeedsOwnRelease) { + tagsToRemove.add("testRelease"); + } + } + + if (tagsToRemove.isEmpty()) { return t; } - Xml.Tag updated = filterTagChildren(t, compilerPluginConfig, - child -> !("source".equals(child.getName()) || "target".equals(child.getName()))); - String existingPropertyRef = getExistingPropertyReference(release, source, target); - String releaseVersionValue; - if (existingPropertyRef != null) { - releaseVersionValue = existingPropertyRef; - } else if (hasJavaVersionProperty(getCursor().firstEnclosingOrThrow(Xml.Document.class))) { - releaseVersionValue = "${java.version}"; - } else { - releaseVersionValue = releaseVersion.toString(); + Xml.Tag updated = filterTagChildren(t, config, + child -> !tagsToRemove.contains(child.getName())); + + // Add/update + if (processMain) { + String existingPropertyRef = getExistingPropertyReference(release, source, target); + String releaseVal; + if (existingPropertyRef != null) { + releaseVal = existingPropertyRef; + } else if (hasJavaVersionProperty(getCursor().firstEnclosingOrThrow(Xml.Document.class))) { + releaseVal = "${java.version}"; + } else { + releaseVal = releaseVersion.toString(); + } + updated = addOrUpdateChild(updated, config, + Xml.Tag.build("" + releaseVal + ""), getCursor().getParentOrThrow()); + } + + // Add/update if test version is higher than proposed release + if (hasTestConfig && testNeedsOwnRelease && testVersionValue != null) { + String testExistingRef = getExistingPropertyReference(testRelease, testSource, testTarget); + String testReleaseVal = testExistingRef != null ? testExistingRef : testVersionValue; + updated = addOrUpdateChild(updated, config, + Xml.Tag.build("" + testReleaseVal + ""), getCursor().getParentOrThrow()); + } + + // Determine which maven.compiler.* properties are now stale and should be removed + boolean releaseConfigured = processMain || release.isPresent(); + boolean testReleaseConfigured = (hasTestConfig && testNeedsOwnRelease) || testRelease.isPresent(); + + if (releaseConfigured) { + markPropertyForRemovalIfUnused("maven.compiler.source", acc); + markPropertyForRemovalIfUnused("maven.compiler.target", acc); + } + if (releaseConfigured || testReleaseConfigured) { + markPropertyForRemovalIfUnused("maven.compiler.testSource", acc); + markPropertyForRemovalIfUnused("maven.compiler.testTarget", acc); + } + + return updated; + } + + private void markPropertyForRemovalIfUnused(String propertyName, Accumulator acc) { + ResolvedGroupArtifactVersion currentGav = getResolutionResult().getPom().getGav(); + + Set usages = acc.propertyUsages.get(propertyName); + if (usages != null) { + for (ResolvedGroupArtifactVersion usingGav : usages) { + if (isAncestorOrSelf(currentGav, usingGav)) { + return; + } + } + } + + doAfterVisit(new MavenIsoVisitor() { + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + Xml.Tag t = super.visitTag(tag, ctx); + if (isPropertyTag() && propertyName.equals(t.getName())) { + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); + maybeUpdateModel(); + } + return t; + } + }); + } + + private boolean isAncestorOrSelf(ResolvedGroupArtifactVersion possibleAncestor, ResolvedGroupArtifactVersion gav) { + if (possibleAncestor.equals(gav)) { + return true; + } + MavenResolutionResult ancestor = getResolutionResult(); + while (ancestor != null) { + if (ancestor.getPom().getGav().equals(possibleAncestor)) { + return true; + } + ancestor = ancestor.getParent(); } - return addOrUpdateChild(updated, compilerPluginConfig, - Xml.Tag.build("" + releaseVersionValue + ""), getCursor().getParentOrThrow()); + return false; } }; } - private boolean currentNewerThanProposed(@SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional config) { + private boolean versionNewerThanProposed(@SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional config) { if (!config.isPresent()) { return false; } + return isHigherVersion(config.get(), releaseVersion.toString()); + } + + private static boolean isHigherVersion(String current, String proposed) { try { - float currentVersion = Float.parseFloat(config.get()); - float proposedVersion = Float.parseFloat(releaseVersion.toString()); - return proposedVersion < currentVersion; + return Float.parseFloat(current) > Float.parseFloat(proposed); } catch (NumberFormatException e) { return false; } } + private static @Nullable String resolveVersion( + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional... configs) { + for (Optional config : configs) { + if (config.isPresent()) { + return config.get(); + } + } + return null; + } + private static @Nullable String getExistingPropertyReference( @SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional... configs) { for (Optional config : configs) { if (config.isPresent()) { String value = config.get(); - if (value.startsWith("${") && value.endsWith("}") && !isDefaultMavenCompilerProperty(value)) { + if (value.startsWith("${") && value.endsWith("}") && !DEFAULT_MAVEN_COMPILER_PROPERTIES.contains(value)) { return value; } } @@ -115,12 +302,6 @@ private boolean currentNewerThanProposed(@SuppressWarnings("OptionalUsedAsFieldO return null; } - private static boolean isDefaultMavenCompilerProperty(String value) { - return "${maven.compiler.source}".equals(value) || - "${maven.compiler.target}".equals(value) || - "${maven.compiler.release}".equals(value); - } - private boolean hasJavaVersionProperty(Xml.Document xml) { return xml.getMarkers().findFirst(MavenResolutionResult.class) .map(r -> r.getPom().getProperties().get("java.version") != null) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java index 8145fcdb474..a71e80ff8be 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java @@ -765,6 +765,657 @@ void skipsPluginWithExecutionConfiguration() { ); } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void removesStaleSourceTargetProperties() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + 8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + ${maven.compiler.source} + ${maven.compiler.target} + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void removesStalePropertiesInheritedByChildModule() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + parent + 1.0.0 + pom + + + 8 + 8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + ${maven.compiler.source} + ${maven.compiler.target} + + + + + + + """, + """ + + 4.0.0 + org.sample + parent + 1.0.0 + pom + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + + """ + ), + mavenProject( + "child", + //language=xml + pomXml(""" + + 4.0.0 + + + org.sample + parent + 1.0.0 + + + child + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + ${maven.compiler.source} + ${maven.compiler.target} + + + + + + """, + """ + + 4.0.0 + + + org.sample + parent + 1.0.0 + + + child + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + """ + ) + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void removesImplicitlyStaleProperties() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + 8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 8 + 8 + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void keepsPropertiesWhenReferencedElsewhere() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + 8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + ${maven.compiler.source} + ${maven.compiler.target} + + + + com.example + some-plugin + + ${maven.compiler.source} + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + com.example + some-plugin + + ${maven.compiler.source} + + + + + + + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void keepsPropertiesWithoutCompilerPlugin() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + 8 + + + + """ + ) + ); + } + + @Test + void replacesTestSourceAndTestTargetWithTestRelease() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(11)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 1.8 + 1.8 + 17 + 17 + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 11 + 17 + + + + + + + """ + ) + ); + } + + @Test + void removesTestSourceTargetWhenSameAsRelease() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(11)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 1.8 + 1.8 + 1.8 + 1.8 + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 11 + + + + + + + """ + ) + ); + } + + @Test + void preservesCustomPropertyRefInTestRelease() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(11)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 17 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 1.8 + 1.8 + ${test.java.version} + ${test.java.version} + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 17 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 11 + ${test.java.version} + + + + + + + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void removesStaleTestSourceTargetProperties() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + 8 + 8 + 11 + 11 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + ${maven.compiler.source} + ${maven.compiler.target} + ${maven.compiler.testSource} + ${maven.compiler.testTarget} + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + + """ + ) + ); + } + + @Test + void removesRedundantTestReleaseWhenCoveredByRelease() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), + //language=xml + pomXml( + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 8 + 8 + 11 + + + + + + + """, + """ + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.0 + + 21 + + + + + + + """ + ) + ); + } + private static Stream mavenCompilerPluginConfig() { return Stream.of( Arguments.of("""