feat: Add "One" import granularity #16372
Conversation
rustbot
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #16389) made this pull request unmergeable. Please resolve the merge conflicts. |
davidsemakula
force-pushed
the
import-granularity-one
branch
from
57c060d
to
7db4117
Compare
| let mut edits = tree.clone().try_merge_from(&mut neighbor, &ctx.config.insert_use); | ||
|
|
||
| if edits.is_none() && ctx.config.insert_use.granularity == ImportGranularity::One { |
There was a problem hiding this comment.
| let mut edits = tree.clone().try_merge_from(&mut neighbor, &ctx.config.insert_use); | |
| if edits.is_none() && ctx.config.insert_use.granularity == ImportGranularity::One { | |
| if ctx.config.insert_use.granularity != ImportGranularity::One { | |
| tree.clone().try_merge_from(&mut neighbor, &ctx.config.insert_use) | |
| else { |
This more reasonable to me, since then we don't do work twice.
There was a problem hiding this comment.
Making this change would break this test (and similar ones for ImportGranularity::One - the test case just above it is the motivation for having this behavior).
But breaking this behavior (at least for ImportGranularity::One only for now) may not be a bad thing because the assist is supposed to "Merges two imports with a common prefix". So may be it's better to provide the functionality equivalent to the test case behavior with a new assist (in a separate PR - something like "normalize import")?
If the above make sense, then I can simplify this implementation even further, while maintaining existing behavior for other import granularities. LMK 🙂
There was a problem hiding this comment.
I see, then that's probably fine. Could you add a https://docs.rs/cov-mark/latest/cov_mark/ here for one of the corresponding tests? That makes this behavior a bit more clear
Veykril
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
Thanks! |
|
☀️ Test successful - checks-actions |
feat: "Normalize import" assist and utilities for normalizing use trees - Add import/use tree normalization utilities - Add "normalize import" assist - Update "merge imports" assist to always apply to the covering use item except for nested use tree selections - Update "merge imports" assist to avoid adding unnecessary braces when merging nested use tree selections See [this discussion](#16372 (comment)) for the motivation for the new "normalize import" assist and changes to the "merge imports" assist.
Adds a new import granularity option "One" that merges all imports into a single use statement as long as they have the same visibility and attributes.
This is similar to rustfmt's
imports_granularity = "One".Fixes: #11361