-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Refactor swayfmt
to generate code from arbitrary lexed trees, not necessarily backed by source code
#6779
Comments
Re-using https://clang.llvm.org/docs/LibTooling.html |
Thanks for the links @tritao! As someone who wrote a lot of static analysis code I couldn't agree more! The approach taken in #6790 is motivated by pragmatic reasons, to get a tool of production quality with acceptable effort. Using low level compiler API (lexed and typed trees) for analysis and On the long term, I would love to have:
|
## Description This PR introduces `forc-migrate`, a Forc tool for migrating Sway projects to the next breaking change version of Sway. The tool addresses two points crucial for code updates caused by breaking changes: - it informs developers about the breaking changes and **assists in planing and executing** the migration. - it **automatically changes source code** where possible, reducing the manual effort needed for code changes. Besides adding the `forc-migrate` tool, the PR: - extends `Diagnostic` to support migration diagnostics aside with errors and warnings. - changes `swayfmt` to support generating source code from arbitrary lexed trees. The change is a minimal one done only in the parts of `swayfmt` that are executed by migration steps written in this PR. Adapting `swayfmt` to fully support arbitrary lexed trees will be done in #6779. The migration for the `references` feature, migrating `ref mut` to `&mut`, is developed only partially, to demonstrate the development and usage of automatic migrations that alter the original source code. The intended usage of the tool is documented in detail in the "forc migrate" chapter of The Sway Book: _Forc reference > Plugins > forc_migrate_. (The generated documentation has issues that are caused by the documentation generation bug explained in #6792. These issues will be fixed in a separate PR that will fix it for all the plugins.) We expect the `forc-migrate` to evolve based on the developer's feedback. Some of the possible extensions of the tool are: - adding additional CLI options, e.g., for executing only specific migration steps, or ignoring them. - passing parameters to migration steps from the CLI. - not allowing updates by default, if the repository contains modified or untracked files. - migrating workspaces. - migrating other artifacts, e.g., Forc.toml files or contract IDs. - migrating between arbitrary versions of Sway. - migrating SDK code. - etc. `forc-migrate` also showed a clear need for better infrastructure for writing static analyzers and transforming Sway code. The approach used in the implementation of this PR should be seen as a pragmatic beginning, based on the reuse of what we currently have. Some future options are discussed in #6836. ## Demo ### `forc migrate show` Shows the breaking change features and related migration steps. This command can be run anywhere and does not require a Sway project. ``` Breaking change features: - storage_domains (#6701) - references (#5063) Migration steps (1 manual and 1 semiautomatic): storage_domains [M] Review explicitly defined slot keys in storage declarations (`in` keywords) references [S] Replace `ref mut` function parameters with `&mut` Experimental feature flags: - for Forc.toml: experimental = { storage_domains = true, references = true } - for CLI: --experimental storage_domains,references ``` ### `forc migrate check` Performs a dry-run of the migration on a concrete Sway project. It outputs all the occurrences in code that need to be reviewed or changed, as well as the migration time effort: ``` info: [storage_domains] Review explicitly defined slot keys in storage declarations (`in` keywords) --> /home/kebradalaonda/Desktop/M Forc migrate tool/src/main.sw:19:10 | ... 19 | y in b256::zero(): u64 = 0, | ------------ 20 | z: u64 = 0, 21 | a in calculate_slot_address(): u64 = 0, | ------------------------ 22 | b in 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20: u64 = 0, | ------------------------------------------------------------------ | = help: If the slot keys used in `in` keywords represent keys generated for `storage` fields = help: by the Sway compiler, those keys might need to be recalculated. = help: = help: The previous formula for calculating storage field keys was: `sha256("storage.<field name>")`. = help: The new formula is: `sha256((0u8, "storage.<field name>"))`. = help: = help: For a detailed migration guide see: #6701 ____ Migration effort: storage_domains [M] Review explicitly defined slot keys in storage declarations (`in` keywords) Occurrences: 3 Migration effort (hh::mm): ~00:06 references [S] Replace `ref mut` function parameters with `&mut` Occurrences: 0 Migration effort (hh::mm): ~00:00 Total migration effort (hh::mm): ~00:06 ``` ### `forc migrate run` Runs the migration steps and guides developers through the migration process. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
In the (near) future, we will want to develop tools that will analyze and transform Sway source code. Two typical examples are:
forc migrate
tool that will migrate existing source code to new versions of Sway, assisting at breaking-changes.Such tools usually operates on the AST, traversing or pattern matching it during the analysis, and transforming it to produce the changed source code.
Sway trees (lexed, parsed, and typed) are not full-fidelity trees in the sense that they do not carry the full source information (e.g. trivia like white-spaces and comments). Also, manipulating the trees "manually" in a program, does not update the spans already contained in those trees.
Supporting full-fidelity trees would require a significant change in the compiler. This issue describes a simpler solution for generating changed code from altered trees.
Although the lexed tree is the least convenient for analysis and tree manipulation, it allows us to reuse
swayfmt
for code generation, assuming theswayfmt
implementation gets changed, as proposed below.To get the idea about the kind of changes, e.g., the
forc migrate
tool will be doing in the lexed tree, here are a few concrete examples.References will require changing
self
function parameters to&self
:fn method(self)
->fn method(&self)
.References will require changing
ref mut
function parameters to&mut
:fn fun(ref mut x: MyType)
->fn method(x: &mut MyType)
.Supporting
PartialEq
andEq
will require inserting trait implementations, e.g.,impl Eq for MyType {}
.In short, we can have new elements in the tree entered, elements removed, renamed, etc.
The proposal for obtaining the altered source code from the altered lexed tree is to reuse the
swayfmt
. It requires adapting the implementation of theswayfmt
to not necessarily parse the code from files, but to take any lexed tree as input, no matter how the tree is created, or if there is actual existing source code behind it.The issue with getting formatted code generated by
swayfmt
from an arbitrary lexed tree lays in the current implementation of theswayfmt
that expects that every element of the lexed tree has a validSpan
that points to the valid text in the source code, that contains the textual representation of that lexical element. This is done also in the cases of keywords and e.g. punctuation where the text is actually known. Essentially, when rendering the formatted output, the text is always taken from the spans, means from the original source code.E.g., referencing like
&mut <type>
is formatted this way:E.g.,
Ident
s are formatted this way (in the example below,self
isIdent
):The problem is, that in the case of the altered trees, the existing spans can become obsolete or completely meaningless (e.g.,
ref mut
is removed from the tree), and the new elements will not have their textual representation in the source file at all (e.g., the inserted&mut
).The proposed solution to the problem is fairly straightforward. Instead of taking the strings from the spans/source code, for punctuations, keywords, etc. we can just use their string values, and for
Ident
s theas_str()
method, which will use the overwritten ident, if available.Essentially, we would have something like:
I've played with this proposal quite some, and tried out various combinations of altered lexed trees and I can confirm that everything what is needed by the
forc migrate
tool can we achieved if we do the above change in the implementation of theswayfmt
. Once available, this approach of altering trees and getting source code out of them can be used in all tools that want to structurally change the code. E.g., our own Clippy equivalent one day.To produce altered trees that will play well with such changed
swayfmt
, one will still need to provide "suitable faked spans". But this will also be straightforward, and we will provide rules for obtaining such spans. Also, altering lexed trees can we supported with an API that will encapsulate the rules.The proposed change shouldn't affect how the
swayfmt
operates, and all the existing clients, likeforc-fmt
, will remain unchanged.The text was updated successfully, but these errors were encountered: