diff --git a/Configurations.md b/Configurations.md index e9035c6ff24..07c8a442d41 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1615,13 +1615,56 @@ pub enum Foo {} pub enum Foo {} ``` +## `imports_granularity` + +Merge together related imports based on their paths. + +- **Default value**: `Preserve` +- **Possible values**: `Preserve`, `Crate`, `Module` +- **Stable**: No + +#### `Preserve` (default): + +Do not perform any merging and preserve the original structure written by the developer. + +```rust +use foo::b; +use foo::b::{f, g}; +use foo::{a, c, d::e}; +use qux::{h, i}; +``` + +#### `Crate`: + +Merge imports from the same crate into a single `use` statement. Conversely, imports from different crates are split into separate statements. + +```rust +use foo::{ + a, b, + b::{f, g}, + c, + d::e, +}; +use qux::{h, i}; +``` + +#### `Module`: + +Merge imports from the same module into a single `use` statement. Conversely, imports from different modules are split into separate statements. + +```rust +use foo::b::{f, g}; +use foo::d::e; +use foo::{a, b, c}; +use qux::{h, i}; +``` + ## `merge_imports` -Merge multiple imports into a single nested import. +This option is deprecated. Use `imports_granularity = "Crate"` instead. - **Default value**: `false` - **Possible values**: `true`, `false` -- **Stable**: No (tracking issue: #3362) #### `false` (default): diff --git a/src/config/config_type.rs b/src/config/config_type.rs index d8fd8f78b38..bd4a847e0f1 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -97,6 +97,7 @@ macro_rules! create_config { match stringify!($i) { "max_width" | "use_small_heuristics" => self.0.set_heuristics(), "license_template_path" => self.0.set_license_template(), + "merge_imports" => self.0.set_merge_imports(), &_ => (), } } @@ -156,6 +157,7 @@ macro_rules! create_config { self.set_heuristics(); self.set_license_template(); self.set_ignore(dir); + self.set_merge_imports(); self } @@ -230,14 +232,15 @@ macro_rules! create_config { match key { "max_width" | "use_small_heuristics" => self.set_heuristics(), "license_template_path" => self.set_license_template(), + "merge_imports" => self.set_merge_imports(), &_ => (), } } #[allow(unreachable_pub)] pub fn is_hidden_option(name: &str) -> bool { - const HIDE_OPTIONS: [&str; 4] = - ["verbose", "verbose_diff", "file_lines", "width_heuristics"]; + const HIDE_OPTIONS: [&str; 5] = + ["verbose", "verbose_diff", "file_lines", "width_heuristics", "merge_imports"]; HIDE_OPTIONS.contains(&name) } @@ -309,6 +312,22 @@ macro_rules! create_config { self.ignore.2.add_prefix(dir); } + fn set_merge_imports(&mut self) { + if self.was_set().merge_imports() { + eprintln!( + "Warning: the `merge_imports` option is deprecated. \ + Use `imports_granularity=Crate` instead" + ); + if !self.was_set().imports_granularity() { + self.imports_granularity.2 = if self.merge_imports() { + ImportGranularity::Crate + } else { + ImportGranularity::Preserve + }; + } + } + } + #[allow(unreachable_pub)] /// Returns `true` if the config key was explicitly set and is the default value. pub fn is_default(&self, key: &str) -> bool { diff --git a/src/config/mod.rs b/src/config/mod.rs index 5b5aed4bc83..a5356fc257e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -64,9 +64,11 @@ create_config! { // Imports imports_indent: IndentStyle, IndentStyle::Block, false, "Indent of imports"; imports_layout: ListTactic, ListTactic::Mixed, false, "Item layout inside a import block"; - merge_imports: bool, false, false, "Merge imports"; + imports_granularity: ImportGranularity, ImportGranularity::Preserve, false, + "Merge or split imports to the provided granularity"; group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false, "Controls the strategy for how imports are grouped together"; + merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; @@ -174,6 +176,7 @@ impl PartialConfig { cloned.verbose = None; cloned.width_heuristics = None; cloned.print_misformatted_file_names = None; + cloned.merge_imports = None; ::toml::to_string(&cloned).map_err(ToTomlError) } @@ -407,6 +410,10 @@ mod test { via the --file-lines option"; width_heuristics: WidthHeuristics, WidthHeuristics::scaled(100), false, "'small' heuristic values"; + // merge_imports deprecation + imports_granularity: ImportGranularity, ImportGranularity::Preserve, false, + "Merge imports"; + merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; // Options that are used by the tests stable_option: bool, false, true, "A stable option"; @@ -529,7 +536,7 @@ fn_single_line = false where_single_line = false imports_indent = "Block" imports_layout = "Mixed" -merge_imports = false +imports_granularity = "Preserve" group_imports = "Preserve" reorder_imports = true reorder_modules = true @@ -615,4 +622,53 @@ make_backup = false // assert_eq!(config.unstable_features(), true); // ::std::env::set_var("CFG_RELEASE_CHANNEL", v); // } + + #[cfg(test)] + mod deprecated_option_merge_imports { + use super::*; + + #[test] + fn test_old_option_set() { + let toml = r#" + unstable_features = true + merge_imports = true + "#; + let config = Config::from_toml(toml, Path::new("")).unwrap(); + assert_eq!(config.imports_granularity(), ImportGranularity::Crate); + } + + #[test] + fn test_both_set() { + let toml = r#" + unstable_features = true + merge_imports = true + imports_granularity = "Preserve" + "#; + let config = Config::from_toml(toml, Path::new("")).unwrap(); + assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); + } + + #[test] + fn test_new_overridden() { + let toml = r#" + unstable_features = true + merge_imports = true + "#; + let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + config.override_value("imports_granularity", "Preserve"); + assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); + } + + #[test] + fn test_old_overridden() { + let toml = r#" + unstable_features = true + imports_granularity = "Module" + "#; + let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + config.override_value("merge_imports", "true"); + // no effect: the new option always takes precedence + assert_eq!(config.imports_granularity(), ImportGranularity::Module); + } + } } diff --git a/src/config/options.rs b/src/config/options.rs index e7a6c414354..690e0b73a3e 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -1,6 +1,7 @@ use std::collections::{hash_set, HashSet}; use std::fmt; use std::path::{Path, PathBuf}; +use std::str::FromStr; use itertools::Itertools; use rustfmt_config_proc_macro::config_type; @@ -111,6 +112,17 @@ pub enum GroupImportsTactic { StdExternalCrate, } +#[config_type] +/// How to merge imports. +pub enum ImportGranularity { + /// Do not merge imports. + Preserve, + /// Use one `use` statement per crate. + Crate, + /// Use one `use` statement per module. + Module, +} + #[config_type] pub enum ReportTactic { Always, @@ -362,7 +374,7 @@ impl IgnoreList { } } -impl ::std::str::FromStr for IgnoreList { +impl FromStr for IgnoreList { type Err = &'static str; fn from_str(_: &str) -> Result { diff --git a/src/imports.rs b/src/imports.rs index d7082d50f88..156bf6da5ab 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -159,7 +159,7 @@ impl UseSegment { } } -pub(crate) fn merge_use_trees(use_trees: Vec) -> Vec { +pub(crate) fn merge_use_trees(use_trees: Vec, merge_by: SharedPrefix) -> Vec { let mut result = Vec::with_capacity(use_trees.len()); for use_tree in use_trees { if use_tree.has_comment() || use_tree.attrs.is_some() { @@ -168,8 +168,11 @@ pub(crate) fn merge_use_trees(use_trees: Vec) -> Vec { } for flattened in use_tree.flatten() { - if let Some(tree) = result.iter_mut().find(|tree| tree.share_prefix(&flattened)) { - tree.merge(&flattened); + if let Some(tree) = result + .iter_mut() + .find(|tree| tree.share_prefix(&flattened, merge_by)) + { + tree.merge(&flattened, merge_by); } else { result.push(flattened); } @@ -527,7 +530,7 @@ impl UseTree { } } - fn share_prefix(&self, other: &UseTree) -> bool { + fn share_prefix(&self, other: &UseTree, shared_prefix: SharedPrefix) -> bool { if self.path.is_empty() || other.path.is_empty() || self.attrs.is_some() @@ -535,7 +538,12 @@ impl UseTree { { false } else { - self.path[0] == other.path[0] + match shared_prefix { + SharedPrefix::Crate => self.path[0] == other.path[0], + SharedPrefix::Module => { + self.path[..self.path.len() - 1] == other.path[..other.path.len() - 1] + } + } } } @@ -573,7 +581,7 @@ impl UseTree { } } - fn merge(&mut self, other: &UseTree) { + fn merge(&mut self, other: &UseTree, merge_by: SharedPrefix) { let mut prefix = 0; for (a, b) in self.path.iter().zip(other.path.iter()) { if *a == *b { @@ -582,20 +590,30 @@ impl UseTree { break; } } - if let Some(new_path) = merge_rest(&self.path, &other.path, prefix) { + if let Some(new_path) = merge_rest(&self.path, &other.path, prefix, merge_by) { self.path = new_path; self.span = self.span.to(other.span); } } } -fn merge_rest(a: &[UseSegment], b: &[UseSegment], mut len: usize) -> Option> { +fn merge_rest( + a: &[UseSegment], + b: &[UseSegment], + mut len: usize, + merge_by: SharedPrefix, +) -> Option> { if a.len() == len && b.len() == len { return None; } if a.len() != len && b.len() != len { - if let UseSegment::List(mut list) = a[len].clone() { - merge_use_trees_inner(&mut list, UseTree::from_path(b[len..].to_vec(), DUMMY_SP)); + if let UseSegment::List(ref list) = a[len] { + let mut list = list.clone(); + merge_use_trees_inner( + &mut list, + UseTree::from_path(b[len..].to_vec(), DUMMY_SP), + merge_by, + ); let mut new_path = b[..len].to_vec(); new_path.push(UseSegment::List(list)); return Some(new_path); @@ -622,20 +640,20 @@ fn merge_rest(a: &[UseSegment], b: &[UseSegment], mut len: usize) -> Option, use_tree: UseTree) { - let similar_trees = trees.iter_mut().filter(|tree| tree.share_prefix(&use_tree)); - if use_tree.path.len() == 1 { +fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: SharedPrefix) { + let similar_trees = trees + .iter_mut() + .filter(|tree| tree.share_prefix(&use_tree, merge_by)); + if use_tree.path.len() == 1 && merge_by == SharedPrefix::Crate { if let Some(tree) = similar_trees.min_by_key(|tree| tree.path.len()) { if tree.path.len() == 1 { return; } } - } else { - if let Some(tree) = similar_trees.max_by_key(|tree| tree.path.len()) { - if tree.path.len() > 1 { - tree.merge(&use_tree); - return; - } + } else if let Some(tree) = similar_trees.max_by_key(|tree| tree.path.len()) { + if tree.path.len() > 1 { + tree.merge(&use_tree, merge_by); + return; } } trees.push(use_tree); @@ -848,6 +866,12 @@ impl Rewrite for UseTree { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum SharedPrefix { + Crate, + Module, +} + #[cfg(test)] mod test { use super::*; @@ -994,44 +1018,72 @@ mod test { } } - #[test] - fn test_use_tree_merge() { - macro_rules! test_merge { - ([$($input:expr),* $(,)*], [$($output:expr),* $(,)*]) => { - assert_eq!( - merge_use_trees(parse_use_trees!($($input,)*)), - parse_use_trees!($($output,)*), - ); - } + macro_rules! test_merge { + ($by:ident, [$($input:expr),* $(,)*], [$($output:expr),* $(,)*]) => { + assert_eq!( + merge_use_trees(parse_use_trees!($($input,)*), SharedPrefix::$by), + parse_use_trees!($($output,)*), + ); } + } - test_merge!(["a::b::{c, d}", "a::b::{e, f}"], ["a::b::{c, d, e, f}"]); - test_merge!(["a::b::c", "a::b"], ["a::{b, b::c}"]); - test_merge!(["a::b", "a::b"], ["a::b"]); - test_merge!(["a", "a::b", "a::b::c"], ["a::{self, b, b::c}"]); + #[test] + fn test_use_tree_merge_crate() { test_merge!( + Crate, + ["a::b::{c, d}", "a::b::{e, f}"], + ["a::b::{c, d, e, f}"] + ); + test_merge!(Crate, ["a::b::c", "a::b"], ["a::{b, b::c}"]); + test_merge!(Crate, ["a::b", "a::b"], ["a::b"]); + test_merge!(Crate, ["a", "a::b", "a::b::c"], ["a::{self, b, b::c}"]); + test_merge!( + Crate, ["a", "a::b", "a::b::c", "a::b::c::d"], ["a::{self, b, b::{c, c::d}}"] ); - test_merge!(["a", "a::b", "a::b::c", "a::b"], ["a::{self, b, b::c}"]); test_merge!( + Crate, + ["a", "a::b", "a::b::c", "a::b"], + ["a::{self, b, b::c}"] + ); + test_merge!( + Crate, ["a::{b::{self, c}, d::e}", "a::d::f"], ["a::{b::{self, c}, d::{e, f}}"] ); test_merge!( + Crate, ["a::d::f", "a::{b::{self, c}, d::e}"], ["a::{b::{self, c}, d::{e, f}}"] ); test_merge!( + Crate, ["a::{c, d, b}", "a::{d, e, b, a, f}", "a::{f, g, c}"], ["a::{a, b, c, d, e, f, g}"] ); test_merge!( + Crate, ["a::{self}", "b::{self as foo}"], ["a::{self}", "b::{self as foo}"] ); } + #[test] + fn test_use_tree_merge_module() { + test_merge!( + Module, + ["foo::b", "foo::{a, c, d::e}"], + ["foo::{a, b, c}", "foo::d::e"] + ); + + test_merge!( + Module, + ["foo::{a::b, a::c, d::e, d::f}"], + ["foo::a::{b, c}", "foo::d::{e, f}"] + ); + } + #[test] fn test_use_tree_flatten() { assert_eq!( diff --git a/src/reorder.rs b/src/reorder.rs index d6b6c1963bd..3ac6bc5f5c4 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -11,8 +11,8 @@ use std::cmp::{Ord, Ordering}; use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; -use crate::config::{Config, GroupImportsTactic}; -use crate::imports::{merge_use_trees, UseSegment, UseTree}; +use crate::config::{Config, GroupImportsTactic, ImportGranularity}; +use crate::imports::{merge_use_trees, SharedPrefix, UseSegment, UseTree}; use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}; use crate::lists::{itemize_list, write_list, ListFormatting, ListItem}; use crate::rewrite::RewriteContext; @@ -107,8 +107,14 @@ fn rewrite_reorderable_or_regroupable_items( for (item, list_item) in normalized_items.iter_mut().zip(list_items) { item.list_item = Some(list_item.clone()); } - if context.config.merge_imports() { - normalized_items = merge_use_trees(normalized_items); + match context.config.imports_granularity() { + ImportGranularity::Crate => { + normalized_items = merge_use_trees(normalized_items, SharedPrefix::Crate) + } + ImportGranularity::Module => { + normalized_items = merge_use_trees(normalized_items, SharedPrefix::Module) + } + ImportGranularity::Preserve => {} } let mut regrouped_items = match context.config.group_imports() { diff --git a/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs index 1a60126d2d1..ea7f6280a64 100644 --- a/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs +++ b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: StdExternalCrate -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/source/configs/imports_layout/merge_mixed.rs b/tests/source/configs/imports_layout/merge_mixed.rs index bd09079a595..477c4aa1684 100644 --- a/tests/source/configs/imports_layout/merge_mixed.rs +++ b/tests/source/configs/imports_layout/merge_mixed.rs @@ -1,5 +1,5 @@ // rustfmt-imports_indent: Block -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str}; diff --git a/tests/source/merge_imports.rs b/tests/source/imports_granularity_crate.rs similarity index 84% rename from tests/source/merge_imports.rs rename to tests/source/imports_granularity_crate.rs index 3838e2c2e07..d16681b01b5 100644 --- a/tests/source/merge_imports.rs +++ b/tests/source/imports_granularity_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate use a::{c,d,b}; use a::{d, e, b, a, f}; @@ -32,3 +32,6 @@ use g::{self, b}; use h::{a}; use i::a::{self}; use j::{a::{self}}; + +use {k::{a, b}, l::{a, b}}; +use {k::{c, d}, l::{c, d}}; diff --git a/tests/source/imports_granularity_module.rs b/tests/source/imports_granularity_module.rs new file mode 100644 index 00000000000..5a4fad5872b --- /dev/null +++ b/tests/source/imports_granularity_module.rs @@ -0,0 +1,18 @@ +// rustfmt-imports_granularity: Module + +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}; + +#[cfg(test)] +use foo::{a::b, c::d}; +use foo::e; + +use bar::{ + // comment + a::b, + // more comment + c::d, + e::f, +}; diff --git a/tests/source/issue-3750.rs b/tests/source/issue-3750.rs index e11d9ab062a..1189a99d2b6 100644 --- a/tests/source/issue-3750.rs +++ b/tests/source/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate pub mod foo { pub mod bar { diff --git a/tests/source/merge_imports_true_compat.rs b/tests/source/merge_imports_true_compat.rs new file mode 100644 index 00000000000..bcea9435129 --- /dev/null +++ b/tests/source/merge_imports_true_compat.rs @@ -0,0 +1,4 @@ +// rustfmt-merge_imports: true + +use a::b; +use a::c; \ No newline at end of file diff --git a/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs index a3d99eb907e..5e4064dd811 100644 --- a/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs +++ b/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: StdExternalCrate -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate use alloc::{alloc::Layout, vec::Vec}; use core::f32; use std::sync::Arc; diff --git a/tests/target/configs/imports_layout/merge_mixed.rs b/tests/target/configs/imports_layout/merge_mixed.rs index 2200d7dec6d..bc0da92fffb 100644 --- a/tests/target/configs/imports_layout/merge_mixed.rs +++ b/tests/target/configs/imports_layout/merge_mixed.rs @@ -1,5 +1,5 @@ // rustfmt-imports_indent: Block -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str, str::FromStr}; diff --git a/tests/target/merge_imports.rs b/tests/target/imports_granularity_crate.rs similarity index 82% rename from tests/target/merge_imports.rs rename to tests/target/imports_granularity_crate.rs index f20498212da..d75906d30f1 100644 --- a/tests/target/merge_imports.rs +++ b/tests/target/imports_granularity_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate use a::{a, b, c, d, e, f, g}; @@ -23,3 +23,6 @@ use g::{self, a, b}; use h::a; use i::a::{self}; use j::a::{self}; + +use k::{a, b, c, d}; +use l::{a, b, c, d}; diff --git a/tests/target/imports_granularity_module.rs b/tests/target/imports_granularity_module.rs new file mode 100644 index 00000000000..9c1387c466a --- /dev/null +++ b/tests/target/imports_granularity_module.rs @@ -0,0 +1,20 @@ +// rustfmt-imports_granularity: Module + +use a::b::c; +use a::d::e; +use a::f; +use a::g::{h, i}; +use a::j::k::{self, l}; +use a::j::{self, m}; +use a::n::o::p; +use a::n::q; +pub use a::r::s; +pub use a::t; + +use foo::e; +#[cfg(test)] +use foo::{a::b, c::d}; + +use bar::a::b; +use bar::c::d; +use bar::e::f; diff --git a/tests/target/issue-3750.rs b/tests/target/issue-3750.rs index 93d4dc6df25..6875f8d3897 100644 --- a/tests/target/issue-3750.rs +++ b/tests/target/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-imports_granularity: Crate pub mod foo { pub mod bar { diff --git a/tests/target/merge_imports_true_compat.rs b/tests/target/merge_imports_true_compat.rs new file mode 100644 index 00000000000..46cd0a3b8a0 --- /dev/null +++ b/tests/target/merge_imports_true_compat.rs @@ -0,0 +1,3 @@ +// rustfmt-merge_imports: true + +use a::{b, c};