Skip to content
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

Tact source code formatter #162

Open
anton-trunov opened this issue Feb 29, 2024 · 10 comments
Open

Tact source code formatter #162

anton-trunov opened this issue Feb 29, 2024 · 10 comments
Labels
big This is a hard task, more like a project and will take a while to implement tooling
Milestone

Comments

@anton-trunov
Copy link
Member

anton-trunov commented Feb 29, 2024

Currently, tact-vscode does source code formatting using a plugin for Prettier.
I believe a formatter should be a part of the language package. Also, we can have better control over how code is formatted.

Btw, since we now have a Tree-sitter grammar for Tact, formatting can be done using Topiary.

The formatter should probably be aware of tactdoc comments: #232. Or at least keep the formatting of comments intact

@anton-trunov anton-trunov added this to the v1.5.0 milestone Feb 29, 2024
@novusnota
Copy link
Member

novusnota commented Feb 29, 2024

Good suggestion. Alternatively, we could just re-use the aforementioned prettier plugin made in tact-lang/tact-vscode.

Also, we can have better control over how code is formatted.

Yes, but if anything, it would be nice to make the formatter "opinionated" and idiomatic, like the psf/black formatter for Python, or the default formatter for Golang, gofmt. That is, to figure out the optimal defaults and allow no change — this would significantly simplify all the formatting means, as all code would be in either of two states: unformatted, or formatted according to the one and only set of official rules.

I believe a formatter should be a part of the language package.

I do agree. Besides, the possible static analyzer (linter) or other similar tooling too.
Perhaps, it's best to make them standalone binaries/scripts (to make use of on their own), then bundle them with the compiler and make a unified interface in the CLI (#136).

@anton-trunov
Copy link
Member Author

it's best to make them standalone binaries/scripts (to make use of on their own), then bundle them with the compiler and make a unified interface in the CLI

Good idea!

The first step could be to indeed move the code from tact-vscode to tact itself and refactor tact-vscode to use it throw the Tact API. And then we can employ our API for the CLI tool. And at some point we can switch the formatter implementation to the one we want (if there is an actual need)

@anton-trunov anton-trunov pinned this issue Apr 19, 2024
@anton-trunov anton-trunov added the big This is a hard task, more like a project and will take a while to implement label Apr 24, 2024
This was referenced Apr 24, 2024
@anton-trunov anton-trunov unpinned this issue May 1, 2024
@byakuren-hijiri
Copy link
Contributor

byakuren-hijiri commented May 13, 2024

The initial implementation of the formatter is available in the 162-formatter branch: main...162-formatter.

@heli0dus could you please take a look? You could simply fork the 162-formatter branch and create a new PR from it.

Before merging this, we should consider the following:

  • Test and fix the existing logic: The current version of the formatter might contain implementation flaws. It makes sense to look at the following cases in particular:
    • Parentheses in complex binary expressions
    • Indentation, including nested code blocks
    • New lines
  • Add unit tests: To avoid regressions in future development, we should cover AST constructions with regular unit tests. The idea is to parse a contract, format it, and compare the result with the formatted code. Code coverage might be a good metric to ensure we cover all the formatter constructions at least once.
  • Additional features needed: If any additional features need to be implemented, we should discuss them and create issues as needed. Here are a few examples:
    • Configuration of the formatter: aspects like indentation level and column length should be configurable
    • Should we force new lines when formatting long expressions?

@heli0dus heli0dus mentioned this issue May 13, 2024
4 tasks
@novusnota
Copy link
Member

Does the formatter handle comments? Can't find mentions of them in its sources.

Configuration of the formatter: aspects like indentation level and column length should be configurable

Also, regarding to configuration, I think that it's best to go into the direction of Go (gofmt), Dart (dart fmt) and Black (Python code formatter), which do not support any configuration and provide "good enough" defaults that form a standard across all code bases. So we can omit this point, I think.

Should we force new lines when formatting long expressions?

We should definitely do so for trailing commas in arguments of initOf (...), static and extension function calls and Struct/Message instantiations. And, probably, chained dot-call sequences have to be split by lines, with one extension function on each.

Those (or alternative/additional) rules should probably also go under style guide

@byakuren-hijiri
Copy link
Contributor

byakuren-hijiri commented May 13, 2024

Does the formatter handle comments? Can't find mentions of them in its sources.

No, since it requires changes in the frontend that should be additionally discussed.

It would be great to implement this later in a separate PR, while the goal of #335 is to introduce an API for third parties and internal use that will help test the compiler itself. Our next step would be make a tool for end-users.

Also, regarding to configuration, I think that it's best to go into the direction of Go (gofmt), Dart (dart fmt) and Black (Python code formatter), which do not support any configuration and provide "good enough" defaults that form a standard across all code bases. So we can omit this point, I think.

That's a good point. I agree that the tool could have only the default configuration. However, it might be beneficial to have configuration options in the API internals, as it could be useful for internal use to have, for example, a more concise format of the formatted code.

We should definitely do so for trailing commas in arguments of initOf (...), static and extension function calls and Struct/Message instantiations. And, probably, chained dot-call sequences have to be split by lines, with one extension function on each.

Agreed. I think we should also add some newlines in long binary expressions, e.g., between big numbers, as I see them a lot in the fuzzer.

@novusnota
Copy link
Member

@anton-trunov
Copy link
Member Author

#335 implemented a pretty-printer, but we still don't have a formatter

@anton-trunov anton-trunov reopened this Jul 27, 2024
@anton-trunov anton-trunov modified the milestones: v1.5.0, v1.6.0 Jul 30, 2024
@jubnzv
Copy link
Member

jubnzv commented Aug 18, 2024

implemented a pretty-printer, but we still don't have a formatter

What do we need to prepare a production version of the formatter?

From my perspective, it would be nice to:

Creating a binary and integrating it to text editors is a quite trivial task (at least for neovim).

@anton-trunov
Copy link
Member Author

Sounds like a great plan!

@novusnota
Copy link
Member

Good points, @jubnzv!

Creating a binary and integrating it to text editors is a quite trivial task (at least for neovim).

We'll only need to add it to the public API, then use that in the LSP — and all the clients that will be using the language server will also get formatting right away!

As a bonus, we may also expose the formatter in Tact's CLI. Something like --fmt or even a separate subcommand will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big This is a hard task, more like a project and will take a while to implement tooling
Projects
None yet
Development

No branches or pull requests

4 participants