Skip to content

feat(fmt): rewrite formatter using Solar and a structured algorithm #10907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 109 commits into
base: master
Choose a base branch
from

Conversation

0xrusowsky
Copy link
Contributor

@0xrusowsky 0xrusowsky commented Jul 2, 2025

tests state

  • ArrayExpressions:
    • STYLE: Is the rest acceptable?
  • BlockComments
  • BlockCommentsFunction
  • ConditionalOperatorExpression
  • ConstructorDefinition
  • ConstructorModifierStyle
  • ContractDefinition
    • STYLE: Is it acceptable?
  • DocComments
    • STYLE: is it acceptable
  • DoWhileStatement
  • EmitStatement
  • EnumDefinition
  • EnumVariants
  • ErrorDefinition
  • EventDefinition
  • ForStatement
    • STATUS Works perfectly with one less digit + breaks as intended (imo nothing else to do -> OK)
  • FunctionCall
  • FunctionCallArgsStatement
    • STYLE: Is it acceptable?
  • FunctionDefinition
    • STYLE: Is it acceptable?
    • TODO: support for custom configs
  • FunctionDefinitionWithFunctionReturns
  • FunctionType
    • STYLE: is it acceptable?
  • HexUnderscore
  • IfStatement
    • STYLE: is it acceptable?
  • IfStatement2
  • ImportDirective
  • InlineDisable
    • FIX: invalid output
  • IntTypes
  • LiteralExpression
    • STYLE: is it acceptable?
  • MappingType
  • ModifierDefinition
  • NamedFunctionCallExpression
    • STYLE: is it acceptable?
  • NumberLiteralUnderscore
  • OperatorExpressions
  • PragmaDirective
  • Repros
    • TODO: check boxes, panics
  • ReturnStatement
    • STYLE: is it acceptable? (inline block logic is inconsistent with 'if stmt' unit test)
  • RevertNamedArgsStatement
    • STYLE: properly break long calls?
  • RevertStatement
  • SimpleComments
  • SortedImports
  • StatementBlock
  • StructDefinition
  • ThisExpression
  • TrailingComma
    • NOTE: solar error
  • TryStatement
    • STYLE: is it acceptable?
  • TypeDefinition
  • UnitExpression
  • UsingDirective
  • VariableAssignment
    • STYLE: is it acceptable?
  • VariableDefinition
    • STYLE: is it acceptable?
  • WhileStatement
  • Yul
    • FIX: spacing
  • YulStrings

--- previous description

Rewrite forge-fmt entirely to use Solar as the Solidity/Yul parser and AST, and using a structured pretty-printing algorithm.

The language-agnostic implementation of the pretty-printer (src/pp) is based on rustc_ast_pretty and prettyplease, both MIT and Apache licensed. See prettyplease's README for algorithm notes.

The goal of this PR is to make progress with #9317, and fixing existing issues with the formatter while trying to maintain compatibility as much as possible.

This PR will not be replacing the formatter yet, opting to create a separate crate crates/fmt-2 to reduce unrelated changes and aid with review. A follow-up PR will replace crates/fmt and deal with fallout in-tree.

WIP

Progress

Tests:

13 passed; 40 failed; 1 ignored

Implementation:

  • items
    • pragma
    • import
    • using
    • contract/interface/library
    • function/modifier
    • variable declaration
    • struct
    • enum
    • UDVT
    • error
    • event
  • types
  • expressions
    • literals
    • ...
  • statements
    • if
    • while
    • do while
    • for
    • try-catch
    • ...
  • yul
    • expressions
    • statements
    • functions

Other:

  • Update README.md
  • Refactor printing delimited groups to handle span/comments
  • Print docs that get discarded during parsing like other comments
  • Add a non-fatal error that returns the formatted source with the errors
  • Add snapshot infra to tests to automatically update 'expected' files

Configs:

  • line_length: usize
  • tab_width: usize
  • bracket_spacing: bool
  • int_types: IntTypes
  • multiline_func_header: MultilineFuncHeaderStyle
  • quote_style: QuoteStyle
  • number_underscore: NumberUnderscore
  • hex_underscore: HexUnderscore
  • single_line_statement_blocks: SingleLineBlockStyle
  • override_spacing: bool
  • wrap_comments: bool
  • ignore: Vec<String>
  • contract_new_lines: bool
  • sort_imports: bool

Issues

Issues that will be fixed with the new implementation. Not closed by this PR, but by the follow-up that will hook the new implementation up; see paragraph above.

@DaniPopes DaniPopes changed the base branch from dani/fmt-solar to master August 6, 2025 18:52
@DaniPopes DaniPopes changed the title wip: fmt solar feat(fmt): rewrite formatter using Solar and a structured algorithm Aug 6, 2025
Copy link
Member

Choose a reason for hiding this comment

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

removed test and files here?

Copy link
Member

Choose a reason for hiding this comment

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

can we keep the original the same here?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, trailing spaces aren't significant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants