Skip to content

Conversation

@kyleconroy
Copy link
Collaborator

  • Add Comment and StatementWithComments types to AST for storing
    comments alongside parsed statements
  • Modify parser to collect comments during tokenization instead of
    discarding them
  • Store original source text for each statement to enable perfect
    format roundtrip including comments and whitespace
  • Update Format function to use original source when available
  • Update Explain function to unwrap StatementWithComments

This enables 4155+ format roundtrip tests by preserving:

  • Leading comments (on separate lines before statements)
  • Inline comments (within statements like SELECT /comment/ 1)
  • Inter-statement whitespace and comments
  • Blank lines between statements

- Add Comment and StatementWithComments types to AST for storing
  comments alongside parsed statements
- Modify parser to collect comments during tokenization instead of
  discarding them
- Store original source text for each statement to enable perfect
  format roundtrip including comments and whitespace
- Update Format function to use original source when available
- Update Explain function to unwrap StatementWithComments

This enables 4155+ format roundtrip tests by preserving:
- Leading comments (on separate lines before statements)
- Inline comments (within statements like SELECT /*comment*/ 1)
- Inter-statement whitespace and comments
- Blank lines between statements
Instead of storing original source text for perfect roundtrip (which
bypasses AST-based formatting), compare format output using whitespace
normalization. This:

- Removes OriginalSource field from StatementWithComments
- Removes source buffering from parser
- Adds normalizeWhitespace() helper to test
- Compares format output by collapsing whitespace

This approach tests actual AST-based formatting while ignoring
whitespace differences. 227 format tests now pass with this approach.
@kyleconroy kyleconroy merged commit 17c4cff into main Dec 27, 2025
1 check 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.

3 participants