Add imports_granularity="Item".#4639

msmorgan · 2021-01-12T19:39:34Z

This option splits all imports into their own use statement.

calebcartwright · 2021-01-12T23:33:36Z

Thank you for the PR!

A few important things:

These changes are causing legitimate test failures, so please take a look at those
The names of the variants for the new option have largely been finalize after extensive discussion, and the variant name for this style needs to be Item, not Flatten
Please include thorough tests for this new variant. At a minimum this should include the pre/post formatting files with a variety of imports to be used for the system and idempotence tests (https://github.com/rust-lang/rustfmt/blob/master/Contributing.md#creating-test-cases), but it would be helpful to include the unit level tests as well along the lines of what exists for the other variants (https://github.com/rust-lang/rustfmt/blob/master/src/formatting/imports.rs#L1015)

msmorgan · 2021-01-14T04:26:29Z

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.

jstarks · 2021-01-15T20:26:51Z

src/formatting/reorder.rs

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


Should this transformation be part of merge_use_trees?

(Although this transformation should be skipped altogether, IIUC)

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:

rustfmt/src/formatting/imports.rs

Line 430 in a97fd77

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

tests/target/imports_granularity_item.rs

calebcartwright

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

msmorgan · 2021-01-18T18:42:27Z

@calebcartwright

Something like this?

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

calebcartwright · 2021-01-20T01:10:21Z

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 · 2021-01-20T01:11:03Z

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 · 2021-01-20T14:29:16Z

Squashed and rebased onto master. Ready to go!

calebcartwright · 2021-01-21T00:19:46Z

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!

…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 · 2021-10-26T11:59:09Z

Backported in #4674

msmorgan force-pushed the granularity_flatten branch from 5ca731a to 22e437d Compare January 12, 2021 20:37

calebcartwright added the pr-not-reviewed label Jan 12, 2021

msmorgan force-pushed the granularity_flatten branch 2 times, most recently from b8b21ef to ba12968 Compare January 13, 2021 03:49

msmorgan changed the title ~~Add imports_granularity="Flatten".~~ Add imports_granularity="Item". Jan 13, 2021

msmorgan force-pushed the granularity_flatten branch from ae2735e to 47fc453 Compare January 14, 2021 03:56

jstarks reviewed Jan 15, 2021

View reviewed changes

tests/target/imports_granularity_item.rs Outdated Show resolved Hide resolved

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

calebcartwright reviewed Jan 17, 2021

View reviewed changes

src/formatting/imports.rs Outdated Show resolved Hide resolved

calebcartwright removed the pr-not-reviewed label Jan 17, 2021

msmorgan force-pushed the granularity_flatten branch from 93b4f7e to ff07d6a Compare January 18, 2021 00:16

calebcartwright added the pr-ready-to-merge label Jan 20, 2021

msmorgan force-pushed the granularity_flatten branch from 9c9ad03 to b333b5d Compare January 20, 2021 14:24

Add imports_granularity="Item".

3e936aa

This option splits all imports into their own `use` statement.

msmorgan force-pushed the granularity_flatten branch from b333b5d to 3e936aa Compare January 20, 2021 14:28

calebcartwright merged commit e16d4f3 into rust-lang:master Jan 21, 2021

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 deleted the granularity_flatten branch January 21, 2021 03:40

calebcartwright mentioned this pull request Jan 29, 2021

update rustfmt to v1.4.34 rust-lang/rust#81501

Merged

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

Conversation

msmorgan commented Jan 12, 2021

Uh oh!

calebcartwright commented Jan 12, 2021

Uh oh!

msmorgan commented Jan 14, 2021

Uh oh!

jstarks Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

jstarks Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

msmorgan Jan 16, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

calebcartwright left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msmorgan commented Jan 18, 2021

Uh oh!

calebcartwright commented Jan 20, 2021

Uh oh!

calebcartwright commented Jan 20, 2021

Uh oh!

msmorgan commented Jan 20, 2021

Uh oh!

calebcartwright commented Jan 21, 2021

Uh oh!

karyon commented Oct 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants