From 6053fb47f4987c31f0f53256c30d1d3d74cdc222 Mon Sep 17 00:00:00 2001 From: shiri Date: Mon, 16 May 2022 20:51:46 +0300 Subject: [PATCH 01/12] change reorder_imports possible options to enum --- Configurations.md | 14 +++++++------- src/config/mod.rs | 5 +++-- src/config/options.rs | 10 ++++++++++ src/reorder.rs | 8 ++++---- tests/config/issue-1111.toml | 2 +- tests/config/small_tabs.toml | 2 +- .../source/configs/group_imports/One-no_reorder.rs | 2 +- .../group_imports/StdExternalCrate-no_reorder.rs | 2 +- tests/source/configs/reorder_imports/false.rs | 2 +- tests/source/configs/reorder_imports/true.rs | 2 +- tests/source/issue-1120.rs | 2 +- tests/source/issue-1124.rs | 2 +- tests/source/issue-3515.rs | 2 +- .../source/issue-3585/reorder_imports_disabled.rs | 2 +- tests/source/issue-3585/reorder_imports_enabled.rs | 2 +- tests/source/long-use-statement-issue-3154.rs | 2 +- .../target/configs/group_imports/One-no_reorder.rs | 2 +- .../group_imports/StdExternalCrate-no_reorder.rs | 2 +- tests/target/configs/reorder_imports/false.rs | 2 +- tests/target/configs/reorder_imports/true.rs | 2 +- tests/target/issue-1120.rs | 2 +- tests/target/issue-1124.rs | 2 +- tests/target/issue-3515.rs | 2 +- .../target/issue-3585/reorder_imports_disabled.rs | 2 +- tests/target/issue-3585/reorder_imports_enabled.rs | 2 +- tests/target/issue-4791/issue_4928.rs | 2 +- tests/target/long-use-statement-issue-3154.rs | 2 +- 27 files changed, 47 insertions(+), 36 deletions(-) diff --git a/Configurations.md b/Configurations.md index 7afd4b5cd6a..045fcd17535 100644 --- a/Configurations.md +++ b/Configurations.md @@ -6,7 +6,7 @@ A possible content of `rustfmt.toml` or `.rustfmt.toml` might look like this: ```toml indent_style = "Block" -reorder_imports = false +reorder_imports = "Off" ``` Each configuration option is either stable or unstable. @@ -2034,14 +2034,14 @@ impl Iterator for Dummy { ## `reorder_imports` -Reorder import and extern crate statements alphabetically in groups (a group is +Reorder import and extern crate statements in groups (a group is separated by a newline). -- **Default value**: `true` -- **Possible values**: `true`, `false` -- **Stable**: Yes +- **Default value**: `"Alphabetically"` +- **Possible values**: `"Alphabetically"`, `"Length"`, `"Off"` +- **Stable**: No -#### `true` (default): +#### `"Alphabetically"` (default): ```rust use dolor; @@ -2050,7 +2050,7 @@ use lorem; use sit; ``` -#### `false`: +#### `"Off"`: ```rust use lorem; diff --git a/src/config/mod.rs b/src/config/mod.rs index 18e1854612b..9ffd0c3ce29 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -90,7 +90,8 @@ create_config! { merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)"; // Ordering - reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically"; + reorder_imports: ReorderImports, ReorderImports::Alphabetically, false, + "Reorder import and extern crate statements"; reorder_modules: bool, true, true, "Reorder module statements alphabetically in group"; reorder_impl_items: bool, false, false, "Reorder impl items"; @@ -583,7 +584,7 @@ imports_indent = "Block" imports_layout = "Mixed" imports_granularity = "Preserve" group_imports = "Preserve" -reorder_imports = true +reorder_imports = "Alphabetically" reorder_modules = true reorder_impl_items = false type_punctuation_density = "Wide" diff --git a/src/config/options.rs b/src/config/options.rs index d857c29be29..c7f17ed58ee 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -457,3 +457,13 @@ pub enum MatchArmLeadingPipe { /// Preserve any existing leading pipes Preserve, } + +/// Reorder imports +#[config_type] +pub enum ReorderImports { + Alphabetically, + + Length, + + Off, +} diff --git a/src/reorder.rs b/src/reorder.rs index 8ae297de25b..5fd518efdcb 100644 --- a/src/reorder.rs +++ b/src/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}; +use crate::config::{Config, GroupImportsTactic, ReorderImports}; use crate::imports::{normalize_use_trees_with_granularity, UseSegment, UseTree}; use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}; use crate::lists::{itemize_list, write_list, ListFormatting, ListItem}; @@ -119,7 +119,7 @@ fn rewrite_reorderable_or_regroupable_items( GroupImportsTactic::StdExternalCrate => group_imports(normalized_items), }; - if context.config.reorder_imports() { + if context.config.reorder_imports() == ReorderImports::Alphabetically { regrouped_items.iter_mut().for_each(|items| items.sort()) } @@ -228,9 +228,9 @@ impl ReorderableItemKind { fn is_reorderable(self, config: &Config) -> bool { match self { - ReorderableItemKind::ExternCrate => config.reorder_imports(), + ReorderableItemKind::ExternCrate => config.reorder_imports() != ReorderImports::Off, ReorderableItemKind::Mod => config.reorder_modules(), - ReorderableItemKind::Use => config.reorder_imports(), + ReorderableItemKind::Use => config.reorder_imports() != ReorderImports::Off, ReorderableItemKind::Other => false, } } diff --git a/tests/config/issue-1111.toml b/tests/config/issue-1111.toml index 44148a2d3c3..d2fa1201eef 100644 --- a/tests/config/issue-1111.toml +++ b/tests/config/issue-1111.toml @@ -1 +1 @@ -reorder_imports = true +reorder_imports = "Alphabetically" diff --git a/tests/config/small_tabs.toml b/tests/config/small_tabs.toml index 35c8fd86467..9570f1a734e 100644 --- a/tests/config/small_tabs.toml +++ b/tests/config/small_tabs.toml @@ -8,5 +8,5 @@ trailing_comma = "Vertical" indent_style = "Block" report_todo = "Always" report_fixme = "Never" -reorder_imports = false +reorder_imports = "Off" format_strings = true diff --git a/tests/source/configs/group_imports/One-no_reorder.rs b/tests/source/configs/group_imports/One-no_reorder.rs index f82f62c7f5b..934ce0ad195 100644 --- a/tests/source/configs/group_imports/One-no_reorder.rs +++ b/tests/source/configs/group_imports/One-no_reorder.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: One -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs b/tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs index 08c9a72ae61..0ca02d54775 100644 --- a/tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs +++ b/tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: StdExternalCrate -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use chrono::Utc; use super::update::convert_publish_payload; diff --git a/tests/source/configs/reorder_imports/false.rs b/tests/source/configs/reorder_imports/false.rs index 4b85684dc01..37f4287ba27 100644 --- a/tests/source/configs/reorder_imports/false.rs +++ b/tests/source/configs/reorder_imports/false.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off // Reorder imports use lorem; diff --git a/tests/source/configs/reorder_imports/true.rs b/tests/source/configs/reorder_imports/true.rs index 2a40f6d069f..135aee0d848 100644 --- a/tests/source/configs/reorder_imports/true.rs +++ b/tests/source/configs/reorder_imports/true.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically // Reorder imports use lorem; diff --git a/tests/source/issue-1120.rs b/tests/source/issue-1120.rs index e85c9af99d4..cc068050d72 100644 --- a/tests/source/issue-1120.rs +++ b/tests/source/issue-1120.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically // Ensure that a use at the start of an inline module is correctly formatted. mod foo {use bar;} diff --git a/tests/source/issue-1124.rs b/tests/source/issue-1124.rs index 35c2197fa34..81529df47f0 100644 --- a/tests/source/issue-1124.rs +++ b/tests/source/issue-1124.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically use d; use c; use b; use a; // The previous line has a space after the `use a;` diff --git a/tests/source/issue-3515.rs b/tests/source/issue-3515.rs index 9f760cb94e8..e34bcf47880 100644 --- a/tests/source/issue-3515.rs +++ b/tests/source/issue-3515.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use std :: fmt :: { self , Display } ; use std :: collections :: HashMap ; diff --git a/tests/source/issue-3585/reorder_imports_disabled.rs b/tests/source/issue-3585/reorder_imports_disabled.rs index 45b1bb9fdd9..06148871fdd 100644 --- a/tests/source/issue-3585/reorder_imports_disabled.rs +++ b/tests/source/issue-3585/reorder_imports_disabled.rs @@ -1,5 +1,5 @@ // rustfmt-inline_attribute_width: 100 -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off #[cfg(unix)] extern crate crateb; diff --git a/tests/source/issue-3585/reorder_imports_enabled.rs b/tests/source/issue-3585/reorder_imports_enabled.rs index 9f433e5ca21..460a7b95f97 100644 --- a/tests/source/issue-3585/reorder_imports_enabled.rs +++ b/tests/source/issue-3585/reorder_imports_enabled.rs @@ -1,5 +1,5 @@ // rustfmt-inline_attribute_width: 100 -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically #[cfg(unix)] extern crate crateb; diff --git a/tests/source/long-use-statement-issue-3154.rs b/tests/source/long-use-statement-issue-3154.rs index 339382b5bbf..11411793fa6 100644 --- a/tests/source/long-use-statement-issue-3154.rs +++ b/tests/source/long-use-statement-issue-3154.rs @@ -1,3 +1,3 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off pub use self :: super :: super :: super :: root::mozilla::detail::StringClassFlags as nsTStringRepr_ClassFlags ; diff --git a/tests/target/configs/group_imports/One-no_reorder.rs b/tests/target/configs/group_imports/One-no_reorder.rs index 015e841d014..bc8a1f25f6d 100644 --- a/tests/target/configs/group_imports/One-no_reorder.rs +++ b/tests/target/configs/group_imports/One-no_reorder.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: One -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use chrono::Utc; use super::update::convert_publish_payload; use juniper::{FieldError, FieldResult}; diff --git a/tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs b/tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs index 76d3d6ccb95..d26dd040b66 100644 --- a/tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs +++ b/tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs @@ -1,5 +1,5 @@ // rustfmt-group_imports: StdExternalCrate -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use alloc::alloc::Layout; use std::sync::Arc; diff --git a/tests/target/configs/reorder_imports/false.rs b/tests/target/configs/reorder_imports/false.rs index 4b85684dc01..37f4287ba27 100644 --- a/tests/target/configs/reorder_imports/false.rs +++ b/tests/target/configs/reorder_imports/false.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off // Reorder imports use lorem; diff --git a/tests/target/configs/reorder_imports/true.rs b/tests/target/configs/reorder_imports/true.rs index e4ff7295fd0..7e8a961d47c 100644 --- a/tests/target/configs/reorder_imports/true.rs +++ b/tests/target/configs/reorder_imports/true.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically // Reorder imports use dolor; diff --git a/tests/target/issue-1120.rs b/tests/target/issue-1120.rs index f44597e7d1e..43823fdc268 100644 --- a/tests/target/issue-1120.rs +++ b/tests/target/issue-1120.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically // Ensure that a use at the start of an inline module is correctly formatted. mod foo { diff --git a/tests/target/issue-1124.rs b/tests/target/issue-1124.rs index f0fc485a3c2..c8985519b93 100644 --- a/tests/target/issue-1124.rs +++ b/tests/target/issue-1124.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically use a; use b; diff --git a/tests/target/issue-3515.rs b/tests/target/issue-3515.rs index b59d03c6c82..ed4cda36285 100644 --- a/tests/target/issue-3515.rs +++ b/tests/target/issue-3515.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off use std::fmt::{self, Display}; use std::collections::HashMap; diff --git a/tests/target/issue-3585/reorder_imports_disabled.rs b/tests/target/issue-3585/reorder_imports_disabled.rs index f9637729b7e..5707f5a71cb 100644 --- a/tests/target/issue-3585/reorder_imports_disabled.rs +++ b/tests/target/issue-3585/reorder_imports_disabled.rs @@ -1,5 +1,5 @@ // rustfmt-inline_attribute_width: 100 -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off #[cfg(unix)] extern crate crateb; #[cfg(unix)] extern crate cratea; diff --git a/tests/target/issue-3585/reorder_imports_enabled.rs b/tests/target/issue-3585/reorder_imports_enabled.rs index d040d0ed341..ba511b4d6ba 100644 --- a/tests/target/issue-3585/reorder_imports_enabled.rs +++ b/tests/target/issue-3585/reorder_imports_enabled.rs @@ -1,5 +1,5 @@ // rustfmt-inline_attribute_width: 100 -// rustfmt-reorder_imports: true +// rustfmt-reorder_imports: Alphabetically #[cfg(unix)] extern crate cratea; #[cfg(unix)] extern crate crateb; diff --git a/tests/target/issue-4791/issue_4928.rs b/tests/target/issue-4791/issue_4928.rs index 588656b535f..d2b58c6822e 100644 --- a/tests/target/issue-4791/issue_4928.rs +++ b/tests/target/issue-4791/issue_4928.rs @@ -9,7 +9,7 @@ // rustfmt-newline_style: Unix // rustfmt-normalize_doc_attributes: true // rustfmt-overflow_delimited_expr: true -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off // rustfmt-reorder_modules: true // rustfmt-struct_field_align_threshold: 20 // rustfmt-tab_spaces: 4 diff --git a/tests/target/long-use-statement-issue-3154.rs b/tests/target/long-use-statement-issue-3154.rs index 877241e3b6f..3ababf161a0 100644 --- a/tests/target/long-use-statement-issue-3154.rs +++ b/tests/target/long-use-statement-issue-3154.rs @@ -1,3 +1,3 @@ -// rustfmt-reorder_imports: false +// rustfmt-reorder_imports: Off pub use self::super::super::super::root::mozilla::detail::StringClassFlags as nsTStringRepr_ClassFlags; From 8e5f251c24e215e54e6dbee7b936c5c5f6efa47d Mon Sep 17 00:00:00 2001 From: shiri Date: Tue, 17 May 2022 18:30:29 +0300 Subject: [PATCH 02/12] add config parameter to normalize function --- src/imports.rs | 100 ++++++++++++++++++++++++++++++++---------------- src/test/mod.rs | 1 + 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index efe4e9498c9..e561d5e2e12 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -11,7 +11,7 @@ use rustc_span::{ use crate::comment::combine_strs_with_missing_comments; use crate::config::lists::*; use crate::config::ImportGranularity; -use crate::config::{Edition, IndentStyle}; +use crate::config::{Config, Edition, IndentStyle}; use crate::lists::{ definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator, }; @@ -357,7 +357,7 @@ impl UseTree { Some(item.attrs.clone()) }, ) - .normalize(), + .normalize(context.config), ), _ => None, } @@ -470,7 +470,7 @@ impl UseTree { } // Do the adjustments that rustfmt does elsewhere to use paths. - pub(crate) fn normalize(mut self) -> UseTree { + pub(crate) fn normalize(mut self, config: &Config) -> UseTree { let mut last = self.path.pop().expect("Empty use tree?"); // Hack around borrow checker. let mut normalize_sole_list = false; @@ -535,7 +535,7 @@ impl UseTree { for seg in &list[0].path { self.path.push(seg.clone()); } - return self.normalize(); + return self.normalize(config); } _ => unreachable!(), } @@ -543,7 +543,10 @@ 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(UseTree::normalize).collect::>(); + let mut list = list + .into_iter() + .map(|member| member.normalize(config)) + .collect::>(); list.sort(); last = UseSegment::List(list); } @@ -1283,67 +1286,98 @@ 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 as foo").normalize(), + parse_use_tree("a::self").normalize(&Config::default()), + parse_use_tree("a") + ); + assert_eq!( + parse_use_tree("a::self as foo").normalize(&Config::default()), parse_use_tree("a as foo") ); assert_eq!( - parse_use_tree("a::{self}").normalize(), + parse_use_tree("a::{self}").normalize(&Config::default()), parse_use_tree("a::{self}") ); - assert_eq!(parse_use_tree("a::{b}").normalize(), parse_use_tree("a::b")); assert_eq!( - parse_use_tree("a::{b, c::self}").normalize(), + parse_use_tree("a::{b}").normalize(&Config::default()), + parse_use_tree("a::b") + ); + assert_eq!( + parse_use_tree("a::{b, c::self}").normalize(&Config::default()), parse_use_tree("a::{b, c}") ); assert_eq!( - parse_use_tree("a::{b as bar, c::self}").normalize(), + parse_use_tree("a::{b as bar, c::self}").normalize(&Config::default()), parse_use_tree("a::{b as bar, c}") ); } #[test] fn test_use_tree_ord() { - 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("a").normalize(&Config::default()) + < parse_use_tree("aa").normalize(&Config::default()) + ); + assert!( + parse_use_tree("a").normalize(&Config::default()) + < parse_use_tree("a::a").normalize(&Config::default()) + ); + assert!( + parse_use_tree("a").normalize(&Config::default()) + < parse_use_tree("*").normalize(&Config::default()) + ); + assert!( + parse_use_tree("a").normalize(&Config::default()) + < parse_use_tree("{a, b}").normalize(&Config::default()) + ); + assert!( + parse_use_tree("*").normalize(&Config::default()) + < parse_use_tree("{a, b}").normalize(&Config::default()) + ); assert!( - parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize() - < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize() + parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize(&Config::default()) + < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}") + .normalize(&Config::default()) + ); + assert!( + parse_use_tree("serde::de::{Deserialize}").normalize(&Config::default()) + < parse_use_tree("serde_json").normalize(&Config::default()) ); assert!( - parse_use_tree("serde::de::{Deserialize}").normalize() - < parse_use_tree("serde_json").normalize() + parse_use_tree("a::b::c").normalize(&Config::default()) + < parse_use_tree("a::b::*").normalize(&Config::default()) ); - assert!(parse_use_tree("a::b::c").normalize() < parse_use_tree("a::b::*").normalize()); assert!( - parse_use_tree("foo::{Bar, Baz}").normalize() - < parse_use_tree("{Bar, Baz}").normalize() + parse_use_tree("foo::{Bar, Baz}").normalize(&Config::default()) + < parse_use_tree("{Bar, Baz}").normalize(&Config::default()) ); assert!( - parse_use_tree("foo::{qux as bar}").normalize() - < parse_use_tree("foo::{self as bar}").normalize() + parse_use_tree("foo::{qux as bar}").normalize(&Config::default()) + < parse_use_tree("foo::{self as bar}").normalize(&Config::default()) ); assert!( - parse_use_tree("foo::{qux as bar}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() + parse_use_tree("foo::{qux as bar}").normalize(&Config::default()) + < parse_use_tree("foo::{baz, qux as bar}").normalize(&Config::default()) ); assert!( - parse_use_tree("foo::{self as bar, baz}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() + parse_use_tree("foo::{self as bar, baz}").normalize(&Config::default()) + < parse_use_tree("foo::{baz, qux as bar}").normalize(&Config::default()) ); - 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("foo").normalize(&Config::default()) + < parse_use_tree("Foo").normalize(&Config::default()) + ); + assert!( + parse_use_tree("foo").normalize(&Config::default()) + < parse_use_tree("foo::Bar").normalize(&Config::default()) + ); assert!( - parse_use_tree("std::cmp::{d, c, b, a}").normalize() - < parse_use_tree("std::cmp::{b, e, g, f}").normalize() + parse_use_tree("std::cmp::{d, c, b, a}").normalize(&Config::default()) + < parse_use_tree("std::cmp::{b, e, g, f}").normalize(&Config::default()) ); } diff --git a/src/test/mod.rs b/src/test/mod.rs index 4bad8e71481..6988080de01 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -818,6 +818,7 @@ fn handle_result( // Ignore LF and CRLF difference for Windows. if !string_eq_ignore_newline_repr(&fmt_text, &text) { let diff = make_diff(&text, &fmt_text, DIFF_CONTEXT_SIZE); + println!("diff: {:#?}", diff); assert!( !diff.is_empty(), "Empty diff? Maybe due to a missing a newline at the end of a file?" From 6349c54da83e35f8e676dd14ddc554e3edcfadaf Mon Sep 17 00:00:00 2001 From: shiri Date: Wed, 18 May 2022 15:59:55 +0300 Subject: [PATCH 03/12] reorder imports according to each namespace's length --- src/imports.rs | 11 ++++++++--- src/reorder.rs | 8 +++++++- src/utils.rs | 10 ++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index e561d5e2e12..afae92b033f 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -10,8 +10,8 @@ use rustc_span::{ use crate::comment::combine_strs_with_missing_comments; use crate::config::lists::*; -use crate::config::ImportGranularity; use crate::config::{Config, Edition, IndentStyle}; +use crate::config::{ImportGranularity, ReorderImports}; use crate::lists::{ definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator, }; @@ -19,7 +19,7 @@ use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::Shape; use crate::source_map::SpanUtils; use crate::spanned::Spanned; -use crate::utils::{is_same_visibility, mk_sp, rewrite_ident}; +use crate::utils::{compare_sliding_order, is_same_visibility, mk_sp, rewrite_ident}; use crate::visitor::FmtVisitor; /// Returns a name imported by a `use` declaration. @@ -547,7 +547,12 @@ impl UseTree { .into_iter() .map(|member| member.normalize(config)) .collect::>(); - list.sort(); + + if config.reorder_imports() == ReorderImports::Alphabetically { + list.sort(); + } else if config.reorder_imports() == ReorderImports::Length { + list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) + } last = UseSegment::List(list); } diff --git a/src/reorder.rs b/src/reorder.rs index 5fd518efdcb..14a24770e62 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -19,7 +19,7 @@ use crate::rewrite::RewriteContext; use crate::shape::Shape; use crate::source_map::LineRangeUtils; use crate::spanned::Spanned; -use crate::utils::{contains_skip, mk_sp}; +use crate::utils::{compare_sliding_order, contains_skip, mk_sp}; use crate::visitor::FmtVisitor; /// Choose the ordering between the given two items. @@ -121,6 +121,12 @@ fn rewrite_reorderable_or_regroupable_items( if context.config.reorder_imports() == ReorderImports::Alphabetically { regrouped_items.iter_mut().for_each(|items| items.sort()) + } else if context.config.reorder_imports() == ReorderImports::Length { + regrouped_items.iter_mut().for_each({ + |items| { + items.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) + } + }) } // 4 = "use ", 1 = ";" diff --git a/src/utils.rs b/src/utils.rs index 35512e78fa6..35f6181d6cf 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::cmp::Ordering; use rustc_ast::ast::{ self, Attribute, CrateSugar, MetaItem, MetaItemKind, NestedMetaItem, NodeId, Path, Visibility, @@ -694,6 +695,15 @@ pub(crate) fn unicode_str_width(s: &str) -> usize { s.width() } +pub(crate) fn compare_sliding_order(s1: &str, s2: &str) -> Ordering { + let mut is_equal = s1.len().cmp(&s2.len()); + if Ordering::is_eq(is_equal) { + is_equal = s1.cmp(&s2); + } + + is_equal +} + #[cfg(test)] mod test { use super::*; From 36e24803573bbe6780fd78c1bd6f83bf2a7e7b71 Mon Sep 17 00:00:00 2001 From: shiri Date: Mon, 23 May 2022 14:51:20 +0300 Subject: [PATCH 04/12] add struct for list-sorting flag --- src/imports.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++++- src/reorder.rs | 12 +++++---- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index afae92b033f..06edfe081bc 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -109,6 +109,12 @@ pub(crate) struct UseTree { attrs: Option>, } +#[derive(Clone)] +pub(crate) struct ImportsVector { + pub(crate) list: Vec, + reorder_config: ReorderImports, +} + impl PartialEq for UseTree { fn eq(&self, other: &UseTree) -> bool { self.path == other.path @@ -198,6 +204,7 @@ pub(crate) fn normalize_use_trees_with_granularity( 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() { + println!("use_tree normalize: {:?}", use_tree); result.push(use_tree); continue; } @@ -205,7 +212,7 @@ pub(crate) fn normalize_use_trees_with_granularity( for mut flattened in use_tree.flatten(import_granularity) { if let Some(tree) = result .iter_mut() - .find(|tree| tree.share_prefix(&flattened, merge_by)) + .find(|tree| {println!("tree normalize: {:?}", tree); tree.share_prefix(&flattened, merge_by)}) { tree.merge(&flattened, merge_by); } else { @@ -213,10 +220,12 @@ pub(crate) fn normalize_use_trees_with_granularity( if merge_by == SharedPrefix::Module { flattened = flattened.nest_trailing_self(); } + println!("flattened normalize: {:?}", flattened); result.push(flattened); } } } + println!("result: {:?}", result); result } @@ -553,6 +562,7 @@ impl UseTree { } else if config.reorder_imports() == ReorderImports::Length { list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) } + println!("list: {:?}", list); last = UseSegment::List(list); } @@ -670,6 +680,67 @@ impl UseTree { } } +impl ImportsVector { + pub(crate) fn new(list: Vec, reorder_config: ReorderImports) -> Self { + ImportsVector{list, reorder_config} + } + + pub(crate) fn sort(&mut self) { + if self.reorder_config == ReorderImports::Alphabetically { + self.list.sort(); + } else if self.reorder_config == ReorderImports::Length { + self.list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())); + } + } + + pub(crate) fn iter(self: &Self) -> impl Iterator { + self.list.iter() + } + + pub(crate) fn into_iter(self: Self) -> impl Iterator { + self.list.into_iter() + } + + pub(crate) fn iter_mut(self: &mut Self) -> impl Iterator { + let mut_iterator = ImportsVectorIterator::new(self.list); + mut_iterator.iter_mut() + + } +} + +/*impl IntoIterator for ImportsVector { + type Item = UseTree; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.list.into_iter() + } +}*/ + +struct ImportsVectorIterator { + list: Vec, + index: usize, +} + +impl ImportsVectorIterator { + fn new(list: Vec) -> Self { + ImportsVectorIterator { + list: list, + index: 0, + } + } +} + +impl Iterator for ImportsVectorIterator { + type Item = UseTree; + + fn next(&mut self) -> Option { + let result = self.list[self.index]; + self.index += 1; + result + } +} + fn merge_rest( a: &[UseSegment], b: &[UseSegment], diff --git a/src/reorder.rs b/src/reorder.rs index 14a24770e62..bc6f83dcb7b 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -12,7 +12,7 @@ use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; use crate::config::{Config, GroupImportsTactic, ReorderImports}; -use crate::imports::{normalize_use_trees_with_granularity, UseSegment, UseTree}; +use crate::imports::{normalize_use_trees_with_granularity, UseSegment, UseTree, ImportsVector}; use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}; use crate::lists::{itemize_list, write_list, ListFormatting, ListItem}; use crate::rewrite::RewriteContext; @@ -90,6 +90,7 @@ fn rewrite_reorderable_or_regroupable_items( .iter() .filter_map(|item| UseTree::from_ast_with_normalization(context, item)) .collect(); + let mut normalized_items = ImportsVector::new(normalized_items, context.config.reorder_imports()); let cloned = normalized_items.clone(); // Add comments before merging. let list_items = itemize_list( @@ -124,7 +125,7 @@ fn rewrite_reorderable_or_regroupable_items( } else if context.config.reorder_imports() == ReorderImports::Length { regrouped_items.iter_mut().for_each({ |items| { - items.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) + items.sort_by(|a, b| {println!("list a: {:?}", a); println!("list b: {:?}", b); compare_sliding_order(&a.to_string(), &b.to_string())}) } }) } @@ -135,17 +136,18 @@ fn rewrite_reorderable_or_regroupable_items( .into_iter() .filter(|use_group| !use_group.is_empty()) .map(|use_group| { + println!("use_group: {:?}", use_group); let item_vec: Vec<_> = use_group .into_iter() - .map(|use_tree| ListItem { + .map(|use_tree| {println!("use_tree: {:?}", 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::>>()?; - + println!("item_vec: {:?}", item_vec); let join_string = format!("\n\n{}", shape.indent.to_string(context.config)); Some(item_vec.join(&join_string)) } From f214390b5fd3ffe62420e7d106ee334d7cda3f9c Mon Sep 17 00:00:00 2001 From: shiri Date: Tue, 24 May 2022 17:47:39 +0300 Subject: [PATCH 05/12] replace Vec uses with ImportsVector --- src/imports.rs | 70 +++++++++++++++++++++++++------------------------- src/reorder.rs | 21 +++++++-------- 2 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index 06edfe081bc..ab71e0dcc60 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -190,11 +190,11 @@ impl UseSegment { } pub(crate) fn normalize_use_trees_with_granularity( - use_trees: Vec, + use_trees: ImportsVector, import_granularity: ImportGranularity, -) -> Vec { +) -> ImportsVector { let merge_by = match import_granularity { - ImportGranularity::Item => return flatten_use_trees(use_trees, ImportGranularity::Item), + ImportGranularity::Item => return ImportsVector::from(flatten_use_trees(use_trees, ImportGranularity::Item), use_trees.reorder_config()), ImportGranularity::Preserve => return use_trees, ImportGranularity::Crate => SharedPrefix::Crate, ImportGranularity::Module => SharedPrefix::Module, @@ -225,12 +225,13 @@ pub(crate) fn normalize_use_trees_with_granularity( } } } - println!("result: {:?}", result); + + let result = ImportsVector::from(use_trees.list, use_trees.reorder_config()); result } fn flatten_use_trees( - use_trees: Vec, + use_trees: ImportsVector, import_granularity: ImportGranularity, ) -> Vec { use_trees @@ -681,10 +682,22 @@ impl UseTree { } impl ImportsVector { - pub(crate) fn new(list: Vec, reorder_config: ReorderImports) -> Self { + pub(crate) fn new(reorder_config: ReorderImports) -> Self { + let list: Vec:: = Vec::::new(); + ImportsVector { + list, + reorder_config, + } + } + + pub(crate) fn from(list: Vec, reorder_config: ReorderImports) -> Self { ImportsVector{list, reorder_config} } + pub(crate) fn reorder_config(&self) -> ReorderImports { + self.reorder_config + } + pub(crate) fn sort(&mut self) { if self.reorder_config == ReorderImports::Alphabetically { self.list.sort(); @@ -693,51 +706,38 @@ impl ImportsVector { } } - pub(crate) fn iter(self: &Self) -> impl Iterator { - self.list.iter() + pub(crate) fn is_empty(&mut self) -> bool { + self.is_empty() } - pub(crate) fn into_iter(self: Self) -> impl Iterator { - self.list.into_iter() + pub(crate) fn push(self: &mut Self, item: UseTree) { + self.list.push(item); } - pub(crate) fn iter_mut(self: &mut Self) -> impl Iterator { - let mut_iterator = ImportsVectorIterator::new(self.list); - mut_iterator.iter_mut() - + pub(crate) fn len(self: &Self) -> usize { + self.list.len() } -} -/*impl IntoIterator for ImportsVector { - type Item = UseTree; - type IntoIter = std::vec::IntoIter; + pub(crate) fn iter(self: &Self) -> impl Iterator { + self.list.iter() + } - fn into_iter(self) -> Self::IntoIter { + pub(crate) fn into_iter(self: Self) -> impl Iterator { self.list.into_iter() } -}*/ -struct ImportsVectorIterator { - list: Vec, - index: usize, -} + pub(crate) fn iter_mut(self: &mut Self) -> std::slice::IterMut<'_, UseTree> { + self.list.iter_mut() -impl ImportsVectorIterator { - fn new(list: Vec) -> Self { - ImportsVectorIterator { - list: list, - index: 0, - } } } -impl Iterator for ImportsVectorIterator { +impl IntoIterator for ImportsVector { type Item = UseTree; + type IntoIter = Box>; - fn next(&mut self) -> Option { - let result = self.list[self.index]; - self.index += 1; - result + fn into_iter(self) -> Self::IntoIter { + Box::new(self.list.into_iter()) } } diff --git a/src/reorder.rs b/src/reorder.rs index bc6f83dcb7b..2e115fc2752 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -90,7 +90,7 @@ fn rewrite_reorderable_or_regroupable_items( .iter() .filter_map(|item| UseTree::from_ast_with_normalization(context, item)) .collect(); - let mut normalized_items = ImportsVector::new(normalized_items, context.config.reorder_imports()); + let mut normalized_items = ImportsVector::from(normalized_items, context.config.reorder_imports()); let cloned = normalized_items.clone(); // Add comments before merging. let list_items = itemize_list( @@ -119,16 +119,18 @@ fn rewrite_reorderable_or_regroupable_items( } GroupImportsTactic::StdExternalCrate => group_imports(normalized_items), }; - - if context.config.reorder_imports() == ReorderImports::Alphabetically { + for regrouped_item in regrouped_items { + regrouped_item.sort(); + } + /*if context.config.reorder_imports() == ReorderImports::Alphabetically { regrouped_items.iter_mut().for_each(|items| items.sort()) } else if context.config.reorder_imports() == ReorderImports::Length { regrouped_items.iter_mut().for_each({ |items| { - items.sort_by(|a, b| {println!("list a: {:?}", a); println!("list b: {:?}", b); compare_sliding_order(&a.to_string(), &b.to_string())}) + items.sort(|a, b| {println!("list a: {:?}", a); println!("list b: {:?}", b); compare_sliding_order(&a.to_string(), &b.to_string())}) } }) - } + }*/ // 4 = "use ", 1 = ";" let nested_shape = shape.offset_left(4)?.sub_width(1)?; @@ -136,7 +138,6 @@ fn rewrite_reorderable_or_regroupable_items( .into_iter() .filter(|use_group| !use_group.is_empty()) .map(|use_group| { - println!("use_group: {:?}", use_group); let item_vec: Vec<_> = use_group .into_iter() .map(|use_tree| {println!("use_tree: {:?}", use_tree); ListItem { @@ -180,10 +181,10 @@ fn contains_macro_use_attr(item: &ast::Item) -> bool { /// Divides imports into three groups, corresponding to standard, external /// and local imports. Sorts each subgroup. -fn group_imports(uts: Vec) -> Vec> { - let mut std_imports = Vec::new(); - let mut external_imports = Vec::new(); - let mut local_imports = Vec::new(); +fn group_imports(uts: ImportsVector) -> Vec { + let mut std_imports = ImportsVector::new(uts.reorder_config()); + let mut external_imports = ImportsVector::new(uts.reorder_config()); + let mut local_imports = ImportsVector::new(uts.reorder_config()); for ut in uts.into_iter() { if ut.path.is_empty() { From 214b0286e12d21ad96843b6209c0741738a4ef64 Mon Sep 17 00:00:00 2001 From: shiri Date: Fri, 27 May 2022 15:32:29 +0300 Subject: [PATCH 06/12] Revert "add config parameter to normalize function" This reverts commit 8e5f251c24e215e54e6dbee7b936c5c5f6efa47d. --- src/imports.rs | 94 ++++++++++++++++--------------------------------- src/test/mod.rs | 1 - 2 files changed, 31 insertions(+), 64 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index ab71e0dcc60..d5adb8c7342 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -367,7 +367,7 @@ impl UseTree { Some(item.attrs.clone()) }, ) - .normalize(context.config), + .normalize(), ), _ => None, } @@ -480,7 +480,7 @@ impl UseTree { } // Do the adjustments that rustfmt does elsewhere to use paths. - pub(crate) fn normalize(mut self, config: &Config) -> UseTree { + pub(crate) 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; @@ -545,7 +545,7 @@ impl UseTree { for seg in &list[0].path { self.path.push(seg.clone()); } - return self.normalize(config); + return self.normalize(); } _ => unreachable!(), } @@ -563,7 +563,6 @@ impl UseTree { } else if config.reorder_imports() == ReorderImports::Length { list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) } - println!("list: {:?}", list); last = UseSegment::List(list); } @@ -1362,98 +1361,67 @@ 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(&Config::default()), - parse_use_tree("a") - ); - assert_eq!( - parse_use_tree("a::self as foo").normalize(&Config::default()), + parse_use_tree("a::self as foo").normalize(), parse_use_tree("a as foo") ); assert_eq!( - parse_use_tree("a::{self}").normalize(&Config::default()), + parse_use_tree("a::{self}").normalize(), parse_use_tree("a::{self}") ); + assert_eq!(parse_use_tree("a::{b}").normalize(), parse_use_tree("a::b")); assert_eq!( - parse_use_tree("a::{b}").normalize(&Config::default()), - parse_use_tree("a::b") - ); - assert_eq!( - parse_use_tree("a::{b, c::self}").normalize(&Config::default()), + 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(&Config::default()), + 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(&Config::default()) - < parse_use_tree("aa").normalize(&Config::default()) - ); - assert!( - parse_use_tree("a").normalize(&Config::default()) - < parse_use_tree("a::a").normalize(&Config::default()) - ); - assert!( - parse_use_tree("a").normalize(&Config::default()) - < parse_use_tree("*").normalize(&Config::default()) - ); - assert!( - parse_use_tree("a").normalize(&Config::default()) - < parse_use_tree("{a, b}").normalize(&Config::default()) - ); - assert!( - parse_use_tree("*").normalize(&Config::default()) - < parse_use_tree("{a, b}").normalize(&Config::default()) - ); + 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(&Config::default()) - < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}") - .normalize(&Config::default()) - ); - assert!( - parse_use_tree("serde::de::{Deserialize}").normalize(&Config::default()) - < parse_use_tree("serde_json").normalize(&Config::default()) + parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize() + < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize() ); assert!( - parse_use_tree("a::b::c").normalize(&Config::default()) - < parse_use_tree("a::b::*").normalize(&Config::default()) + 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(&Config::default()) - < parse_use_tree("{Bar, Baz}").normalize(&Config::default()) + parse_use_tree("foo::{Bar, Baz}").normalize() + < parse_use_tree("{Bar, Baz}").normalize() ); assert!( - parse_use_tree("foo::{qux as bar}").normalize(&Config::default()) - < parse_use_tree("foo::{self as bar}").normalize(&Config::default()) + parse_use_tree("foo::{qux as bar}").normalize() + < parse_use_tree("foo::{self as bar}").normalize() ); assert!( - parse_use_tree("foo::{qux as bar}").normalize(&Config::default()) - < parse_use_tree("foo::{baz, qux as bar}").normalize(&Config::default()) + 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(&Config::default()) - < parse_use_tree("foo::{baz, qux as bar}").normalize(&Config::default()) + parse_use_tree("foo::{self as bar, baz}").normalize() + < parse_use_tree("foo::{baz, qux as bar}").normalize() ); - assert!( - parse_use_tree("foo").normalize(&Config::default()) - < parse_use_tree("Foo").normalize(&Config::default()) - ); - assert!( - parse_use_tree("foo").normalize(&Config::default()) - < parse_use_tree("foo::Bar").normalize(&Config::default()) - ); + 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(&Config::default()) - < parse_use_tree("std::cmp::{b, e, g, f}").normalize(&Config::default()) + parse_use_tree("std::cmp::{d, c, b, a}").normalize() + < parse_use_tree("std::cmp::{b, e, g, f}").normalize() ); } diff --git a/src/test/mod.rs b/src/test/mod.rs index 6988080de01..4bad8e71481 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -818,7 +818,6 @@ fn handle_result( // Ignore LF and CRLF difference for Windows. if !string_eq_ignore_newline_repr(&fmt_text, &text) { let diff = make_diff(&text, &fmt_text, DIFF_CONTEXT_SIZE); - println!("diff: {:#?}", diff); assert!( !diff.is_empty(), "Empty diff? Maybe due to a missing a newline at the end of a file?" From ef2835edd82dc1fdee3b474ae3b937b3e67f7e05 Mon Sep 17 00:00:00 2001 From: shiri Date: Tue, 31 May 2022 11:17:54 +0300 Subject: [PATCH 07/12] pass reorder_imports as a function parameter --- src/config/config_type.rs | 5 ++ src/imports.rs | 105 +++++++++----------------------------- src/reorder.rs | 20 ++++---- 3 files changed, 37 insertions(+), 93 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 7fc4486ddcd..90b8f345e78 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -296,6 +296,11 @@ macro_rules! create_config { } )+ } + + /*#[allow(unreachable_pub)] + pub fn reorder_imports(&self) -> ReorderImports { + self.reorder_imports + }*/ fn set_width_heuristics(&mut self, heuristics: WidthHeuristics) { let max_width = self.max_width.2; diff --git a/src/imports.rs b/src/imports.rs index d5adb8c7342..5a29c47fc48 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -190,11 +190,12 @@ impl UseSegment { } pub(crate) fn normalize_use_trees_with_granularity( - use_trees: ImportsVector, + use_trees: Vec, import_granularity: ImportGranularity, -) -> ImportsVector { + reorder_imports: ReorderImports, +) -> Vec { let merge_by = match import_granularity { - ImportGranularity::Item => return ImportsVector::from(flatten_use_trees(use_trees, ImportGranularity::Item), use_trees.reorder_config()), + ImportGranularity::Item => return flatten_use_trees(use_trees, ImportGranularity::Item), ImportGranularity::Preserve => return use_trees, ImportGranularity::Crate => SharedPrefix::Crate, ImportGranularity::Module => SharedPrefix::Module, @@ -204,7 +205,6 @@ pub(crate) fn normalize_use_trees_with_granularity( 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() { - println!("use_tree normalize: {:?}", use_tree); result.push(use_tree); continue; } @@ -212,26 +212,23 @@ pub(crate) fn normalize_use_trees_with_granularity( for mut flattened in use_tree.flatten(import_granularity) { if let Some(tree) = result .iter_mut() - .find(|tree| {println!("tree normalize: {:?}", tree); tree.share_prefix(&flattened, merge_by)}) + .find(|tree| tree.share_prefix(&flattened, merge_by)) { - tree.merge(&flattened, merge_by); + tree.merge(&flattened, merge_by, reorder_imports); } else { // If this is the first tree with this prefix, handle potential trailing ::self if merge_by == SharedPrefix::Module { flattened = flattened.nest_trailing_self(); } - println!("flattened normalize: {:?}", flattened); result.push(flattened); } } } - - let result = ImportsVector::from(use_trees.list, use_trees.reorder_config()); result } fn flatten_use_trees( - use_trees: ImportsVector, + use_trees: Vec, import_granularity: ImportGranularity, ) -> Vec { use_trees @@ -367,7 +364,7 @@ impl UseTree { Some(item.attrs.clone()) }, ) - .normalize(), + .normalize(context.config.reorder_imports()), ), _ => None, } @@ -480,7 +477,7 @@ impl UseTree { } // Do the adjustments that rustfmt does elsewhere to use paths. - pub(crate) fn normalize(mut self) -> UseTree { + pub(crate) fn normalize(mut self, reorder_imports: ReorderImports) -> UseTree { let mut last = self.path.pop().expect("Empty use tree?"); // Hack around borrow checker. let mut normalize_sole_list = false; @@ -545,7 +542,7 @@ impl UseTree { for seg in &list[0].path { self.path.push(seg.clone()); } - return self.normalize(); + return self.normalize(reorder_imports); } _ => unreachable!(), } @@ -555,12 +552,12 @@ impl UseTree { if let UseSegment::List(list) = last { let mut list = list .into_iter() - .map(|member| member.normalize(config)) + .map(|member| member.normalize(reorder_imports)) .collect::>(); - if config.reorder_imports() == ReorderImports::Alphabetically { + if reorder_imports == ReorderImports::Alphabetically { list.sort(); - } else if config.reorder_imports() == ReorderImports::Length { + } else if reorder_imports == ReorderImports::Length { list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) } last = UseSegment::List(list); @@ -651,7 +648,7 @@ impl UseTree { } } - fn merge(&mut self, other: &UseTree, merge_by: SharedPrefix) { + fn merge(&mut self, other: &UseTree, merge_by: SharedPrefix, reorder_imports: ReorderImports) { let mut prefix = 0; for (a, b) in self.path.iter().zip(other.path.iter()) { // only discard the alias at the root of the tree @@ -661,7 +658,7 @@ impl UseTree { break; } } - if let Some(new_path) = merge_rest(&self.path, &other.path, prefix, merge_by) { + if let Some(new_path) = merge_rest(&self.path, &other.path, prefix, merge_by, reorder_imports) { self.path = new_path; self.span = self.span.to(other.span); } @@ -680,71 +677,12 @@ impl UseTree { } } -impl ImportsVector { - pub(crate) fn new(reorder_config: ReorderImports) -> Self { - let list: Vec:: = Vec::::new(); - ImportsVector { - list, - reorder_config, - } - } - - pub(crate) fn from(list: Vec, reorder_config: ReorderImports) -> Self { - ImportsVector{list, reorder_config} - } - - pub(crate) fn reorder_config(&self) -> ReorderImports { - self.reorder_config - } - - pub(crate) fn sort(&mut self) { - if self.reorder_config == ReorderImports::Alphabetically { - self.list.sort(); - } else if self.reorder_config == ReorderImports::Length { - self.list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())); - } - } - - pub(crate) fn is_empty(&mut self) -> bool { - self.is_empty() - } - - pub(crate) fn push(self: &mut Self, item: UseTree) { - self.list.push(item); - } - - pub(crate) fn len(self: &Self) -> usize { - self.list.len() - } - - pub(crate) fn iter(self: &Self) -> impl Iterator { - self.list.iter() - } - - pub(crate) fn into_iter(self: Self) -> impl Iterator { - self.list.into_iter() - } - - pub(crate) fn iter_mut(self: &mut Self) -> std::slice::IterMut<'_, UseTree> { - self.list.iter_mut() - - } -} - -impl IntoIterator for ImportsVector { - type Item = UseTree; - type IntoIter = Box>; - - fn into_iter(self) -> Self::IntoIter { - Box::new(self.list.into_iter()) - } -} - fn merge_rest( a: &[UseSegment], b: &[UseSegment], mut len: usize, merge_by: SharedPrefix, + reorder_imports: ReorderImports, ) -> Option> { if a.len() == len && b.len() == len { return None; @@ -756,6 +694,7 @@ fn merge_rest( &mut list, UseTree::from_path(b[len..].to_vec(), DUMMY_SP), merge_by, + reorder_imports, ); let mut new_path = b[..len].to_vec(); new_path.push(UseSegment::List(list)); @@ -789,7 +728,7 @@ fn merge_rest( Some(new_path) } -fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: SharedPrefix) { +fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: SharedPrefix, reorder_imports: ReorderImports) { struct SimilarTree<'a> { similarity: usize, path_len: usize, @@ -831,18 +770,20 @@ fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: } else if merge_by == SharedPrefix::One { if let Some(sim_tree) = similar_trees.max_by_key(|tree| tree.similarity) { if sim_tree.similarity > 0 { - sim_tree.tree.merge(&use_tree, merge_by); + sim_tree.tree.merge(&use_tree, merge_by, reorder_imports); return; } } } else if let Some(sim_tree) = similar_trees.max_by_key(|tree| tree.path_len) { if sim_tree.path_len > 1 { - sim_tree.tree.merge(&use_tree, merge_by); + sim_tree.tree.merge(&use_tree, merge_by, reorder_imports); return; } } trees.push(use_tree); - trees.sort(); + if reorder_imports == ReorderImports::Alphabetically { + trees.sort(); + } } impl PartialOrd for UseSegment { diff --git a/src/reorder.rs b/src/reorder.rs index 2e115fc2752..a899f3da449 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -90,7 +90,6 @@ fn rewrite_reorderable_or_regroupable_items( .iter() .filter_map(|item| UseTree::from_ast_with_normalization(context, item)) .collect(); - let mut normalized_items = ImportsVector::from(normalized_items, context.config.reorder_imports()); let cloned = normalized_items.clone(); // Add comments before merging. let list_items = itemize_list( @@ -111,6 +110,7 @@ fn rewrite_reorderable_or_regroupable_items( normalized_items = normalize_use_trees_with_granularity( normalized_items, context.config.imports_granularity(), + context.config.reorder_imports(), ); let mut regrouped_items = match context.config.group_imports() { @@ -119,18 +119,16 @@ fn rewrite_reorderable_or_regroupable_items( } GroupImportsTactic::StdExternalCrate => group_imports(normalized_items), }; - for regrouped_item in regrouped_items { - regrouped_item.sort(); - } - /*if context.config.reorder_imports() == ReorderImports::Alphabetically { + + if context.config.reorder_imports() == ReorderImports::Alphabetically { regrouped_items.iter_mut().for_each(|items| items.sort()) } else if context.config.reorder_imports() == ReorderImports::Length { regrouped_items.iter_mut().for_each({ |items| { - items.sort(|a, b| {println!("list a: {:?}", a); println!("list b: {:?}", b); compare_sliding_order(&a.to_string(), &b.to_string())}) + items.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())) } }) - }*/ + } // 4 = "use ", 1 = ";" let nested_shape = shape.offset_left(4)?.sub_width(1)?; @@ -181,10 +179,10 @@ fn contains_macro_use_attr(item: &ast::Item) -> bool { /// Divides imports into three groups, corresponding to standard, external /// and local imports. Sorts each subgroup. -fn group_imports(uts: ImportsVector) -> Vec { - let mut std_imports = ImportsVector::new(uts.reorder_config()); - let mut external_imports = ImportsVector::new(uts.reorder_config()); - let mut local_imports = ImportsVector::new(uts.reorder_config()); +fn group_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() { From 3ade0075afc99d9b9742a293ddffbf2ab11447f9 Mon Sep 17 00:00:00 2001 From: shiri Date: Tue, 31 May 2022 17:59:57 +0300 Subject: [PATCH 08/12] pass existing tests --- src/config/config_type.rs | 1 - src/imports.rs | 119 ++++++++++++++++++++++++++------------ src/reorder.rs | 6 +- 3 files changed, 84 insertions(+), 42 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 90b8f345e78..56e20d99299 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -296,7 +296,6 @@ macro_rules! create_config { } )+ } - /*#[allow(unreachable_pub)] pub fn reorder_imports(&self) -> ReorderImports { self.reorder_imports diff --git a/src/imports.rs b/src/imports.rs index 5a29c47fc48..8c2486ef86f 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -10,7 +10,7 @@ use rustc_span::{ use crate::comment::combine_strs_with_missing_comments; use crate::config::lists::*; -use crate::config::{Config, Edition, IndentStyle}; +use crate::config::{Edition, IndentStyle}; use crate::config::{ImportGranularity, ReorderImports}; use crate::lists::{ definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator, @@ -109,12 +109,6 @@ pub(crate) struct UseTree { attrs: Option>, } -#[derive(Clone)] -pub(crate) struct ImportsVector { - pub(crate) list: Vec, - reorder_config: ReorderImports, -} - impl PartialEq for UseTree { fn eq(&self, other: &UseTree) -> bool { self.path == other.path @@ -658,7 +652,9 @@ impl UseTree { break; } } - if let Some(new_path) = merge_rest(&self.path, &other.path, prefix, merge_by, reorder_imports) { + if let Some(new_path) = + merge_rest(&self.path, &other.path, prefix, merge_by, reorder_imports) + { self.path = new_path; self.span = self.span.to(other.span); } @@ -722,13 +718,22 @@ fn merge_rest( UseTree::from_path(a[len..].to_vec(), DUMMY_SP), UseTree::from_path(b[len..].to_vec(), DUMMY_SP), ]; - list.sort(); + if reorder_imports == ReorderImports::Alphabetically { + list.sort(); + } else if reorder_imports == ReorderImports::Length { + list.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())); + } let mut new_path = b[..len].to_vec(); new_path.push(UseSegment::List(list)); Some(new_path) } -fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: SharedPrefix, reorder_imports: ReorderImports) { +fn merge_use_trees_inner( + trees: &mut Vec, + use_tree: UseTree, + merge_by: SharedPrefix, + reorder_imports: ReorderImports, +) { struct SimilarTree<'a> { similarity: usize, path_len: usize, @@ -783,6 +788,8 @@ fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: trees.push(use_tree); if reorder_imports == ReorderImports::Alphabetically { trees.sort(); + } else if reorder_imports == ReorderImports::Length { + trees.sort_by(|a, b| compare_sliding_order(&a.to_string(), &b.to_string())); } } @@ -1147,6 +1154,7 @@ mod test { normalize_use_trees_with_granularity( parse_use_trees!($($input,)*), ImportGranularity::$by, + ReorderImports::Alphabetically, ), parse_use_trees!($($output,)*), ); @@ -1302,67 +1310,102 @@ 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 as foo").normalize(), + parse_use_tree("a::self").normalize(ReorderImports::Alphabetically), + parse_use_tree("a") + ); + assert_eq!( + parse_use_tree("a::self as foo").normalize(ReorderImports::Alphabetically), parse_use_tree("a as foo") ); assert_eq!( - parse_use_tree("a::{self}").normalize(), + parse_use_tree("a::{self}").normalize(ReorderImports::Alphabetically), parse_use_tree("a::{self}") ); - assert_eq!(parse_use_tree("a::{b}").normalize(), parse_use_tree("a::b")); assert_eq!( - parse_use_tree("a::{b, c::self}").normalize(), + parse_use_tree("a::{b}").normalize(ReorderImports::Alphabetically), + parse_use_tree("a::b") + ); + assert_eq!( + parse_use_tree("a::{b, c::self}").normalize(ReorderImports::Alphabetically), parse_use_tree("a::{b, c}") ); assert_eq!( - parse_use_tree("a::{b as bar, c::self}").normalize(), + parse_use_tree("a::{b as bar, c::self}").normalize(ReorderImports::Alphabetically), parse_use_tree("a::{b as bar, c}") ); } #[test] fn test_use_tree_ord() { - 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("a").normalize(ReorderImports::Alphabetically) + < parse_use_tree("aa").normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("a").normalize(ReorderImports::Alphabetically) + < parse_use_tree("a::a").normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("a").normalize(ReorderImports::Alphabetically) + < parse_use_tree("*").normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("a").normalize(ReorderImports::Alphabetically) + < parse_use_tree("{a, b}").normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("*").normalize(ReorderImports::Alphabetically) + < parse_use_tree("{a, b}").normalize(ReorderImports::Alphabetically) + ); assert!( - parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize() - < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize() + parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}") + .normalize(ReorderImports::Alphabetically) + < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}") + .normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("serde::de::{Deserialize}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("serde_json").normalize(ReorderImports::Alphabetically) ); assert!( - parse_use_tree("serde::de::{Deserialize}").normalize() - < parse_use_tree("serde_json").normalize() + parse_use_tree("a::b::c").normalize(ReorderImports::Alphabetically) + < parse_use_tree("a::b::*").normalize(ReorderImports::Alphabetically) ); - assert!(parse_use_tree("a::b::c").normalize() < parse_use_tree("a::b::*").normalize()); assert!( - parse_use_tree("foo::{Bar, Baz}").normalize() - < parse_use_tree("{Bar, Baz}").normalize() + parse_use_tree("foo::{Bar, Baz}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("{Bar, Baz}").normalize(ReorderImports::Alphabetically) ); assert!( - parse_use_tree("foo::{qux as bar}").normalize() - < parse_use_tree("foo::{self as bar}").normalize() + parse_use_tree("foo::{qux as bar}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("foo::{self as bar}").normalize(ReorderImports::Alphabetically) ); assert!( - parse_use_tree("foo::{qux as bar}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() + parse_use_tree("foo::{qux as bar}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("foo::{baz, qux as bar}") + .normalize(ReorderImports::Alphabetically) ); assert!( - parse_use_tree("foo::{self as bar, baz}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() + parse_use_tree("foo::{self as bar, baz}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("foo::{baz, qux as bar}") + .normalize(ReorderImports::Alphabetically) ); - 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("foo").normalize(ReorderImports::Alphabetically) + < parse_use_tree("Foo").normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree("foo").normalize(ReorderImports::Alphabetically) + < parse_use_tree("foo::Bar").normalize(ReorderImports::Alphabetically) + ); assert!( - parse_use_tree("std::cmp::{d, c, b, a}").normalize() - < parse_use_tree("std::cmp::{b, e, g, f}").normalize() + parse_use_tree("std::cmp::{d, c, b, a}").normalize(ReorderImports::Alphabetically) + < parse_use_tree("std::cmp::{b, e, g, f}") + .normalize(ReorderImports::Alphabetically) ); } diff --git a/src/reorder.rs b/src/reorder.rs index a899f3da449..51915ecd604 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -12,7 +12,7 @@ use rustc_ast::ast; use rustc_span::{symbol::sym, Span}; use crate::config::{Config, GroupImportsTactic, ReorderImports}; -use crate::imports::{normalize_use_trees_with_granularity, UseSegment, UseTree, ImportsVector}; +use crate::imports::{normalize_use_trees_with_granularity, UseSegment, UseTree}; use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}; use crate::lists::{itemize_list, write_list, ListFormatting, ListItem}; use crate::rewrite::RewriteContext; @@ -138,10 +138,10 @@ fn rewrite_reorderable_or_regroupable_items( .map(|use_group| { let item_vec: Vec<_> = use_group .into_iter() - .map(|use_tree| {println!("use_tree: {:?}", use_tree); ListItem { + .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) }) From 86b63f67bde5502ba9383d2a1ec632f0cdd478e2 Mon Sep 17 00:00:00 2001 From: shiri Date: Sun, 12 Jun 2022 01:47:22 -0700 Subject: [PATCH 09/12] test new length module --- src/imports.rs | 140 +++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 79 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index 8c2486ef86f..366c4e34cc7 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -1133,7 +1133,6 @@ mod test { } } } - let mut parser = Parser { input: s.chars().peekable(), }; @@ -1161,6 +1160,32 @@ mod test { } } + macro_rules! test_normalize_eq { + ($first:expr, $second:expr) => { + assert_eq!( + parse_use_tree($first).normalize(ReorderImports::Alphabetically), + parse_use_tree($second) + ); + assert_eq!( + parse_use_tree($first).normalize(ReorderImports::Length), + parse_use_tree($second) + ); + }; + } + + macro_rules! test_normalize_smaller { + ($first:expr, $second:expr) => { + assert!( + parse_use_tree($first).normalize(ReorderImports::Alphabetically) + < parse_use_tree($second).normalize(ReorderImports::Alphabetically) + ); + assert!( + parse_use_tree($first).normalize(ReorderImports::Length) + < parse_use_tree($second).normalize(ReorderImports::Length) + ); + }; + } + #[test] fn test_use_tree_merge_crate() { test_merge!( @@ -1310,22 +1335,10 @@ mod test { #[test] fn test_use_tree_normalize() { - assert_eq!( - parse_use_tree("a::self").normalize(ReorderImports::Alphabetically), - parse_use_tree("a") - ); - assert_eq!( - parse_use_tree("a::self as foo").normalize(ReorderImports::Alphabetically), - parse_use_tree("a as foo") - ); - assert_eq!( - parse_use_tree("a::{self}").normalize(ReorderImports::Alphabetically), - parse_use_tree("a::{self}") - ); - assert_eq!( - parse_use_tree("a::{b}").normalize(ReorderImports::Alphabetically), - parse_use_tree("a::b") - ); + test_normalize_eq!("a::self", "a"); + test_normalize_eq!("a::self as foo", "a as foo"); + test_normalize_eq!("a::{self}", "a::{self}"); + test_normalize_eq!("a::{b}", "a::b"); assert_eq!( parse_use_tree("a::{b, c::self}").normalize(ReorderImports::Alphabetically), parse_use_tree("a::{b, c}") @@ -1338,74 +1351,43 @@ mod test { #[test] fn test_use_tree_ord() { - assert!( - parse_use_tree("a").normalize(ReorderImports::Alphabetically) - < parse_use_tree("aa").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("a").normalize(ReorderImports::Alphabetically) - < parse_use_tree("a::a").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("a").normalize(ReorderImports::Alphabetically) - < parse_use_tree("*").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("a").normalize(ReorderImports::Alphabetically) - < parse_use_tree("{a, b}").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("*").normalize(ReorderImports::Alphabetically) - < parse_use_tree("{a, b}").normalize(ReorderImports::Alphabetically) + test_normalize_smaller!("a", "aa"); + test_normalize_smaller!("a", "a::a"); + test_normalize_smaller!("a", "*"); + test_normalize_smaller!("a", "{a, b}"); + test_normalize_smaller!("*", "{a, b}"); + + test_normalize_smaller!( + "aaaaaaaaaaaaaaa::{bb, cc, dddddddd}", + "aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}" ); + test_normalize_smaller!("serde::de::{Deserialize}", "serde_json"); + test_normalize_smaller!("a::b::c", "a::b::*"); + test_normalize_smaller!("foo::{Bar, Baz}", "{Bar, Baz}"); - assert!( - parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}") - .normalize(ReorderImports::Alphabetically) - < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}") - .normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("serde::de::{Deserialize}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("serde_json").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("a::b::c").normalize(ReorderImports::Alphabetically) - < parse_use_tree("a::b::*").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("foo::{Bar, Baz}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("{Bar, Baz}").normalize(ReorderImports::Alphabetically) - ); + test_normalize_smaller!("foo::{qux as bar}", "foo::{self as bar}"); + test_normalize_smaller!("foo::{qux as bar}", "foo::{baz, qux as bar}"); + test_normalize_smaller!("foo::{self as bar, baz}", "foo::{baz, qux as bar}"); - assert!( - parse_use_tree("foo::{qux as bar}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("foo::{self as bar}").normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("foo::{qux as bar}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("foo::{baz, qux as bar}") - .normalize(ReorderImports::Alphabetically) - ); - assert!( - parse_use_tree("foo::{self as bar, baz}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("foo::{baz, qux as bar}") - .normalize(ReorderImports::Alphabetically) - ); + test_normalize_smaller!("foo", "Foo"); + test_normalize_smaller!("foo", "foo::Bar"); - assert!( - parse_use_tree("foo").normalize(ReorderImports::Alphabetically) - < parse_use_tree("Foo").normalize(ReorderImports::Alphabetically) + test_normalize_smaller!("std::cmp::{d, c, b, a}", "std::cmp::{b, e, g, f}"); + } + + #[test] + fn test_use_tree_length() { + assert_eq!( + parse_use_tree("aaaaa::{aa, bbbb, cc, d}").normalize(ReorderImports::Length), + parse_use_tree("aaaaa::{d, aa, cc, bbbb}") ); - assert!( - parse_use_tree("foo").normalize(ReorderImports::Alphabetically) - < parse_use_tree("foo::Bar").normalize(ReorderImports::Alphabetically) + assert_eq!( + parse_use_tree("a::{b, c::self}").normalize(ReorderImports::Length), + parse_use_tree("a::{b, c}") ); - - assert!( - parse_use_tree("std::cmp::{d, c, b, a}").normalize(ReorderImports::Alphabetically) - < parse_use_tree("std::cmp::{b, e, g, f}") - .normalize(ReorderImports::Alphabetically) + assert_eq!( + parse_use_tree("a::{b as bar, c::self}").normalize(ReorderImports::Length), + parse_use_tree("a::{c, b as bar}") ); } From cfe1c4d640eca8ecfff7796bc427ebe19f5f1bff Mon Sep 17 00:00:00 2001 From: shiri Date: Sun, 12 Jun 2022 03:03:52 -0700 Subject: [PATCH 10/12] remove print --- src/reorder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/reorder.rs b/src/reorder.rs index 51915ecd604..f9bb713a281 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -146,7 +146,7 @@ fn rewrite_reorderable_or_regroupable_items( wrap_reorderable_items(context, &item_vec, nested_shape) }) .collect::>>()?; - println!("item_vec: {:?}", item_vec); + let join_string = format!("\n\n{}", shape.indent.to_string(context.config)); Some(item_vec.join(&join_string)) } From 4c6a91750ffaad033d5e3f02f713e088931d110c Mon Sep 17 00:00:00 2001 From: shiri Date: Mon, 13 Jun 2022 21:59:01 -0700 Subject: [PATCH 11/12] fix merge conflict --- tests/config/small_tabs.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/config/small_tabs.toml b/tests/config/small_tabs.toml index 9570f1a734e..a3661593774 100644 --- a/tests/config/small_tabs.toml +++ b/tests/config/small_tabs.toml @@ -6,7 +6,5 @@ brace_style = "SameLineWhere" fn_args_layout = "Tall" trailing_comma = "Vertical" indent_style = "Block" -report_todo = "Always" -report_fixme = "Never" reorder_imports = "Off" format_strings = true From e3d15b795fa3fb8beb80e33abf5362d50a323f39 Mon Sep 17 00:00:00 2001 From: shiri Date: Tue, 14 Jun 2022 00:24:02 -0700 Subject: [PATCH 12/12] remove commented function --- src/config/config_type.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 38fd1c2a1e6..e37ed798cb5 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -290,10 +290,6 @@ macro_rules! create_config { } )+ } - /*#[allow(unreachable_pub)] - pub fn reorder_imports(&self) -> ReorderImports { - self.reorder_imports - }*/ fn set_width_heuristics(&mut self, heuristics: WidthHeuristics) { let max_width = self.max_width.2;