From c1e2d1ad3115a0250e548ec2a55d73fcd821c634 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 30 Jul 2026 19:32:18 +0200 Subject: [PATCH 1/3] Adopt upstream XML trailing-comment formatting openrewrite/rewrite#8353 made XML auto-format keep a comment on the same line as the element it trails, instead of reflowing it onto its own line. `AddMockitoJavaAgentToMavenSurefirePlugin` relied on that reflow: when its `` is appended into an existing ``, the accompanying `` is no longer the first content of the tag, so it stayed glued to the preceding sibling and read as annotating that element rather than the `` below it. Seed the comment with a newline prefix so auto-format keeps it on its own line either way. `JpaCachePropertiesTest` expectations reflowed comments that the input had placed on the same line; they now assert the input's own placement. --- ...MockitoJavaAgentToMavenSurefirePlugin.java | 3 +- .../java/migrate/JpaCachePropertiesTest.java | 78 ++++++------------- 2 files changed, 27 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index 9b96d5da4e..939e422547 100644 --- a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java +++ b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java @@ -70,7 +70,8 @@ public class AddMockitoJavaAgentToMavenSurefirePlugin extends Recipe { @Override public TreeVisitor getVisitor() { return Preconditions.check(new DependencyInsight("org.mockito", "mockito-core", "test", null, false), new MavenIsoVisitor() { - private final String CONFIGURATION_TAG_TEMPLATE = "%s"; + // The newline before the comment keeps auto-format from attaching it to a preceding sibling as a trailing comment + private final String CONFIGURATION_TAG_TEMPLATE = "\n%s"; private String getArgLineJavaAgentArgument() { String mockitoCoreVersion = getResolutionResult().getDependencies().getOrDefault(Scope.Test, emptyList()).stream() diff --git a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java index 28f944a3f0..5ccafb6e6c 100644 --- a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java @@ -58,9 +58,7 @@ void set_set_set1() { NONE NONE - - - + @@ -96,9 +94,7 @@ void set_set_set2() { ALL NONE - - - + @@ -134,9 +130,7 @@ void set_set_set3() { ALL NONE - - - + @@ -173,8 +167,7 @@ void set_set_set4() { NONE - - + @@ -209,8 +202,7 @@ void set_set_notset1() { NONE NONE - - + @@ -245,8 +237,7 @@ void set_set_notset2() { ALL NONE - - + @@ -281,8 +272,7 @@ void set_set_notset3() { NONE NONE - - + @@ -317,8 +307,7 @@ void set_set_notset4() { ALL NONE - - + @@ -410,8 +399,7 @@ void set_notset_set2() { ALL NONE - - + @@ -446,8 +434,7 @@ void set_notset_set3() { ALL NONE - - + @@ -605,10 +592,8 @@ void notset_set_set1() { NONE - - - - + + @@ -642,10 +627,8 @@ void notset_set_set2() { NONE - - - - + + @@ -679,10 +662,8 @@ void notset_set_set3() { NONE - - - - + + @@ -783,8 +764,7 @@ void notset_set_notset3() { NONE - - + @@ -841,8 +821,7 @@ void notset_notset_set1() { NONE NONE - - + @@ -877,8 +856,7 @@ void notset_notset_set2() { ALL NONE - - + @@ -909,14 +887,12 @@ void notset_notset_set3() { - - + ENABLE_SELECTIVE NONE - - + @@ -947,14 +923,12 @@ void notset_notset_set4() { - - + DISABLE_SELECTIVE NONE - - + @@ -1074,8 +1048,7 @@ void openjpa_cache1_flagged() { NONE NONE - - + @@ -1139,8 +1112,7 @@ void openjpa_cache3_flagged() { - - + NONE NONE From 6bb6de9b231064b7aca8521d1241dcfff347fffd Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 30 Jul 2026 20:24:52 +0200 Subject: [PATCH 2/3] Revert JpaCachePropertiesTest expectations to own-line comments Restores the pre-existing expectations, which assert that auto-format moves a trailing comment onto its own line. All 20 cases fail against the current rewrite snapshot: openrewrite/rewrite#8353 now keeps such comments on the line of the element they trail. Leaving the expectations as the desired behaviour rather than absorbing the new output into the test. Red until the formatting is addressed. --- .../java/migrate/JpaCachePropertiesTest.java | 78 +++++++++++++------ 1 file changed, 53 insertions(+), 25 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java index 5ccafb6e6c..28f944a3f0 100644 --- a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java @@ -58,7 +58,9 @@ void set_set_set1() { NONE NONE - + + + @@ -94,7 +96,9 @@ void set_set_set2() { ALL NONE - + + + @@ -130,7 +134,9 @@ void set_set_set3() { ALL NONE - + + + @@ -167,7 +173,8 @@ void set_set_set4() { NONE - + + @@ -202,7 +209,8 @@ void set_set_notset1() { NONE NONE - + + @@ -237,7 +245,8 @@ void set_set_notset2() { ALL NONE - + + @@ -272,7 +281,8 @@ void set_set_notset3() { NONE NONE - + + @@ -307,7 +317,8 @@ void set_set_notset4() { ALL NONE - + + @@ -399,7 +410,8 @@ void set_notset_set2() { ALL NONE - + + @@ -434,7 +446,8 @@ void set_notset_set3() { ALL NONE - + + @@ -592,8 +605,10 @@ void notset_set_set1() { NONE - - + + + + @@ -627,8 +642,10 @@ void notset_set_set2() { NONE - - + + + + @@ -662,8 +679,10 @@ void notset_set_set3() { NONE - - + + + + @@ -764,7 +783,8 @@ void notset_set_notset3() { NONE - + + @@ -821,7 +841,8 @@ void notset_notset_set1() { NONE NONE - + + @@ -856,7 +877,8 @@ void notset_notset_set2() { ALL NONE - + + @@ -887,12 +909,14 @@ void notset_notset_set3() { - + + ENABLE_SELECTIVE NONE - + + @@ -923,12 +947,14 @@ void notset_notset_set4() { - + + DISABLE_SELECTIVE NONE - + + @@ -1048,7 +1074,8 @@ void openjpa_cache1_flagged() { NONE NONE - + + @@ -1112,7 +1139,8 @@ void openjpa_cache3_flagged() { - + + NONE NONE From b7ce74dd34cfdaea4fba96e71c6e5b88fcfa57d1 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 30 Jul 2026 21:06:13 +0200 Subject: [PATCH 3/3] Route JpaCacheProperties removals through RemoveContentVisitor openrewrite/rewrite#8355 taught `RemoveContentVisitor` to hand a removed element's prefix to a comment that trailed it, so the comment keeps its own line instead of collapsing onto the preceding sibling. `JpaCacheProperties` removed via `filterTagChildren`, which drops content outright and so never saw that fix; switching the two single-element removals over picks it up and restores the original expectations for 13 of the 20 cases. The remaining 7 remove nothing: the element the comment trails is retained (or merely has an attribute updated), so #8353 now faithfully reproduces the inline placement the input used. Those expectations were asserting the old reflow and are updated to match the source. --- .../java/migrate/JpaCacheProperties.java | 14 +++++++--- .../java/migrate/JpaCachePropertiesTest.java | 27 +++++++------------ 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/JpaCacheProperties.java b/src/main/java/org/openrewrite/java/migrate/JpaCacheProperties.java index 42cd88f511..2c1813eb69 100644 --- a/src/main/java/org/openrewrite/java/migrate/JpaCacheProperties.java +++ b/src/main/java/org/openrewrite/java/migrate/JpaCacheProperties.java @@ -20,6 +20,7 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; +import org.openrewrite.xml.RemoveContentVisitor; import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.XmlVisitor; import org.openrewrite.xml.tree.Content; @@ -31,7 +32,6 @@ import java.util.Optional; import static org.openrewrite.xml.AddOrUpdateChild.addOrUpdateChild; -import static org.openrewrite.xml.FilterTagChildrenVisitor.filterTagChildren; @EqualsAndHashCode(callSuper = false) @Value @@ -185,7 +185,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { if (sdh.openJPACacheProperty != null) { String attrValue = getAttributeValue("value", sdh.openJPACacheProperty); if ("true".equalsIgnoreCase(attrValue) || "false".equalsIgnoreCase(attrValue)) { - sdh.propertiesElement = filterTagChildren(sdh.propertiesElement, child -> child != sdh.openJPACacheProperty); + sdh.propertiesElement = removeContent(sdh.propertiesElement, sdh.openJPACacheProperty, ctx); t = addOrUpdateChild(t, sdh.propertiesElement, getCursor().getParentOrThrow()); } } @@ -193,12 +193,20 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { // if both shared-cache-mode and javax cache property are set, delete the // javax cache property if (sdh.sharedCacheModeElement != null && sdh.sharedCacheModeProperty != null) { - sdh.propertiesElement = filterTagChildren(sdh.propertiesElement, child -> child != sdh.sharedCacheModeProperty); + sdh.propertiesElement = removeContent(sdh.propertiesElement, sdh.sharedCacheModeProperty, ctx); t = addOrUpdateChild(t, sdh.propertiesElement, getCursor().getParentOrThrow()); } return t; } + /** + * Unlike {@code filterTagChildren}, this hands the removed element's prefix to a comment that trailed it, + * so the comment keeps its own line rather than collapsing onto the preceding sibling. + */ + private static Xml.Tag removeContent(Xml.Tag parent, Xml.Tag child, ExecutionContext ctx) { + return (Xml.Tag) new RemoveContentVisitor(child, false, false).visitNonNull(parent, ctx); + } + private SharedDataHolder extractData(Xml.Tag puNode) { SharedDataHolder sdh = new SharedDataHolder(); diff --git a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java index 28f944a3f0..94baafd4aa 100644 --- a/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java @@ -607,8 +607,7 @@ void notset_set_set1() { - - + @@ -644,8 +643,7 @@ void notset_set_set2() { - - + @@ -681,8 +679,7 @@ void notset_set_set3() { - - + @@ -783,8 +780,7 @@ void notset_set_notset3() { NONE - - + @@ -909,14 +905,12 @@ void notset_notset_set3() { - - + ENABLE_SELECTIVE NONE - - + @@ -947,14 +941,12 @@ void notset_notset_set4() { - - + DISABLE_SELECTIVE NONE - - + @@ -1139,8 +1131,7 @@ void openjpa_cache3_flagged() { - - + NONE NONE