From 13964b739e84dc6238700187e0ef01e915c3b6f7 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 13:16:36 +0900 Subject: [PATCH 1/6] Update tests 1. snake_case < CamelCase < UPPER_SNAKE_CASE 2. Use vertical layout for list with nested imports. --- tests/source/issue-2256.rs | 6 +++--- tests/target/imports.rs | 31 ++++++++++++++++++++----------- tests/target/issue-2256.rs | 7 +++---- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/tests/source/issue-2256.rs b/tests/source/issue-2256.rs index b505b96e2b3..a206e8db6cf 100644 --- a/tests/source/issue-2256.rs +++ b/tests/source/issue-2256.rs @@ -2,11 +2,11 @@ use std::{}; use std::borrow::Cow; -/* comment */ use std::{}; -/* comment */ use std::{}; +/* comment 1 */ use std::{}; +/* comment 2 */ use std::{}; -/* comment */ use std::{}; +/* comment 3 */ use std::{}; diff --git a/tests/target/imports.rs b/tests/target/imports.rs index c8f5921afb3..236fee95e82 100644 --- a/tests/target/imports.rs +++ b/tests/target/imports.rs @@ -19,12 +19,11 @@ use list::{// Another item use test::{/* A */ self /* B */, Other /* C */}; -use Foo::{Bar, Baz}; use syntax; pub use syntax::ast::{Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath, Expr_}; +use Foo::{Bar, Baz}; use {Bar /* comment */, /* Pre-comment! */ Foo}; -use self; use std::io; use std::io; @@ -54,14 +53,14 @@ use foo::{self as bar, baz}; use foo::{baz, qux as bar}; // With absolute paths -use Foo; use foo; use foo::Bar; use foo::{Bar, Baz}; +use Foo; use {Bar, Baz}; // Root globs -use ::*; +use *; use *; // spaces used to cause glob imports to disappear (#1356) @@ -73,14 +72,24 @@ use foo::issue_1356::*; use self::unix::{}; // nested imports -use foo::{a, b, boo, c, - bar::{baz, qux, xxxxxxxxxxx, yyyyyyyyyyyyy, zzzzzzzzzzzzzzzz, - foo::{a, b, cxxxxxxxxxxxxx, yyyyyyyyyyyyyy, zzzzzzzzzzzzzzzz}}}; - -use fooo::{bar, x, y, z, - baar::foobar::{xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy, +use foo::{a, + b, + bar::{baz, + foo::{a, b, cxxxxxxxxxxxxx, yyyyyyyyyyyyyy, zzzzzzzzzzzzzzzz}, + qux, + xxxxxxxxxxx, + yyyyyyyyyyyyy, + zzzzzzzzzzzzzzzz}, + boo, + c}; + +use fooo::{baar::foobar::{xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy, zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz}, - bar::*}; + bar, + bar::*, + x, + y, + z}; // nested imports with a single sub-tree. use a::b::c::d; diff --git a/tests/target/issue-2256.rs b/tests/target/issue-2256.rs index d4e594515b3..0a59c308394 100644 --- a/tests/target/issue-2256.rs +++ b/tests/target/issue-2256.rs @@ -1,8 +1,7 @@ // こんにちは use std::borrow::Cow; -/* comment */ +/* comment 1 */ +/* comment 2 */ -/* comment */ - -/* comment */ +/* comment 3 */ From a8022f38621b7c8c71b225cf1782fb796a9a0e37 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 13:18:53 +0900 Subject: [PATCH 2/6] Do not insert newline when item is empty This change is necessary when we remove unused imports (`use std::{};`). --- src/lists.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lists.rs b/src/lists.rs index 05b1a7ce296..de39d03356f 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -257,7 +257,9 @@ where result.push(' '); } } - DefinitiveListTactic::Vertical if !first => { + DefinitiveListTactic::Vertical + if !first && !inner_item.is_empty() && !result.is_empty() => + { result.push('\n'); result.push_str(indent_str); } From 2b682b8ed5331986b13c6f9ef8a0ff2933d098b5 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 13:19:55 +0900 Subject: [PATCH 3/6] Do not include separator to post comment This prevents the trailing `;` on use item to be treated as comment. --- src/lists.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lists.rs b/src/lists.rs index de39d03356f..61c9c413ac2 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -619,6 +619,8 @@ where let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') { post_snippet[1..].trim_matches(white_space) + } else if post_snippet.starts_with(self.separator) { + post_snippet[self.separator.len()..].trim_matches(white_space) } else if post_snippet.ends_with(',') { post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space) } else { From 01311c63ec14f2a323a9e4819ab713f195ac4109 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 13:21:13 +0900 Subject: [PATCH 4/6] Format normalized use item This commit implements `Rewrite` trait on `UseTree`, which is a normalized form of `ast::UseTree` for rustfmt. --- src/imports.rs | 901 ++++++++++++++++++++++++++++++++++--------------- src/lists.rs | 4 +- src/reorder.rs | 567 ++++++------------------------- 3 files changed, 727 insertions(+), 745 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index f631f4b9c26..8054fbdd451 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -11,7 +11,7 @@ use std::cmp::Ordering; use config::lists::*; -use syntax::ast; +use syntax::ast::{self, UseTreeKind}; use syntax::codemap::{BytePos, Span}; use codemap::SpanUtils; @@ -19,112 +19,30 @@ use config::IndentStyle; use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator}; use rewrite::{Rewrite, RewriteContext}; use shape::Shape; -use types::{rewrite_path, PathContext}; -use utils::{format_visibility, mk_sp}; +use spanned::Spanned; +use utils::mk_sp; use visitor::FmtVisitor; +use std::borrow::Cow; + /// Returns a name imported by a `use` declaration. e.g. returns `Ordering` /// for `std::cmp::Ordering` and `self` for `std::cmp::self`. pub fn path_to_imported_ident(path: &ast::Path) -> ast::Ident { path.segments.last().unwrap().identifier } -pub fn same_rename(opt_ident: &Option, path: &ast::Path) -> bool { - opt_ident.map_or(true, |ident| path_to_imported_ident(path) == ident) -} - -fn rewrite_prefix(path: &ast::Path, context: &RewriteContext, shape: Shape) -> Option { - if path.segments.len() > 1 && path_to_imported_ident(path).to_string() == "self" { - let path = &ast::Path { - span: path.span, - segments: path.segments[..path.segments.len() - 1].to_owned(), - }; - rewrite_path(context, PathContext::Import, None, path, shape) - } else { - rewrite_path(context, PathContext::Import, None, path, shape) - } -} - -impl Rewrite for ast::UseTree { - fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option { - match self.kind { - ast::UseTreeKind::Nested(ref items) => { - rewrite_nested_use_tree(shape, &self.prefix, items, self.span, context) - } - ast::UseTreeKind::Glob => { - let prefix_shape = shape.sub_width(3)?; - - if !self.prefix.segments.is_empty() { - let path_str = rewrite_prefix(&self.prefix, context, prefix_shape)?; - Some(format!("{}::*", path_str)) - } else { - Some("*".to_owned()) - } - } - ast::UseTreeKind::Simple(opt_ident) => { - if same_rename(&opt_ident, &self.prefix) { - rewrite_prefix(&self.prefix, context, shape) - .or_else(|| Some(context.snippet(self.prefix.span).to_owned())) - } else { - let ident_str = opt_ident?.to_string(); - // 4 = " as ".len() - let prefix_shape = shape.sub_width(ident_str.len() + 4)?; - let path_str = rewrite_prefix(&self.prefix, context, prefix_shape) - .unwrap_or_else(|| context.snippet(self.prefix.span).to_owned()); - Some(format!("{} as {}", path_str, ident_str)) - } - } - } - } -} - -fn is_unused_import(tree: &ast::UseTree, attrs: &[ast::Attribute]) -> bool { - attrs.is_empty() && is_unused_import_inner(tree) -} - -fn is_unused_import_inner(tree: &ast::UseTree) -> bool { - match tree.kind { - ast::UseTreeKind::Nested(ref items) => match items.len() { - 0 => true, - 1 => is_unused_import_inner(&items[0].0), - _ => false, - }, - _ => false, - } -} - -// Rewrite `use foo;` WITHOUT attributes. -pub fn rewrite_import( - context: &RewriteContext, - vis: &ast::Visibility, - tree: &ast::UseTree, - attrs: &[ast::Attribute], - shape: Shape, -) -> Option { - let vis = format_visibility(vis); - // 4 = `use `, 1 = `;` - let rw = shape - .offset_left(vis.len() + 4) - .and_then(|shape| shape.sub_width(1)) - .and_then(|shape| { - // If we have an empty nested group with no attributes, we erase it - if is_unused_import(tree, attrs) { - Some("".to_owned()) - } else { - tree.rewrite(context, shape) - } - }); - match rw { - Some(ref s) if !s.is_empty() => Some(format!("{}use {};", vis, s)), - _ => rw, - } -} - impl<'a> FmtVisitor<'a> { pub fn format_import(&mut self, item: &ast::Item, tree: &ast::UseTree) { let span = item.span; let shape = self.shape(); - let rw = rewrite_import(&self.get_context(), &item.vis, tree, &item.attrs, shape); + let rw = UseTree::from_ast( + &self.get_context(), + tree, + None, + Some(item.vis.clone()), + Some(item.span.lo()), + Some(item.attrs.clone()), + ).rewrite_top_level(&self.get_context(), shape); match rw { Some(ref s) if s.is_empty() => { // Format up to last newline @@ -152,204 +70,416 @@ impl<'a> FmtVisitor<'a> { } } -fn rewrite_nested_use_tree_single( - context: &RewriteContext, - path_str: &str, - tree: &ast::UseTree, - shape: Shape, -) -> Option { - match tree.kind { - ast::UseTreeKind::Simple(opt_rename) => { - let mut item_str = rewrite_prefix(&tree.prefix, context, shape)?; - if item_str == "self" { - item_str = "".to_owned(); - } +// Ordering of imports - let path_item_str = if path_str.is_empty() { - if item_str.is_empty() { - "self".to_owned() - } else { - item_str - } - } else if item_str.is_empty() { - path_str.to_owned() - } else { - format!("{}::{}", path_str, item_str) - }; - - Some(if same_rename(&opt_rename, &tree.prefix) { - path_item_str - } else { - format!("{} as {}", path_item_str, opt_rename?) - }) - } - ast::UseTreeKind::Glob | ast::UseTreeKind::Nested(..) => { - // 2 = "::" - let nested_shape = shape.offset_left(path_str.len() + 2)?; - tree.rewrite(context, nested_shape) - .map(|item| format!("{}::{}", path_str, item)) - } - } +// We order imports by translating to our own representation and then sorting. +// The Rust AST data structures are really bad for this. Rustfmt applies a bunch +// of normalisations to imports and since we want to sort based on the result +// of these (and to maintain idempotence) we must apply the same normalisations +// to the data structures for sorting. +// +// We sort `self` and `super` before other imports, then identifier imports, +// then glob imports, then lists of imports. We do not take aliases into account +// when ordering unless the imports are identical except for the alias (rare in +// practice). + +// FIXME(#2531) - we should unify the comparison code here with the formatting +// code elsewhere since we are essentially string-ifying twice. Furthermore, by +// parsing to our own format on comparison, we repeat a lot of work when +// sorting. + +// FIXME we do a lot of allocation to make our own representation. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum UseSegment { + Ident(String, Option), + Slf(Option), + Super(Option), + Glob, + List(Vec), } -#[derive(Eq, PartialEq)] -enum ImportItem<'a> { - // `self` or `self as a` - SelfImport(&'a str), - // name_one, name_two, ... - SnakeCase(&'a str), - // NameOne, NameTwo, ... - CamelCase(&'a str), - // NAME_ONE, NAME_TWO, ... - AllCaps(&'a str), - // Failed to format the import item - Invalid, +#[derive(Debug, Clone)] +pub struct UseTree { + pub path: Vec, + pub span: Span, + // Comment information within nested use tree. + list_item: Option, + // Additional fields for top level use items. + // Should we have another struct for top-level use items rather than reusing this? + visibility: Option, + attrs: Option>, +} + +impl PartialEq for UseTree { + fn eq(&self, other: &UseTree) -> bool { + self.path == other.path + } } +impl Eq for UseTree {} -impl<'a> ImportItem<'a> { - fn from_str(s: &str) -> ImportItem { - if s == "self" || s.starts_with("self as") { - ImportItem::SelfImport(s) - } else if s.chars().all(|c| c.is_lowercase() || c == '_' || c == ' ') { - ImportItem::SnakeCase(s) - } else if s.chars().all(|c| c.is_uppercase() || c == '_' || c == ' ') { - ImportItem::AllCaps(s) +impl UseSegment { + // Clone a version of self with any top-level alias removed. + fn remove_alias(&self) -> UseSegment { + match *self { + UseSegment::Ident(ref s, _) => UseSegment::Ident(s.clone(), None), + UseSegment::Slf(_) => UseSegment::Slf(None), + UseSegment::Super(_) => UseSegment::Super(None), + _ => self.clone(), + } + } + + fn from_path_segment(path_seg: &ast::PathSegment) -> Option { + let name = path_seg.identifier.name.as_str(); + if name == "{{root}}" { + return None; + } + Some(if name == "self" { + UseSegment::Slf(None) + } else if name == "super" { + UseSegment::Super(None) } else { - ImportItem::CamelCase(s) + UseSegment::Ident((*name).to_owned(), None) + }) + } +} + +impl UseTree { + // Rewrite use tree with `use ` and a trailing `;`. + pub fn rewrite_top_level(&self, context: &RewriteContext, shape: Shape) -> Option { + let mut result = String::with_capacity(256); + if let Some(ref attrs) = self.attrs { + result.push_str(&attrs.rewrite(context, shape)?); + if !result.is_empty() { + result.push_str(&shape.indent.to_string_with_newline(context.config)); + } } + + let vis = self.visibility + .as_ref() + .map_or(Cow::from(""), |vis| ::utils::format_visibility(&vis)); + result.push_str(&self.rewrite(context, shape.offset_left(vis.len())?) + .map(|s| { + if s.is_empty() { + s.to_owned() + } else { + format!("{}use {};", vis, s) + } + })?); + Some(result) } - fn from_opt_str(s: Option<&String>) -> ImportItem { - s.map_or(ImportItem::Invalid, |s| ImportItem::from_str(s)) + pub fn from_ast_with_normalization( + context: &RewriteContext, + item: &ast::Item, + ) -> Option { + match item.node { + ast::ItemKind::Use(ref use_tree) => Some( + UseTree::from_ast( + context, + use_tree, + None, + Some(item.vis.clone()), + Some(item.span().lo()), + if item.attrs.is_empty() { + None + } else { + Some(item.attrs.clone()) + }, + ).normalize(context.config.reorder_imported_names()), + ), + _ => None, + } } - fn to_str(&self) -> Option<&str> { - match *self { - ImportItem::SelfImport(s) - | ImportItem::SnakeCase(s) - | ImportItem::CamelCase(s) - | ImportItem::AllCaps(s) => Some(s), - ImportItem::Invalid => None, + fn from_ast( + context: &RewriteContext, + a: &ast::UseTree, + list_item: Option, + visibility: Option, + opt_lo: Option, + attrs: Option>, + ) -> UseTree { + let span = if let Some(lo) = opt_lo { + mk_sp(lo, a.span.hi()) + } else { + a.span + }; + let mut result = UseTree { + path: vec![], + span, + list_item, + visibility, + attrs, + }; + for p in &a.prefix.segments { + if let Some(use_segment) = UseSegment::from_path_segment(p) { + result.path.push(use_segment); + } + } + match a.kind { + UseTreeKind::Glob => { + result.path.push(UseSegment::Glob); + } + UseTreeKind::Nested(ref list) => { + // Extract comments between nested use items. + // This needs to be done before sorting use items. + let items: Vec<_> = itemize_list( + context.snippet_provider, + list.iter().map(|(tree, _)| tree), + "}", + ",", + |tree| tree.span.lo(), + |tree| tree.span.hi(), + |_| Some("".to_owned()), // We only need comments for now. + context.snippet_provider.span_after(a.span, "{"), + a.span.hi(), + false, + ).collect(); + result.path.push(UseSegment::List( + list.iter() + .zip(items.into_iter()) + .map(|(t, list_item)| { + Self::from_ast(context, &t.0, Some(list_item), None, None, None) + }) + .collect(), + )); + } + UseTreeKind::Simple(ref rename) => { + let mut name = (*path_to_imported_ident(&a.prefix).name.as_str()).to_owned(); + let alias = rename.and_then(|ident| { + if ident == path_to_imported_ident(&a.prefix) { + None + } else { + Some(ident.to_string()) + } + }); + + let segment = if &name == "self" { + UseSegment::Slf(alias) + } else if &name == "super" { + UseSegment::Super(alias) + } else { + UseSegment::Ident(name, alias) + }; + + // `name` is already in result. + result.path.pop(); + result.path.push(segment); + } } + result } - fn to_u32(&self) -> u32 { - match *self { - ImportItem::SelfImport(..) => 0, - ImportItem::SnakeCase(..) => 1, - ImportItem::CamelCase(..) => 2, - ImportItem::AllCaps(..) => 3, - ImportItem::Invalid => 4, + // Do the adjustments that rustfmt does elsewhere to use paths. + pub fn normalize(mut self, do_sort: bool) -> UseTree { + let mut last = self.path.pop().expect("Empty use tree?"); + // Hack around borrow checker. + let mut normalize_sole_list = false; + let mut aliased_self = false; + + // Remove foo::{} or self without attributes. + match last { + _ if self.attrs.is_some() => (), + UseSegment::List(ref list) if list.is_empty() => { + self.path = vec![]; + return self; + } + UseSegment::Slf(None) if self.path.is_empty() && self.visibility.is_some() => { + self.path = vec![]; + return self; + } + _ => (), + } + + // Normalise foo::self -> foo. + if let UseSegment::Slf(None) = last { + if self.path.len() > 0 { + return self; + } + } + + // Normalise foo::self as bar -> foo as bar. + if let UseSegment::Slf(_) = last { + match self.path.last() { + None => {} + Some(UseSegment::Ident(_, None)) => { + aliased_self = true; + } + _ => unreachable!(), + } + } + + if aliased_self { + match self.path.last() { + Some(UseSegment::Ident(_, ref mut old_rename)) => { + assert!(old_rename.is_none()); + if let UseSegment::Slf(Some(rename)) = last { + *old_rename = Some(rename); + return self; + } + } + _ => unreachable!(), + } + } + + // Normalise foo::{bar} -> foo::bar + if let UseSegment::List(ref list) = last { + if list.len() == 1 { + normalize_sole_list = true; + } + } + + if normalize_sole_list { + match last { + UseSegment::List(list) => { + for seg in &list[0].path { + self.path.push(seg.clone()); + } + return self.normalize(do_sort); + } + _ => unreachable!(), + } + } + + // Recursively normalize elements of a list use (including sorting the list). + if let UseSegment::List(list) = last { + let mut list = list.into_iter() + .map(|ut| ut.normalize(do_sort)) + .collect::>(); + if do_sort { + list.sort(); + } + last = UseSegment::List(list); } + + self.path.push(last); + self } } -impl<'a> PartialOrd for ImportItem<'a> { - fn partial_cmp(&self, other: &ImportItem<'a>) -> Option { +impl PartialOrd for UseSegment { + fn partial_cmp(&self, other: &UseSegment) -> Option { Some(self.cmp(other)) } } - -impl<'a> Ord for ImportItem<'a> { - fn cmp(&self, other: &ImportItem<'a>) -> Ordering { - let res = self.to_u32().cmp(&other.to_u32()); - if res != Ordering::Equal { - return res; - } - self.to_str().map_or(Ordering::Greater, |self_str| { - other - .to_str() - .map_or(Ordering::Less, |other_str| self_str.cmp(other_str)) - }) +impl PartialOrd for UseTree { + fn partial_cmp(&self, other: &UseTree) -> Option { + Some(self.cmp(other)) } } +impl Ord for UseSegment { + fn cmp(&self, other: &UseSegment) -> Ordering { + use self::UseSegment::*; -// Pretty prints a multi-item import. -// If the path list is empty, it leaves the braces empty. -fn rewrite_nested_use_tree( - shape: Shape, - path: &ast::Path, - trees: &[(ast::UseTree, ast::NodeId)], - span: Span, - context: &RewriteContext, -) -> Option { - // Returns a different option to distinguish `::foo` and `foo` - let path_str = rewrite_path(context, PathContext::Import, None, path, shape)?; - - match trees.len() { - 0 => { - let shape = shape.offset_left(path_str.len() + 3)?; - return rewrite_path(context, PathContext::Import, None, path, shape) - .map(|path_str| format!("{}::{{}}", path_str)); + fn is_upper_snake_case(s: &str) -> bool { + s.chars().all(|c| c.is_uppercase() || c == '_') } - 1 => { - return rewrite_nested_use_tree_single(context, &path_str, &trees[0].0, shape); + + match (self, other) { + (&Slf(ref a), &Slf(ref b)) | (&Super(ref a), &Super(ref b)) => a.cmp(b), + (&Glob, &Glob) => Ordering::Equal, + (&Ident(ref ia, ref aa), &Ident(ref ib, ref ab)) => { + // snake_case < CamelCase < UPPER_SNAKE_CASE + if ia.starts_with(char::is_uppercase) && ib.starts_with(char::is_lowercase) { + return Ordering::Greater; + } + if ia.starts_with(char::is_lowercase) && ib.starts_with(char::is_uppercase) { + return Ordering::Less; + } + if is_upper_snake_case(ia) && !is_upper_snake_case(ib) { + return Ordering::Greater; + } + if !is_upper_snake_case(ia) && is_upper_snake_case(ib) { + return Ordering::Less; + } + let ident_ord = ia.cmp(ib); + if ident_ord != Ordering::Equal { + return ident_ord; + } + if aa.is_none() && ab.is_some() { + return Ordering::Less; + } + if aa.is_some() && ab.is_none() { + return Ordering::Greater; + } + aa.cmp(ab) + } + (&List(ref a), &List(ref b)) => { + for (a, b) in a.iter().zip(b.iter()) { + let ord = a.cmp(b); + if ord != Ordering::Equal { + return ord; + } + } + + a.len().cmp(&b.len()) + } + (&Slf(_), _) => Ordering::Less, + (_, &Slf(_)) => Ordering::Greater, + (&Super(_), _) => Ordering::Less, + (_, &Super(_)) => Ordering::Greater, + (&Ident(..), _) => Ordering::Less, + (_, &Ident(..)) => Ordering::Greater, + (&Glob, _) => Ordering::Less, + (_, &Glob) => Ordering::Greater, } - _ => (), } +} +impl Ord for UseTree { + fn cmp(&self, other: &UseTree) -> Ordering { + for (a, b) in self.path.iter().zip(other.path.iter()) { + let ord = a.cmp(b); + // The comparison without aliases is a hack to avoid situations like + // comparing `a::b` to `a as c` - where the latter should be ordered + // first since it is shorter. + if ord != Ordering::Equal && a.remove_alias().cmp(&b.remove_alias()) != Ordering::Equal + { + return ord; + } + } - let path_str = if path_str.is_empty() { - path_str - } else { - format!("{}::", path_str) - }; - - // 2 = "{}" - let remaining_width = shape.width.checked_sub(path_str.len() + 2).unwrap_or(0); - let nested_indent = match context.config.imports_indent() { - IndentStyle::Block => shape.indent.block_indent(context.config), - // 1 = `{` - IndentStyle::Visual => shape.visual_indent(path_str.len() + 1).indent, - }; + self.path.len().cmp(&other.path.len()) + } +} +fn rewrite_nested_use_tree( + context: &RewriteContext, + use_tree_list: &[UseTree], + shape: Shape, +) -> Option { + let mut list_items = Vec::with_capacity(use_tree_list.len()); let nested_shape = match context.config.imports_indent() { - IndentStyle::Block => Shape::indented(nested_indent, context.config).sub_width(1)?, - IndentStyle::Visual => Shape::legacy(remaining_width, nested_indent), - }; - - let mut items = { - // Dummy value, see explanation below. - let mut items = vec![ListItem::from_str("")]; - let iter = itemize_list( - context.snippet_provider, - trees.iter().map(|tree| &tree.0), - "}", - ",", - |tree| tree.span.lo(), - |tree| tree.span.hi(), - |tree| tree.rewrite(context, nested_shape), - context.snippet_provider.span_after(span, "{"), - span.hi(), - false, - ); - items.extend(iter); - items + IndentStyle::Block => shape + .block_indent(context.config.tab_spaces()) + .with_max_width(context.config) + .sub_width(1)?, + IndentStyle::Visual => shape.visual_indent(0), }; - - // We prefixed the item list with a dummy value so that we can - // potentially move "self" to the front of the vector without touching - // the rest of the items. - let has_self = move_self_to_front(&mut items); - let first_index = if has_self { 0 } else { 1 }; - - if context.config.reorder_imported_names() { - items[1..].sort_by(|a, b| { - let a = ImportItem::from_opt_str(a.item.as_ref()); - let b = ImportItem::from_opt_str(b.item.as_ref()); - a.cmp(&b) - }); + for use_tree in use_tree_list { + let mut list_item = use_tree.list_item.clone()?; + list_item.item = use_tree.rewrite(context, nested_shape); + list_items.push(list_item); } - - let tactic = definitive_tactic( - &items[first_index..], - context.config.imports_layout(), - Separator::Comma, - remaining_width, - ); - + let tactic = if use_tree_list.iter().any(|use_segment| { + use_segment + .path + .last() + .map_or(false, |last_segment| match last_segment { + UseSegment::List(..) => true, + _ => false, + }) + }) { + DefinitiveListTactic::Vertical + } else { + definitive_tactic( + &list_items, + context.config.imports_layout(), + Separator::Comma, + shape.width.checked_sub(2).unwrap_or(0), + ) + }; let ends_with_newline = context.config.imports_indent() == IndentStyle::Block && tactic != DefinitiveListTactic::Horizontal; - let fmt = ListFormatting { tactic, separator: ",", @@ -364,33 +494,258 @@ fn rewrite_nested_use_tree( preserve_newline: true, config: context.config, }; - let list_str = write_list(&items[first_index..], &fmt)?; + + let list_str = write_list(&list_items, &fmt)?; let result = if list_str.contains('\n') && context.config.imports_indent() == IndentStyle::Block { format!( - "{}{{\n{}{}\n{}}}", - path_str, + "{{\n{}{}\n{}}}", nested_shape.indent.to_string(context.config), list_str, shape.indent.to_string(context.config) ) } else { - format!("{}{{{}}}", path_str, list_str) + format!("{{{}}}", list_str) }; + Some(result) } -// Returns true when self item was found. -fn move_self_to_front(items: &mut Vec) -> bool { - match items - .iter() - .position(|item| item.item.as_ref().map(|x| &x[..]) == Some("self")) - { - Some(pos) => { - items[0] = items.remove(pos); - true +impl Rewrite for UseSegment { + fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option { + Some(match *self { + UseSegment::Ident(ref ident, Some(ref rename)) => format!("{} as {}", ident, rename), + UseSegment::Ident(ref ident, None) => ident.clone(), + UseSegment::Slf(Some(ref rename)) => format!("self as {}", rename), + UseSegment::Slf(None) => "self".to_owned(), + UseSegment::Super(Some(ref rename)) => format!("super as {}", rename), + UseSegment::Super(None) => "super".to_owned(), + UseSegment::Glob => "*".to_owned(), + UseSegment::List(ref use_tree_list) => rewrite_nested_use_tree( + context, + use_tree_list, + // 1 = "{" and "}" + shape.offset_left(1)?.sub_width(1)?, + )?, + }) + } +} + +impl Rewrite for UseTree { + // This does NOT format attributes and visibility or add a trailing `;`. + fn rewrite(&self, context: &RewriteContext, mut shape: Shape) -> Option { + let mut result = String::with_capacity(256); + let mut iter = self.path.iter().peekable(); + while let Some(ref segment) = iter.next() { + let segment_str = segment.rewrite(context, shape)?; + result.push_str(&segment_str); + if iter.peek().is_some() { + result.push_str("::"); + // 2 = "::" + shape = shape.offset_left(2 + segment_str.len())?; + } + } + Some(result) + } +} + +#[cfg(test)] +mod test { + use super::*; + use syntax::codemap::DUMMY_SP; + + // Parse the path part of an import. This parser is not robust and is only + // suitable for use in a test harness. + fn parse_use_tree(s: &str) -> UseTree { + use std::iter::Peekable; + use std::mem::swap; + use std::str::Chars; + + struct Parser<'a> { + input: Peekable>, + } + + impl<'a> Parser<'a> { + fn bump(&mut self) { + self.input.next().unwrap(); + } + fn eat(&mut self, c: char) { + assert!(self.input.next().unwrap() == c); + } + fn push_segment( + result: &mut Vec, + buf: &mut String, + alias_buf: &mut Option, + ) { + if !buf.is_empty() { + let mut alias = None; + swap(alias_buf, &mut alias); + if buf == "self" { + result.push(UseSegment::Slf(alias)); + *buf = String::new(); + *alias_buf = None; + } else if buf == "super" { + result.push(UseSegment::Super(alias)); + *buf = String::new(); + *alias_buf = None; + } else { + let mut name = String::new(); + swap(buf, &mut name); + result.push(UseSegment::Ident(name, alias)); + } + } + } + fn parse_in_list(&mut self) -> UseTree { + let mut result = vec![]; + let mut buf = String::new(); + let mut alias_buf = None; + while let Some(&c) = self.input.peek() { + match c { + '{' => { + assert!(buf.is_empty()); + self.bump(); + result.push(UseSegment::List(self.parse_list())); + self.eat('}'); + } + '*' => { + assert!(buf.is_empty()); + self.bump(); + result.push(UseSegment::Glob); + } + ':' => { + self.bump(); + self.eat(':'); + Self::push_segment(&mut result, &mut buf, &mut alias_buf); + } + '}' | ',' => { + Self::push_segment(&mut result, &mut buf, &mut alias_buf); + return UseTree { + path: result, + span: DUMMY_SP, + list_item: None, + visibility: None, + attrs: None, + }; + } + ' ' => { + self.bump(); + self.eat('a'); + self.eat('s'); + self.eat(' '); + alias_buf = Some(String::new()); + } + c => { + self.bump(); + if let Some(ref mut buf) = alias_buf { + buf.push(c); + } else { + buf.push(c); + } + } + } + } + Self::push_segment(&mut result, &mut buf, &mut alias_buf); + UseTree { + path: result, + span: DUMMY_SP, + list_item: None, + visibility: None, + attrs: None, + } + } + + fn parse_list(&mut self) -> Vec { + let mut result = vec![]; + loop { + match self.input.peek().unwrap() { + ',' | ' ' => self.bump(), + '}' => { + return result; + } + _ => result.push(self.parse_in_list()), + } + } + } } - None => false, + + let mut parser = Parser { + input: s.chars().peekable(), + }; + parser.parse_in_list() + } + + #[test] + fn test_use_tree_normalize() { + assert_eq!( + parse_use_tree("a::self").normalize(true), + parse_use_tree("a") + ); + assert_eq!( + parse_use_tree("a::self as foo").normalize(true), + parse_use_tree("a as foo") + ); + assert_eq!( + parse_use_tree("a::{self}").normalize(true), + parse_use_tree("a") + ); + assert_eq!( + parse_use_tree("a::{b}").normalize(true), + parse_use_tree("a::b") + ); + assert_eq!( + parse_use_tree("a::{b, c::self}").normalize(true), + parse_use_tree("a::{b, c}") + ); + assert_eq!( + parse_use_tree("a::{b as bar, c::self}").normalize(true), + parse_use_tree("a::{b as bar, c}") + ); + } + + #[test] + fn test_use_tree_ord() { + assert!(parse_use_tree("a").normalize(true) < parse_use_tree("aa").normalize(true)); + assert!(parse_use_tree("a").normalize(true) < parse_use_tree("a::a").normalize(true)); + assert!(parse_use_tree("a").normalize(true) < parse_use_tree("*").normalize(true)); + assert!(parse_use_tree("a").normalize(true) < parse_use_tree("{a, b}").normalize(true)); + assert!(parse_use_tree("*").normalize(true) < parse_use_tree("{a, b}").normalize(true)); + + assert!( + parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize(true) + < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize(true) + ); + assert!( + parse_use_tree("serde::de::{Deserialize}").normalize(true) + < parse_use_tree("serde_json").normalize(true) + ); + assert!( + parse_use_tree("a::b::c").normalize(true) < parse_use_tree("a::b::*").normalize(true) + ); + assert!( + parse_use_tree("foo::{Bar, Baz}").normalize(true) + < parse_use_tree("{Bar, Baz}").normalize(true) + ); + + assert!( + parse_use_tree("foo::{self as bar}").normalize(true) + < parse_use_tree("foo::{qux as bar}").normalize(true) + ); + assert!( + parse_use_tree("foo::{qux as bar}").normalize(true) + < parse_use_tree("foo::{baz, qux as bar}").normalize(true) + ); + assert!( + parse_use_tree("foo::{self as bar, baz}").normalize(true) + < parse_use_tree("foo::{baz, qux as bar}").normalize(true) + ); + + assert!(parse_use_tree("foo").normalize(true) < parse_use_tree("Foo").normalize(true)); + assert!(parse_use_tree("foo").normalize(true) < parse_use_tree("foo::Bar").normalize(true)); + + assert!( + parse_use_tree("std::cmp::{d, c, b, a}").normalize(true) + < parse_use_tree("std::cmp::{b, e, g, f}").normalize(true) + ); } } diff --git a/src/lists.rs b/src/lists.rs index 61c9c413ac2..43ba7f9d673 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -56,7 +56,7 @@ impl AsRef for ListItem { } } -#[derive(PartialEq, Eq, Debug)] +#[derive(PartialEq, Eq, Debug, Copy, Clone)] pub enum ListItemCommentStyle { // Try to keep the comment on the same line with the item. SameLine, @@ -66,7 +66,7 @@ pub enum ListItemCommentStyle { None, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ListItem { // None for comments mean that they are not present. pub pre_comment: Option, diff --git a/src/reorder.rs b/src/reorder.rs index 38d2a09bd08..ae9cada01d2 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -16,29 +16,22 @@ // TODO(#2455): Reorder trait items. -use config::{Config, lists::*}; -use syntax::ast::UseTreeKind; +use config::{lists::*, Config}; use syntax::{ast, attr, codemap::Span}; use attr::filter_inline_attrs; use codemap::LineRangeUtils; use comment::combine_strs_with_missing_comments; -use imports::{path_to_imported_ident, rewrite_import}; +use imports::UseTree; use items::{is_mod_decl, rewrite_extern_crate, rewrite_mod}; -use lists::{itemize_list, write_list, ListFormatting}; +use lists::{itemize_list, write_list, ListFormatting, ListItem}; use rewrite::{Rewrite, RewriteContext}; use shape::Shape; use spanned::Spanned; use utils::mk_sp; use visitor::FmtVisitor; -use std::cmp::{Ord, Ordering, PartialOrd}; - -fn compare_use_trees(a: &ast::UseTree, b: &ast::UseTree) -> Ordering { - let aa = UseTree::from_ast(a).normalize(); - let bb = UseTree::from_ast(b).normalize(); - aa.cmp(&bb) -} +use std::cmp::{Ord, Ordering}; /// Choose the ordering between the given two items. fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { @@ -46,9 +39,6 @@ fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { (&ast::ItemKind::Mod(..), &ast::ItemKind::Mod(..)) => { a.ident.name.as_str().cmp(&b.ident.name.as_str()) } - (&ast::ItemKind::Use(ref a_tree), &ast::ItemKind::Use(ref b_tree)) => { - compare_use_trees(a_tree, b_tree) - } (&ast::ItemKind::ExternCrate(ref a_name), &ast::ItemKind::ExternCrate(ref b_name)) => { // `extern crate foo as bar;` // ^^^ Comparing this. @@ -74,58 +64,11 @@ fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { } } -/// Rewrite a list of items with reordering. Every item in `items` must have -/// the same `ast::ItemKind`. -fn rewrite_reorderable_items( +fn wrap_reorderable_items( context: &RewriteContext, - reorderable_items: &[&ast::Item], + list_items: &[ListItem], shape: Shape, - span: Span, ) -> Option { - let items = itemize_list( - context.snippet_provider, - reorderable_items.iter(), - "", - ";", - |item| item.span().lo(), - |item| item.span().hi(), - |item| { - let attrs = filter_inline_attrs(&item.attrs, item.span()); - let attrs_str = attrs.rewrite(context, shape)?; - - let missed_span = if attrs.is_empty() { - mk_sp(item.span.lo(), item.span.lo()) - } else { - mk_sp(attrs.last().unwrap().span.hi(), item.span.lo()) - }; - - let item_str = match item.node { - ast::ItemKind::Use(ref tree) => { - rewrite_import(context, &item.vis, tree, &item.attrs, shape)? - } - ast::ItemKind::ExternCrate(..) => rewrite_extern_crate(context, item)?, - ast::ItemKind::Mod(..) => rewrite_mod(item), - _ => return None, - }; - - combine_strs_with_missing_comments( - context, - &attrs_str, - &item_str, - missed_span, - shape, - false, - ) - }, - span.lo(), - span.hi(), - false, - ); - - let mut item_pair_vec: Vec<_> = items.zip(reorderable_items.iter()).collect(); - item_pair_vec.sort_by(|a, b| compare_items(a.1, b.1)); - let item_vec: Vec<_> = item_pair_vec.into_iter().map(|pair| pair.0).collect(); - let fmt = ListFormatting { tactic: DefinitiveListTactic::Vertical, separator: "", @@ -137,7 +80,97 @@ fn rewrite_reorderable_items( config: context.config, }; - write_list(&item_vec, &fmt) + write_list(list_items, &fmt) +} + +fn rewrite_reorderable_item( + context: &RewriteContext, + item: &ast::Item, + shape: Shape, +) -> Option { + let attrs = filter_inline_attrs(&item.attrs, item.span()); + let attrs_str = attrs.rewrite(context, shape)?; + + let missed_span = if attrs.is_empty() { + mk_sp(item.span.lo(), item.span.lo()) + } else { + mk_sp(attrs.last().unwrap().span.hi(), item.span.lo()) + }; + + let item_str = match item.node { + ast::ItemKind::ExternCrate(..) => rewrite_extern_crate(context, item)?, + ast::ItemKind::Mod(..) => rewrite_mod(item), + _ => return None, + }; + + combine_strs_with_missing_comments( + context, + &attrs_str, + &item_str, + missed_span, + shape, + false, + ) +} + +/// Rewrite a list of items with reordering. Every item in `items` must have +/// the same `ast::ItemKind`. +fn rewrite_reorderable_items( + context: &RewriteContext, + reorderable_items: &[&ast::Item], + shape: Shape, + span: Span, +) -> Option { + match reorderable_items[0].node { + // FIXME: Remove duplicated code. + ast::ItemKind::Use(..) => { + let normalized_items: Vec<_> = reorderable_items + .iter() + .filter_map(|item| UseTree::from_ast_with_normalization(context, item)) + .collect(); + + // 4 = "use ", 1 = ";" + let nested_shape = shape.offset_left(4)?.sub_width(1)?; + let list_items = itemize_list( + context.snippet_provider, + normalized_items.iter(), + "", + ";", + |item| item.span.lo(), + |item| item.span.hi(), + |item| item.rewrite_top_level(context, nested_shape), + span.lo(), + span.hi(), + false, + ); + + let mut item_pair_vec: Vec<_> = list_items.zip(&normalized_items).collect(); + item_pair_vec.sort_by(|a, b| a.1.cmp(b.1)); + let item_vec: Vec<_> = item_pair_vec.into_iter().map(|pair| pair.0).collect(); + + wrap_reorderable_items(context, &item_vec, nested_shape) + } + _ => { + let list_items = itemize_list( + context.snippet_provider, + reorderable_items.iter(), + "", + ";", + |item| item.span().lo(), + |item| item.span().hi(), + |item| rewrite_reorderable_item(context, item, shape), + span.lo(), + span.hi(), + false, + ); + + let mut item_pair_vec: Vec<_> = list_items.zip(reorderable_items.iter()).collect(); + item_pair_vec.sort_by(|a, b| compare_items(a.1, b.1)); + let item_vec: Vec<_> = item_pair_vec.into_iter().map(|pair| pair.0).collect(); + + wrap_reorderable_items(context, &item_vec, shape) + } + } } fn contains_macro_use_attr(item: &ast::Item) -> bool { @@ -255,409 +288,3 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } } } - -// Ordering of imports - -// We order imports by translating to our own representation and then sorting. -// The Rust AST data structures are really bad for this. Rustfmt applies a bunch -// of normalisations to imports and since we want to sort based on the result -// of these (and to maintain idempotence) we must apply the same normalisations -// to the data structures for sorting. -// -// We sort `self` and `super` before other imports, then identifier imports, -// then glob imports, then lists of imports. We do not take aliases into account -// when ordering unless the imports are identical except for the alias (rare in -// practice). - -// FIXME(#2531) - we should unify the comparison code here with the formatting -// code elsewhere since we are essentially string-ifying twice. Furthermore, by -// parsing to our own format on comparison, we repeat a lot of work when -// sorting. - -// FIXME we do a lot of allocation to make our own representation. -#[derive(Debug, Clone, Eq, PartialEq)] -enum UseSegment { - Ident(String, Option), - Slf(Option), - Super(Option), - Glob, - List(Vec), -} - -#[derive(Debug, Clone, Eq, PartialEq)] -struct UseTree { - path: Vec, -} - -impl UseSegment { - // Clone a version of self with any top-level alias removed. - fn remove_alias(&self) -> UseSegment { - match *self { - UseSegment::Ident(ref s, _) => UseSegment::Ident(s.clone(), None), - UseSegment::Slf(_) => UseSegment::Slf(None), - UseSegment::Super(_) => UseSegment::Super(None), - _ => self.clone(), - } - } - - fn from_path_segment(path_seg: &ast::PathSegment) -> UseSegment { - let name = path_seg.identifier.name.as_str(); - if name == "self" { - UseSegment::Slf(None) - } else if name == "super" { - UseSegment::Super(None) - } else { - UseSegment::Ident((*name).to_owned(), None) - } - } -} - -impl UseTree { - fn from_ast(a: &ast::UseTree) -> UseTree { - let mut result = UseTree { path: vec![] }; - for p in &a.prefix.segments { - result.path.push(UseSegment::from_path_segment(p)); - } - match a.kind { - UseTreeKind::Glob => { - result.path.push(UseSegment::Glob); - } - UseTreeKind::Nested(ref list) => { - result.path.push(UseSegment::List( - list.iter().map(|t| Self::from_ast(&t.0)).collect(), - )); - } - UseTreeKind::Simple(ref rename) => { - let mut name = (*path_to_imported_ident(&a.prefix).name.as_str()).to_owned(); - let alias = rename.and_then(|ident| { - if ident == path_to_imported_ident(&a.prefix) { - None - } else { - Some(ident.to_string()) - } - }); - - let segment = if &name == "self" { - UseSegment::Slf(alias) - } else if &name == "super" { - UseSegment::Super(alias) - } else { - UseSegment::Ident(name, alias) - }; - - // `name` is already in result. - result.path.pop(); - result.path.push(segment); - } - } - result - } - - // Do the adjustments that rustfmt does elsewhere to use paths. - 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; - let mut aliased_self = false; - - // Normalise foo::self -> foo. - if let UseSegment::Slf(None) = last { - return self; - } - - // Normalise foo::self as bar -> foo as bar. - if let UseSegment::Slf(_) = last { - match self.path.last() { - None => {} - Some(UseSegment::Ident(_, None)) => { - aliased_self = true; - } - _ => unreachable!(), - } - } - - if aliased_self { - match self.path.last() { - Some(UseSegment::Ident(_, ref mut old_rename)) => { - assert!(old_rename.is_none()); - if let UseSegment::Slf(Some(rename)) = last { - *old_rename = Some(rename); - return self; - } - } - _ => unreachable!(), - } - } - - // Normalise foo::{bar} -> foo::bar - if let UseSegment::List(ref list) = last { - if list.len() == 1 && list[0].path.len() == 1 { - normalize_sole_list = true; - } - } - - if normalize_sole_list { - match last { - UseSegment::List(list) => { - self.path.push(list[0].path[0].clone()); - return self.normalize(); - } - _ => unreachable!(), - } - } - - // Recursively normalize elements of a list use (including sorting the list). - if let UseSegment::List(list) = last { - let mut list: Vec<_> = list.into_iter().map(|ut| ut.normalize()).collect(); - list.sort(); - last = UseSegment::List(list); - } - - self.path.push(last); - self - } -} - -impl PartialOrd for UseSegment { - fn partial_cmp(&self, other: &UseSegment) -> Option { - Some(self.cmp(other)) - } -} -impl PartialOrd for UseTree { - fn partial_cmp(&self, other: &UseTree) -> Option { - Some(self.cmp(other)) - } -} -impl Ord for UseSegment { - fn cmp(&self, other: &UseSegment) -> Ordering { - use self::UseSegment::*; - - match (self, other) { - (&Slf(ref a), &Slf(ref b)) | (&Super(ref a), &Super(ref b)) => a.cmp(b), - (&Glob, &Glob) => Ordering::Equal, - (&Ident(ref ia, ref aa), &Ident(ref ib, ref ab)) => { - let ident_ord = ia.cmp(ib); - if ident_ord != Ordering::Equal { - return ident_ord; - } - if aa.is_none() && ab.is_some() { - return Ordering::Less; - } - if aa.is_some() && ab.is_none() { - return Ordering::Greater; - } - aa.cmp(ab) - } - (&List(ref a), &List(ref b)) => { - for (a, b) in a.iter().zip(b.iter()) { - let ord = a.cmp(b); - if ord != Ordering::Equal { - return ord; - } - } - - a.len().cmp(&b.len()) - } - (&Slf(_), _) => Ordering::Less, - (_, &Slf(_)) => Ordering::Greater, - (&Super(_), _) => Ordering::Less, - (_, &Super(_)) => Ordering::Greater, - (&Ident(..), _) => Ordering::Less, - (_, &Ident(..)) => Ordering::Greater, - (&Glob, _) => Ordering::Less, - (_, &Glob) => Ordering::Greater, - } - } -} -impl Ord for UseTree { - fn cmp(&self, other: &UseTree) -> Ordering { - for (a, b) in self.path.iter().zip(other.path.iter()) { - let ord = a.cmp(b); - // The comparison without aliases is a hack to avoid situations like - // comparing `a::b` to `a as c` - where the latter should be ordered - // first since it is shorter. - if ord != Ordering::Equal && a.remove_alias().cmp(&b.remove_alias()) != Ordering::Equal - { - return ord; - } - } - - self.path.len().cmp(&other.path.len()) - } -} - -#[cfg(test)] -mod test { - use super::*; - - // Parse the path part of an import. This parser is not robust and is only - // suitable for use in a test harness. - fn parse_use_tree(s: &str) -> UseTree { - use std::iter::Peekable; - use std::mem::swap; - use std::str::Chars; - - struct Parser<'a> { - input: Peekable>, - } - - impl<'a> Parser<'a> { - fn bump(&mut self) { - self.input.next().unwrap(); - } - fn eat(&mut self, c: char) { - assert!(self.input.next().unwrap() == c); - } - fn push_segment( - result: &mut Vec, - buf: &mut String, - alias_buf: &mut Option, - ) { - if !buf.is_empty() { - let mut alias = None; - swap(alias_buf, &mut alias); - if buf == "self" { - result.push(UseSegment::Slf(alias)); - *buf = String::new(); - *alias_buf = None; - } else if buf == "super" { - result.push(UseSegment::Super(alias)); - *buf = String::new(); - *alias_buf = None; - } else { - let mut name = String::new(); - swap(buf, &mut name); - result.push(UseSegment::Ident(name, alias)); - } - } - } - fn parse_in_list(&mut self) -> UseTree { - let mut result = vec![]; - let mut buf = String::new(); - let mut alias_buf = None; - while let Some(&c) = self.input.peek() { - match c { - '{' => { - assert!(buf.is_empty()); - self.bump(); - result.push(UseSegment::List(self.parse_list())); - self.eat('}'); - } - '*' => { - assert!(buf.is_empty()); - self.bump(); - result.push(UseSegment::Glob); - } - ':' => { - self.bump(); - self.eat(':'); - Self::push_segment(&mut result, &mut buf, &mut alias_buf); - } - '}' | ',' => { - Self::push_segment(&mut result, &mut buf, &mut alias_buf); - return UseTree { path: result }; - } - ' ' => { - self.bump(); - self.eat('a'); - self.eat('s'); - self.eat(' '); - alias_buf = Some(String::new()); - } - c => { - self.bump(); - if let Some(ref mut buf) = alias_buf { - buf.push(c); - } else { - buf.push(c); - } - } - } - } - Self::push_segment(&mut result, &mut buf, &mut alias_buf); - UseTree { path: result } - } - - fn parse_list(&mut self) -> Vec { - let mut result = vec![]; - loop { - match self.input.peek().unwrap() { - ',' | ' ' => self.bump(), - '}' => { - return result; - } - _ => result.push(self.parse_in_list()), - } - } - } - } - - let mut parser = Parser { - input: s.chars().peekable(), - }; - parser.parse_in_list() - } - - #[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 as foo") - ); - assert_eq!(parse_use_tree("a::{self}").normalize(), parse_use_tree("a")); - assert_eq!(parse_use_tree("a::{b}").normalize(), parse_use_tree("a::b")); - assert_eq!( - parse_use_tree("a::{b, c::self}").normalize(), - 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}") - ); - } - - #[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("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize() - < parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize() - ); - assert!( - 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() - < parse_use_tree("{Bar, Baz}").normalize() - ); - - assert!( - parse_use_tree("foo::{self as bar}").normalize() - < parse_use_tree("foo::{qux as bar}").normalize() - ); - assert!( - parse_use_tree("foo::{qux as bar}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() - ); - assert!( - parse_use_tree("foo::{self as bar, baz}").normalize() - < parse_use_tree("foo::{baz, qux as bar}").normalize() - ); - - 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() - < parse_use_tree("std::cmp::{b, e, g, f}").normalize() - ); - } -} From f6c0a0f4ed16699af45af618e35ab0d18fb7d711 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 14:23:20 +0900 Subject: [PATCH 5/6] Cargo fmt --- src/attr.rs | 2 +- src/comment.rs | 2 +- src/config/lists.rs | 2 +- src/lib.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/attr.rs b/src/attr.rs index 3c4cc390e78..68d6a288896 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -10,8 +10,8 @@ //! Format attributes and meta items. -use config::IndentStyle; use config::lists::*; +use config::IndentStyle; use syntax::ast; use syntax::codemap::Span; diff --git a/src/comment.rs b/src/comment.rs index 05b423d1898..431be847678 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -10,7 +10,7 @@ // Formatting and tools for comments. -use std::{self, iter, borrow::Cow}; +use std::{self, borrow::Cow, iter}; use itertools::{multipeek, MultiPeek}; use syntax::codemap::Span; diff --git a/src/config/lists.rs b/src/config/lists.rs index 53cf7ca7083..04406e8d566 100644 --- a/src/config/lists.rs +++ b/src/config/lists.rs @@ -10,8 +10,8 @@ //! Configuration options related to rewriting a list. -use config::IndentStyle; use config::config_type::ConfigType; +use config::IndentStyle; /// The definitive formatting tactic for lists. #[derive(Eq, PartialEq, Debug, Copy, Clone)] diff --git a/src/lib.rs b/src/lib.rs index 77589038c66..31cb49dd0d5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,8 +54,8 @@ use shape::Indent; use utils::use_colored_tty; use visitor::{FmtVisitor, SnippetProvider}; -pub use config::Config; pub use config::summary::Summary; +pub use config::Config; #[macro_use] mod utils; From 78e09bd05cff61edbee139eda3c8bc57d00f68ff Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 31 Mar 2018 14:47:50 +0900 Subject: [PATCH 6/6] Cargo fmt --- src/reorder.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/reorder.rs b/src/reorder.rs index ae9cada01d2..7cff6fd5719 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -103,14 +103,7 @@ fn rewrite_reorderable_item( _ => return None, }; - combine_strs_with_missing_comments( - context, - &attrs_str, - &item_str, - missed_span, - shape, - false, - ) + combine_strs_with_missing_comments(context, &attrs_str, &item_str, missed_span, shape, false) } /// Rewrite a list of items with reordering. Every item in `items` must have