Skip to content

Commit c691c9f

Browse files
authored
Adopt upstream XML trailing-comment formatting (#1180)
* 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 `<argLine>` is appended into an existing `<configuration>`, the accompanying `<!--suppress MavenModelInspection -->` 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 `<argLine>` 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. * 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. * 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.
1 parent 908a523 commit c691c9f

3 files changed

Lines changed: 22 additions & 22 deletions

File tree

src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public class AddMockitoJavaAgentToMavenSurefirePlugin extends Recipe {
7070
@Override
7171
public TreeVisitor<?, ExecutionContext> getVisitor() {
7272
return Preconditions.check(new DependencyInsight("org.mockito", "mockito-core", "test", null, false), new MavenIsoVisitor<ExecutionContext>() {
73-
private final String CONFIGURATION_TAG_TEMPLATE = "<configuration><!--suppress MavenModelInspection --><argLine>%s</argLine></configuration>";
73+
// The newline before the comment keeps auto-format from attaching it to a preceding sibling as a trailing comment
74+
private final String CONFIGURATION_TAG_TEMPLATE = "<configuration>\n<!--suppress MavenModelInspection --><argLine>%s</argLine></configuration>";
7475

7576
private String getArgLineJavaAgentArgument() {
7677
String mockitoCoreVersion = getResolutionResult().getDependencies().getOrDefault(Scope.Test, emptyList()).stream()

src/main/java/org/openrewrite/java/migrate/JpaCacheProperties.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.*;
2222
import org.openrewrite.internal.ListUtils;
23+
import org.openrewrite.xml.RemoveContentVisitor;
2324
import org.openrewrite.xml.XPathMatcher;
2425
import org.openrewrite.xml.XmlVisitor;
2526
import org.openrewrite.xml.tree.Content;
@@ -31,7 +32,6 @@
3132
import java.util.Optional;
3233

3334
import static org.openrewrite.xml.AddOrUpdateChild.addOrUpdateChild;
34-
import static org.openrewrite.xml.FilterTagChildrenVisitor.filterTagChildren;
3535

3636
@EqualsAndHashCode(callSuper = false)
3737
@Value
@@ -185,20 +185,28 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
185185
if (sdh.openJPACacheProperty != null) {
186186
String attrValue = getAttributeValue("value", sdh.openJPACacheProperty);
187187
if ("true".equalsIgnoreCase(attrValue) || "false".equalsIgnoreCase(attrValue)) {
188-
sdh.propertiesElement = filterTagChildren(sdh.propertiesElement, child -> child != sdh.openJPACacheProperty);
188+
sdh.propertiesElement = removeContent(sdh.propertiesElement, sdh.openJPACacheProperty, ctx);
189189
t = addOrUpdateChild(t, sdh.propertiesElement, getCursor().getParentOrThrow());
190190
}
191191
}
192192

193193
// if both shared-cache-mode and javax cache property are set, delete the
194194
// javax cache property
195195
if (sdh.sharedCacheModeElement != null && sdh.sharedCacheModeProperty != null) {
196-
sdh.propertiesElement = filterTagChildren(sdh.propertiesElement, child -> child != sdh.sharedCacheModeProperty);
196+
sdh.propertiesElement = removeContent(sdh.propertiesElement, sdh.sharedCacheModeProperty, ctx);
197197
t = addOrUpdateChild(t, sdh.propertiesElement, getCursor().getParentOrThrow());
198198
}
199199
return t;
200200
}
201201

202+
/**
203+
* Unlike {@code filterTagChildren}, this hands the removed element's prefix to a comment that trailed it,
204+
* so the comment keeps its own line rather than collapsing onto the preceding sibling.
205+
*/
206+
private static Xml.Tag removeContent(Xml.Tag parent, Xml.Tag child, ExecutionContext ctx) {
207+
return (Xml.Tag) new RemoveContentVisitor<ExecutionContext>(child, false, false).visitNonNull(parent, ctx);
208+
}
209+
202210
private SharedDataHolder extractData(Xml.Tag puNode) {
203211
SharedDataHolder sdh = new SharedDataHolder();
204212

src/test/java/org/openrewrite/java/migrate/JpaCachePropertiesTest.java

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,7 @@ void notset_set_set1() {
607607
<properties>
608608
<!-- Connection properties -->
609609
<!-- remove -->
610-
<property name="javax.persistence.sharedCache.mode" value="NONE"/>
611-
<!-- leave -->
610+
<property name="javax.persistence.sharedCache.mode" value="NONE"/><!-- leave -->
612611
</properties>
613612
</persistence-unit>
614613
</persistence>
@@ -644,8 +643,7 @@ void notset_set_set2() {
644643
<properties>
645644
<!-- Connection properties -->
646645
<!-- remove -->
647-
<property name="javax.persistence.sharedCache.mode" value="ALL"/>
648-
<!-- leave -->
646+
<property name="javax.persistence.sharedCache.mode" value="ALL"/><!-- leave -->
649647
</properties>
650648
</persistence-unit>
651649
</persistence>
@@ -681,8 +679,7 @@ void notset_set_set3() {
681679
<properties>
682680
<!-- Connection properties -->
683681
<!-- remove -->
684-
<property name="javax.persistence.sharedCache.mode" value="ALL"/>
685-
<!-- change to ALL -->
682+
<property name="javax.persistence.sharedCache.mode" value="ALL"/><!-- change to ALL -->
686683
</properties>
687684
</persistence-unit>
688685
</persistence>
@@ -783,8 +780,7 @@ void notset_set_notset3() {
783780
<validation-mode>NONE</validation-mode>
784781
<properties>
785782
<!-- Connection properties -->
786-
<property name="javax.persistence.sharedCache.mode" value="NONE"/>
787-
<!-- change to NONE -->
783+
<property name="javax.persistence.sharedCache.mode" value="NONE"/><!-- change to NONE -->
788784
</properties>
789785
</persistence-unit>
790786
</persistence>
@@ -909,14 +905,12 @@ void notset_notset_set3() {
909905
<?xml version="1.0" encoding="UTF-8"?>
910906
<persistence version="2.0" xmlns="http://java.sun.com/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd">
911907
<persistence-unit name="notset_notset_set3">
912-
<!-- flag -->
913-
<!-- add shared-cache-mode ENABLE_SELECTIVE -->
908+
<!-- flag --> <!-- add shared-cache-mode ENABLE_SELECTIVE -->
914909
<shared-cache-mode>ENABLE_SELECTIVE</shared-cache-mode>
915910
<validation-mode>NONE</validation-mode>
916911
<properties>
917912
<!-- Connection properties -->
918-
<property name="openjpa.DataCache" value="truE(Types=foo.bar.Person;foo.bar.Employee)"/>
919-
<!-- leave - manual fix-->
913+
<property name="openjpa.DataCache" value="truE(Types=foo.bar.Person;foo.bar.Employee)"/><!-- leave - manual fix-->
920914
</properties>
921915
</persistence-unit>
922916
</persistence>
@@ -947,14 +941,12 @@ void notset_notset_set4() {
947941
<?xml version="1.0" encoding="UTF-8"?>
948942
<persistence version="2.0" xmlns="http://java.sun.com/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd">
949943
<persistence-unit name="notset_notset_set4">
950-
<!-- flag -->
951-
<!-- add shared-cache-mode DISABLE_SELECTIVE -->
944+
<!-- flag --> <!-- add shared-cache-mode DISABLE_SELECTIVE -->
952945
<shared-cache-mode>DISABLE_SELECTIVE</shared-cache-mode>
953946
<validation-mode>NONE</validation-mode>
954947
<properties>
955948
<!-- Connection properties -->
956-
<property name="openjpa.DataCache" value="TRUE(ExcludedTypes=foo.bar.Person;foo.bar.Employee)"/>
957-
<!-- leave - manual fix -->
949+
<property name="openjpa.DataCache" value="TRUE(ExcludedTypes=foo.bar.Person;foo.bar.Employee)"/><!-- leave - manual fix -->
958950
</properties>
959951
</persistence-unit>
960952
</persistence>
@@ -1139,8 +1131,7 @@ void openjpa_cache3_flagged() {
11391131
<?xml version="1.0" encoding="UTF-8"?>
11401132
<persistence version="2.0" xmlns="http://java.sun.com/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd">
11411133
<persistence-unit name="openjpa_cache3_flagged">
1142-
<!-- flag -->
1143-
<!-- create shared-cache-mode NONE -->
1134+
<!-- flag --><!-- create shared-cache-mode NONE -->
11441135
<shared-cache-mode>NONE</shared-cache-mode>
11451136
<validation-mode>NONE</validation-mode>
11461137
<properties>

0 commit comments

Comments
 (0)