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

Add imports_granularity="Item". #4639

Merged
merged 1 commit into from
Jan 21, 2021

Conversation

msmorgan
Copy link
Contributor

This option splits all imports into their own use statement.

@calebcartwright
Copy link
Member

Thank you for the PR!

A few important things:

@msmorgan msmorgan force-pushed the granularity_flatten branch 2 times, most recently from b8b21ef to ba12968 Compare January 13, 2021 03:49
@msmorgan msmorgan changed the title Add imports_granularity="Flatten". Add imports_granularity="Item". Jan 13, 2021
@msmorgan msmorgan force-pushed the granularity_flatten branch from ae2735e to 47fc453 Compare January 14, 2021 03:56
@msmorgan
Copy link
Contributor Author

Done! Let me know what you think.

I also filed #4641 after noticing some strange inconsistencies with use statements ending in self or {self}. It depends on this PR.

}
ImportGranularity::Preserve => normalized_items,
};
for item in normalized_items.iter_mut() {
Copy link

Choose a reason for hiding this comment

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

Should this transformation be part of merge_use_trees?

Copy link

Choose a reason for hiding this comment

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

(Although this transformation should be skipped altogether, IIUC)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the logic to merge_use_trees, and changed it to rewrite paths ending in ::self into ::{self}. This also allowed me to remove some logic from the start of UseTree::flatten, which did the same thing but only for one-element lists.

Perhaps this should be part of UseTree::normalize, and that should be called again post-merge? Also, I noticed it seems to remove final ::self:

// Remove foo::{} or self without attributes.

@msmorgan msmorgan force-pushed the granularity_flatten branch 2 times, most recently from 9e16646 to 93b4f7e Compare January 16, 2021 19:04
Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Looking good overall, though left some inline feedback to see if there's an opportunity to shuffle the logic around a bit

src/formatting/imports.rs Outdated Show resolved Hide resolved
@msmorgan
Copy link
Contributor Author

@calebcartwright

Something like this?

The logic to make self into {self} is duplicated in UseTree::flatten and flatten_use_trees now though.

@calebcartwright
Copy link
Member

Something like this?

Excellent! I think this is a better approach 👍

The logic to make self into {self} is duplicated in UseTree::flatten and flatten_use_trees now though

Yeah, though I'm happy to accept that tradeoff all things considered. It's a tiny piece of code and the implementation for Item reads more clearly now IMO.

@calebcartwright
Copy link
Member

Do you want to squash up the commits a bit? Otherwise I can just squash on merging

This option splits all imports into their own `use` statement.
@msmorgan msmorgan force-pushed the granularity_flatten branch from b333b5d to 3e936aa Compare January 20, 2021 14:28
@msmorgan
Copy link
Contributor Author

Squashed and rebased onto master. Ready to go!

@calebcartwright
Copy link
Member

Squashed and rebased onto master. Ready to go!

Fantastic, and thanks again for this! A lot of folks seemed quite keen on having this style available so I anticipate there will be many ready to utilize this addition!

@calebcartwright calebcartwright merged commit e16d4f3 into rust-lang:master Jan 21, 2021
@calebcartwright calebcartwright added the 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release label Jan 21, 2021
@msmorgan msmorgan deleted the granularity_flatten branch January 21, 2021 03:40
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 30, 2021
…anxiyn

update rustfmt to v1.4.34

Short summary: Various formatting fixes (several const generic related) and introduction of `imports_granularity` config option

Long summary copied from changelog:

#### Changed
- `merge_imports` configuration has been deprecated in favor of the new `imports_granularity` option. Any existing usage of `merge_imports` will be automatically mapped to the corresponding value on `imports_granularity` with a warning message printed to encourage users to update their config files.

#### Added
- New `imports_granularity` option has been added which succeeds `merge_imports`. This new option supports several additional variants which allow users to merge imports at different levels (crate or module), and even flatten imports to have a single use statement per item. ([PR rust-lang/rustfmt#4634](rust-lang/rustfmt#4634), [PR rust-lang/rustfmt#4639](rust-lang/rustfmt#4639))

See the section on the configuration site for more information
https://rust-lang.github.io/rustfmt/?version=v1.4.33&search=#imports_granularity

#### Fixed
- Fix erroneous removal of `const` keyword on const trait impl ([rust-lang/rustfmt#4084](rust-lang/rustfmt#4084))
- Fix incorrect span usage wit const generics in supertraits ([rust-lang/rustfmt#4204](rust-lang/rustfmt#4204))
- Use correct span for const generic params ([rust-lang/rustfmt#4263](rust-lang/rustfmt#4263))
- Correct span on const generics to include type bounds ([rust-lang/rustfmt#4310](rust-lang/rustfmt#4310))
- Idempotence issue on blocks containing only empty statements ([rust-lang/rustfmt#4627](rust-lang/rustfmt#4627) and [rust-lang#3868](rust-lang/rustfmt#3868))
- Fix issue with semicolon placement on required functions that have a trailing comment that ends in a line-style comment before the semicolon ([rust-lang/rustfmt#4646](rust-lang/rustfmt#4646))
- Avoid shared interned cfg_if symbol since rustfmt can re-initialize the rustc_ast globals on multiple inputs ([rust-lang/rustfmt#4656](rust-lang/rustfmt#4656))
- Don't insert trailing comma on (base-less) rest in struct literals within macros ([rust-lang/rustfmt#4675](rust-lang/rustfmt#4675))
@karyon
Copy link
Contributor

karyon commented Oct 26, 2021

Backported in #4674

@karyon karyon added 1x-backport:completed and removed pr-ready-to-merge 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release labels Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants