Skip to content

Stop MergeYamlVisitor from grafting the document end comment onto inserted entries - #8432

Merged
timtebeek merged 2 commits into
mainfrom
tim/cape-town-v3
Aug 8, 2026
Merged

Stop MergeYamlVisitor from grafting the document end comment onto inserted entries#8432
timtebeek merged 2 commits into
mainfrom
tim/cape-town-v3

Conversation

@timtebeek

@timtebeek timtebeek commented Aug 8, 2026

Copy link
Copy Markdown
Member

Fixes #8416

An inline comment on the last line of a document is stored on Yaml.Document.End. When MergeYamlVisitor appends entries to the block that comment trails, it has to copy the comment onto the first newly inserted entry, otherwise the comment slides down to the last line of the inserted block. Removing the copy it moved away from was left entirely to MergeYaml, through cursor messages on ancestors that MergeYamlVisitor does not traverse itself, so anyone using the visitor directly got the comment rendered twice.

MergeYamlVisitor now consumes those messages for the documents and mappings it does traverse, so applying it to the enclosing Yaml.Documents/Yaml.Document produces the same result as the MergeYaml recipe. That is now stated in the class javadoc. Merging into a block without passing the enclosing document remains unable to clean up Yaml.Document.End — the visitor returns only the block it was given — hence the javadoc note.

While confirming the recipe path, three related defects surfaced in how the document end prefix is treated. The first two only show up on documents ending in a line break, which RewriteTest strips from its text blocks, which is why no existing test caught them:

  • The whole prefix was hoisted as the comment, so value: true # inline\n inserted a blank line before the new entry and dropped the file's trailing line break. Only the part before the first line break is taken now, and the rest is retained.
  • A comment on a line of its own at the end of a document was hoisted too, moving it above the inserted block. It now stays put.
  • The createNewKeys branch of MergeYaml.visitDocument returned before the cleanup ran, so the comment was duplicated onto the key created on the spot. Both branches now fall through to a single tail.

…erted entries

An inline comment on the last line of a document is stored on `Yaml.Document.End`,
so it has to be moved onto the first newly inserted entry to stay on its own line.
Removing the original copy was left entirely to `MergeYaml`, which meant the comment
was rendered twice when `MergeYamlVisitor` was used directly. It now consumes those
cursor messages itself for the documents and mappings it traverses.

Only the part of the document end prefix before the first line break is treated as
an inline comment now; a comment on a line of its own stays where it is, and a
trailing line break is no longer swallowed.

Fixes #8416
The `createNewKeys` branch of `visitDocument` returned before the cleanup ran, so a
trailing inline comment was duplicated onto the key that was created on the spot.
Both branches now fall through to a single tail.
@timtebeek
timtebeek merged commit cf8ad1a into main Aug 8, 2026
1 check passed
@timtebeek
timtebeek deleted the tim/cape-town-v3 branch August 8, 2026 21:56
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

MergeYamlVisitor grafts Document.End inline comment onto inserted entries when used directly

1 participant