Skip to content

Conversation

@bartolomej
Copy link
Owner

@bartolomej bartolomej commented Jul 26, 2024

Description

POC for preserving comments in Cadence AST. When complete, this should enable us to make a fully working a pretty printer for Cadence.

This is a follow-up to my discussion in Discord some time ago: https://discord.com/channels/613813861610684416/1108479699732152503/1240749220357607475

Closes onflow#308

Design

I decided to start tracking trivia tokens in the form of leading/trailing trivia structs attached to Token's as discussed in onflow#308 (comment).

I believe this could simplify some parsing logic, as the lexer already computes which tokens are associated with which comments + it removes the need to skip comments when parsing.

At the AST level, I added leading/trailing comments fields to some AST nodes. The other trivia types such as spaces or newlines are just discarded for now, as I don't think we'd need them. Maybe that also means we don't even need to track them at the lexer level, but I still think that is a good think for checking correctness in unit testing if nothing else.

Note that I've currently implemented this only for FunctionDeclaration and Block AST nodes, but I'll keep expanding that to other AST nodes.

Notes

  • This change will touch quite a lot of the parsing code, as we'd need to update how we parse declaration/statement/expression (and possibly other) AST nodes to attach leading/trailing comments to them. These changes shouldn't be difficult to implement but are pretty sensitive regardless, so updating and expanding the test suite will be important.
  • We should also test that the comments in the real-world code on mainnet/testnet are preserved as expected + possibly add a runtime check to see if all the comments are correctly preserved when doing prettifying, similar as it's done in prettier: https://github.com/prettier/prettier/blob/251ecabbdee509954f7b0d33b38da9ec4ae93ad8/src/main/comments/print.js#L206-L222
  • I'm tracking some of the leftover work with TODO(preserve-comments) in code
  • I'm not sure if old parser is still being used at all. If not, should that package be removed from source code (I saw there was already some related work here, but the source wasn't removed yet: Remove old parser onflow/cadence#243)? For now, I'm updating it so that the tests pass.

Definition of Done

  • Add AST elements for comments:
    • Block comment
    • Line comment
  • Add a leading/trailing comments property
    • Declarations: Line and block comments
    • Statements: Line and block comments
    • Expressions: Block comments
  • Add comments to AST elements when parsing
    • Declarations (WIP)
    • Statements
    • Expressions
  • Tests

Description


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@bartolomej
Copy link
Owner Author

Closing in favor of onflow#3509

@bartolomej bartolomej closed this Aug 4, 2024
bartolomej pushed a commit that referenced this pull request Nov 18, 2025
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.

Retain comments in AST

2 participants