Skip to content

Don't strip newlines that sit directly before a comment - #8057

Merged
peterbud merged 1 commit into
sqlfluff:mainfrom
CodeBlackwell:crg-farm/6801
Jul 10, 2026
Merged

peterbud merged 1 commit into
sqlfluff:mainfrom
CodeBlackwell:crg-farm/6801

Conversation

@CodeBlackwell

@CodeBlackwell CodeBlackwell commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6801

What was happening

When a layout constraint asks for newlines to be stripped between two tokens (for example spacing_within = single:inline on an expression), the reflow code could delete a newline that sat right in front of a comment. The result was that the token before the comment got glued straight onto the -- marker:

SELECT * FROM t WHERE a AND
-- comment
b

came back as

SELECT * FROM t WHERE a AND-- comment
b

which quietly changes what the statement means, since everything after -- is now commented out on that line.

Root cause

In determine_constraints (src/sqlfluff/utils/reflow/respace.py), the guard that keeps newlines around comments only looked at the previous block. If the previous block was a comment it kept the newline, but if the next block was the comment it did nothing, so an inherited strip_newlines=True went ahead and removed the newline in front of it.

The fix

Check the next block for comment segments as well as the previous one. If either side of the point is a comment, we leave the newline alone. A newline that sits immediately before a comment now survives the respace.

Before / after

  • Before: a AND\n-- comment\nb -> a AND-- comment\nb (meaning changed)
  • After: a AND\n-- comment\nb -> unchanged

Tests

Added test_reflow__respace_does_not_strip_newline_before_comment in test/utils/reflow/respace_test.py, which sets the single:inline expression constraint and asserts the newline before the comment is preserved. The existing reflow suite (test/utils/reflow/) still passes.


Summary by cubic

Stop stripping a newline that sits directly before a comment during respace. Prevents code being glued to the -- marker and changing SQL meaning. Fixes #6801.

  • Bug Fixes
    • In determine_constraints, also check the next block for comment segments; if previous or next is a comment, do not strip newlines.
    • Added regression test test_reflow__respace_does_not_strip_newline_before_comment; existing reflow tests still pass.

Written for commit 56e06b5. Summary will update on new commits.

Review in cubic

When a parent layout constraint sets strip_newlines (e.g.
spacing_within = single:inline on an expression), determine_constraints
only checked whether the *previous* block was a comment before allowing
the newline to be removed. If the *next* block was a comment, the newline
in front of it got stripped, gluing the preceding token straight onto the
-- marker and silently changing what the statement means.

Check both the previous and the next block for comment segments, so a
newline that sits immediately before a comment survives as well.

Fixes sqlfluff#6801
@CodeBlackwell
CodeBlackwell marked this pull request as ready for review July 7, 2026 19:13

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Coverage Results ✅

Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL   22209      0   100%

267 files skipped due to complete coverage.

@peterbud peterbud left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CodeBlackwell !

@peterbud
peterbud added this pull request to the merge queue Jul 10, 2026
Merged via the queue into sqlfluff:main with commit 5f26884 Jul 10, 2026
49 checks passed
benfdking added a commit to quarylabs/sqruff that referenced this pull request Oct 5, 2026
Port SQLFluff commit 5f268847afc97b1ab8218d00d81add2ffb84e191 from sqlfluff/sqlfluff#8057.

Do not strip a newline when the following block is a comment; add a regression test for an inline spacing constraint that previously glued AND to a line comment.
benfdking added a commit to quarylabs/sqruff that referenced this pull request Oct 5, 2026
Port SQLFluff commit 5f268847afc97b1ab8218d00d81add2ffb84e191 from sqlfluff/sqlfluff#8057.

Do not strip a newline when the following block is a comment; add a regression test for an inline spacing constraint that previously glued AND to a line comment.
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.

Formatting AND followed by blank/comment line with binary_operation spacing_after="any" changes meaning of the code

2 participants