From 38056e4baf3f1c242f8ea17cc4a8f1e676702cd3 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Tue, 29 Sep 2020 18:26:52 -0400 Subject: [PATCH 01/12] Add config option. It might actually be better to have a three-way option. --- Configurations.md | 27 +++++++++++++++++++++++++++ src/config.rs | 2 ++ 2 files changed, 29 insertions(+) diff --git a/Configurations.md b/Configurations.md index 08e60b1c837..32fb3156673 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2006,6 +2006,33 @@ use dolor; use sit; ``` +## `reorder_imports_opinionated` + +Reorder imports, creating three groups for: +- std (core, alloc would also fit here) +- external crates +- this module. + +Within each group, imports are sorted as with `reorder_imports`. + +This has no effect is `reorder_imports` is `false`. + +- **Default value**: `false` +- **Possible values**: `true`, `false` +- **Stable**: No + +#### `true`: + +```rust +use lorem; // TODO +``` + +#### `false` (default): + +```rust +use lorem; // TODO +``` + ## `reorder_modules` diff --git a/src/config.rs b/src/config.rs index 737194c8e9b..c0e72da24fb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -81,6 +81,7 @@ create_config! { // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; + reorder_imports_opinionated: bool, false, false, "Reorder imports in blocks"; reorder_modules: bool, true, true, "Reorder module statements alphabetically in group"; reorder_impl_items: bool, false, false, "Reorder impl items"; @@ -594,6 +595,7 @@ imports_indent = "Block" imports_layout = "Mixed" merge_imports = false reorder_imports = true +reorder_imports_opinionated = false reorder_modules = true reorder_impl_items = false type_punctuation_density = "Wide" From 5ac3b72cd4f17ad947270376e3f9eb080f1de217 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Tue, 29 Sep 2020 23:51:33 -0400 Subject: [PATCH 02/12] Add test case for opinionated reordering --- tests/source/import_opinionated.rs | 12 ++++++++++++ tests/target/import_opinionated.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/source/import_opinionated.rs create mode 100644 tests/target/import_opinionated.rs diff --git a/tests/source/import_opinionated.rs b/tests/source/import_opinionated.rs new file mode 100644 index 00000000000..a7bdd9e6f1d --- /dev/null +++ b/tests/source/import_opinionated.rs @@ -0,0 +1,12 @@ +// rustfmt:reorder_imports_opinionated: true +use chrono::Utc; +use super::update::convert_publish_payload; + +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; +use std::sync::Arc; + +use broker::database::PooledConnection; + +use super::schema::{Context, Payload}; +use crate::models::Event; diff --git a/tests/target/import_opinionated.rs b/tests/target/import_opinionated.rs new file mode 100644 index 00000000000..69d9ba420f5 --- /dev/null +++ b/tests/target/import_opinionated.rs @@ -0,0 +1,12 @@ +// rustfmt:reorder_imports_opinionated: true +use std::sync::Arc; + +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + +use broker::database::PooledConnection; + +use crate::models::Event; +use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; From d0fe771f8c3b14b65f9938b5c8f37fb2fe2bc96e Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Tue, 29 Sep 2020 23:51:46 -0400 Subject: [PATCH 03/12] Opinionated reordering --- src/formatting/reorder.rs | 69 ++++++++++++++++++++++++------ src/test/mod.rs | 7 +++ tests/source/import_opinionated.rs | 2 +- tests/target/import_opinionated.rs | 7 ++- 4 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index 92360aa3450..ab0bca284ee 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -12,6 +12,7 @@ use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; use crate::config::Config; +use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ imports::{merge_use_trees, UseTree}, @@ -227,19 +228,32 @@ fn rewrite_reorderable_items( if context.config.merge_imports() { normalized_items = merge_use_trees(normalized_items); } - normalized_items.sort(); + + let reordered_imports = if context.config.reorder_imports_opinionated() { + group_and_sort_imports(normalized_items) + } else { + normalized_items.sort(); + vec![normalized_items] + }; // 4 = "use ", 1 = ";" let nested_shape = shape.offset_left(4)?.sub_width(1)?; - let item_vec: Vec<_> = normalized_items + let item_vec: Vec<_> = reordered_imports .into_iter() - .map(|use_tree| ListItem { - item: use_tree.rewrite_top_level(context, nested_shape), - ..use_tree.list_item.unwrap_or_else(ListItem::empty) + .filter(|use_group| !use_group.is_empty()) + .map(|use_group| { + let item_vec: Vec<_> = use_group + .into_iter() + .map(|use_tree| ListItem { + item: use_tree.rewrite_top_level(context, nested_shape), + ..use_tree.list_item.unwrap_or_else(ListItem::empty) + }) + .collect(); + wrap_reorderable_items(context, &item_vec, nested_shape) }) - .collect(); + .collect::>>()?; - wrap_reorderable_items(context, &item_vec, nested_shape) + Some(item_vec.join("\n\n")) } _ => { let list_items = itemize_list( @@ -268,6 +282,38 @@ fn contains_macro_use_attr(attrs: &[ast::Attribute]) -> bool { crate::formatting::attr::contains_name(attrs, sym::macro_use) } +/// Divides imports into three groups, corresponding to standard, external +/// and local imports. Sorts each subgroup. +fn group_and_sort_imports(uts: Vec) -> Vec> { + let mut std_imports = Vec::new(); + let mut external_imports = Vec::new(); + let mut local_imports = Vec::new(); + + for ut in uts.into_iter() { + if ut.path.is_empty() { + external_imports.push(ut); + continue; + } + match &ut.path[0] { + UseSegment::Ident(id, _) => match id.as_ref() { + "std" | "alloc" | "core" => std_imports.push(ut), + _ => external_imports.push(ut), + }, + UseSegment::Slf(_) | UseSegment::Super(_) | UseSegment::Crate(_) => { + local_imports.push(ut) + } + // These are probably illegal here + UseSegment::Glob | UseSegment::List(_) => external_imports.push(ut), + } + } + + std_imports.sort(); + external_imports.sort(); + local_imports.sort(); + + vec![std_imports, external_imports, local_imports] +} + /// A simplified version of `ast::ItemKind`. #[derive(Debug, PartialEq, Eq, Copy, Clone)] enum ReorderableItemKind { @@ -311,11 +357,10 @@ impl ReorderableItemKind { } } - fn in_group(self) -> bool { + fn in_group(self, config: &Config) -> bool { match self { - ReorderableItemKind::ExternCrate - | ReorderableItemKind::Mod - | ReorderableItemKind::Use => true, + ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod => true, + ReorderableItemKind::Use => !config.reorder_imports_opinionated(), ReorderableItemKind::Other => false, } } @@ -376,7 +421,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let item_kind = ReorderableItemKind::from(items[0], self.file_mod_map); if item_kind.is_reorderable(self.config) { let visited_items_num = - self.walk_reorderable_items(items, item_kind, item_kind.in_group()); + self.walk_reorderable_items(items, item_kind, item_kind.in_group(self.config)); let (_, rest) = items.split_at(visited_items_num); items = rest; } else { diff --git a/src/test/mod.rs b/src/test/mod.rs index f5eaafc45fc..3f05125ad32 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -197,6 +197,13 @@ fn system_tests() { }); } +#[test] +fn temp() { + let vpb = vec![Path::new("tests/source/import_opinionated.rs").to_path_buf()]; + let (_reports, _count, fails) = check_files(vpb, &None); + assert_eq!(fails, 0); +} + #[test] fn checkstyle_test() { init_log(); diff --git a/tests/source/import_opinionated.rs b/tests/source/import_opinionated.rs index a7bdd9e6f1d..cd23aa2d433 100644 --- a/tests/source/import_opinionated.rs +++ b/tests/source/import_opinionated.rs @@ -1,4 +1,4 @@ -// rustfmt:reorder_imports_opinionated: true +// rustfmt-reorder_imports_opinionated: true use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/target/import_opinionated.rs b/tests/target/import_opinionated.rs index 69d9ba420f5..4355be70da2 100644 --- a/tests/target/import_opinionated.rs +++ b/tests/target/import_opinionated.rs @@ -1,12 +1,11 @@ -// rustfmt:reorder_imports_opinionated: true +// rustfmt-reorder_imports_opinionated: true use std::sync::Arc; +use broker::database::PooledConnection; use chrono::Utc; use juniper::{FieldError, FieldResult}; use uuid::Uuid; -use broker::database::PooledConnection; - -use crate::models::Event; use super::schema::{Context, Payload}; use super::update::convert_publish_payload; +use crate::models::Event; From ce79ded0f750f7dd9a6ca27ffbb81dcac1e3b586 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Wed, 30 Sep 2020 17:23:01 -0400 Subject: [PATCH 04/12] Update Documentation.md examples --- Configurations.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/Configurations.md b/Configurations.md index 32fb3156673..f33896278fd 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2024,13 +2024,37 @@ This has no effect is `reorder_imports` is `false`. #### `true`: ```rust -use lorem; // TODO +use alloc::alloc::Layout; +use core::f32; +use std::sync::Arc; + +use broker::database::PooledConnection; +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + +use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; +use crate::models::Event; ``` #### `false` (default): ```rust -use lorem; // TODO +use chrono::Utc; +use super::update::convert_publish_payload; + +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; +use alloc::alloc::Layout; + +use std::sync::Arc; + +use broker::database::PooledConnection; + +use super::schema::{Context, Payload}; +use core::f32; +use crate::models::Event; ``` From a46997616d6cf6315f1ffe6e343e6389fc8886ce Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Wed, 30 Sep 2020 17:35:49 -0400 Subject: [PATCH 05/12] Rename tests --- Configurations.md | 8 ++++---- .../imports-reorder-opinionated.rs} | 12 ++++++++---- .../imports-reorder-opinionated.rs} | 11 ++++++----- 3 files changed, 18 insertions(+), 13 deletions(-) rename tests/{target/import_opinionated.rs => source/imports-reorder-opinionated.rs} (87%) rename tests/{source/import_opinionated.rs => target/imports-reorder-opinionated.rs} (87%) diff --git a/Configurations.md b/Configurations.md index f33896278fd..a0fea246ef0 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2009,7 +2009,7 @@ use sit; ## `reorder_imports_opinionated` Reorder imports, creating three groups for: -- std (core, alloc would also fit here) +- `std`, `core` and `alloc`, - external crates - this module. @@ -2041,20 +2041,20 @@ use crate::models::Event; #### `false` (default): ```rust -use chrono::Utc; use super::update::convert_publish_payload; +use chrono::Utc; +use alloc::alloc::Layout; use juniper::{FieldError, FieldResult}; use uuid::Uuid; -use alloc::alloc::Layout; use std::sync::Arc; use broker::database::PooledConnection; use super::schema::{Context, Payload}; -use core::f32; use crate::models::Event; +use core::f32; ``` diff --git a/tests/target/import_opinionated.rs b/tests/source/imports-reorder-opinionated.rs similarity index 87% rename from tests/target/import_opinionated.rs rename to tests/source/imports-reorder-opinionated.rs index 4355be70da2..a14bdf076fd 100644 --- a/tests/target/import_opinionated.rs +++ b/tests/source/imports-reorder-opinionated.rs @@ -1,11 +1,15 @@ // rustfmt-reorder_imports_opinionated: true -use std::sync::Arc; - -use broker::database::PooledConnection; use chrono::Utc; +use super::update::convert_publish_payload; + use juniper::{FieldError, FieldResult}; use uuid::Uuid; +use alloc::alloc::Layout; + +use std::sync::Arc; + +use broker::database::PooledConnection; use super::schema::{Context, Payload}; -use super::update::convert_publish_payload; +use core::f32; use crate::models::Event; diff --git a/tests/source/import_opinionated.rs b/tests/target/imports-reorder-opinionated.rs similarity index 87% rename from tests/source/import_opinionated.rs rename to tests/target/imports-reorder-opinionated.rs index cd23aa2d433..6b7b5f9d0c7 100644 --- a/tests/source/import_opinionated.rs +++ b/tests/target/imports-reorder-opinionated.rs @@ -1,12 +1,13 @@ // rustfmt-reorder_imports_opinionated: true -use chrono::Utc; -use super::update::convert_publish_payload; - -use juniper::{FieldError, FieldResult}; -use uuid::Uuid; +use alloc::alloc::Layout; +use core::f32; use std::sync::Arc; use broker::database::PooledConnection; +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; use crate::models::Event; From eba49fe71142076a6a775825fc1efaafc923e754 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Wed, 30 Sep 2020 17:37:25 -0400 Subject: [PATCH 06/12] Removed temp test --- Configurations.md | 37 ++++++++++++++++++------------------- src/config.rs | 3 ++- src/test/mod.rs | 7 ------- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/Configurations.md b/Configurations.md index a0fea246ef0..6f695b9d153 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2008,10 +2008,10 @@ use sit; ## `reorder_imports_opinionated` -Reorder imports, creating three groups for: -- `std`, `core` and `alloc`, -- external crates -- this module. +Discard existing import groups, and create three groups for: +1. `std`, `core` and `alloc`, +2. external crates, +3. this module. Within each group, imports are sorted as with `reorder_imports`. @@ -2021,43 +2021,42 @@ This has no effect is `reorder_imports` is `false`. - **Possible values**: `true`, `false` - **Stable**: No -#### `true`: +#### `false` (default): ```rust +use super::update::convert_publish_payload; +use chrono::Utc; + use alloc::alloc::Layout; -use core::f32; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + use std::sync::Arc; use broker::database::PooledConnection; -use chrono::Utc; -use juniper::{FieldError, FieldResult}; -use uuid::Uuid; use super::schema::{Context, Payload}; -use super::update::convert_publish_payload; use crate::models::Event; +use core::f32; ``` -#### `false` (default): +#### `true`: ```rust -use super::update::convert_publish_payload; -use chrono::Utc; - use alloc::alloc::Layout; -use juniper::{FieldError, FieldResult}; -use uuid::Uuid; - +use core::f32; use std::sync::Arc; use broker::database::PooledConnection; +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; use crate::models::Event; -use core::f32; ``` - ## `reorder_modules` Reorder `mod` declarations alphabetically in group. diff --git a/src/config.rs b/src/config.rs index c0e72da24fb..be9e45bd87b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -81,7 +81,8 @@ create_config! { // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; - reorder_imports_opinionated: bool, false, false, "Reorder imports in blocks"; + reorder_imports_opinionated: bool, false, false, + "Create blocks for standard, external, and local imports"; reorder_modules: bool, true, true, "Reorder module statements alphabetically in group"; reorder_impl_items: bool, false, false, "Reorder impl items"; diff --git a/src/test/mod.rs b/src/test/mod.rs index 3f05125ad32..f5eaafc45fc 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -197,13 +197,6 @@ fn system_tests() { }); } -#[test] -fn temp() { - let vpb = vec![Path::new("tests/source/import_opinionated.rs").to_path_buf()]; - let (_reports, _count, fails) = check_files(vpb, &None); - assert_eq!(fails, 0); -} - #[test] fn checkstyle_test() { init_log(); From 16fd87bc9fdf20b47f6790d962d5c6374f0751b8 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Thu, 1 Oct 2020 18:18:42 -0400 Subject: [PATCH 07/12] Rename reorder_imports_opinionated -> group_imports --- Configurations.md | 12 ++++++------ src/config.rs | 6 +++--- src/config/options.rs | 12 ++++++++++++ src/formatting/reorder.rs | 6 +++--- ...ports-reorder-opinionated.rs => group_imports.rs} | 2 +- ...ports-reorder-opinionated.rs => group_imports.rs} | 2 +- 6 files changed, 26 insertions(+), 14 deletions(-) rename tests/source/{imports-reorder-opinionated.rs => group_imports.rs} (86%) rename tests/target/{imports-reorder-opinionated.rs => group_imports.rs} (86%) diff --git a/Configurations.md b/Configurations.md index 6f695b9d153..0ca4e8c0819 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2006,22 +2006,22 @@ use dolor; use sit; ``` -## `reorder_imports_opinionated` +## `group_imports` Discard existing import groups, and create three groups for: 1. `std`, `core` and `alloc`, 2. external crates, -3. this module. +3. `self`, `super` and `crate` imports. Within each group, imports are sorted as with `reorder_imports`. This has no effect is `reorder_imports` is `false`. -- **Default value**: `false` -- **Possible values**: `true`, `false` +- **Default value**: `None` +- **Possible values**: `None`, `StdExternalCrate` - **Stable**: No -#### `false` (default): +#### `None` (default): ```rust use super::update::convert_publish_payload; @@ -2040,7 +2040,7 @@ use crate::models::Event; use core::f32; ``` -#### `true`: +#### `StdExternalCrate`: ```rust use alloc::alloc::Layout; diff --git a/src/config.rs b/src/config.rs index be9e45bd87b..6b16f15c6e2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -78,11 +78,11 @@ create_config! { 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"; + group_imports: GroupImportsTactic, GroupImportsTactic::None, false, + "Reorganize import groups"; // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; - reorder_imports_opinionated: bool, false, false, - "Create blocks for standard, external, and local imports"; reorder_modules: bool, true, true, "Reorder module statements alphabetically in group"; reorder_impl_items: bool, false, false, "Reorder impl items"; @@ -595,8 +595,8 @@ where_single_line = false imports_indent = "Block" imports_layout = "Mixed" merge_imports = false +group_imports = "None" reorder_imports = true -reorder_imports_opinionated = false reorder_modules = true reorder_impl_items = false type_punctuation_density = "Wide" diff --git a/src/config/options.rs b/src/config/options.rs index bdf6d2766f5..46eadbb56a7 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -106,6 +106,18 @@ impl Density { } } +#[config_type] +/// Configuration for import groups, i.e. sets of imports separated by newlines. +pub enum GroupImportsTactic { + /// Keep groups as they are. + None, + /// Discard existing groups, and create new groups for + /// 1. `std` / `core` / `alloc` imports + /// 2. other imports + /// 3. `self` / `crate` / `super` imports + StdExternalCrate, +} + #[config_type] pub enum ReportTactic { Always, diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index ab0bca284ee..8eef38555f5 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; +use crate::config::{Config, GroupImportsTactic}; use crate::formatting::imports::UseSegment; use crate::formatting::modules::{get_mod_inner_attrs, FileModMap}; use crate::formatting::{ @@ -229,7 +229,7 @@ fn rewrite_reorderable_items( normalized_items = merge_use_trees(normalized_items); } - let reordered_imports = if context.config.reorder_imports_opinionated() { + let reordered_imports = if context.config.group_imports() != GroupImportsTactic::None { group_and_sort_imports(normalized_items) } else { normalized_items.sort(); @@ -360,7 +360,7 @@ impl ReorderableItemKind { fn in_group(self, config: &Config) -> bool { match self { ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod => true, - ReorderableItemKind::Use => !config.reorder_imports_opinionated(), + ReorderableItemKind::Use => config.group_imports() == GroupImportsTactic::None, ReorderableItemKind::Other => false, } } diff --git a/tests/source/imports-reorder-opinionated.rs b/tests/source/group_imports.rs similarity index 86% rename from tests/source/imports-reorder-opinionated.rs rename to tests/source/group_imports.rs index a14bdf076fd..d49c8941e6d 100644 --- a/tests/source/imports-reorder-opinionated.rs +++ b/tests/source/group_imports.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports_opinionated: true +// rustfmt-group_imports: StdExternalCrate use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/target/imports-reorder-opinionated.rs b/tests/target/group_imports.rs similarity index 86% rename from tests/target/imports-reorder-opinionated.rs rename to tests/target/group_imports.rs index 6b7b5f9d0c7..08025796898 100644 --- a/tests/target/imports-reorder-opinionated.rs +++ b/tests/target/group_imports.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports_opinionated: true +// rustfmt-group_imports: StdExternalCrate use alloc::alloc::Layout; use core::f32; use std::sync::Arc; From 4f39fc497e4b83b6b64637c05ef8e4e8292e3169 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Thu, 1 Oct 2020 18:24:54 -0400 Subject: [PATCH 08/12] Add extra tests Tests for interaction with `no_reorder` and `merge_imports`. --- tests/source/group_imports-merge_imports.rs | 17 +++++++++++++++++ tests/source/group_imports-no_reorder.rs | 19 +++++++++++++++++++ tests/target/group_imports-merge_imports.rs | 16 ++++++++++++++++ tests/target/group_imports-no_reorder.rs | 19 +++++++++++++++++++ 4 files changed, 71 insertions(+) create mode 100644 tests/source/group_imports-merge_imports.rs create mode 100644 tests/source/group_imports-no_reorder.rs create mode 100644 tests/target/group_imports-merge_imports.rs create mode 100644 tests/target/group_imports-no_reorder.rs diff --git a/tests/source/group_imports-merge_imports.rs b/tests/source/group_imports-merge_imports.rs new file mode 100644 index 00000000000..1a60126d2d1 --- /dev/null +++ b/tests/source/group_imports-merge_imports.rs @@ -0,0 +1,17 @@ +// rustfmt-group_imports: StdExternalCrate +// rustfmt-merge_imports: true +use chrono::Utc; +use super::update::convert_publish_payload; + +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; +use alloc::alloc::Layout; + +use std::sync::Arc; +use alloc::vec::Vec; + +use broker::database::PooledConnection; + +use super::schema::{Context, Payload}; +use core::f32; +use crate::models::Event; diff --git a/tests/source/group_imports-no_reorder.rs b/tests/source/group_imports-no_reorder.rs new file mode 100644 index 00000000000..2e283987aa1 --- /dev/null +++ b/tests/source/group_imports-no_reorder.rs @@ -0,0 +1,19 @@ +// rustfmt-group_imports: StdExternalCrate +// rustfmt-reorder_imports: false + +// group_imports has no effect when reorder_imports is disabled. + +use chrono::Utc; +use super::update::convert_publish_payload; + +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; +use alloc::alloc::Layout; + +use std::sync::Arc; + +use broker::database::PooledConnection; + +use super::schema::{Context, Payload}; +use core::f32; +use crate::models::Event; diff --git a/tests/target/group_imports-merge_imports.rs b/tests/target/group_imports-merge_imports.rs new file mode 100644 index 00000000000..a3d99eb907e --- /dev/null +++ b/tests/target/group_imports-merge_imports.rs @@ -0,0 +1,16 @@ +// rustfmt-group_imports: StdExternalCrate +// rustfmt-merge_imports: true +use alloc::{alloc::Layout, vec::Vec}; +use core::f32; +use std::sync::Arc; + +use broker::database::PooledConnection; +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + +use super::{ + schema::{Context, Payload}, + update::convert_publish_payload, +}; +use crate::models::Event; diff --git a/tests/target/group_imports-no_reorder.rs b/tests/target/group_imports-no_reorder.rs new file mode 100644 index 00000000000..2e283987aa1 --- /dev/null +++ b/tests/target/group_imports-no_reorder.rs @@ -0,0 +1,19 @@ +// rustfmt-group_imports: StdExternalCrate +// rustfmt-reorder_imports: false + +// group_imports has no effect when reorder_imports is disabled. + +use chrono::Utc; +use super::update::convert_publish_payload; + +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; +use alloc::alloc::Layout; + +use std::sync::Arc; + +use broker::database::PooledConnection; + +use super::schema::{Context, Payload}; +use core::f32; +use crate::models::Event; From 93b7cc37acc3e11845ecc0b2beccbc37840af979 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Sun, 4 Oct 2020 10:34:21 -0400 Subject: [PATCH 09/12] Decouple reordering and grouping --- src/formatting/reorder.rs | 58 ++++++++++++++++-------- tests/source/group_imports-no_reorder.rs | 2 - tests/target/group_imports-no_reorder.rs | 12 ++--- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index 8eef38555f5..c0418afea0e 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -193,9 +193,10 @@ fn rewrite_reorderable_item( } } -/// Rewrite a list of items with reordering. Every item in `items` must have -/// the same `ast::ItemKind`. -fn rewrite_reorderable_items( +/// Rewrite a list of items with reordering and/or regrouping. Every item +/// in `items` must have the same `ast::ItemKind`. Whether reordering, regrouping, +/// or both ore done is determined from the `context`. +fn rewrite_reorderable_or_regroupable_items( context: &RewriteContext<'_>, reorderable_items: &[&ast::Item], shape: Shape, @@ -229,16 +230,20 @@ fn rewrite_reorderable_items( normalized_items = merge_use_trees(normalized_items); } - let reordered_imports = if context.config.group_imports() != GroupImportsTactic::None { - group_and_sort_imports(normalized_items) + let mut regrouped_items = if context.config.group_imports() != GroupImportsTactic::None + { + group_imports(normalized_items) } else { - normalized_items.sort(); vec![normalized_items] }; + if context.config.reorder_imports() { + regrouped_items.iter_mut().for_each(|items| items.sort()) + } + // 4 = "use ", 1 = ";" let nested_shape = shape.offset_left(4)?.sub_width(1)?; - let item_vec: Vec<_> = reordered_imports + let item_vec: Vec<_> = regrouped_items .into_iter() .filter(|use_group| !use_group.is_empty()) .map(|use_group| { @@ -284,7 +289,7 @@ fn contains_macro_use_attr(attrs: &[ast::Attribute]) -> bool { /// Divides imports into three groups, corresponding to standard, external /// and local imports. Sorts each subgroup. -fn group_and_sort_imports(uts: Vec) -> Vec> { +fn group_imports(uts: Vec) -> Vec> { let mut std_imports = Vec::new(); let mut external_imports = Vec::new(); let mut local_imports = Vec::new(); @@ -307,10 +312,6 @@ fn group_and_sort_imports(uts: Vec) -> Vec> { } } - std_imports.sort(); - external_imports.sort(); - local_imports.sort(); - vec![std_imports, external_imports, local_imports] } @@ -357,6 +358,15 @@ impl ReorderableItemKind { } } + fn is_regroupable(self, config: &Config) -> bool { + match self { + ReorderableItemKind::ExternCrate + | ReorderableItemKind::Mod + | ReorderableItemKind::Other => false, + ReorderableItemKind::Use => config.group_imports() != GroupImportsTactic::None, + } + } + fn in_group(self, config: &Config) -> bool { match self { ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod => true, @@ -367,10 +377,10 @@ impl ReorderableItemKind { } impl<'b, 'a: 'b> FmtVisitor<'a> { - /// Format items with the same item kind and reorder them. If `in_group` is - /// `true`, then the items separated by an empty line will not be reordered - /// together. - fn walk_reorderable_items( + /// Format items with the same item kind and reorder them, regroup them, or + /// both. If `in_group` is `true`, then the items separated by an empty line + /// will not be reordered together. + fn walk_reorderable_or_regroupable_items( &mut self, items: &[&ast::Item], item_kind: ReorderableItemKind, @@ -400,7 +410,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let lo = items.first().unwrap().span().lo(); let hi = items.last().unwrap().span().hi(); let span = mk_sp(lo, hi); - let rw = rewrite_reorderable_items(&self.get_context(), items, self.shape(), span); + let rw = rewrite_reorderable_or_regroupable_items( + &self.get_context(), + items, + self.shape(), + span, + ); self.push_rewrite(span, rw); } else { for item in items { @@ -419,9 +434,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // subsequent items that have the same item kind to be reordered within // `walk_reorderable_items`. Otherwise, just format the next item for output. let item_kind = ReorderableItemKind::from(items[0], self.file_mod_map); - if item_kind.is_reorderable(self.config) { - let visited_items_num = - self.walk_reorderable_items(items, item_kind, item_kind.in_group(self.config)); + if item_kind.is_reorderable(self.config) || item_kind.is_regroupable(self.config) { + let visited_items_num = self.walk_reorderable_or_regroupable_items( + items, + item_kind, + item_kind.in_group(self.config), + ); let (_, rest) = items.split_at(visited_items_num); items = rest; } else { diff --git a/tests/source/group_imports-no_reorder.rs b/tests/source/group_imports-no_reorder.rs index 2e283987aa1..08c9a72ae61 100644 --- a/tests/source/group_imports-no_reorder.rs +++ b/tests/source/group_imports-no_reorder.rs @@ -1,8 +1,6 @@ // rustfmt-group_imports: StdExternalCrate // rustfmt-reorder_imports: false -// group_imports has no effect when reorder_imports is disabled. - use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/target/group_imports-no_reorder.rs b/tests/target/group_imports-no_reorder.rs index 2e283987aa1..76d3d6ccb95 100644 --- a/tests/target/group_imports-no_reorder.rs +++ b/tests/target/group_imports-no_reorder.rs @@ -1,19 +1,15 @@ // rustfmt-group_imports: StdExternalCrate // rustfmt-reorder_imports: false -// group_imports has no effect when reorder_imports is disabled. +use alloc::alloc::Layout; +use std::sync::Arc; +use core::f32; use chrono::Utc; -use super::update::convert_publish_payload; - use juniper::{FieldError, FieldResult}; use uuid::Uuid; -use alloc::alloc::Layout; - -use std::sync::Arc; - use broker::database::PooledConnection; +use super::update::convert_publish_payload; use super::schema::{Context, Payload}; -use core::f32; use crate::models::Event; From ec219b52cd22e35bd7ce9a13f3fca36fbf2d8cdd Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Sun, 4 Oct 2020 10:50:31 -0400 Subject: [PATCH 10/12] Change None -> Preserve for group_imports config Also reword configuration to be less specific. --- Configurations.md | 22 +++++++++++----------- src/config.rs | 4 ++-- src/config/options.rs | 2 +- src/formatting/reorder.rs | 14 ++++++-------- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/Configurations.md b/Configurations.md index 0ca4e8c0819..00fe6ec00b1 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2008,20 +2008,15 @@ use sit; ## `group_imports` -Discard existing import groups, and create three groups for: -1. `std`, `core` and `alloc`, -2. external crates, -3. `self`, `super` and `crate` imports. - -Within each group, imports are sorted as with `reorder_imports`. - -This has no effect is `reorder_imports` is `false`. +Controls the strategy for how imports are grouped together. -- **Default value**: `None` -- **Possible values**: `None`, `StdExternalCrate` +- **Default value**: `Preserve` +- **Possible values**: `Preserve`, `StdExternalCrate` - **Stable**: No -#### `None` (default): +#### `Preserve` (default): + +Preserve the source file's import groups. ```rust use super::update::convert_publish_payload; @@ -2042,6 +2037,11 @@ use core::f32; #### `StdExternalCrate`: +Discard existing import groups, and create three groups for: +1. `std`, `core` and `alloc`, +2. external crates, +3. `self`, `super` and `crate` imports. + ```rust use alloc::alloc::Layout; use core::f32; diff --git a/src/config.rs b/src/config.rs index 6b16f15c6e2..b28fe3d9e36 100644 --- a/src/config.rs +++ b/src/config.rs @@ -78,7 +78,7 @@ create_config! { 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"; - group_imports: GroupImportsTactic, GroupImportsTactic::None, false, + group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false, "Reorganize import groups"; // Ordering @@ -595,7 +595,7 @@ where_single_line = false imports_indent = "Block" imports_layout = "Mixed" merge_imports = false -group_imports = "None" +group_imports = "Preserve" reorder_imports = true reorder_modules = true reorder_impl_items = false diff --git a/src/config/options.rs b/src/config/options.rs index 46eadbb56a7..aea67749a45 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -110,7 +110,7 @@ impl Density { /// Configuration for import groups, i.e. sets of imports separated by newlines. pub enum GroupImportsTactic { /// Keep groups as they are. - None, + Preserve, /// Discard existing groups, and create new groups for /// 1. `std` / `core` / `alloc` imports /// 2. other imports diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index c0418afea0e..f43cd10f45d 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -195,7 +195,7 @@ fn rewrite_reorderable_item( /// Rewrite a list of items with reordering and/or regrouping. Every item /// in `items` must have the same `ast::ItemKind`. Whether reordering, regrouping, -/// or both ore done is determined from the `context`. +/// or both are done is determined from the `context`. fn rewrite_reorderable_or_regroupable_items( context: &RewriteContext<'_>, reorderable_items: &[&ast::Item], @@ -230,11 +230,9 @@ fn rewrite_reorderable_or_regroupable_items( normalized_items = merge_use_trees(normalized_items); } - let mut regrouped_items = if context.config.group_imports() != GroupImportsTactic::None - { - group_imports(normalized_items) - } else { - vec![normalized_items] + let mut regrouped_items = match context.config.group_imports() { + GroupImportsTactic::Preserve => vec![normalized_items], + GroupImportsTactic::StdExternalCrate => group_imports(normalized_items), }; if context.config.reorder_imports() { @@ -363,14 +361,14 @@ impl ReorderableItemKind { ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod | ReorderableItemKind::Other => false, - ReorderableItemKind::Use => config.group_imports() != GroupImportsTactic::None, + ReorderableItemKind::Use => config.group_imports() != GroupImportsTactic::Preserve, } } fn in_group(self, config: &Config) -> bool { match self { ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod => true, - ReorderableItemKind::Use => config.group_imports() == GroupImportsTactic::None, + ReorderableItemKind::Use => config.group_imports() == GroupImportsTactic::Preserve, ReorderableItemKind::Other => false, } } From a0b4a3bc0bd12fa3e3630a28e5e165afc606c486 Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Mon, 5 Oct 2020 22:07:30 -0400 Subject: [PATCH 11/12] Move test files; change description wording. --- src/config.rs | 2 +- .../group_imports/StdExternalCrate-merge_imports.rs} | 0 .../group_imports/StdExternalCrate-no_reorder.rs} | 0 .../group_imports/StdExternalCrate.rs} | 0 .../group_imports/StdExternalCrate-merge_imports.rs} | 0 .../group_imports/StdExternalCrate-no_reorder.rs} | 0 .../group_imports/StdExternalCrate.rs} | 0 7 files changed, 1 insertion(+), 1 deletion(-) rename tests/source/{group_imports-merge_imports.rs => configs/group_imports/StdExternalCrate-merge_imports.rs} (100%) rename tests/source/{group_imports-no_reorder.rs => configs/group_imports/StdExternalCrate-no_reorder.rs} (100%) rename tests/source/{group_imports.rs => configs/group_imports/StdExternalCrate.rs} (100%) rename tests/target/{group_imports-merge_imports.rs => configs/group_imports/StdExternalCrate-merge_imports.rs} (100%) rename tests/target/{group_imports-no_reorder.rs => configs/group_imports/StdExternalCrate-no_reorder.rs} (100%) rename tests/target/{group_imports.rs => configs/group_imports/StdExternalCrate.rs} (100%) diff --git a/src/config.rs b/src/config.rs index b28fe3d9e36..e604e5b4b2c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -79,7 +79,7 @@ create_config! { imports_layout: ListTactic, ListTactic::Mixed, false, "Item layout inside a import block"; merge_imports: bool, false, false, "Merge imports"; group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false, - "Reorganize import groups"; + "Controls the strategy for how imports are grouped together"; // Ordering reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; diff --git a/tests/source/group_imports-merge_imports.rs b/tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs similarity index 100% rename from tests/source/group_imports-merge_imports.rs rename to tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs diff --git a/tests/source/group_imports-no_reorder.rs b/tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs similarity index 100% rename from tests/source/group_imports-no_reorder.rs rename to tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs diff --git a/tests/source/group_imports.rs b/tests/source/configs/group_imports/StdExternalCrate.rs similarity index 100% rename from tests/source/group_imports.rs rename to tests/source/configs/group_imports/StdExternalCrate.rs diff --git a/tests/target/group_imports-merge_imports.rs b/tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs similarity index 100% rename from tests/target/group_imports-merge_imports.rs rename to tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs diff --git a/tests/target/group_imports-no_reorder.rs b/tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs similarity index 100% rename from tests/target/group_imports-no_reorder.rs rename to tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs diff --git a/tests/target/group_imports.rs b/tests/target/configs/group_imports/StdExternalCrate.rs similarity index 100% rename from tests/target/group_imports.rs rename to tests/target/configs/group_imports/StdExternalCrate.rs From f8f55c78f4d8809b79c3b85cc596cc917c94b0de Mon Sep 17 00:00:00 2001 From: Matthieu Felix Date: Mon, 26 Oct 2020 20:06:00 -0400 Subject: [PATCH 12/12] Handle indented import groups --- src/formatting/reorder.rs | 3 ++- .../configs/group_imports/StdExternalCrate-nested.rs | 6 ++++++ .../configs/group_imports/StdExternalCrate-nested.rs | 7 +++++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/source/configs/group_imports/StdExternalCrate-nested.rs create mode 100644 tests/target/configs/group_imports/StdExternalCrate-nested.rs diff --git a/src/formatting/reorder.rs b/src/formatting/reorder.rs index f43cd10f45d..379aa109b14 100644 --- a/src/formatting/reorder.rs +++ b/src/formatting/reorder.rs @@ -256,7 +256,8 @@ fn rewrite_reorderable_or_regroupable_items( }) .collect::>>()?; - Some(item_vec.join("\n\n")) + let join_string = format!("\n\n{}", shape.indent.to_string(context.config)); + Some(item_vec.join(&join_string)) } _ => { let list_items = itemize_list( diff --git a/tests/source/configs/group_imports/StdExternalCrate-nested.rs b/tests/source/configs/group_imports/StdExternalCrate-nested.rs new file mode 100644 index 00000000000..08f4e07b704 --- /dev/null +++ b/tests/source/configs/group_imports/StdExternalCrate-nested.rs @@ -0,0 +1,6 @@ +// rustfmt-group_imports: StdExternalCrate +mod test { + use crate::foo::bar; + use std::path; + use crate::foo::bar2; +} diff --git a/tests/target/configs/group_imports/StdExternalCrate-nested.rs b/tests/target/configs/group_imports/StdExternalCrate-nested.rs new file mode 100644 index 00000000000..daf23375c0e --- /dev/null +++ b/tests/target/configs/group_imports/StdExternalCrate-nested.rs @@ -0,0 +1,7 @@ +// rustfmt-group_imports: StdExternalCrate +mod test { + use std::path; + + use crate::foo::bar; + use crate::foo::bar2; +}