diff --git a/Configurations.md b/Configurations.md index ac638ff91e6..e2774ec4535 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1864,7 +1864,7 @@ Similar to other `import` related configuration options, this option operates wi Note that rustfmt will not modify the granularity of imports containing comments if doing so could potentially lose or misplace said comments. - **Default value**: `Preserve` -- **Possible values**: `Preserve`, `Crate`, `Module`, `Item`, `One` +- **Possible values**: `Preserve`, `Crate`, `Module`, `ModuleCondensed`, `Item`, `One` - **Stable**: No (tracking issue: [#4991](https://github.com/rust-lang/rustfmt/issues/4991)) @@ -1904,6 +1904,16 @@ use foo::{a, b, c}; use qux::{h, i}; ``` +#### `ModuleCondensed`: + +Like `Module`, but singleton imports are included in parent modules' `use` statements where it doesn't introduce nested braces. + +```rust +use foo::b::{f, g}; +use foo::{a, b, c, d::e}; +use qux::{h, i}; +``` + #### `Item`: Flatten imports so that each has its own `use` statement. diff --git a/src/config/options.rs b/src/config/options.rs index 408017d2432..f0daaac1f11 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -125,6 +125,11 @@ pub enum ImportGranularity { Crate, /// Use one `use` statement per module. Module, + /// Use one `use` statement per module, but merge singleton imports into + /// parent modules' `use` statements where it doesn't introduce nested + /// braces. + #[unstable_variant] + ModuleCondensed, /// Use one `use` statement per imported item. Item, /// Use one `use` statement including all items. diff --git a/src/imports.rs b/src/imports.rs index 339e5cef5af..6b9a58ab41c 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -218,7 +218,7 @@ pub(crate) fn normalize_use_trees_with_granularity( ImportGranularity::Item => return flatten_use_trees(use_trees, ImportGranularity::Item), ImportGranularity::Preserve => return use_trees, ImportGranularity::Crate => SharedPrefix::Crate, - ImportGranularity::Module => SharedPrefix::Module, + ImportGranularity::Module | ImportGranularity::ModuleCondensed => SharedPrefix::Module, ImportGranularity::One => SharedPrefix::One, }; @@ -244,9 +244,83 @@ pub(crate) fn normalize_use_trees_with_granularity( } } } + + if import_granularity == ImportGranularity::ModuleCondensed { + condense_use_trees(&mut result); + } + result } +/// Merge singleton imports into parent modules' use-trees where it does't +/// introduce nested braces. +fn condense_use_trees(use_trees: &mut Vec) { + // The implementation uses a layer of indirection to avoid reordering the + // `use_trees` vector. Specifically, `use_trees_sorted` is a permutation of + // `0..use_trees.len()`. The elements of `use_trees_sorted` can be thought + // of as indices into the `use_trees` vector. + let mut use_trees_sorted = (0..use_trees.len()).collect::>(); + + // Sort by path length to put singletons after the trees into which they + // may be absorbed. + use_trees_sorted.sort_by_key(|&index| use_trees[index].path.len()); + + // Search for singletons back-to-front to preserve the just mentioned + // relative order between absorbing trees and singletons. Note that a + // singleton that is found and merged into _some_ tree is `swap_remove`d + // from the `use_trees_sorted` vector. + for n in (0..use_trees_sorted.len()).rev() { + let index = use_trees_sorted[n]; + let singleton = &use_trees[index]; + + if !singleton.is_singleton() || singleton.attrs.is_some() || singleton.contains_comment() { + continue; + } + + let mut best_indirect_index_and_len = None; + + // Search front-to-back for a tree to merge the singleton into. If + // multiple trees are equally good candidates, prefer the earliest one. + // Like above, these precautions help preserve the relative order + // between absorbing trees and singletons. + for curr_indirect_index in 0..n { + let curr_index = use_trees_sorted[curr_indirect_index]; + let curr_tree = &use_trees[curr_index]; + let curr_len = curr_tree.shared_prefix_len(singleton); + if best_indirect_index_and_len.map_or(false, |(_, best_len)| best_len >= curr_len) { + continue; + } + if curr_len < 1 + || !curr_tree.is_singleton() && curr_len + 1 != curr_tree.path.len() + || curr_tree.attrs.is_some() + || curr_tree.contains_comment() + || !curr_tree.same_visibility(singleton) + { + continue; + } + best_indirect_index_and_len = Some((curr_indirect_index, curr_len)); + } + + if let Some((best_indirect_index, _)) = best_indirect_index_and_len { + let singleton_index = use_trees_sorted.swap_remove(n); + let singleton = use_trees.remove(singleton_index); + + // `singleton` was removed from `use_trees`. So any indices larger + // than `singleton_index` must be decremented. Note that this could + // affect `best_index`. So the decrementing must occur before + // `best_index` is fetched from `use_trees_sorted`. + for index_ref in &mut use_trees_sorted { + if *index_ref >= singleton_index { + *index_ref -= 1; + } + } + + let best_index = use_trees_sorted[best_indirect_index]; + use_trees[best_index].merge(&singleton, SharedPrefix::Crate); + } + } +} + fn flatten_use_trees( use_trees: Vec, import_granularity: ImportGranularity, @@ -669,6 +743,20 @@ impl UseTree { } } + fn shared_prefix_len(&self, other: &UseTree) -> usize { + self.path + .iter() + .zip(other.path.iter()) + .take_while(|(a, b)| a == b) + .count() + } + + fn is_singleton(&self) -> bool { + self.path.last().map_or(false, |use_segment| { + !matches!(use_segment.kind, UseSegmentKind::List(_)) + }) + } + fn flatten(self, import_granularity: ImportGranularity) -> Vec { if self.path.is_empty() || self.contains_comment() { return vec![self]; @@ -1328,6 +1416,62 @@ mod test { ); } + #[test] + fn test_use_tree_merge_module_condensed() { + test_merge!( + ModuleCondensed, + ["foo::b", "foo::{a, c, d::e}"], + ["foo::{a, b, c, d::e}"] + ); + + test_merge!( + ModuleCondensed, + ["foo::{a::b, a::c, d::e, d::f}"], + ["foo::a::{b, c}", "foo::d::{e, f}"] + ); + } + + /// This test exercises the "prefer the earliest one" logic in [`condense_use_trees`]. In + /// particular, if this line: + /// ``` + /// for curr_index in 0..n { + /// ``` + /// were changed to this line: + /// ``` + /// for curr_index in (0..n).rev() { + /// ``` + /// the test would fail. An explanation follows. + /// + /// Note that [`condense_use_trees`]'s outer loop visits the trees back-to-front. So in this + /// test, `foo::e::f` will be the first singleton considered for merging. + /// + /// Next note that `foo::a::b` and `foo::c::d` are equally good candidates to absorb + /// `foo::e::f`. Because the earliest candidate (i.e., `foo::a::b`) is selected, we get the + /// following intermediate state: + /// ``` + /// foo::{a::b, e::f}; + /// foo::c::d; + /// ``` + /// Then, when `foo::c::d` is visited by the outer loop, it will notice that this singleton can + /// be merged into `foo::{a::b, e::f}`, and we get the desired output. + /// + /// On the other hand, if `foo::c::d` were selected, we would get the following intermediate + /// state: + /// ``` + /// foo::a::b; + /// foo::{c::d, e::f}; + /// ``` + /// From this state, no progress could be made, because the only singleton (`foo::a::b`) appears + /// before all trees that could absorb it. + #[test] + fn test_use_tree_merge_module_condensed_relative_order() { + test_merge!( + ModuleCondensed, + ["foo::a::b", "foo::c::d", "foo::e::f"], + ["foo::{a::b, c::d, e::f}"] + ); + } + #[test] fn test_use_tree_merge_one() { test_merge!(One, ["a", "b"], ["{a, b}"]); diff --git a/tests/source/imports/imports_granularity_module-One-no_reorder.rs b/tests/source/imports/imports_granularity_module-One-no_reorder.rs new file mode 100644 index 00000000000..bd7e1efaffa --- /dev/null +++ b/tests/source/imports/imports_granularity_module-One-no_reorder.rs @@ -0,0 +1,49 @@ +// rustfmt-imports_granularity: Module +// rustfmt-group_imports: One +// rustfmt-reorder_imports: false + +use a::{b::c, d::e}; +use a::{f, g::{h, i}}; +use a::{j::{self, k::{self, l}, m}, n::{o::p, q}}; +pub use a::{r::s, t}; +use b::{c::d, self}; + +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; + +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; + +use b::{f::g, h::{i, j} /* After b::h group */}; +use b::e; +use b::{/* Before b::l group */ l::{self, m, n::o, p::*}, q}; +use b::d; +use b::r; // After b::r +use b::q::{self /* After b::q::self */}; +use b::u::{ + a, + b, +}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::s::{ + a, + b, // After b::s::b +}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::t::{/* Before b::t::self */ self}; +use b::c; diff --git a/tests/source/imports/imports_granularity_module_condensed-One-no_reorder.rs b/tests/source/imports/imports_granularity_module_condensed-One-no_reorder.rs new file mode 100644 index 00000000000..d994b3785c3 --- /dev/null +++ b/tests/source/imports/imports_granularity_module_condensed-One-no_reorder.rs @@ -0,0 +1,49 @@ +// rustfmt-imports_granularity: ModuleCondensed +// rustfmt-group_imports: One +// rustfmt-reorder_imports: false + +use a::{b::c, d::e}; +use a::{f, g::{h, i}}; +use a::{j::{self, k::{self, l}, m}, n::{o::p, q}}; +pub use a::{r::s, t}; +use b::{c::d, self}; + +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; + +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; + +use b::{f::g, h::{i, j} /* After b::h group */}; +use b::e; +use b::{/* Before b::l group */ l::{self, m, n::o, p::*}, q}; +use b::d; +use b::r; // After b::r +use b::q::{self /* After b::q::self */}; +use b::u::{ + a, + b, +}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::s::{ + a, + b, // After b::s::b +}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::t::{/* Before b::t::self */ self}; +use b::c; diff --git a/tests/source/imports/imports_granularity_module_condensed.rs b/tests/source/imports/imports_granularity_module_condensed.rs new file mode 100644 index 00000000000..fde3018432b --- /dev/null +++ b/tests/source/imports/imports_granularity_module_condensed.rs @@ -0,0 +1,47 @@ +// rustfmt-imports_granularity: ModuleCondensed + +use a::{b::c, d::e}; +use a::{f, g::{h, i}}; +use a::{j::{self, k::{self, l}, m}, n::{o::p, q}}; +pub use a::{r::s, t}; +use b::{c::d, self}; + +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; + +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; + +use b::{f::g, h::{i, j} /* After b::h group */}; +use b::e; +use b::{/* Before b::l group */ l::{self, m, n::o, p::*}, q}; +use b::d; +use b::r; // After b::r +use b::q::{self /* After b::q::self */}; +use b::u::{ + a, + b, +}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::s::{ + a, + b, // After b::s::b +}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::t::{/* Before b::t::self */ self}; +use b::c; diff --git a/tests/target/imports/imports_granularity_module-One-no_reorder.rs b/tests/target/imports/imports_granularity_module-One-no_reorder.rs new file mode 100644 index 00000000000..92423aab459 --- /dev/null +++ b/tests/target/imports/imports_granularity_module-One-no_reorder.rs @@ -0,0 +1,53 @@ +// rustfmt-imports_granularity: Module +// rustfmt-group_imports: One +// rustfmt-reorder_imports: false + +use a::b::c; +use a::d::e; +use a::f; +use a::g::{h, i}; +use a::j::{self, m}; +use a::j::k::{self, l}; +use a::n::o::p; +use a::n::q; +pub use a::r::s; +pub use a::t; +use b::{self, c, d, e}; +use b::c::d; +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; +use b::{ + f::g, + h::{i, j}, /* After b::h group */ +}; +use b::{ + /* Before b::l group */ l::{self, m, n::o, p::*}, + q, +}; +use b::r; // After b::r +use b::q::{self /* After b::q::self */}; +use b::u::{a, b}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::s::{ + a, + b, // After b::s::b +}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::t::{/* Before b::t::self */ self}; diff --git a/tests/target/imports/imports_granularity_module_condensed-One-no_reorder.rs b/tests/target/imports/imports_granularity_module_condensed-One-no_reorder.rs new file mode 100644 index 00000000000..5f2c0e3fe79 --- /dev/null +++ b/tests/target/imports/imports_granularity_module_condensed-One-no_reorder.rs @@ -0,0 +1,48 @@ +// rustfmt-imports_granularity: ModuleCondensed +// rustfmt-group_imports: One +// rustfmt-reorder_imports: false + +use a::{b::c, d::e, f}; +use a::g::{h, i}; +use a::j::{self, m}; +use a::j::k::{self, l}; +use a::n::{o::p, q}; +pub use a::{r::s, t}; +use b::{self, c, c::d, d, e}; +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; +use b::{ + f::g, + h::{i, j}, /* After b::h group */ +}; +use b::{ + /* Before b::l group */ l::{self, m, n::o, p::*}, + q, +}; +use b::r; // After b::r +use b::q::{self /* After b::q::self */}; +use b::u::{a, b}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::s::{ + a, + b, // After b::s::b +}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::t::{/* Before b::t::self */ self}; diff --git a/tests/target/imports/imports_granularity_module_condensed.rs b/tests/target/imports/imports_granularity_module_condensed.rs new file mode 100644 index 00000000000..055ab2288dc --- /dev/null +++ b/tests/target/imports/imports_granularity_module_condensed.rs @@ -0,0 +1,50 @@ +// rustfmt-imports_granularity: ModuleCondensed + +use a::g::{h, i}; +use a::j::k::{self, l}; +use a::j::{self, m}; +use a::n::{o::p, q}; +use a::{b::c, d::e, f}; +pub use a::{r::s, t}; +use b::{self, c::d}; + +use foo::e; +#[cfg(test)] +use foo::{a::b, c::d}; + +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; + +use b::q::{self /* After b::q::self */}; +use b::r; // After b::r +use b::s::{ + a, + b, // After b::s::b +}; +use b::t::{/* Before b::t::self */ self}; +use b::t::{ + // Before b::t::a + a, + b, +}; +use b::u::{a, b}; +use b::v::{ + // Before b::v::a + a, + // Before b::v::b + b, +}; +use b::{c, d, e}; +use b::{ + f::g, + h::{i, j}, /* After b::h group */ +}; +use b::{ + /* Before b::l group */ l::{self, m, n::o, p::*}, + q, +};