Skip to content

Fix duplicated comments in for..of formatter#8395

Merged
cknitt merged 2 commits intorescript-lang:masterfrom
cknitt:fix-for-of-formatting
Apr 27, 2026
Merged

Fix duplicated comments in for..of formatter#8395
cknitt merged 2 commits intorescript-lang:masterfrom
cknitt:fix-for-of-formatting

Conversation

@cknitt
Copy link
Copy Markdown
Member

@cknitt cknitt commented Apr 27, 2026

Summary

Fixes #8394.

  • Fix comment attachment for for..of and for await..of loops.
  • Add formatter regression coverage for comments in both loop forms.
  • Confirm range for and while loops are not affected by the issue.

Details

Issue #8394 was caused by the comment walker for Pexp_for_of and
Pexp_for_await_of passing the same full comment list to the loop pattern,
iterable expression, and body. That allowed a single source comment to attach to
multiple nodes, so formatting duplicated it and repeated formatting compounded
the duplication.

The fix partitions comments through the pattern, iterable expression, and body,
matching the ownership approach already used by normal range for loops.

Verification

  • Reproduced the original duplication locally before the fix.

  • Checked for..of, for await..of, range for, and while with body
    comments after the fix.

  • Ran targeted printer fixture diff:

    _build/install/default/bin/res_parser tests/syntax_tests/data/printer/comments/expr.res > /tmp/expr8394.out
    diff -u tests/syntax_tests/data/printer/comments/expected/expr.res.txt /tmp/expr8394.out
  • Ran a two-pass formatter roundtrip for the reproducer.

  • Ran make checkformat.

@cknitt cknitt requested a review from fhammerschmidt April 27, 2026 10:11
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 27, 2026

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8395

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8395

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8395

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8395

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8395

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8395

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8395

commit: 9368d41

@cknitt cknitt merged commit 872c596 into rescript-lang:master Apr 27, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comments duplicated when formatting for..of loop

2 participants