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

Update rustfmt #9669

Merged
merged 2 commits into from Apr 29, 2024
Merged

Update rustfmt #9669

merged 2 commits into from Apr 29, 2024

Conversation

alshdavid
Copy link
Contributor

@alshdavid alshdavid commented Apr 24, 2024

This PR adds

imports_granularity = "Item"
group_imports = "StdExternalCrate"

to the Rustfmt.

https://rust-lang.github.io/rustfmt/?version=v1.7.0&search=imports_granularity#imports_granularity
https://rust-lang.github.io/rustfmt/?version=v1.7.0&search=#group_imports

This will change Rust imports so that they are split into individual line items that are grouped (in order) as std, extern and crate

example:

use swc_core::ecma::ast::{Atom, Span};
crate::{Baz, Buzz};
use std::foo::{Foo, Bar};

becomes

use std::foo::Bar;
use std::foo::Foo;

use swc_core::ecma::ast::Atom;
use swc_core::ecma::ast::Span;

use crate::Baz;
use crate::Buzz;

The import groupings rule enforces the import grouping convention used by the Rust community whist the import granularity rule makes it easier to read/tidy-up Rust imports and makes it easier to see changes in PRs

Most of the code changes are the formatting rules being applied.

Notable changes in the PR are:

An example case where the applied rule improves readability
image

Copy link
Contributor

@yamadapc yamadapc left a comment

Choose a reason for hiding this comment

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

I don't think we should change default formatting settings.

Most rust code that exists will use the default settings. This is great because it means a lot of rust code has consistent style.

For example, the following projects do not change these settings

@alshdavid
Copy link
Contributor Author

This is great because it means a lot of rust code has consistent style.

I agree in general however in this case - with the import style - we have inconsistent usage throughout the codebase.

I believe that the addition of these style rules will improve code consistency within our codebase in a way that remains congruent with the styling and conventions used broadly by the Rust community.

@alshdavid alshdavid enabled auto-merge (squash) April 29, 2024 06:51
@alshdavid alshdavid merged commit 3ff10bc into v2 Apr 29, 2024
16 of 17 checks passed
@mischnic mischnic deleted the alsh/rustfmt branch April 29, 2024 08:03
@mischnic
Copy link
Member

mischnic commented Apr 29, 2024

Apart from causing loads of merge conflicts, this is also not being enforced with the current setup

Warning: can't set `imports_granularity = Item`, unstable features are only available in nightly channel.
Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel.

because we have it set to stable

stable

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

Successfully merging this pull request may close these issues.

None yet

4 participants