From 24279c84235324d7c75aa86c8f73145c2afc03c7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 8 Apr 2026 13:29:00 +0200 Subject: [PATCH 1/9] Remove stale maven.compiler.source/target properties after release replacement Convert UseMavenCompilerPluginReleaseConfiguration from Recipe to ScanningRecipe so it can detect and remove maven.compiler.source/maven.compiler.target properties that become unused after source/target is replaced with release. Fixes openrewrite/rewrite-migrate-java#668 --- ...venCompilerPluginReleaseConfiguration.java | 126 +++++++++- ...ompilerPluginReleaseConfigurationTest.java | 229 ++++++++++++++++++ 2 files changed, 346 insertions(+), 9 deletions(-) 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..fd2d5695a9a 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,29 @@ 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))}"); @Option( displayName = "Release version", @@ -44,17 +51,77 @@ 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. Will not downgrade the Java version if the current version is higher. " + + "Also removes stale `maven.compiler.source` and `maven.compiler.target` properties that are no longer referenced."; + + public static class Accumulator { + Map> propertyUsages = new HashMap<>(); + Set pomsWithRelease = new HashSet<>(); + } @Override - public TreeVisitor getVisitor() { + 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) { Xml.Tag t = super.visitTag(tag, ctx); - if (!isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { - return t; + + // Track ${maven.compiler.source} and ${maven.compiler.target} usages + // outside of the section + if (!isPropertyTag()) { + Optional value = t.getValue(); + if (value.isPresent()) { + 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()); + } + } } + + // Track POMs that have in the compiler plugin configuration + if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + Optional config = t.getChild("configuration"); + if (config.isPresent() && config.get().getChildValue("release").isPresent()) { + acc.pomsWithRelease.add(getResolutionResult().getPom().getGav()); + } + } + + return t; + } + }; + } + + @Override + 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); + + // Handle compiler plugin source/target → release replacement (existing logic) + if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + t = handleCompilerPlugin(t); + } + + // Handle stale maven.compiler.source/target property removal + if (isPropertyTag() && + ("maven.compiler.source".equals(t.getName()) || "maven.compiler.target".equals(t.getName()))) { + if (isPropertyStale(t.getName(), acc)) { + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); + maybeUpdateModel(); + } + } + + return t; + } + + private Xml.Tag handleCompilerPlugin(Xml.Tag t) { Optional maybeCompilerPluginConfig = t.getChild("configuration"); if (!maybeCompilerPluginConfig.isPresent()) { return t; @@ -64,7 +131,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { 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 + return t; } if (currentNewerThanProposed(source) || currentNewerThanProposed(target) || @@ -86,6 +153,47 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { return addOrUpdateChild(updated, compilerPluginConfig, Xml.Tag.build("" + releaseVersionValue + ""), getCursor().getParentOrThrow()); } + + private boolean isPropertyStale(String propertyName, Accumulator acc) { + ResolvedGroupArtifactVersion currentGav = getResolutionResult().getPom().getGav(); + + // Check if any POM references this property + Set usages = acc.propertyUsages.get(propertyName); + if (usages != null) { + for (ResolvedGroupArtifactVersion usingGav : usages) { + if (isAncestorOrSelf(currentGav, usingGav)) { + return false; + } + } + } + + // Only remove if this POM (or an ancestor in the project) has configured + if (acc.pomsWithRelease.contains(currentGav)) { + return true; + } + // Check if any module in the project that uses this POM as ancestor has release + for (ResolvedGroupArtifactVersion releaseGav : acc.pomsWithRelease) { + if (isAncestorOrSelf(currentGav, releaseGav)) { + return true; + } + } + return false; + } + + private boolean isAncestorOrSelf(ResolvedGroupArtifactVersion possibleAncestor, ResolvedGroupArtifactVersion gav) { + if (possibleAncestor.equals(gav)) { + return true; + } + // Walk up from the current resolution result to check ancestor chain + MavenResolutionResult ancestor = getResolutionResult(); + while (ancestor != null) { + if (ancestor.getPom().getGav().equals(possibleAncestor)) { + return true; + } + ancestor = ancestor.getParent(); + } + return false; + } }; } 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..c2487c50306 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,235 @@ void skipsPluginWithExecutionConfiguration() { ); } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/668") + @Test + void removesStaleSourceTargetProperties() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) + .cycles(2).expectedCyclesThatMakeChanges(2), + //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 removesImplicitlyStaleProperties() { + rewriteRun( + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) + .cycles(2).expectedCyclesThatMakeChanges(2), + //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)) + .cycles(2).expectedCyclesThatMakeChanges(2), + //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 + + + + """ + ) + ); + } + private static Stream mavenCompilerPluginConfig() { return Stream.of( Arguments.of(""" From 7c524792d2ddcc4e45e50a1a81428d2ddc821abc Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 9 Apr 2026 18:13:02 +0200 Subject: [PATCH 2/9] Also scan execution configurations for when detecting stale properties --- ...venCompilerPluginReleaseConfiguration.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) 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 fd2d5695a9a..f96fbf08a0a 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -85,15 +85,34 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } // Track POMs that have in the compiler plugin configuration + // (including per-execution configurations) if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { - Optional config = t.getChild("configuration"); - if (config.isPresent() && config.get().getChildValue("release").isPresent()) { + if (hasReleaseInConfiguration(t)) { acc.pomsWithRelease.add(getResolutionResult().getPom().getGav()); } } return t; } + + private boolean hasReleaseInConfiguration(Xml.Tag pluginTag) { + // Check top-level + Optional config = pluginTag.getChild("configuration"); + if (config.isPresent() && config.get().getChildValue("release").isPresent()) { + return true; + } + // Check // + Optional executions = pluginTag.getChild("executions"); + if (executions.isPresent()) { + for (Xml.Tag execution : executions.get().getChildren("execution")) { + Optional execConfig = execution.getChild("configuration"); + if (execConfig.isPresent() && execConfig.get().getChildValue("release").isPresent()) { + return true; + } + } + } + return false; + } }; } From e31c5efbd19beb4d97097e55f3640635d9fff491 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 9 Apr 2026 18:25:09 +0200 Subject: [PATCH 3/9] =?UTF-8?q?Handle=20testSource/testTarget=20=E2=86=92?= =?UTF-8?q?=20testRelease=20conversion=20and=20stale=20test=20property=20c?= =?UTF-8?q?leanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When test sources use a different Java version than main sources, introduce to preserve the higher version. Also clean up stale maven.compiler.testSource/testTarget properties when or is configured. --- ...venCompilerPluginReleaseConfiguration.java | 221 +++++++++---- ...ompilerPluginReleaseConfigurationTest.java | 303 ++++++++++++++++++ 2 files changed, 468 insertions(+), 56 deletions(-) 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 f96fbf08a0a..881ec93ef6e 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -39,7 +39,12 @@ public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe { private static final Pattern MAVEN_COMPILER_PROPERTY_PATTERN = - Pattern.compile("\\$\\{(maven\\.compiler\\.(?:source|target))}"); + 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", @@ -51,12 +56,15 @@ public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe> propertyUsages = new HashMap<>(); Set pomsWithRelease = new HashSet<>(); + Set pomsWithTestRelease = new HashSet<>(); } @Override @@ -71,8 +79,7 @@ public TreeVisitor getScanner(Accumulator acc) { public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - // Track ${maven.compiler.source} and ${maven.compiler.target} usages - // outside of the section + // Track ${maven.compiler.*} property usages outside of the section if (!isPropertyTag()) { Optional value = t.getValue(); if (value.isPresent()) { @@ -84,34 +91,42 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } } - // Track POMs that have in the compiler plugin configuration + // Track POMs that have or in the compiler plugin configuration // (including per-execution configurations) if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { - if (hasReleaseInConfiguration(t)) { - acc.pomsWithRelease.add(getResolutionResult().getPom().getGav()); - } + scanCompilerPluginForRelease(t, acc); } return t; } - private boolean hasReleaseInConfiguration(Xml.Tag pluginTag) { + private void scanCompilerPluginForRelease(Xml.Tag pluginTag, Accumulator acc) { + ResolvedGroupArtifactVersion gav = getResolutionResult().getPom().getGav(); // Check top-level Optional config = pluginTag.getChild("configuration"); - if (config.isPresent() && config.get().getChildValue("release").isPresent()) { - return true; + if (config.isPresent()) { + if (config.get().getChildValue("release").isPresent()) { + acc.pomsWithRelease.add(gav); + } + if (config.get().getChildValue("testRelease").isPresent()) { + acc.pomsWithTestRelease.add(gav); + } } // Check // Optional executions = pluginTag.getChild("executions"); if (executions.isPresent()) { for (Xml.Tag execution : executions.get().getChildren("execution")) { Optional execConfig = execution.getChild("configuration"); - if (execConfig.isPresent() && execConfig.get().getChildValue("release").isPresent()) { - return true; + if (execConfig.isPresent()) { + if (execConfig.get().getChildValue("release").isPresent()) { + acc.pomsWithRelease.add(gav); + } + if (execConfig.get().getChildValue("testRelease").isPresent()) { + acc.pomsWithTestRelease.add(gav); + } } } } - return false; } }; } @@ -123,15 +138,20 @@ public TreeVisitor getVisitor(Accumulator acc) { public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - // Handle compiler plugin source/target → release replacement (existing logic) + // Handle compiler plugin source/target → release replacement if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { t = handleCompilerPlugin(t); } - // Handle stale maven.compiler.source/target property removal - if (isPropertyTag() && - ("maven.compiler.source".equals(t.getName()) || "maven.compiler.target".equals(t.getName()))) { - if (isPropertyStale(t.getName(), acc)) { + // Handle stale maven.compiler.* property removal + if (isPropertyTag()) { + String name = t.getName(); + if (("maven.compiler.source".equals(name) || "maven.compiler.target".equals(name)) && + isPropertyStale(name, acc.pomsWithRelease, acc)) { + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); + maybeUpdateModel(); + } else if (("maven.compiler.testSource".equals(name) || "maven.compiler.testTarget".equals(name)) && + isTestPropertyStale(name, acc)) { doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); } @@ -141,39 +161,101 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } private Xml.Tag handleCompilerPlugin(Xml.Tag t) { - Optional maybeCompilerPluginConfig = t.getChild("configuration"); - if (!maybeCompilerPluginConfig.isPresent()) { + Optional maybeConfig = t.getChild("configuration"); + if (!maybeConfig.isPresent()) { 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()) { + 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; } - if (currentNewerThanProposed(source) || - currentNewerThanProposed(target) || - currentNewerThanProposed(release)) { + + // 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 (isDefaultMavenCompilerProperty("${" + extractPropertyName(testVersionValue) + "}")) { + // Default maven property like ${maven.compiler.testSource} - release covers it + testNeedsOwnRelease = false; + } else if (testVersionValue.startsWith("${")) { + // Custom property reference - preserve as testRelease + testNeedsOwnRelease = true; + } else { + testNeedsOwnRelease = isHigherVersion(testVersionValue, releaseVersion.toString()); + } + } + } + + if (!processMain && !hasTestConfig) { 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(); + // Build the set of tags to remove from configuration + Set tagsToRemove = new HashSet<>(); + if (processMain) { + tagsToRemove.add("source"); + tagsToRemove.add("target"); } - return addOrUpdateChild(updated, compilerPluginConfig, - Xml.Tag.build("" + releaseVersionValue + ""), getCursor().getParentOrThrow()); + if (hasTestConfig) { + tagsToRemove.add("testSource"); + tagsToRemove.add("testTarget"); + if (!testNeedsOwnRelease) { + tagsToRemove.add("testRelease"); + } + } + + if (tagsToRemove.isEmpty()) { + return t; + } + + 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()); + } + + return updated; } - private boolean isPropertyStale(String propertyName, Accumulator acc) { + private boolean isPropertyStale(String propertyName, Set pomsWithConfig, Accumulator acc) { ResolvedGroupArtifactVersion currentGav = getResolutionResult().getPom().getGav(); // Check if any POM references this property @@ -186,24 +268,30 @@ private boolean isPropertyStale(String propertyName, Accumulator acc) { } } - // Only remove if this POM (or an ancestor in the project) has configured - if (acc.pomsWithRelease.contains(currentGav)) { + // Only remove if this POM (or a descendant in the project) has the relevant config + if (pomsWithConfig.contains(currentGav)) { return true; } - // Check if any module in the project that uses this POM as ancestor has release - for (ResolvedGroupArtifactVersion releaseGav : acc.pomsWithRelease) { - if (isAncestorOrSelf(currentGav, releaseGav)) { + for (ResolvedGroupArtifactVersion configGav : pomsWithConfig) { + if (isAncestorOrSelf(currentGav, configGav)) { return true; } } return false; } + private boolean isTestPropertyStale(String propertyName, Accumulator acc) { + // Test properties are stale if either or is configured + // (since covers both main and test compilation) + Set combined = new HashSet<>(acc.pomsWithRelease); + combined.addAll(acc.pomsWithTestRelease); + return isPropertyStale(propertyName, combined, acc); + } + private boolean isAncestorOrSelf(ResolvedGroupArtifactVersion possibleAncestor, ResolvedGroupArtifactVersion gav) { if (possibleAncestor.equals(gav)) { return true; } - // Walk up from the current resolution result to check ancestor chain MavenResolutionResult ancestor = getResolutionResult(); while (ancestor != null) { if (ancestor.getPom().getGav().equals(possibleAncestor)) { @@ -216,19 +304,31 @@ private boolean isAncestorOrSelf(ResolvedGroupArtifactVersion possibleAncestor, }; } - 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) { @@ -243,9 +343,18 @@ private boolean currentNewerThanProposed(@SuppressWarnings("OptionalUsedAsFieldO } private static boolean isDefaultMavenCompilerProperty(String value) { - return "${maven.compiler.source}".equals(value) || - "${maven.compiler.target}".equals(value) || - "${maven.compiler.release}".equals(value); + return DEFAULT_MAVEN_COMPILER_PROPERTIES.contains(value); + } + + /** + * Extracts the property name from a value that may or may not be a property reference. + * For "${foo.bar}" returns "foo.bar", for "11" returns "11". + */ + private static String extractPropertyName(String value) { + if (value.startsWith("${") && value.endsWith("}")) { + return value.substring(2, value.length() - 1); + } + return value; } private boolean hasJavaVersionProperty(Xml.Document xml) { 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 c2487c50306..e97d208e36e 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java @@ -994,6 +994,309 @@ void keepsPropertiesWithoutCompilerPlugin() { ); } + @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)) + .cycles(2).expectedCyclesThatMakeChanges(2), + //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(""" From ddf4d8c7a7fba0af35936400d5f80fd63059a968 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 12 Apr 2026 15:24:57 +0200 Subject: [PATCH 4/9] Inline isDefaultMavenCompilerProperty method --- .../maven/UseMavenCompilerPluginReleaseConfiguration.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 881ec93ef6e..6fa5746746c 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -192,7 +192,7 @@ private Xml.Tag handleCompilerPlugin(Xml.Tag t) { if (hasTestConfig) { testVersionValue = resolveVersion(testRelease, testSource, testTarget); if (testVersionValue != null) { - if (isDefaultMavenCompilerProperty("${" + extractPropertyName(testVersionValue) + "}")) { + if (DEFAULT_MAVEN_COMPILER_PROPERTIES.contains("${" + extractPropertyName(testVersionValue) + "}")) { // Default maven property like ${maven.compiler.testSource} - release covers it testNeedsOwnRelease = false; } else if (testVersionValue.startsWith("${")) { @@ -334,7 +334,7 @@ private static boolean isHigherVersion(String current, String proposed) { 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; } } @@ -342,10 +342,6 @@ private static boolean isHigherVersion(String current, String proposed) { return null; } - private static boolean isDefaultMavenCompilerProperty(String value) { - return DEFAULT_MAVEN_COMPILER_PROPERTIES.contains(value); - } - /** * Extracts the property name from a value that may or may not be a property reference. * For "${foo.bar}" returns "foo.bar", for "11" returns "11". From 2da32ee4553bfa921dba3100a5824a79d86ea035 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 12 Apr 2026 15:33:30 +0200 Subject: [PATCH 5/9] Remove stale maven.compiler.source/target properties after release replacement Convert UseMavenCompilerPluginReleaseConfiguration from Recipe to ScanningRecipe so it can detect and remove maven.compiler.source/maven.compiler.target properties that become unused after source/target is replaced with release. The scanner excludes property references from the compiler plugin's own source/target/testSource/testTarget tags (since the visitor replaces them), allowing the visitor to remove stale properties in a single cycle via doAfterVisit rather than requiring two recipe cycles. Fixes openrewrite/rewrite-migrate-java#668 --- ...venCompilerPluginReleaseConfiguration.java | 146 ++++++++---------- ...ompilerPluginReleaseConfigurationTest.java | 8 +- 2 files changed, 71 insertions(+), 83 deletions(-) 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 6fa5746746c..4ad7da03567 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -46,6 +46,10 @@ public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe COMPILER_SOURCE_TARGET_TAG_NAMES = new HashSet<>(Arrays.asList( + "source", "target", "testSource", "testTarget" + )); + @Option( displayName = "Release version", description = "The new value for the release configuration. This recipe prefers ${java.version} if defined.", @@ -62,9 +66,9 @@ public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe> propertyUsages = new HashMap<>(); - Set pomsWithRelease = new HashSet<>(); - Set pomsWithTestRelease = new HashSet<>(); } @Override @@ -75,65 +79,70 @@ public Accumulator getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(Accumulator acc) { return new MavenIsoVisitor() { + private boolean insideCompilerPlugin; + @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + boolean wasInsideCompilerPlugin = insideCompilerPlugin; + if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { + insideCompilerPlugin = true; + } + Xml.Tag t = super.visitTag(tag, ctx); - // Track ${maven.compiler.*} property usages outside of the section + // Track ${maven.compiler.*} property usages outside of , + // but skip usages from compiler plugin source/target tags (those will be replaced) if (!isPropertyTag()) { Optional value = t.getValue(); if (value.isPresent()) { - 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()); + boolean isCompilerSourceTargetTag = insideCompilerPlugin && + COMPILER_SOURCE_TARGET_TAG_NAMES.contains(t.getName()); + if (!isCompilerSourceTargetTag) { + 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()); + } } } } - // Track POMs that have or in the compiler plugin configuration - // (including per-execution configurations) - if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) { - scanCompilerPluginForRelease(t, acc); - } - + insideCompilerPlugin = wasInsideCompilerPlugin; return t; } - - private void scanCompilerPluginForRelease(Xml.Tag pluginTag, Accumulator acc) { - ResolvedGroupArtifactVersion gav = getResolutionResult().getPom().getGav(); - // Check top-level - Optional config = pluginTag.getChild("configuration"); - if (config.isPresent()) { - if (config.get().getChildValue("release").isPresent()) { - acc.pomsWithRelease.add(gav); - } - if (config.get().getChildValue("testRelease").isPresent()) { - acc.pomsWithTestRelease.add(gav); - } - } - // Check // - Optional executions = pluginTag.getChild("executions"); - if (executions.isPresent()) { - for (Xml.Tag execution : executions.get().getChildren("execution")) { - Optional execConfig = execution.getChild("configuration"); - if (execConfig.isPresent()) { - if (execConfig.get().getChildValue("release").isPresent()) { - acc.pomsWithRelease.add(gav); - } - if (execConfig.get().getChildValue("testRelease").isPresent()) { - acc.pomsWithTestRelease.add(gav); - } - } - } - } - } }; } @Override public TreeVisitor getVisitor(Accumulator acc) { return new MavenIsoVisitor() { + private final Set propertiesToRemove = new HashSet<>(); + + @Override + public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { + propertiesToRemove.clear(); + Xml.Document d = super.visitDocument(document, ctx); + + // Schedule property removal as doAfterVisit so it runs in the same cycle, + // even though property tags appear before the compiler plugin in the POM + if (!propertiesToRemove.isEmpty()) { + Set toRemove = new HashSet<>(propertiesToRemove); + doAfterVisit(new MavenIsoVisitor() { + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + Xml.Tag t = super.visitTag(tag, ctx); + if (isPropertyTag() && toRemove.contains(t.getName())) { + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); + maybeUpdateModel(); + } + return t; + } + }); + } + + return d; + } + @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); @@ -143,20 +152,6 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { t = handleCompilerPlugin(t); } - // Handle stale maven.compiler.* property removal - if (isPropertyTag()) { - String name = t.getName(); - if (("maven.compiler.source".equals(name) || "maven.compiler.target".equals(name)) && - isPropertyStale(name, acc.pomsWithRelease, acc)) { - doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); - maybeUpdateModel(); - } else if (("maven.compiler.testSource".equals(name) || "maven.compiler.testTarget".equals(name)) && - isTestPropertyStale(name, acc)) { - doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); - maybeUpdateModel(); - } - } - return t; } @@ -193,10 +188,8 @@ private Xml.Tag handleCompilerPlugin(Xml.Tag t) { testVersionValue = resolveVersion(testRelease, testSource, testTarget); if (testVersionValue != null) { if (DEFAULT_MAVEN_COMPILER_PROPERTIES.contains("${" + extractPropertyName(testVersionValue) + "}")) { - // Default maven property like ${maven.compiler.testSource} - release covers it testNeedsOwnRelease = false; } else if (testVersionValue.startsWith("${")) { - // Custom property reference - preserve as testRelease testNeedsOwnRelease = true; } else { testNeedsOwnRelease = isHigherVersion(testVersionValue, releaseVersion.toString()); @@ -252,40 +245,35 @@ private Xml.Tag handleCompilerPlugin(Xml.Tag t) { 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 boolean isPropertyStale(String propertyName, Set pomsWithConfig, Accumulator acc) { + private void markPropertyForRemovalIfUnused(String propertyName, Accumulator acc) { ResolvedGroupArtifactVersion currentGav = getResolutionResult().getPom().getGav(); - // Check if any POM references this property Set usages = acc.propertyUsages.get(propertyName); if (usages != null) { for (ResolvedGroupArtifactVersion usingGav : usages) { if (isAncestorOrSelf(currentGav, usingGav)) { - return false; + return; } } } - // Only remove if this POM (or a descendant in the project) has the relevant config - if (pomsWithConfig.contains(currentGav)) { - return true; - } - for (ResolvedGroupArtifactVersion configGav : pomsWithConfig) { - if (isAncestorOrSelf(currentGav, configGav)) { - return true; - } - } - return false; - } - - private boolean isTestPropertyStale(String propertyName, Accumulator acc) { - // Test properties are stale if either or is configured - // (since covers both main and test compilation) - Set combined = new HashSet<>(acc.pomsWithRelease); - combined.addAll(acc.pomsWithTestRelease); - return isPropertyStale(propertyName, combined, acc); + propertiesToRemove.add(propertyName); } private boolean isAncestorOrSelf(ResolvedGroupArtifactVersion possibleAncestor, ResolvedGroupArtifactVersion gav) { 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 e97d208e36e..6fa2d749bc8 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java @@ -770,7 +770,7 @@ void skipsPluginWithExecutionConfiguration() { void removesStaleSourceTargetProperties() { rewriteRun( spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(2).expectedCyclesThatMakeChanges(2), + .cycles(1).expectedCyclesThatMakeChanges(1), //language=xml pomXml( """ @@ -832,7 +832,7 @@ void removesStaleSourceTargetProperties() { void removesImplicitlyStaleProperties() { rewriteRun( spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(2).expectedCyclesThatMakeChanges(2), + .cycles(1).expectedCyclesThatMakeChanges(1), //language=xml pomXml( """ @@ -894,7 +894,7 @@ void removesImplicitlyStaleProperties() { void keepsPropertiesWhenReferencedElsewhere() { rewriteRun( spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(2).expectedCyclesThatMakeChanges(2), + .cycles(1).expectedCyclesThatMakeChanges(1), //language=xml pomXml( """ @@ -1180,7 +1180,7 @@ void preservesCustomPropertyRefInTestRelease() { void removesStaleTestSourceTargetProperties() { rewriteRun( spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(2).expectedCyclesThatMakeChanges(2), + .cycles(1).expectedCyclesThatMakeChanges(1), //language=xml pomXml( """ From 620393b886622bd5832d6443f9ca639fff571891 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 12 Apr 2026 15:34:19 +0200 Subject: [PATCH 6/9] Remove redundant single-cycle test configuration --- ...eMavenCompilerPluginReleaseConfigurationTest.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 6fa2d749bc8..c1b07e908cf 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java @@ -769,8 +769,7 @@ void skipsPluginWithExecutionConfiguration() { @Test void removesStaleSourceTargetProperties() { rewriteRun( - spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(1).expectedCyclesThatMakeChanges(1), + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), //language=xml pomXml( """ @@ -831,8 +830,7 @@ void removesStaleSourceTargetProperties() { @Test void removesImplicitlyStaleProperties() { rewriteRun( - spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(1).expectedCyclesThatMakeChanges(1), + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), //language=xml pomXml( """ @@ -893,8 +891,7 @@ void removesImplicitlyStaleProperties() { @Test void keepsPropertiesWhenReferencedElsewhere() { rewriteRun( - spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(1).expectedCyclesThatMakeChanges(1), + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), //language=xml pomXml( """ @@ -1179,8 +1176,7 @@ void preservesCustomPropertyRefInTestRelease() { @Test void removesStaleTestSourceTargetProperties() { rewriteRun( - spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)) - .cycles(1).expectedCyclesThatMakeChanges(1), + spec -> spec.recipe(new UseMavenCompilerPluginReleaseConfiguration(21)), //language=xml pomXml( """ From a6f4b0eee7ad23d9a1122f8830d28b13026c6126 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 12 Apr 2026 16:11:38 +0200 Subject: [PATCH 7/9] Simplify scanner by returning early for compiler plugin tags Instead of tracking insideCompilerPlugin state, return tag early when visiting the compiler plugin to skip its children entirely. The visitor handles those tags directly, so the scanner doesn't need to see them. --- ...venCompilerPluginReleaseConfiguration.java | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) 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 4ad7da03567..93a71b2d36b 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -46,10 +46,6 @@ public class UseMavenCompilerPluginReleaseConfiguration extends ScanningRecipe COMPILER_SOURCE_TARGET_TAG_NAMES = new HashSet<>(Arrays.asList( - "source", "target", "testSource", "testTarget" - )); - @Option( displayName = "Release version", description = "The new value for the release configuration. This recipe prefers ${java.version} if defined.", @@ -79,35 +75,28 @@ public Accumulator getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(Accumulator acc) { return new MavenIsoVisitor() { - private boolean insideCompilerPlugin; - @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { - boolean wasInsideCompilerPlugin = insideCompilerPlugin; + // 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")) { - insideCompilerPlugin = true; + return tag; } Xml.Tag t = super.visitTag(tag, ctx); - // Track ${maven.compiler.*} property usages outside of , - // but skip usages from compiler plugin source/target tags (those will be replaced) + // Track ${maven.compiler.*} property usages outside of if (!isPropertyTag()) { Optional value = t.getValue(); if (value.isPresent()) { - boolean isCompilerSourceTargetTag = insideCompilerPlugin && - COMPILER_SOURCE_TARGET_TAG_NAMES.contains(t.getName()); - if (!isCompilerSourceTargetTag) { - 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()); - } + 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()); } } } - insideCompilerPlugin = wasInsideCompilerPlugin; return t; } }; From 82f16b1f11d5793f7b1520760f5a3725ee308d53 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 12 Apr 2026 16:25:15 +0200 Subject: [PATCH 8/9] Simplify stale property removal by calling doAfterVisit directly Remove the intermediate propertiesToRemove set and visitDocument override. Each markPropertyForRemovalIfUnused call now schedules its own doAfterVisit directly. Also simplify testNeedsOwnRelease check and remove unused extractPropertyName method. --- ...venCompilerPluginReleaseConfiguration.java | 54 +++++-------------- 1 file changed, 13 insertions(+), 41 deletions(-) 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 93a71b2d36b..4ef22991ffd 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfiguration.java @@ -88,7 +88,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { // Track ${maven.compiler.*} property usages outside of if (!isPropertyTag()) { Optional value = t.getValue(); - if (value.isPresent()) { + 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<>()) @@ -105,33 +105,6 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { @Override public TreeVisitor getVisitor(Accumulator acc) { return new MavenIsoVisitor() { - private final Set propertiesToRemove = new HashSet<>(); - - @Override - public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { - propertiesToRemove.clear(); - Xml.Document d = super.visitDocument(document, ctx); - - // Schedule property removal as doAfterVisit so it runs in the same cycle, - // even though property tags appear before the compiler plugin in the POM - if (!propertiesToRemove.isEmpty()) { - Set toRemove = new HashSet<>(propertiesToRemove); - doAfterVisit(new MavenIsoVisitor() { - @Override - public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { - Xml.Tag t = super.visitTag(tag, ctx); - if (isPropertyTag() && toRemove.contains(t.getName())) { - doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); - maybeUpdateModel(); - } - return t; - } - }); - } - - return d; - } - @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); @@ -176,7 +149,7 @@ private Xml.Tag handleCompilerPlugin(Xml.Tag t) { if (hasTestConfig) { testVersionValue = resolveVersion(testRelease, testSource, testTarget); if (testVersionValue != null) { - if (DEFAULT_MAVEN_COMPILER_PROPERTIES.contains("${" + extractPropertyName(testVersionValue) + "}")) { + if (DEFAULT_MAVEN_COMPILER_PROPERTIES.contains(testVersionValue)) { testNeedsOwnRelease = false; } else if (testVersionValue.startsWith("${")) { testNeedsOwnRelease = true; @@ -262,7 +235,17 @@ private void markPropertyForRemovalIfUnused(String propertyName, Accumulator acc } } - propertiesToRemove.add(propertyName); + 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) { @@ -319,17 +302,6 @@ private static boolean isHigherVersion(String current, String proposed) { return null; } - /** - * Extracts the property name from a value that may or may not be a property reference. - * For "${foo.bar}" returns "foo.bar", for "11" returns "11". - */ - private static String extractPropertyName(String value) { - if (value.startsWith("${") && value.endsWith("}")) { - return value.substring(2, value.length() - 1); - } - return value; - } - private boolean hasJavaVersionProperty(Xml.Document xml) { return xml.getMarkers().findFirst(MavenResolutionResult.class) .map(r -> r.getPom().getProperties().get("java.version") != null) From fe19f7eede29fd3ec9c6aabe734a6666c9788c0e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 14 Apr 2026 18:24:22 +0200 Subject: [PATCH 9/9] Add multi-module test for stale property removal with inherited properties --- ...ompilerPluginReleaseConfigurationTest.java | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) 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 c1b07e908cf..a71e80ff8be 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UseMavenCompilerPluginReleaseConfigurationTest.java @@ -826,6 +826,129 @@ void removesStaleSourceTargetProperties() { ); } + @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() {