Skip to content

Conversation

@bartolomej
Copy link

@bartolomej bartolomej commented Aug 4, 2024

Description

POC for preserving comments in Cadence AST. This should enable us to make a fully working, 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 #308

Design

I decided to start tracking comments in the form of leading/trailing comment structs attached to lexer tokens and AST nodes as discussed in #308 (comment).

I believe this approach (instead of the old one where comments were tracked as AST nodes) could simplify comment parsing logic, as the lexer already computes which tokens are associated with which comments + it removes the need to skip comments when parsing.

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

Definition of Done

  • Add AST elements for comments:
    • Block comment
    • Line comment
  • Add comments to AST elements when parsing
    • Declarations
      • Variable declaration (let/var)
      • Function declaration
      • Import declaration
      • Event declaration
      • Struct declaration
      • Resource declaration
      • Entitlement declaration
      • Attachment declaration
      • Contract declaration
      • Enum declaration
      • Transaction declaration
      • Pragma declaration
    • Statements
      • Return statement
      • Break statement
      • Continue statement
      • If statement
      • Switch statement
      • While statement
      • For statement
      • Emit statement
      • Remove statement
      • Function declaration statement
      • Function expression statement
      • Expression statement
      • Assignment statement
      • Swap statement
    • Expressions
      • Binary expression
      • Unary expression
      • Function expression
      • Identifier expression
      • Array expression
      • Dictionary expression
      • Member expression
      • Index expression
      • Conditional expression
      • Invocation expression
      • Integer literal expression
      • Fixed-point literal expression
      • String literal expression
      • Composite literal expression
      • Create expression
      • Destroy expression
      • Reference expression
      • Force expression
      • Path expression
      • Type expression
  • Tests

TODOs

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 bartolomej changed the title Retain comments in AST [WIP] Retain comments in AST Aug 4, 2024
@SupunS
Copy link
Member

SupunS commented Oct 18, 2024

hey @bartolomej , we recently restructured the cadence repo and as part of that, moved all the subdirectories under runtime package (e.g: runtime/ast, runtime/interpreter, etc) to the root/top level. Please take a pull and update the branch. Github should be able to identify the changes to the moved files, and correctly map them to the new locations. So hopefully there shouldn't be much conflicts.

Let me know if you run into any issues/conflicts, I can help with resolving those.

@SupunS SupunS added the Feature label Oct 23, 2024
# Conflicts:
#	ast/comments.go
#	ast/identifier.go
#	ast/transaction_declaration_test.go
#	old_parser/comment.go
#	old_parser/expression_test.go
#	parser/comment.go
#	parser/expression_test.go
#	parser/lexer/lexer.go
#	parser/parser.go
#	parser/statement.go
#	tools/maprange/go.sum
#	tools/storage-explorer/go.mod
#	tools/storage-explorer/go.sum
# Conflicts:
#	parser/comment.go
#	parser/declaration.go
#	parser/expression.go
#	parser/function.go
#	parser/lexer/state.go
#	parser/parser.go
#	parser/statement.go
#	parser/transaction.go
#	parser/type.go
#	sema/check_composite_declaration.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retain comments in AST

3 participants