Skip to content

fix: keep leading block comment of clause item on its own line#952

Merged
nene merged 2 commits into
sql-formatter-org:masterfrom
sarathfrancis90:fix-block-comment-idempotency
Jun 4, 2026
Merged

fix: keep leading block comment of clause item on its own line#952
nene merged 2 commits into
sql-formatter-org:masterfrom
sarathfrancis90:fix-block-comment-idempotency

Conversation

@sarathfrancis90
Copy link
Copy Markdown
Contributor

@sarathfrancis90 sarathfrancis90 commented Jun 3, 2026

A block comment before the first item of a clause (e.g. SELECT /* c */ foo FROM tbl) was formatted inline on the first pass but moved to its own line on the second — so formatting wasn't idempotent.

The inline-vs-standalone decision used only the comment's precedingWhitespace. Since the formatter emits a newline before a clause's first item, the second pass sees that structural newline and flips the decision. Now a block comment is also treated as standalone when it's already at the start of the line in the output being built, so the first pass already matches the fixpoint. Genuinely-inline comments are unaffected.

Added tests covering comments in more positions (between items, after FROM/JOIN, in UPDATE/SET) to confirm it generalizes.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved block comment formatting to intelligently determine placement based on content and position context. Block comments now consistently appear on their own lines when appropriate, with enhanced support for idempotent formatting to ensure results remain stable across multiple formatting passes.

A block comment placed before the first item of a clause (e.g.
`SELECT /* c */ foo FROM tbl`) was formatted inline on the first pass,
but moved onto its own line when the result was formatted again, making
formatting non-idempotent.

The inline-vs-standalone decision for a block comment was based solely on
its `precedingWhitespace`. The formatter always emits a newline before the
first item of a clause body, so on a second pass that structural newline
becomes the comment's preceding whitespace and flips the decision.

Treat a block comment as standalone whenever it is already at the start of
a line in the output being built, in addition to the existing checks. This
makes the first pass place such comments on their own line directly, so the
output is stable under repeated formatting. Inline block comments that
follow other tokens on the same line (trailing comments, comments between a
function name and its parentheses, or around the `.` of a qualified name)
are unaffected.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d17ffee-d6e1-41c8-9ecd-6706ed0b3e30

📥 Commits

Reviewing files that changed from the base of the PR and between 2b70a66 and 54d4cf2.

📒 Files selected for processing (1)
  • test/features/comments.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/features/comments.ts

📝 Walkthrough

Walkthrough

The PR makes block comment emission position-aware: Layout gains isAtStartOfLine(), ExpressionFormatter uses a new isStandaloneBlockComment to decide splitting vs inline emission, and tests validate idempotent formatting for block comments in several clause positions.

Changes

Block Comment Standalone Formatting

Layer / File(s) Summary
Layout line position detection
src/formatter/Layout.ts
New public isAtStartOfLine() method inspects the internal items buffer to determine whether the next token would be placed at the start of a new line, accounting for trailing indentation.
Expression formatter standalone block comment logic
src/formatter/ExpressionFormatter.ts
New isStandaloneBlockComment helper classifies block comments as standalone when they are multiline (or have multiline preceding whitespace) or when the formatter is at the start of a line. formatBlockComment now uses this to decide whether to split the comment onto separate lines or emit it inline.
Block comment formatting tests
test/features/comments.ts
Adds Jest tests: one that reformats a clause-item-leading inline block comment onto its own line and asserts idempotency, and another that runs multiple SQL samples with block comments in varied clause positions and asserts idempotency.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I nibble whitespace, tidy and bright,
If a block sits lone at the start of the line, I alight.
I split or tuck it with careful delight,
Rerun the formatter — the output stays right.
🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: keeping leading block comments of clause items on their own line to resolve idempotency issues.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nene
Copy link
Copy Markdown
Collaborator

nene commented Jun 3, 2026

Thanks. This looks like a reasonable fix. At least on first look. I need to ponder it some more before merging.

I would definitely appreciate not reading through a massive amount of AI-generated descriptions of this whole thing.

I also would like to have some more tests for this. Currently we're only checking for one scenario: a comment right after SELECT keyword. But how about comments in other places that should either behave the same or not behave the same, for example:

SELECT foo /*comment */, /* comment */ bar FROM /*comment */ tbl1 JOIN /* comment */ tbl2;
UPDATE /* comment */ tbl /*comment */ SET /* comment */ x = 10;

These are just random examples. I'm trying to come up with scenarios that could possibly cause problems. But mainly we should just have a few more tests to ensure that this fix does indeed work in general, not just for one single case.

@sarathfrancis90
Copy link
Copy Markdown
Contributor Author

Good call — added a test that runs comments in more positions (between items, after FROM/JOIN, in UPDATE/SET) through formatting twice to confirm they're all idempotent; full suite passes. Also trimmed the description. Thanks for taking a look!

@nene nene merged commit 54d4cf2 into sql-formatter-org:master Jun 4, 2026
1 check passed
@nene
Copy link
Copy Markdown
Collaborator

nene commented Jun 4, 2026

OK. Thanks. It's merged now, and released in 15.8.1

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.

2 participants