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

Properly handle Punctuated without trailing comma everywhere in swayfmt #6805

Open
ironcev opened this issue Jan 5, 2025 · 0 comments
Open
Labels
bug Something isn't working formatter

Comments

@ironcev
Copy link
Member

ironcev commented Jan 5, 2025

Punctuated<T, P> consist of two parts, the value_separator_pairs describing all the values followed by the separator, and an optional final_value_opt describing the final value not followed by a separator.

Lexical elements that are defined using Punctuated, like, e.g., structs, enums, configurables, etc. are not all properly formatted if their elements do not end in a separator. In that case, the final_value_opt will be Some and will be formatted, but not always properly. In particular, if the formatted element has field alignment defined (FieldAlignment::AlignFields), the alignment will be ignored for the last element. There are other issues as well, e.g., the closing curly bracket will not be properly positioned.

@ironcev ironcev added bug Something isn't working formatter labels Jan 5, 2025
IGI-111 pushed a commit that referenced this issue Jan 7, 2025
## Description

This PR refactors `swayfmt` to be able generate code from arbitrary
lexed trees. Arbitrary means a lexed tree that can be fully or partially
created in-memory by manipulating the tree structure in code. It does
not need to necessarily be backed by source code. This is needed to be
able to reuse `swayfmt` for generating source code from tools that
analyze and modify Sway code, as explained in #6779.

This PR makes the `swayfmt` independent of spans backed by the source
code, by doing the following changes:
- Keywords are rendered by using theirs `AS_STR` associated constants.
- The `Token` trait is extended with the `AS_STR` associated constant,
and tokens are rendered by using that constant.
- `Ident`s are rendered by using theirs `as_str()` methods. This method
takes the textual implementation from `Ident::name_override_opt` if it
is provided, and ignores the span in that case.
- `Literal`s are rendered based on the literal value and not their
spans. The exception are numeric literals that do not have empty spans.
Those are considered to be backed by source code and are rendered as
written in code, e.g., `1_000_000u64`.

The PR also fixes some existing bugs in `swayfmt`:
- `use path::{single_import,};` will be formatted as `use
path::single_import;`
- `use path::{a,b,};` will be formatted as `use path::{a, b};`
- partial addresses #6802 by rendering annotations for final values.
- partial addresses #6805 by properly rendering final values, but still
not using the expected field alignment.

The PR also removes the `path` parameter from the `parse_file`. It was
used only to provide an unnecessary dummy `source_id` which was always
pointing to the package root file.

Closes #6779.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter
Projects
None yet
Development

No branches or pull requests

1 participant