Fix AddCommentToXmlTag for self-closing and empty tags#7779
Conversation
The recipe previously silently skipped self-closing tags and threw IndexOutOfBoundsException on empty tags. Route through withContent consistently so the Closing tag is materialized when needed.
| "<root><foo bar=\"baz\"/></root>", | ||
| "<root><foo bar=\"baz\"><!-- hello --></foo></root>" |
There was a problem hiding this comment.
I wonder if we should prepend the comment to the self closing tag instead of adding a new close tag to add the comment within the tag. What are your thoughts on that?
My motivation would be a more minimal diff, with a clear line introduced before the existing line in a well formatted XML document.
There was a problem hiding this comment.
I also preferred that, however, that is a bigger change as there is already expectations of comments being added inside the tags.
Would you want to have different behavior when tag is empty vs non-empty?
There was a problem hiding this comment.
I'd be fine with slightly different behavior if that leads to less surprises for the self closing case.
There was a problem hiding this comment.
implemented this change here.
Previously the recipe always inserted the comment inside the matched tag, which forced self-closing tags like <foo bar="baz"/> to be opened up into <foo bar="baz"><!-- ... --></foo>. Reviewer feedback prefers keeping the tag self-closing and emitting the comment on the line just before it, which yields a smaller, more idiomatic diff. Empty tags (<foo></foo>) and tags with content keep the existing in-tag behavior.
Summary
AddCommentToXmlTagsilently skipped self-closing tags (<foo/>) and threwIndexOutOfBoundsExceptionon empty tags (<foo></foo>).""when content is null/empty) and always routes throughXml.Tag#withContent, which already materializes aClosingtag when transitioning a self-closing tag to one with content.tag.getContent()(pre-recursion) witht.withContent(...), dropping nested transformations.Test plan
<foo bar="baz"/>→<foo bar="baz"><!-- hello --></foo>).<foo></foo>→<foo><!-- hello --></foo>) with no exception.addCommentToDependencyBlocktest still passes../gradlew :rewrite-xml:test --tests "org.openrewrite.xml.AddCommentToXmlTagTest"→ BUILD SUCCESSFUL.