From ba602f2dc5920b91ee85afb3ae812338bf7b3a06 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Tue, 15 Dec 2020 17:19:54 -0800 Subject: [PATCH 1/6] Make `merge_imports` more granular with a `Module` option. --- Configurations.md | 37 +++++++++++----- src/config.rs | 20 ++++----- src/config/options.rs | 11 +++++ src/formatting/imports.rs | 42 +++++++++++++++++++ src/formatting/reorder.rs | 12 ++++-- src/rustfmt/main.rs | 2 +- .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- tests/source/issue-3750.rs | 2 +- ...erge_imports.rs => merge_imports_crate.rs} | 2 +- tests/source/merge_imports_module.rs | 18 ++++++++ .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- tests/target/issue-3750.rs | 2 +- ...erge_imports.rs => merge_imports_crate.rs} | 2 +- tests/target/merge_imports_module.rs | 20 +++++++++ 16 files changed, 145 insertions(+), 33 deletions(-) rename tests/source/{merge_imports.rs => merge_imports_crate.rs} (93%) create mode 100644 tests/source/merge_imports_module.rs rename tests/target/{merge_imports.rs => merge_imports_crate.rs} (92%) create mode 100644 tests/target/merge_imports_module.rs diff --git a/Configurations.md b/Configurations.md index 7130e80bfdc..40d2aa53687 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1707,26 +1707,43 @@ pub enum Foo {} ## `merge_imports` -Merge multiple imports into a single nested import. +Merge together related imports based on their paths. -- **Default value**: `false` -- **Possible values**: `true`, `false` +This option requires `reorder_imports`, which is enabled by default. + +- **Default value**: `Never` +- **Possible values**: `Never`, `Crate`, `Module` - **Stable**: No (tracking issue: [#3362](https://github.com/rust-lang/rustfmt/issues/3362)) -#### `false` (default): +#### `Never` (default): ```rust -use foo::{a, c, d}; -use foo::{b, g}; -use foo::{e, f}; +use foo::b; +use foo::b::{f, g}; +use foo::{a, c, d::e}; +use qux::{h, i}; ``` -#### `true`: +#### `Crate`: ```rust -use foo::{a, b, c, d, e, f, g}; +use foo::{ + a, b, + b::{f, g}, + c, + d::e, +}; +use qux::{h, i}; ``` +#### `Module`: + +```rust +use foo::b::{f, g}; +use foo::d::e; +use foo::{a, b, c}; +use qux::{h, i}; +``` ## `newline_style` @@ -2560,7 +2577,7 @@ Enable unstable features on stable and beta channels (unstable features are avai For example: ```bash -rustfmt src/lib.rs --config unstable_features=true merge_imports=true +rustfmt src/lib.rs --config unstable_features=true merge_imports=Crate ``` ## `use_field_init_shorthand` diff --git a/src/config.rs b/src/config.rs index 6a0c46f6275..6f3e4c17dbd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -77,7 +77,7 @@ 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"; + merge_imports: MergeImports, MergeImports::Never, false, "Merge imports"; group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false, "Controls the strategy for how imports are grouped together"; @@ -595,7 +595,7 @@ fn_single_line = false where_single_line = false imports_indent = "Block" imports_layout = "Mixed" -merge_imports = false +merge_imports = "Never" group_imports = "Preserve" reorder_imports = true reorder_modules = true @@ -716,13 +716,13 @@ ignore = [] } let toml = r#" unstable_features = true - merge_imports = true + merge_imports = "Crate" "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); assert_eq!(config.was_set().unstable_features(), true); assert_eq!(config.was_set().merge_imports(), true); assert_eq!(config.unstable_features(), true); - assert_eq!(config.merge_imports(), true); + assert_eq!(config.merge_imports(), MergeImports::Crate); } #[test] @@ -731,9 +731,9 @@ ignore = [] // This test requires non-nightly return; } - let config = Config::from_toml("merge_imports = true", Path::new("")).unwrap(); + let config = Config::from_toml("merge_imports = Crate", Path::new("")).unwrap(); assert_eq!(config.was_set().merge_imports(), false); - assert_eq!(config.merge_imports(), false); + assert_eq!(config.merge_imports(), MergeImports::Never); } #[test] @@ -778,12 +778,12 @@ ignore = [] } let mut config = Config::default(); assert_eq!(config.unstable_features(), false); - config.override_value("merge_imports", "true"); - assert_eq!(config.merge_imports(), false); + config.override_value("merge_imports", "Crate"); + assert_eq!(config.merge_imports(), MergeImports::Crate); config.override_value("unstable_features", "true"); assert_eq!(config.unstable_features(), true); - config.override_value("merge_imports", "true"); - assert_eq!(config.merge_imports(), true); + config.override_value("merge_imports", "Crate"); + assert_eq!(config.merge_imports(), MergeImports::Crate); } #[test] diff --git a/src/config/options.rs b/src/config/options.rs index aea67749a45..ee6f6002046 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -118,6 +118,17 @@ pub enum GroupImportsTactic { StdExternalCrate, } +#[config_type] +/// How to merge imports. +pub enum MergeImports { + /// Do not merge imports. + Never, + /// Use one `use` statement per crate. + Crate, + /// Use one `use` statement per module. + Module, +} + #[config_type] pub enum ReportTactic { Always, diff --git a/src/formatting/imports.rs b/src/formatting/imports.rs index 72aff70987a..df518265e73 100644 --- a/src/formatting/imports.rs +++ b/src/formatting/imports.rs @@ -179,6 +179,48 @@ pub(crate) fn merge_use_trees(use_trees: Vec) -> Vec { result } +/// Split apart nested imports in use trees. +pub(crate) fn unnest_use_trees(mut use_trees: Vec) -> Vec { + let mut result = Vec::with_capacity(use_trees.len()); + while let Some(mut use_tree) = use_trees.pop() { + if !use_tree.has_comment() && use_tree.attrs.is_none() { + if let Some((UseSegment::List(list), ref prefix)) = use_tree.path.split_last_mut() { + let span = use_tree.span; + let visibility = &use_tree.visibility; + list.retain(|nested_use_tree| { + if matches!( + nested_use_tree.path[..], + [UseSegment::Ident(..)] | [UseSegment::Slf(..)] | [UseSegment::Glob] + ) { + return true; + } + if nested_use_tree.has_comment() { + return true; + } + // nested item detected; flatten once, but process it again + // in case it has more nesting + use_trees.push(UseTree { + path: prefix + .iter() + .cloned() + .chain(nested_use_tree.path.iter().cloned()) + .collect(), + span, + list_item: None, + visibility: visibility.clone(), + attrs: None, + }); + // remove this item + false + }); + use_tree = use_tree.normalize(); + } + } + result.push(use_tree); + } + result +} + impl fmt::Debug for UseTree { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self, f) diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index 379aa109b14..ba81b9ecacc 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -11,11 +11,11 @@ use std::cmp::{Ord, Ordering}; use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; -use crate::config::{Config, GroupImportsTactic}; +use crate::config::{Config, GroupImportsTactic, MergeImports}; use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ - imports::{merge_use_trees, UseTree}, + imports::{merge_use_trees, unnest_use_trees, UseTree}, items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}, lists::{itemize_list, write_list, ListFormatting, ListItem}, rewrite::RewriteContext, @@ -226,8 +226,12 @@ 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.merge_imports() { + MergeImports::Crate => normalized_items = merge_use_trees(normalized_items), + MergeImports::Module => { + normalized_items = unnest_use_trees(merge_use_trees(normalized_items)) + } + MergeImports::Never => {} } let mut regrouped_items = match context.config.group_imports() { diff --git a/src/rustfmt/main.rs b/src/rustfmt/main.rs index 41ff2e5396a..66838d1afa9 100644 --- a/src/rustfmt/main.rs +++ b/src/rustfmt/main.rs @@ -100,7 +100,7 @@ struct Opt { /// Set options from command line. /// /// Set configuration options via command line by specifying a list of key-value pairs - /// separated by commas (e.g., rustfmt --config=max_width=100,merge_imports=true). + /// separated by commas (e.g., rustfmt --config=max_width=100,merge_imports=Crate). /// These settings precedes any other settings specified in configuration files. #[structopt(long = "config")] inline_config: Option>, diff --git a/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs index 1a60126d2d1..c5a848e972f 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-merge_imports: 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..17dd3613c7f 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-merge_imports: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str}; diff --git a/tests/source/issue-3750.rs b/tests/source/issue-3750.rs index e11d9ab062a..a8ce9f27002 100644 --- a/tests/source/issue-3750.rs +++ b/tests/source/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-merge_imports: Crate pub mod foo { pub mod bar { diff --git a/tests/source/merge_imports.rs b/tests/source/merge_imports_crate.rs similarity index 93% rename from tests/source/merge_imports.rs rename to tests/source/merge_imports_crate.rs index 3838e2c2e07..fad119efee5 100644 --- a/tests/source/merge_imports.rs +++ b/tests/source/merge_imports_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-merge_imports: Crate use a::{c,d,b}; use a::{d, e, b, a, f}; diff --git a/tests/source/merge_imports_module.rs b/tests/source/merge_imports_module.rs new file mode 100644 index 00000000000..9fce55dac83 --- /dev/null +++ b/tests/source/merge_imports_module.rs @@ -0,0 +1,18 @@ +// rustfmt-merge_imports: 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/target/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs index a3d99eb907e..ec724286e97 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-merge_imports: 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..a9a2b5ef1a1 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-merge_imports: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str, str::FromStr}; diff --git a/tests/target/issue-3750.rs b/tests/target/issue-3750.rs index 93d4dc6df25..9801a310138 100644 --- a/tests/target/issue-3750.rs +++ b/tests/target/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-merge_imports: Crate pub mod foo { pub mod bar { diff --git a/tests/target/merge_imports.rs b/tests/target/merge_imports_crate.rs similarity index 92% rename from tests/target/merge_imports.rs rename to tests/target/merge_imports_crate.rs index f20498212da..2ecbebff276 100644 --- a/tests/target/merge_imports.rs +++ b/tests/target/merge_imports_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: true +// rustfmt-merge_imports: Crate use a::{a, b, c, d, e, f, g}; diff --git a/tests/target/merge_imports_module.rs b/tests/target/merge_imports_module.rs new file mode 100644 index 00000000000..f0385b25662 --- /dev/null +++ b/tests/target/merge_imports_module.rs @@ -0,0 +1,20 @@ +// rustfmt-merge_imports: 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; From 03e7e07ae23867c1572ad0bd19af6c6a0fb68776 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Mon, 21 Dec 2020 12:26:24 -0800 Subject: [PATCH 2/6] Continue to parse merge_imports={true,false} --- src/config/config_type.rs | 8 ++++++- src/config/options.rs | 49 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index b36b78f8e6c..375a636a1ac 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -1,5 +1,5 @@ use crate::config::file_lines::FileLines; -use crate::config::options::{IgnoreList, WidthHeuristics}; +use crate::config::options::{IgnoreList, MergeImports, WidthHeuristics}; /// Trait for types that can be used in `Config`. pub(crate) trait ConfigType: Sized { @@ -50,6 +50,12 @@ impl ConfigType for IgnoreList { } } +impl ConfigType for MergeImports { + fn doc_hint() -> String { + "[Never|Crate|Module]".to_owned() + } +} + macro_rules! update_config { ($config:ident, ignore = $val:ident, $dir:ident) => { $config.ignore.1 = true; diff --git a/src/config/options.rs b/src/config/options.rs index ee6f6002046..a596c84d5e4 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 rustfmt_config_proc_macro::config_type; use serde::de::{SeqAccess, Visitor}; @@ -118,7 +119,7 @@ pub enum GroupImportsTactic { StdExternalCrate, } -#[config_type] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Serialize)] /// How to merge imports. pub enum MergeImports { /// Do not merge imports. @@ -129,6 +130,50 @@ pub enum MergeImports { Module, } +impl FromStr for MergeImports { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + if "false".eq_ignore_ascii_case(s) { + eprintln!("Warning: merge_imports=false is being renamed to Never"); + return Ok(MergeImports::Never); + } + if "true".eq_ignore_ascii_case(s) { + eprintln!("Warning: merge_imports=true is being renamed to Crate"); + return Ok(MergeImports::Crate); + } + if "Never".eq_ignore_ascii_case(s) { + return Ok(MergeImports::Never); + } + if "Crate".eq_ignore_ascii_case(s) { + return Ok(MergeImports::Crate); + } + if "Module".eq_ignore_ascii_case(s) { + return Ok(MergeImports::Module); + } + return Err("Bad variant, expected one of: `Never` `Crate` `Module`"); + } +} + +impl fmt::Display for MergeImports { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +impl<'de> Deserialize<'de> for MergeImports { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + Self::from_str(&s).map_err(|_| { + static ALLOWED: &'static [&str] = &["Never", "Crate", "Module"]; + serde::de::Error::unknown_variant(&s, ALLOWED) + }) + } +} + #[config_type] pub enum ReportTactic { Always, @@ -328,7 +373,7 @@ impl IgnoreList { } } -impl std::str::FromStr for IgnoreList { +impl FromStr for IgnoreList { type Err = &'static str; fn from_str(_: &str) -> Result { From 21e1c3cd2050e0e203f4bed7eac310fa60d07148 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Thu, 24 Dec 2020 14:12:37 -0800 Subject: [PATCH 3/6] Rename option to imports_merge_style, still parse the old option --- Configurations.md | 31 +++++-- src/config.rs | 80 ++++++++++++++++--- src/config/config_type.rs | 35 ++++++-- src/config/options.rs | 50 +----------- src/formatting/reorder.rs | 10 +-- src/rustfmt/main.rs | 2 +- .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- ..._crate.rs => imports_merge_style_crate.rs} | 2 +- ...odule.rs => imports_merge_style_module.rs} | 2 +- tests/source/issue-3750.rs | 2 +- tests/source/merge_imports_true_compat.rs | 4 + .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- ..._crate.rs => imports_merge_style_crate.rs} | 2 +- ...odule.rs => imports_merge_style_module.rs} | 2 +- tests/target/issue-3750.rs | 2 +- tests/target/merge_imports_true_compat.rs | 3 + 18 files changed, 147 insertions(+), 88 deletions(-) rename tests/source/{merge_imports_crate.rs => imports_merge_style_crate.rs} (92%) rename tests/source/{merge_imports_module.rs => imports_merge_style_module.rs} (86%) create mode 100644 tests/source/merge_imports_true_compat.rs rename tests/target/{merge_imports_crate.rs => imports_merge_style_crate.rs} (90%) rename tests/target/{merge_imports_module.rs => imports_merge_style_module.rs} (86%) create mode 100644 tests/target/merge_imports_true_compat.rs diff --git a/Configurations.md b/Configurations.md index 40d2aa53687..75563b9514f 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1705,17 +1705,17 @@ pub enum Foo {} pub enum Foo {} ``` -## `merge_imports` +## `imports_merge_style` Merge together related imports based on their paths. This option requires `reorder_imports`, which is enabled by default. -- **Default value**: `Never` -- **Possible values**: `Never`, `Crate`, `Module` +- **Default value**: `Preserve` +- **Possible values**: `Preserve`, `Crate`, `Module` - **Stable**: No (tracking issue: [#3362](https://github.com/rust-lang/rustfmt/issues/3362)) -#### `Never` (default): +#### `Preserve` (default): ```rust use foo::b; @@ -1745,6 +1745,27 @@ use foo::{a, b, c}; use qux::{h, i}; ``` +## `merge_imports` + +This option is deprecated. Use `imports_merge_style = "Crate"` instead. + +- **Default value**: `false` +- **Possible values**: `true`, `false` + +#### `false` (default): + +```rust +use foo::{a, c, d}; +use foo::{b, g}; +use foo::{e, f}; +``` + +#### `true`: + +```rust +use foo::{a, b, c, d, e, f, g}; +``` + ## `newline_style` Unix or Windows line endings @@ -2577,7 +2598,7 @@ Enable unstable features on stable and beta channels (unstable features are avai For example: ```bash -rustfmt src/lib.rs --config unstable_features=true merge_imports=Crate +rustfmt src/lib.rs --config unstable_features=true imports_merge_style=Crate ``` ## `use_field_init_shorthand` diff --git a/src/config.rs b/src/config.rs index 6f3e4c17dbd..bc4e293dae6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -77,9 +77,10 @@ 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: MergeImports, MergeImports::Never, false, "Merge imports"; + imports_merge_style: ImportMergeStyle, ImportMergeStyle::Preserve, false, "Merge imports"; group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false, "Controls the strategy for how imports are grouped together"; + merge_imports: bool, false, false, "(deprecated: use imports_merge_style instead)"; // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; @@ -175,6 +176,7 @@ impl PartialConfig { // Non-user-facing options can't be specified in TOML let mut cloned = self.clone(); cloned.file_lines = None; + cloned.merge_imports = None; ::toml::to_string(&cloned).map_err(ToTomlError) } @@ -444,6 +446,10 @@ mod test { chain_width: usize, 60, true, "Maximum length of a chain to fit on a single line."; single_line_if_else_max_width: usize, 50, true, "Maximum line length for single \ line if-else expressions. A value of zero means always break if-else expressions."; + // merge_imports deprecation + imports_merge_style: ImportMergeStyle, ImportMergeStyle::Preserve, false, + "Merge imports"; + merge_imports: bool, false, false, "(deprecated: use imports_merge_style instead)"; unstable_features: bool, false, true, "Enables unstable features on stable and beta channels \ @@ -595,7 +601,7 @@ fn_single_line = false where_single_line = false imports_indent = "Block" imports_layout = "Mixed" -merge_imports = "Never" +imports_merge_style = "Preserve" group_imports = "Preserve" reorder_imports = true reorder_modules = true @@ -716,13 +722,13 @@ ignore = [] } let toml = r#" unstable_features = true - merge_imports = "Crate" + imports_merge_style = "Crate" "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); assert_eq!(config.was_set().unstable_features(), true); - assert_eq!(config.was_set().merge_imports(), true); + assert_eq!(config.was_set().imports_merge_style(), true); assert_eq!(config.unstable_features(), true); - assert_eq!(config.merge_imports(), MergeImports::Crate); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::Crate); } #[test] @@ -731,9 +737,10 @@ ignore = [] // This test requires non-nightly return; } - let config = Config::from_toml("merge_imports = Crate", Path::new("")).unwrap(); - assert_eq!(config.was_set().merge_imports(), false); - assert_eq!(config.merge_imports(), MergeImports::Never); + let config = + Config::from_toml("imports_merge_style = \"Crate\"", Path::new("")).unwrap(); + assert_eq!(config.was_set().imports_merge_style(), false); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); } #[test] @@ -778,12 +785,12 @@ ignore = [] } let mut config = Config::default(); assert_eq!(config.unstable_features(), false); - config.override_value("merge_imports", "Crate"); - assert_eq!(config.merge_imports(), MergeImports::Crate); + config.override_value("imports_merge_style", "Crate"); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); config.override_value("unstable_features", "true"); assert_eq!(config.unstable_features(), true); - config.override_value("merge_imports", "Crate"); - assert_eq!(config.merge_imports(), MergeImports::Crate); + config.override_value("imports_merge_style", "Crate"); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::Crate); } #[test] @@ -1036,4 +1043,53 @@ ignore = [] assert_eq!(config.single_line_if_else_max_width(), 100); } } + + #[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_merge_style(), ImportMergeStyle::Crate); + } + + #[test] + fn test_both_set() { + let toml = r#" + unstable_features = true + merge_imports = true + imports_merge_style = "Preserve" + "#; + let config = Config::from_toml(toml, Path::new("")).unwrap(); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::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_merge_style", "Preserve"); + assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); + } + + #[test] + fn test_old_overridden() { + let toml = r#" + unstable_features = true + imports_merge_style = "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_merge_style(), ImportMergeStyle::Module); + } + } } diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 375a636a1ac..1b903d6b770 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -1,5 +1,5 @@ use crate::config::file_lines::FileLines; -use crate::config::options::{IgnoreList, MergeImports, WidthHeuristics}; +use crate::config::options::{IgnoreList, WidthHeuristics}; /// Trait for types that can be used in `Config`. pub(crate) trait ConfigType: Sized { @@ -50,12 +50,6 @@ impl ConfigType for IgnoreList { } } -impl ConfigType for MergeImports { - fn doc_hint() -> String { - "[Never|Crate|Module]".to_owned() - } -} - macro_rules! update_config { ($config:ident, ignore = $val:ident, $dir:ident) => { $config.ignore.1 = true; @@ -66,6 +60,12 @@ macro_rules! update_config { $config.ignore.2 = old_ignored.merge_into(new_ignored); }; + ($config:ident, merge_imports = $val:ident, $dir:ident) => { + $config.merge_imports.1 = true; + $config.merge_imports.2 = $val; + $config.set_merge_imports(); + }; + ($config:ident, $i:ident = $val:ident, $dir:ident) => { $config.$i.1 = true; $config.$i.2 = $val; @@ -127,6 +127,7 @@ macro_rules! create_config { | "array_width" | "chain_width" => self.0.set_heuristics(), "license_template_path" => self.0.set_license_template(), + "merge_imports" => self.0.set_merge_imports(), &_ => (), } } @@ -278,14 +279,16 @@ macro_rules! create_config { | "array_width" | "chain_width" => 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; 1] = [ + const HIDE_OPTIONS: [&str; 2] = [ "file_lines", + "merge_imports", ]; HIDE_OPTIONS.contains(&name) } @@ -429,6 +432,22 @@ macro_rules! create_config { } } + fn set_merge_imports(&mut self) { + if self.was_set().merge_imports() { + eprintln!( + "Warning: the `merge_imports` option is deprecated. \ + Use `imports_merge_style=Crate` instead" + ); + if !self.was_set().imports_merge_style() { + self.imports_merge_style.2 = if self.merge_imports() { + ImportMergeStyle::Crate + } else { + ImportMergeStyle::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/options.rs b/src/config/options.rs index a596c84d5e4..ae5eafa2dcf 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -119,61 +119,17 @@ pub enum GroupImportsTactic { StdExternalCrate, } -#[derive(Debug, Copy, Clone, Eq, PartialEq, Serialize)] +#[config_type] /// How to merge imports. -pub enum MergeImports { +pub enum ImportMergeStyle { /// Do not merge imports. - Never, + Preserve, /// Use one `use` statement per crate. Crate, /// Use one `use` statement per module. Module, } -impl FromStr for MergeImports { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - if "false".eq_ignore_ascii_case(s) { - eprintln!("Warning: merge_imports=false is being renamed to Never"); - return Ok(MergeImports::Never); - } - if "true".eq_ignore_ascii_case(s) { - eprintln!("Warning: merge_imports=true is being renamed to Crate"); - return Ok(MergeImports::Crate); - } - if "Never".eq_ignore_ascii_case(s) { - return Ok(MergeImports::Never); - } - if "Crate".eq_ignore_ascii_case(s) { - return Ok(MergeImports::Crate); - } - if "Module".eq_ignore_ascii_case(s) { - return Ok(MergeImports::Module); - } - return Err("Bad variant, expected one of: `Never` `Crate` `Module`"); - } -} - -impl fmt::Display for MergeImports { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self) - } -} - -impl<'de> Deserialize<'de> for MergeImports { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let s = String::deserialize(deserializer)?; - Self::from_str(&s).map_err(|_| { - static ALLOWED: &'static [&str] = &["Never", "Crate", "Module"]; - serde::de::Error::unknown_variant(&s, ALLOWED) - }) - } -} - #[config_type] pub enum ReportTactic { Always, diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index ba81b9ecacc..323441b483a 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -11,7 +11,7 @@ use std::cmp::{Ord, Ordering}; use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; -use crate::config::{Config, GroupImportsTactic, MergeImports}; +use crate::config::{Config, GroupImportsTactic, ImportMergeStyle}; use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ @@ -226,12 +226,12 @@ 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()); } - match context.config.merge_imports() { - MergeImports::Crate => normalized_items = merge_use_trees(normalized_items), - MergeImports::Module => { + match context.config.imports_merge_style() { + ImportMergeStyle::Crate => normalized_items = merge_use_trees(normalized_items), + ImportMergeStyle::Module => { normalized_items = unnest_use_trees(merge_use_trees(normalized_items)) } - MergeImports::Never => {} + ImportMergeStyle::Preserve => {} } let mut regrouped_items = match context.config.group_imports() { diff --git a/src/rustfmt/main.rs b/src/rustfmt/main.rs index 66838d1afa9..ea2f705b7fb 100644 --- a/src/rustfmt/main.rs +++ b/src/rustfmt/main.rs @@ -100,7 +100,7 @@ struct Opt { /// Set options from command line. /// /// Set configuration options via command line by specifying a list of key-value pairs - /// separated by commas (e.g., rustfmt --config=max_width=100,merge_imports=Crate). + /// separated by commas (e.g., rustfmt --config=max_width=100,imports_merge_style=Crate). /// These settings precedes any other settings specified in configuration files. #[structopt(long = "config")] inline_config: Option>, diff --git a/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs index c5a848e972f..a3c7be06a0d 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: Crate +// rustfmt-imports_merge_style: 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 17dd3613c7f..48e22764470 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: Crate +// rustfmt-imports_merge_style: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str}; diff --git a/tests/source/merge_imports_crate.rs b/tests/source/imports_merge_style_crate.rs similarity index 92% rename from tests/source/merge_imports_crate.rs rename to tests/source/imports_merge_style_crate.rs index fad119efee5..56b30bac632 100644 --- a/tests/source/merge_imports_crate.rs +++ b/tests/source/imports_merge_style_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Crate +// rustfmt-imports_merge_style: Crate use a::{c,d,b}; use a::{d, e, b, a, f}; diff --git a/tests/source/merge_imports_module.rs b/tests/source/imports_merge_style_module.rs similarity index 86% rename from tests/source/merge_imports_module.rs rename to tests/source/imports_merge_style_module.rs index 9fce55dac83..a02bf718003 100644 --- a/tests/source/merge_imports_module.rs +++ b/tests/source/imports_merge_style_module.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Module +// rustfmt-imports_merge_style: Module use a::{b::c, d::e}; use a::{f, g::{h, i}}; diff --git a/tests/source/issue-3750.rs b/tests/source/issue-3750.rs index a8ce9f27002..c5f0556eeef 100644 --- a/tests/source/issue-3750.rs +++ b/tests/source/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Crate +// rustfmt-imports_merge_style: 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 ec724286e97..60333c5806c 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: Crate +// rustfmt-imports_merge_style: 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 a9a2b5ef1a1..c429c3e972b 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: Crate +// rustfmt-imports_merge_style: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str, str::FromStr}; diff --git a/tests/target/merge_imports_crate.rs b/tests/target/imports_merge_style_crate.rs similarity index 90% rename from tests/target/merge_imports_crate.rs rename to tests/target/imports_merge_style_crate.rs index 2ecbebff276..8230a9d58d2 100644 --- a/tests/target/merge_imports_crate.rs +++ b/tests/target/imports_merge_style_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Crate +// rustfmt-imports_merge_style: Crate use a::{a, b, c, d, e, f, g}; diff --git a/tests/target/merge_imports_module.rs b/tests/target/imports_merge_style_module.rs similarity index 86% rename from tests/target/merge_imports_module.rs rename to tests/target/imports_merge_style_module.rs index f0385b25662..c1e4a2cfebb 100644 --- a/tests/target/merge_imports_module.rs +++ b/tests/target/imports_merge_style_module.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Module +// rustfmt-imports_merge_style: Module use a::b::c; use a::d::e; diff --git a/tests/target/issue-3750.rs b/tests/target/issue-3750.rs index 9801a310138..f9ddfa5baff 100644 --- a/tests/target/issue-3750.rs +++ b/tests/target/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-merge_imports: Crate +// rustfmt-imports_merge_style: 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}; From 442615afa04aa048ec50f39415275d0a34e918a5 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Thu, 31 Dec 2020 17:45:54 -0800 Subject: [PATCH 4/6] Improve documentation for imports_merge_style --- Configurations.md | 10 +++++++--- tests/source/imports_merge_style_crate.rs | 3 +++ tests/target/imports_merge_style_crate.rs | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Configurations.md b/Configurations.md index 75563b9514f..e8d8b187903 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1709,14 +1709,14 @@ pub enum Foo {} Merge together related imports based on their paths. -This option requires `reorder_imports`, which is enabled by default. - - **Default value**: `Preserve` - **Possible values**: `Preserve`, `Crate`, `Module` -- **Stable**: No (tracking issue: [#3362](https://github.com/rust-lang/rustfmt/issues/3362)) +- **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}; @@ -1726,6 +1726,8 @@ 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, @@ -1738,6 +1740,8 @@ 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; diff --git a/tests/source/imports_merge_style_crate.rs b/tests/source/imports_merge_style_crate.rs index 56b30bac632..51552a0bfb5 100644 --- a/tests/source/imports_merge_style_crate.rs +++ b/tests/source/imports_merge_style_crate.rs @@ -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/target/imports_merge_style_crate.rs b/tests/target/imports_merge_style_crate.rs index 8230a9d58d2..1268d8522bd 100644 --- a/tests/target/imports_merge_style_crate.rs +++ b/tests/target/imports_merge_style_crate.rs @@ -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}; From 28c08e40eef6641e429345784eb2c509504855bf Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Thu, 31 Dec 2020 17:46:05 -0800 Subject: [PATCH 5/6] Share code with merge_use_trees --- src/formatting/imports.rs | 152 ++++++++++++++++++++------------------ src/formatting/reorder.rs | 10 ++- 2 files changed, 89 insertions(+), 73 deletions(-) diff --git a/src/formatting/imports.rs b/src/formatting/imports.rs index df518265e73..9cb089a29e5 100644 --- a/src/formatting/imports.rs +++ b/src/formatting/imports.rs @@ -160,7 +160,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() { @@ -169,8 +169,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); } @@ -179,48 +182,6 @@ pub(crate) fn merge_use_trees(use_trees: Vec) -> Vec { result } -/// Split apart nested imports in use trees. -pub(crate) fn unnest_use_trees(mut use_trees: Vec) -> Vec { - let mut result = Vec::with_capacity(use_trees.len()); - while let Some(mut use_tree) = use_trees.pop() { - if !use_tree.has_comment() && use_tree.attrs.is_none() { - if let Some((UseSegment::List(list), ref prefix)) = use_tree.path.split_last_mut() { - let span = use_tree.span; - let visibility = &use_tree.visibility; - list.retain(|nested_use_tree| { - if matches!( - nested_use_tree.path[..], - [UseSegment::Ident(..)] | [UseSegment::Slf(..)] | [UseSegment::Glob] - ) { - return true; - } - if nested_use_tree.has_comment() { - return true; - } - // nested item detected; flatten once, but process it again - // in case it has more nesting - use_trees.push(UseTree { - path: prefix - .iter() - .cloned() - .chain(nested_use_tree.path.iter().cloned()) - .collect(), - span, - list_item: None, - visibility: visibility.clone(), - attrs: None, - }); - // remove this item - false - }); - use_tree = use_tree.normalize(); - } - } - result.push(use_tree); - } - result -} - impl fmt::Debug for UseTree { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self, f) @@ -568,7 +529,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() @@ -576,7 +537,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] + } + } } } @@ -610,7 +576,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 { @@ -619,20 +585,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); @@ -659,9 +635,11 @@ 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; @@ -669,7 +647,7 @@ fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree) { } } else if let Some(tree) = similar_trees.max_by_key(|tree| tree.path.len()) { if tree.path.len() > 1 { - tree.merge(&use_tree); + tree.merge(&use_tree, merge_by); return; } } @@ -872,6 +850,12 @@ impl Rewrite for UseTree { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum SharedPrefix { + Crate, + Module, +} + #[cfg(test)] mod test { use super::*; @@ -1018,44 +1002,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/formatting/reorder.rs b/src/formatting/reorder.rs index 323441b483a..0a01a670729 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -15,7 +15,7 @@ use crate::config::{Config, GroupImportsTactic, ImportMergeStyle}; use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ - imports::{merge_use_trees, unnest_use_trees, UseTree}, + imports::{merge_use_trees, UseTree}, items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}, lists::{itemize_list, write_list, ListFormatting, ListItem}, rewrite::RewriteContext, @@ -26,6 +26,8 @@ use crate::formatting::{ visitor::FmtVisitor, }; +use super::imports::SharedPrefix; + /// Compare strings according to version sort (roughly equivalent to `strverscmp`) pub(crate) fn compare_as_versions(left: &str, right: &str) -> Ordering { let mut left = left.chars().peekable(); @@ -227,9 +229,11 @@ fn rewrite_reorderable_or_regroupable_items( item.list_item = Some(list_item.clone()); } match context.config.imports_merge_style() { - ImportMergeStyle::Crate => normalized_items = merge_use_trees(normalized_items), + ImportMergeStyle::Crate => { + normalized_items = merge_use_trees(normalized_items, SharedPrefix::Crate) + } ImportMergeStyle::Module => { - normalized_items = unnest_use_trees(merge_use_trees(normalized_items)) + normalized_items = merge_use_trees(normalized_items, SharedPrefix::Module) } ImportMergeStyle::Preserve => {} } From 622d040ba5192bc17f0724a8d970a779002802e1 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Thu, 31 Dec 2020 17:53:00 -0800 Subject: [PATCH 6/6] Rename to imports_granularity --- Configurations.md | 6 +-- src/config.rs | 45 ++++++++++--------- src/config/config_type.rs | 10 ++--- src/config/options.rs | 2 +- src/formatting/reorder.rs | 10 ++--- src/rustfmt/main.rs | 2 +- .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- ..._crate.rs => imports_granularity_crate.rs} | 2 +- ...odule.rs => imports_granularity_module.rs} | 2 +- tests/source/issue-3750.rs | 2 +- .../StdExternalCrate-merge_imports.rs | 2 +- .../configs/imports_layout/merge_mixed.rs | 2 +- ..._crate.rs => imports_granularity_crate.rs} | 2 +- ...odule.rs => imports_granularity_module.rs} | 2 +- tests/target/issue-3750.rs | 2 +- 16 files changed, 48 insertions(+), 47 deletions(-) rename tests/source/{imports_merge_style_crate.rs => imports_granularity_crate.rs} (93%) rename tests/source/{imports_merge_style_module.rs => imports_granularity_module.rs} (86%) rename tests/target/{imports_merge_style_crate.rs => imports_granularity_crate.rs} (91%) rename tests/target/{imports_merge_style_module.rs => imports_granularity_module.rs} (86%) diff --git a/Configurations.md b/Configurations.md index e8d8b187903..4833a6ecbc4 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1705,7 +1705,7 @@ pub enum Foo {} pub enum Foo {} ``` -## `imports_merge_style` +## `imports_granularity` Merge together related imports based on their paths. @@ -1751,7 +1751,7 @@ use qux::{h, i}; ## `merge_imports` -This option is deprecated. Use `imports_merge_style = "Crate"` instead. +This option is deprecated. Use `imports_granularity = "Crate"` instead. - **Default value**: `false` - **Possible values**: `true`, `false` @@ -2602,7 +2602,7 @@ Enable unstable features on stable and beta channels (unstable features are avai For example: ```bash -rustfmt src/lib.rs --config unstable_features=true imports_merge_style=Crate +rustfmt src/lib.rs --config unstable_features=true imports_granularity=Crate ``` ## `use_field_init_shorthand` diff --git a/src/config.rs b/src/config.rs index bc4e293dae6..8d2bbe7305f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -77,10 +77,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"; - imports_merge_style: ImportMergeStyle, ImportMergeStyle::Preserve, 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_merge_style instead)"; + merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; @@ -447,9 +448,9 @@ mod test { single_line_if_else_max_width: usize, 50, true, "Maximum line length for single \ line if-else expressions. A value of zero means always break if-else expressions."; // merge_imports deprecation - imports_merge_style: ImportMergeStyle, ImportMergeStyle::Preserve, false, + imports_granularity: ImportGranularity, ImportGranularity::Preserve, false, "Merge imports"; - merge_imports: bool, false, false, "(deprecated: use imports_merge_style instead)"; + merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; unstable_features: bool, false, true, "Enables unstable features on stable and beta channels \ @@ -601,7 +602,7 @@ fn_single_line = false where_single_line = false imports_indent = "Block" imports_layout = "Mixed" -imports_merge_style = "Preserve" +imports_granularity = "Preserve" group_imports = "Preserve" reorder_imports = true reorder_modules = true @@ -722,13 +723,13 @@ ignore = [] } let toml = r#" unstable_features = true - imports_merge_style = "Crate" + imports_granularity = "Crate" "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); assert_eq!(config.was_set().unstable_features(), true); - assert_eq!(config.was_set().imports_merge_style(), true); + assert_eq!(config.was_set().imports_granularity(), true); assert_eq!(config.unstable_features(), true); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Crate); + assert_eq!(config.imports_granularity(), ImportGranularity::Crate); } #[test] @@ -738,9 +739,9 @@ ignore = [] return; } let config = - Config::from_toml("imports_merge_style = \"Crate\"", Path::new("")).unwrap(); - assert_eq!(config.was_set().imports_merge_style(), false); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); + Config::from_toml("imports_granularity = \"Crate\"", Path::new("")).unwrap(); + assert_eq!(config.was_set().imports_granularity(), false); + assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); } #[test] @@ -785,12 +786,12 @@ ignore = [] } let mut config = Config::default(); assert_eq!(config.unstable_features(), false); - config.override_value("imports_merge_style", "Crate"); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); + config.override_value("imports_granularity", "Crate"); + assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); config.override_value("unstable_features", "true"); assert_eq!(config.unstable_features(), true); - config.override_value("imports_merge_style", "Crate"); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Crate); + config.override_value("imports_granularity", "Crate"); + assert_eq!(config.imports_granularity(), ImportGranularity::Crate); } #[test] @@ -1055,7 +1056,7 @@ ignore = [] merge_imports = true "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Crate); + assert_eq!(config.imports_granularity(), ImportGranularity::Crate); } #[test] @@ -1063,10 +1064,10 @@ ignore = [] let toml = r#" unstable_features = true merge_imports = true - imports_merge_style = "Preserve" + imports_granularity = "Preserve" "#; let config = Config::from_toml(toml, Path::new("")).unwrap(); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); + assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); } #[test] @@ -1076,20 +1077,20 @@ ignore = [] merge_imports = true "#; let mut config = Config::from_toml(toml, Path::new("")).unwrap(); - config.override_value("imports_merge_style", "Preserve"); - assert_eq!(config.imports_merge_style(), ImportMergeStyle::Preserve); + 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_merge_style = "Module" + 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_merge_style(), ImportMergeStyle::Module); + assert_eq!(config.imports_granularity(), ImportGranularity::Module); } } } diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 1b903d6b770..75d3521bcc3 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -436,13 +436,13 @@ macro_rules! create_config { if self.was_set().merge_imports() { eprintln!( "Warning: the `merge_imports` option is deprecated. \ - Use `imports_merge_style=Crate` instead" + Use `imports_granularity=Crate` instead" ); - if !self.was_set().imports_merge_style() { - self.imports_merge_style.2 = if self.merge_imports() { - ImportMergeStyle::Crate + if !self.was_set().imports_granularity() { + self.imports_granularity.2 = if self.merge_imports() { + ImportGranularity::Crate } else { - ImportMergeStyle::Preserve + ImportGranularity::Preserve }; } } diff --git a/src/config/options.rs b/src/config/options.rs index ae5eafa2dcf..f09481898c7 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -121,7 +121,7 @@ pub enum GroupImportsTactic { #[config_type] /// How to merge imports. -pub enum ImportMergeStyle { +pub enum ImportGranularity { /// Do not merge imports. Preserve, /// Use one `use` statement per crate. diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index 0a01a670729..9d1777d82df 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -11,7 +11,7 @@ use std::cmp::{Ord, Ordering}; use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; -use crate::config::{Config, GroupImportsTactic, ImportMergeStyle}; +use crate::config::{Config, GroupImportsTactic, ImportGranularity}; use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ @@ -228,14 +228,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()); } - match context.config.imports_merge_style() { - ImportMergeStyle::Crate => { + match context.config.imports_granularity() { + ImportGranularity::Crate => { normalized_items = merge_use_trees(normalized_items, SharedPrefix::Crate) } - ImportMergeStyle::Module => { + ImportGranularity::Module => { normalized_items = merge_use_trees(normalized_items, SharedPrefix::Module) } - ImportMergeStyle::Preserve => {} + ImportGranularity::Preserve => {} } let mut regrouped_items = match context.config.group_imports() { diff --git a/src/rustfmt/main.rs b/src/rustfmt/main.rs index ea2f705b7fb..571eb072b2c 100644 --- a/src/rustfmt/main.rs +++ b/src/rustfmt/main.rs @@ -100,7 +100,7 @@ struct Opt { /// Set options from command line. /// /// Set configuration options via command line by specifying a list of key-value pairs - /// separated by commas (e.g., rustfmt --config=max_width=100,imports_merge_style=Crate). + /// separated by commas (e.g., rustfmt --config=max_width=100,imports_granularity=Crate). /// These settings precedes any other settings specified in configuration files. #[structopt(long = "config")] inline_config: Option>, diff --git a/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs index a3c7be06a0d..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-imports_merge_style: Crate +// 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 48e22764470..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-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str}; diff --git a/tests/source/imports_merge_style_crate.rs b/tests/source/imports_granularity_crate.rs similarity index 93% rename from tests/source/imports_merge_style_crate.rs rename to tests/source/imports_granularity_crate.rs index 51552a0bfb5..d16681b01b5 100644 --- a/tests/source/imports_merge_style_crate.rs +++ b/tests/source/imports_granularity_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate use a::{c,d,b}; use a::{d, e, b, a, f}; diff --git a/tests/source/imports_merge_style_module.rs b/tests/source/imports_granularity_module.rs similarity index 86% rename from tests/source/imports_merge_style_module.rs rename to tests/source/imports_granularity_module.rs index a02bf718003..5a4fad5872b 100644 --- a/tests/source/imports_merge_style_module.rs +++ b/tests/source/imports_granularity_module.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Module +// rustfmt-imports_granularity: Module use a::{b::c, d::e}; use a::{f, g::{h, i}}; diff --git a/tests/source/issue-3750.rs b/tests/source/issue-3750.rs index c5f0556eeef..1189a99d2b6 100644 --- a/tests/source/issue-3750.rs +++ b/tests/source/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate pub mod foo { pub mod bar { diff --git a/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs b/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs index 60333c5806c..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-imports_merge_style: Crate +// 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 c429c3e972b..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-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate // rustfmt-imports_layout: Mixed use std::{fmt, io, str, str::FromStr}; diff --git a/tests/target/imports_merge_style_crate.rs b/tests/target/imports_granularity_crate.rs similarity index 91% rename from tests/target/imports_merge_style_crate.rs rename to tests/target/imports_granularity_crate.rs index 1268d8522bd..d75906d30f1 100644 --- a/tests/target/imports_merge_style_crate.rs +++ b/tests/target/imports_granularity_crate.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate use a::{a, b, c, d, e, f, g}; diff --git a/tests/target/imports_merge_style_module.rs b/tests/target/imports_granularity_module.rs similarity index 86% rename from tests/target/imports_merge_style_module.rs rename to tests/target/imports_granularity_module.rs index c1e4a2cfebb..9c1387c466a 100644 --- a/tests/target/imports_merge_style_module.rs +++ b/tests/target/imports_granularity_module.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Module +// rustfmt-imports_granularity: Module use a::b::c; use a::d::e; diff --git a/tests/target/issue-3750.rs b/tests/target/issue-3750.rs index f9ddfa5baff..6875f8d3897 100644 --- a/tests/target/issue-3750.rs +++ b/tests/target/issue-3750.rs @@ -1,4 +1,4 @@ -// rustfmt-imports_merge_style: Crate +// rustfmt-imports_granularity: Crate pub mod foo { pub mod bar {