Rename option to imports_merge_style, still parse the old option

Author: goffrie

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`

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);
+        }
+    }
 }

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 {

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<Self, Self::Err> {
-        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<D>(deserializer: D) -> Result<Self, D::Error>
-    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,

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() {

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<Vec<InlineConfig>>,

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;
 

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};