Merge pull request #2605 from nrc/import-squelch

Coalesce some of the import options

Author: nrc

Configurations.md

@@ -6,7 +6,7 @@ A possible content of `rustfmt.toml` or `.rustfmt.toml` might look like this:
 
 ```toml
 indent_style = "Block"
-reorder_imported_names = true
+reorder_imports = false
 ```
 
 Each configuration option is either stable or unstable.
@@ -1240,33 +1240,13 @@ fn dolor() -> usize {}
 fn adipiscing() -> usize {}
 ```
 
-## `reorder_imported_names`
-
-Reorder lists of names in import statements alphabetically
-
-- **Default value**: `false`
-- **Possible values**: `true`, `false`
-- **Stable**: No
-
-#### `false` (default):
-
-```rust
-use super::{lorem, ipsum, dolor, sit};
-```
-
-#### `true`:
-
-```rust
-use super::{dolor, ipsum, lorem, sit};
-```
-
-See also [`reorder_imports`](#reorder_imports).
 
 ## `reorder_imports`
 
-Reorder import statements alphabetically
+Reorder import and extern crate statements alphabetically in groups (a group is
+separated by a newline).
 
-- **Default value**: `false`
+- **Default value**: `true`
 - **Possible values**: `true`, `false`
 - **Stable**: No
 
@@ -1288,98 +1268,6 @@ use lorem;
 use sit;
 ```
 
-See also [`reorder_imported_names`](#reorder_imported_names), [`reorder_imports_in_group`](#reorder_imports_in_group).
-
-## `reorder_imports_in_group`
-
-Reorder import statements in group
-
-- **Default value**: `false`
-- **Possible values**: `true`, `false`
-- **Stable**: No
-
-**Note:** This option takes effect only when [`reorder_imports`](#reorder_imports) is set to `true`.
-
-#### `true` (default):
-
-```rust
-use std::io;
-use std::mem;
-
-use dolor;
-use ipsum;
-use lorem;
-use sit;
-```
-
-#### `false`:
-
-
-```rust
-use dolor;
-use ipsum;
-use lorem;
-use sit;
-use std::io;
-use std::mem;
-```
-
-See also [`reorder_imports`](#reorder_imports).
-
-## `reorder_extern_crates`
-
-Reorder `extern crate` statements alphabetically
-
-- **Default value**: `true`
-- **Possible values**: `true`, `false`
-- **Stable**: No
-
-#### `true` (default):
-
-```rust
-extern crate dolor;
-extern crate ipsum;
-extern crate lorem;
-extern crate sit;
-```
-
-#### `false`:
-
-```rust
-extern crate lorem;
-extern crate ipsum;
-
-extern crate dolor;
-extern crate sit;
-```
-
-See also [`reorder_extern_crates_in_group`](#reorder_extern_crates_in_group).
-
-## `reorder_extern_crates_in_group`
-
-Reorder `extern crate` statements in group
-
-- **Default value**: `true`
-- **Possible values**: `true`, `false`
-- **Stable**: No
-
-#### `false` (default):
-
-This value has no influence beyond the effect of the [`reorder_extern_crates`](#reorder_extern_crates) option. Set [`reorder_extern_crates`](#reorder_extern_crates) to `false` if you do not want `extern crate` groups to be collapsed and ordered.
-
-#### `true`:
-
-**Note:** This only takes effect when [`reorder_extern_crates`](#reorder_extern_crates) is set to `true`.
-
-```rust
-extern crate a;
-extern crate b;
-
-extern crate dolor;
-extern crate ipsum;
-extern crate lorem;
-extern crate sit;
-```
 
 ## `reorder_modules`
 

src/config/mod.rs

@@ -68,13 +68,8 @@ create_config! {
     imports_layout: ListTactic, ListTactic::Mixed, false, "Item layout inside a import block";
 
     // Ordering
-    reorder_extern_crates: bool, true, false, "Reorder extern crate statements alphabetically";
-    reorder_extern_crates_in_group: bool, true, false, "Reorder extern crate statements in group";
-    reorder_imports: bool, true, false, "Reorder import statements alphabetically";
-    reorder_imports_in_group: bool, true, false, "Reorder import statements in group";
-    reorder_imported_names: bool, true, false,
-        "Reorder lists of names in import statements alphabetically";
-    reorder_modules: bool, true, false, "Reorder module statemtents alphabetically in group";
+    reorder_imports: bool, true, false, "Reorder import and extern crate statements alphabetically";
+    reorder_modules: bool, true, false, "Reorder module statements alphabetically in group";
     reorder_impl_items: bool, false, false, "Reorder impl items";
 
     // Spaces around punctuation

src/imports.rs

@@ -185,7 +185,7 @@ impl UseTree {
                     } else {
                         Some(item.attrs.clone())
                     },
-                ).normalize(context.config.reorder_imported_names()),
+                ).normalize(),
             ),
             _ => None,
         }
@@ -271,7 +271,7 @@ impl UseTree {
     }
 
     // Do the adjustments that rustfmt does elsewhere to use paths.
-    pub fn normalize(mut self, do_sort: bool) -> UseTree {
+    pub fn normalize(mut self) -> UseTree {
         let mut last = self.path.pop().expect("Empty use tree?");
         // Hack around borrow checker.
         let mut normalize_sole_list = false;
@@ -340,7 +340,7 @@ impl UseTree {
                     for seg in &list[0].path {
                         self.path.push(seg.clone());
                     }
-                    return self.normalize(do_sort);
+                    return self.normalize();
                 }
                 _ => unreachable!(),
             }
@@ -349,11 +349,9 @@ impl UseTree {
         // Recursively normalize elements of a list use (including sorting the list).
         if let UseSegment::List(list) = last {
             let mut list = list.into_iter()
-                .map(|ut| ut.normalize(do_sort))
+                .map(|ut| ut.normalize())
                 .collect::<Vec<_>>();
-            if do_sort {
-                list.sort();
-            }
+            list.sort();
             last = UseSegment::List(list);
         }
 
@@ -485,6 +483,7 @@ fn rewrite_nested_use_tree(
         );
         (tactic, remaining_width)
     };
+
     let ends_with_newline = context.config.imports_indent() == IndentStyle::Block
         && tactic != DefinitiveListTactic::Horizontal;
     let fmt = ListFormatting {
@@ -685,75 +684,64 @@ mod test {
 
     #[test]
     fn test_use_tree_normalize() {
+        assert_eq!(parse_use_tree("a::self").normalize(), parse_use_tree("a"));
         assert_eq!(
-            parse_use_tree("a::self").normalize(true),
-            parse_use_tree("a")
-        );
-        assert_eq!(
-            parse_use_tree("a::self as foo").normalize(true),
+            parse_use_tree("a::self as foo").normalize(),
             parse_use_tree("a as foo")
         );
+        assert_eq!(parse_use_tree("a::{self}").normalize(), parse_use_tree("a"));
+        assert_eq!(parse_use_tree("a::{b}").normalize(), parse_use_tree("a::b"));
         assert_eq!(
-            parse_use_tree("a::{self}").normalize(true),
-            parse_use_tree("a")
-        );
-        assert_eq!(
-            parse_use_tree("a::{b}").normalize(true),
-            parse_use_tree("a::b")
-        );
-        assert_eq!(
-            parse_use_tree("a::{b, c::self}").normalize(true),
+            parse_use_tree("a::{b, c::self}").normalize(),
             parse_use_tree("a::{b, c}")
         );
         assert_eq!(
-            parse_use_tree("a::{b as bar, c::self}").normalize(true),
+            parse_use_tree("a::{b as bar, c::self}").normalize(),
             parse_use_tree("a::{b as bar, c}")
         );
     }
 
     #[test]
     fn test_use_tree_ord() {
-        assert!(parse_use_tree("a").normalize(true) < parse_use_tree("aa").normalize(true));
-        assert!(parse_use_tree("a").normalize(true) < parse_use_tree("a::a").normalize(true));
-        assert!(parse_use_tree("a").normalize(true) < parse_use_tree("*").normalize(true));
-        assert!(parse_use_tree("a").normalize(true) < parse_use_tree("{a, b}").normalize(true));
-        assert!(parse_use_tree("*").normalize(true) < parse_use_tree("{a, b}").normalize(true));
+        assert!(parse_use_tree("a").normalize() < parse_use_tree("aa").normalize());
+        assert!(parse_use_tree("a").normalize() < parse_use_tree("a::a").normalize());
+        assert!(parse_use_tree("a").normalize() < parse_use_tree("*").normalize());
+        assert!(parse_use_tree("a").normalize() < parse_use_tree("{a, b}").normalize());
+        assert!(parse_use_tree("*").normalize() < parse_use_tree("{a, b}").normalize());
 
         assert!(
-            parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize(true)
-                < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize(true)
-        );
-        assert!(
-            parse_use_tree("serde::de::{Deserialize}").normalize(true)
-                < parse_use_tree("serde_json").normalize(true)
+            parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize()
+                < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize()
         );
         assert!(
-            parse_use_tree("a::b::c").normalize(true) < parse_use_tree("a::b::*").normalize(true)
+            parse_use_tree("serde::de::{Deserialize}").normalize()
+                < parse_use_tree("serde_json").normalize()
         );
+        assert!(parse_use_tree("a::b::c").normalize() < parse_use_tree("a::b::*").normalize());
         assert!(
-            parse_use_tree("foo::{Bar, Baz}").normalize(true)
-                < parse_use_tree("{Bar, Baz}").normalize(true)
+            parse_use_tree("foo::{Bar, Baz}").normalize()
+                < parse_use_tree("{Bar, Baz}").normalize()
         );
 
         assert!(
-            parse_use_tree("foo::{self as bar}").normalize(true)
-                < parse_use_tree("foo::{qux as bar}").normalize(true)
+            parse_use_tree("foo::{self as bar}").normalize()
+                < parse_use_tree("foo::{qux as bar}").normalize()
         );
         assert!(
-            parse_use_tree("foo::{qux as bar}").normalize(true)
-                < parse_use_tree("foo::{baz, qux as bar}").normalize(true)
+            parse_use_tree("foo::{qux as bar}").normalize()
+                < parse_use_tree("foo::{baz, qux as bar}").normalize()
         );
         assert!(
-            parse_use_tree("foo::{self as bar, baz}").normalize(true)
-                < parse_use_tree("foo::{baz, qux as bar}").normalize(true)
+            parse_use_tree("foo::{self as bar, baz}").normalize()
+                < parse_use_tree("foo::{baz, qux as bar}").normalize()
         );
 
-        assert!(parse_use_tree("foo").normalize(true) < parse_use_tree("Foo").normalize(true));
-        assert!(parse_use_tree("foo").normalize(true) < parse_use_tree("foo::Bar").normalize(true));
+        assert!(parse_use_tree("foo").normalize() < parse_use_tree("Foo").normalize());
+        assert!(parse_use_tree("foo").normalize() < parse_use_tree("foo::Bar").normalize());
 
         assert!(
-            parse_use_tree("std::cmp::{d, c, b, a}").normalize(true)
-                < parse_use_tree("std::cmp::{b, e, g, f}").normalize(true)
+            parse_use_tree("std::cmp::{d, c, b, a}").normalize()
+                < parse_use_tree("std::cmp::{b, e, g, f}").normalize()
         );
     }
 }

src/reorder.rs

@@ -198,18 +198,18 @@ impl ReorderableItemKind {
 
     pub fn is_reorderable(&self, config: &Config) -> bool {
         match *self {
-            ReorderableItemKind::ExternCrate => config.reorder_extern_crates(),
+            ReorderableItemKind::ExternCrate => config.reorder_imports(),
             ReorderableItemKind::Mod => config.reorder_modules(),
             ReorderableItemKind::Use => config.reorder_imports(),
             ReorderableItemKind::Other => false,
         }
     }
 
-    pub fn in_group(&self, config: &Config) -> bool {
+    pub fn in_group(&self) -> bool {
         match *self {
-            ReorderableItemKind::ExternCrate => config.reorder_extern_crates_in_group(),
-            ReorderableItemKind::Mod => config.reorder_modules(),
-            ReorderableItemKind::Use => config.reorder_imports_in_group(),
+            ReorderableItemKind::ExternCrate
+            | ReorderableItemKind::Mod
+            | ReorderableItemKind::Use => true,
             ReorderableItemKind::Other => false,
         }
     }
@@ -268,7 +268,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             let item_kind = ReorderableItemKind::from(items[0]);
             if item_kind.is_reorderable(self.config) {
                 let visited_items_num =
-                    self.walk_reorderable_items(items, item_kind, item_kind.in_group(self.config));
+                    self.walk_reorderable_items(items, item_kind, item_kind.in_group());
                 let (_, rest) = items.split_at(visited_items_num);
                 items = rest;
             } else {