Skip to content

Preserve trailing newline after a statement's semicolon (fix lossless round-trip) - #938

Open
sammist2 wants to merge 1 commit into
andialbrecht:masterfrom
sammist2:fix-trailing-newline-roundtrip
Open

sammist2 wants to merge 1 commit into
andialbrecht:masterfrom
sammist2:fix-trailing-newline-roundtrip

Conversation

@sammist2

Copy link
Copy Markdown

Summary
sqlparse round-trips losslessly — ''.join(str(s) for s in sqlparse.parse(sql)) should equal sql (asserted in test_split_comment_end_of_line). But a trailing newline after the final ; was dropped: str(sqlparse.parse("select 1;\n")[0]) returned "select 1;", while a trailing space was kept — inconsistent. Reading a normal .sql file (which ends in ;\n) and re-emitting it silently lost the final newline.
Cause
In StatementSplitter.process, the “statement finished” check used ttype not in (T.Whitespace, T.Comment.Single) — a tuple == test that doesn’t match T.Whitespace.Newline (a subtype of T.Whitespace). A trailing newline therefore started a new whitespace-only statement that the end-of-stream guard discarded.
Fix
Treat newlines as the whitespace they are when deciding whether the finished statement continues, so trailing whitespace (newlines included) stays with its statement. split() strips statements, so its output is unchanged.
Tests
Adds test_split_preserves_trailing_whitespace. Existing split/parse tests still pass; the fix was verified with the full suite green and a ~5,900-case round-trip fuzz clean.

The splitter treated a newline after a terminating ";" as non-whitespace,
cutting a whitespace-only statement that was dropped at end of stream. A
trailing space or tab was kept but a trailing newline was lost, so
str(parse("select 1;\n")[0]) returned "select 1;" and round-tripping failed
for the common "...;\n" end-of-file case.

Treat newlines as the whitespace they are when deciding whether a finished
statement continues, so trailing whitespace (newlines included) stays with
its statement. Adds a regression test.
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.

1 participant