diff --git a/Configurations.md b/Configurations.md index 2b9cd13ad33..f7fa88b61f2 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1297,28 +1297,28 @@ Reorder import statements in group **Note:** This option takes effect only when [`reorder_imports`](#reorder_imports) is set to `true`. -#### `false` (default): +#### `true` (default): ```rust -use std::mem; use std::io; +use std::mem; -use lorem; -use ipsum; use dolor; +use ipsum; +use lorem; use sit; ``` -#### `true`: +#### `false`: -```rust -use std::io; -use std::mem; +```rust use dolor; use ipsum; use lorem; use sit; +use std::io; +use std::mem; ``` See also [`reorder_imports`](#reorder_imports). @@ -1360,7 +1360,11 @@ Reorder `extern crate` statements in group - **Possible values**: `true`, `false` - **Stable**: No -#### `true` (default): +#### `false` (default): + +This value has no influence beyond the effect of the [`reorder_extern_crates`](#reorder_extern_crates) option. Set [`reorder_extern_crates`](#reorder_extern_crates) to `false` if you do not want `extern crate` groups to be collapsed and ordered. + +#### `true`: **Note:** This only takes effect when [`reorder_extern_crates`](#reorder_extern_crates) is set to `true`. @@ -1374,10 +1378,6 @@ extern crate lorem; extern crate sit; ``` -#### `false`: - -This value has no influence beyond the effect of the [`reorder_extern_crates`](#reorder_extern_crates) option. Set [`reorder_extern_crates`](#reorder_extern_crates) to `false` if you do not want `extern crate` groups to be collapsed and ordered. - ## `reorder_modules` Reorder `mod` declarations alphabetically in group. @@ -1386,7 +1386,7 @@ Reorder `mod` declarations alphabetically in group. - **Possible values**: `true`, `false` - **Stable**: No -#### `true` +#### `true` (default) ```rust mod a; diff --git a/bootstrap.sh b/bootstrap.sh index d3878d9d61c..ae37b043c05 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -7,9 +7,8 @@ cargo build --release target/release/rustfmt --write-mode=overwrite src/lib.rs -target/release/rustfmt --write-mode=overwrite src/bin/rustfmt.rs -target/release/rustfmt --write-mode=overwrite src/bin/cargo-fmt.rs -target/release/rustfmt --write-mode=overwrite tests/system.rs +target/release/rustfmt --write-mode=overwrite src/bin/main.rs +target/release/rustfmt --write-mode=overwrite src/cargo-fmt/main.rs for filename in tests/target/*.rs; do if ! grep -q "rustfmt-" "$filename"; then diff --git a/src/bin/main.rs b/src/bin/main.rs index c89c25a90fe..b59e47930e0 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -14,16 +14,16 @@ extern crate env_logger; extern crate getopts; extern crate rustfmt_nightly as rustfmt; -use std::{env, error}; use std::fs::File; use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::{env, error}; use getopts::{Matches, Options}; -use rustfmt::config::{get_toml_path, Color, Config, WriteMode}; use rustfmt::config::file_lines::FileLines; +use rustfmt::config::{get_toml_path, Color, Config, WriteMode}; use rustfmt::{run, FileName, Input, Summary}; type FmtError = Box; diff --git a/src/chains.rs b/src/chains.rs index e26f4966628..d798981415e 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -77,8 +77,8 @@ use std::borrow::Cow; use std::cmp::min; use std::iter; -use syntax::{ast, ptr}; use syntax::codemap::Span; +use syntax::{ast, ptr}; pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -> Option { debug!("rewrite_chain {:?}", shape); diff --git a/src/closures.rs b/src/closures.rs index 65052d766eb..2ec31afa391 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -9,9 +9,9 @@ // except according to those terms. use config::lists::*; -use syntax::{ast, ptr}; use syntax::codemap::Span; use syntax::parse::classify; +use syntax::{ast, ptr}; use codemap::SpanUtils; use expr::{block_contains_comment, is_simple_block, is_unsafe_block, rewrite_cond, ToExpr}; diff --git a/src/codemap.rs b/src/codemap.rs index f6c05f5be40..c5f24004fcf 100644 --- a/src/codemap.rs +++ b/src/codemap.rs @@ -12,8 +12,8 @@ //! This includes extension traits and methods for looking up spans and line ranges for AST nodes. use config::file_lines::LineRange; -use visitor::SnippetProvider; use syntax::codemap::{BytePos, CodeMap, Span}; +use visitor::SnippetProvider; use comment::FindUncommented; diff --git a/src/config/file_lines.rs b/src/config/file_lines.rs index 714f33c03dc..cf3f827c00d 100644 --- a/src/config/file_lines.rs +++ b/src/config/file_lines.rs @@ -10,9 +10,9 @@ //! This module contains types and functions to support formatting specific line ranges. -use std::{cmp, iter, str}; use std::collections::HashMap; use std::rc::Rc; +use std::{cmp, iter, str}; use serde::de::{Deserialize, Deserializer}; use serde_json as json; diff --git a/src/config/license.rs b/src/config/license.rs index b2babd5ac19..d49fdbe7eba 100644 --- a/src/config/license.rs +++ b/src/config/license.rs @@ -1,6 +1,6 @@ -use std::io; use std::fmt; use std::fs::File; +use std::io; use std::io::Read; use regex; diff --git a/src/config/mod.rs b/src/config/mod.rs index 301b9e06b01..5d5e91ebaed 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -8,12 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::{env, fs}; use std::cell::Cell; use std::default::Default; use std::fs::File; use std::io::{Error, ErrorKind, Read}; use std::path::{Path, PathBuf}; +use std::{env, fs}; use regex::Regex; @@ -23,9 +23,9 @@ mod config_type; mod options; pub mod file_lines; +pub mod license; pub mod lists; pub mod summary; -pub mod license; use config::config_type::ConfigType; use config::file_lines::FileLines; @@ -70,11 +70,11 @@ create_config! { // Ordering reorder_extern_crates: bool, true, false, "Reorder extern crate statements alphabetically"; reorder_extern_crates_in_group: bool, true, false, "Reorder extern crate statements in group"; - reorder_imports: bool, false, false, "Reorder import statements alphabetically"; - reorder_imports_in_group: bool, false, false, "Reorder import statements in group"; + reorder_imports: bool, true, false, "Reorder import statements alphabetically"; + reorder_imports_in_group: bool, true, false, "Reorder import statements in group"; reorder_imported_names: bool, true, false, "Reorder lists of names in import statements alphabetically"; - reorder_modules: bool, false, false, "Reorder module statemtents alphabetically in group"; + reorder_modules: bool, true, false, "Reorder module statemtents alphabetically in group"; // Spaces around punctuation binop_separator: SeparatorPlace, SeparatorPlace::Front, false, diff --git a/src/config/summary.rs b/src/config/summary.rs index b0be5678a0b..99bf752f2b9 100644 --- a/src/config/summary.rs +++ b/src/config/summary.rs @@ -8,8 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::time::{Duration, Instant}; use std::default::Default; +use std::time::{Duration, Instant}; #[must_use] #[derive(Debug, Default, Clone, Copy)] diff --git a/src/expr.rs b/src/expr.rs index e144c8307b6..d364da20128 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -13,8 +13,8 @@ use std::cmp::min; use std::iter::repeat; use config::lists::*; -use syntax::{ast, ptr}; use syntax::codemap::{BytePos, CodeMap, Span}; +use syntax::{ast, ptr}; use chains::rewrite_chain; use closures; diff --git a/src/format-diff/main.rs b/src/format-diff/main.rs index 6633da208e8..c871395f72a 100644 --- a/src/format-diff/main.rs +++ b/src/format-diff/main.rs @@ -23,10 +23,10 @@ extern crate regex; extern crate serde_derive; extern crate serde_json as json; -use std::{env, fmt, process}; use std::collections::HashSet; use std::error::Error; use std::io::{self, BufRead}; +use std::{env, fmt, process}; use regex::Regex; diff --git a/src/items.rs b/src/items.rs index 6306767f00c..2e0e869d12a 100644 --- a/src/items.rs +++ b/src/items.rs @@ -15,9 +15,9 @@ use std::cmp::min; use config::lists::*; use regex::Regex; -use syntax::{abi, ast, ptr, symbol}; use syntax::codemap::{self, BytePos, Span}; use syntax::visit; +use syntax::{abi, ast, ptr, symbol}; use codemap::{LineRangeUtils, SpanUtils}; use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed, @@ -26,8 +26,8 @@ use config::{BraceStyle, Config, Density, IndentStyle}; use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs, rewrite_assign_rhs_with, ExprType, RhsTactics}; use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator}; -use rewrite::{Rewrite, RewriteContext}; use overflow; +use rewrite::{Rewrite, RewriteContext}; use shape::{Indent, Shape}; use spanned::Spanned; use types::TraitTyParamBounds; diff --git a/src/lib.rs b/src/lib.rs index eff414b5708..4fdb7d0f47d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,11 +38,11 @@ use std::path::PathBuf; use std::rc::Rc; use std::time::Duration; -use syntax::errors::{DiagnosticBuilder, Handler}; -use syntax::errors::emitter::{ColorConfig, EmitterWriter}; use syntax::ast; -use syntax::codemap::{CodeMap, FilePathMapping}; pub use syntax::codemap::FileName; +use syntax::codemap::{CodeMap, FilePathMapping}; +use syntax::errors::emitter::{ColorConfig, EmitterWriter}; +use syntax::errors::{DiagnosticBuilder, Handler}; use syntax::parse::{self, ParseSess}; use checkstyle::{output_footer, output_header}; diff --git a/src/lists.rs b/src/lists.rs index 49797a785bb..fdb022db077 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -56,7 +56,7 @@ impl AsRef for ListItem { } } -#[derive(PartialEq, Eq)] +#[derive(PartialEq, Eq, Debug)] pub enum ListItemCommentStyle { // Try to keep the comment on the same line with the item. SameLine, @@ -66,6 +66,7 @@ pub enum ListItemCommentStyle { None, } +#[derive(Debug)] pub struct ListItem { // None for comments mean that they are not present. pub pre_comment: Option, @@ -118,6 +119,18 @@ impl ListItem { new_lines: false, } } + + // true if the item causes something to be written. + fn is_substantial(&self) -> bool { + fn empty(s: &Option) -> bool { + match *s { + Some(ref s) if !s.is_empty() => false, + _ => true, + } + } + + !(empty(&self.pre_comment) && empty(&self.item) && empty(&self.post_comment)) + } } /// The type of separator for lists. @@ -220,6 +233,10 @@ where item_last_line_width -= indent_str.len(); } + if !item.is_substantial() { + continue; + } + match tactic { DefinitiveListTactic::Horizontal if !first => { result.push(' '); @@ -276,26 +293,28 @@ where rewrite_comment(comment, block_mode, formatting.shape, formatting.config)?; result.push_str(&comment); - if tactic == DefinitiveListTactic::Vertical { - // We cannot keep pre-comments on the same line if the comment if normalized. - let keep_comment = if formatting.config.normalize_comments() - || item.pre_comment_style == ListItemCommentStyle::DifferentLine - { - false + if !inner_item.is_empty() { + if tactic == DefinitiveListTactic::Vertical { + // We cannot keep pre-comments on the same line if the comment if normalized. + let keep_comment = if formatting.config.normalize_comments() + || item.pre_comment_style == ListItemCommentStyle::DifferentLine + { + false + } else { + // We will try to keep the comment on the same line with the item here. + // 1 = ` ` + let total_width = total_item_width(item) + item_sep_len + 1; + total_width <= formatting.shape.width + }; + if keep_comment { + result.push(' '); + } else { + result.push('\n'); + result.push_str(indent_str); + } } else { - // We will try to keep the comment on the same line with the item here. - // 1 = ` ` - let total_width = total_item_width(item) + item_sep_len + 1; - total_width <= formatting.shape.width - }; - if keep_comment { result.push(' '); - } else { - result.push('\n'); - result.push_str(indent_str); } - } else { - result.push(' '); } item_max_width = None; } @@ -304,7 +323,7 @@ where result.push_str(formatting.separator.trim()); result.push(' '); } - result.push_str(&inner_item[..]); + result.push_str(inner_item); // Post-comments if tactic != DefinitiveListTactic::Vertical && item.post_comment.is_some() { diff --git a/src/macros.rs b/src/macros.rs index 784e6c5798b..ec1ca29d173 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -22,7 +22,6 @@ use std::collections::HashMap; use config::lists::*; -use syntax::{ast, ptr}; use syntax::codemap::{BytePos, Span}; use syntax::parse::new_parser_from_tts; use syntax::parse::parser::Parser; @@ -31,6 +30,7 @@ use syntax::print::pprust; use syntax::symbol; use syntax::tokenstream::{Cursor, ThinTokenStream, TokenStream, TokenTree}; use syntax::util::ThinVec; +use syntax::{ast, ptr}; use codemap::SpanUtils; use comment::{contains_comment, remove_trailing_white_spaces, CharClasses, FindUncommented, diff --git a/src/missed_spans.rs b/src/missed_spans.rs index dff6b94bd75..f5794c65c4c 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -104,19 +104,38 @@ impl<'a> FmtVisitor<'a> { } fn push_vertical_spaces(&mut self, mut newline_count: usize) { - // The buffer already has a trailing newline. - let offset = if self.buffer.ends_with('\n') { 0 } else { 1 }; - let newline_upper_bound = self.config.blank_lines_upper_bound() + offset; - let newline_lower_bound = self.config.blank_lines_lower_bound() + offset; - if newline_count > newline_upper_bound { - newline_count = newline_upper_bound; - } else if newline_count < newline_lower_bound { - newline_count = newline_lower_bound; + let offset = self.count_trailing_newlines(); + let newline_upper_bound = self.config.blank_lines_upper_bound() + 1; + let newline_lower_bound = self.config.blank_lines_lower_bound() + 1; + + if newline_count + offset > newline_upper_bound { + if offset >= newline_upper_bound { + newline_count = 0; + } else { + newline_count = newline_upper_bound - offset; + } + } else if newline_count + offset < newline_lower_bound { + if offset >= newline_lower_bound { + newline_count = 0; + } else { + newline_count = newline_lower_bound - offset; + } } + let blank_lines: String = repeat('\n').take(newline_count).collect(); self.push_str(&blank_lines); } + fn count_trailing_newlines(&self) -> usize { + let mut buf = &*self.buffer; + let mut result = 0; + while buf.ends_with('\n') { + buf = &buf[..buf.len() - 1]; + result += 1; + } + result + } + fn write_snippet(&mut self, span: Span, process_last_snippet: F) where F: Fn(&mut FmtVisitor, &str, &str), diff --git a/src/overflow.rs b/src/overflow.rs index 294c257d31f..c76b67c461e 100644 --- a/src/overflow.rs +++ b/src/overflow.rs @@ -17,9 +17,9 @@ use syntax::codemap::Span; use closures; use codemap::SpanUtils; +use expr::{is_nested_call, maybe_get_args_offset, ToExpr}; use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator}; use rewrite::{Rewrite, RewriteContext}; -use expr::{is_nested_call, maybe_get_args_offset, ToExpr}; use shape::Shape; use spanned::Spanned; use utils::{count_newlines, extra_offset, first_line_width, last_line_width, mk_sp, paren_overhead}; diff --git a/src/reorder.rs b/src/reorder.rs index a43f56d595b..5595fa5b237 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -17,6 +17,7 @@ // TODO(#2455): Reorder trait items. use config::{Config, lists::*}; +use syntax::ast::UseTreeKind; use syntax::{ast, attr, codemap::Span}; use attr::filter_inline_attrs; @@ -31,86 +32,12 @@ use spanned::Spanned; use utils::mk_sp; use visitor::FmtVisitor; -use std::cmp::Ordering; +use std::cmp::{Ord, Ordering, PartialOrd}; -fn compare_path_segments(a: &ast::PathSegment, b: &ast::PathSegment) -> Ordering { - a.identifier.name.as_str().cmp(&b.identifier.name.as_str()) -} - -fn compare_paths(a: &ast::Path, b: &ast::Path) -> Ordering { - for segment in a.segments.iter().zip(b.segments.iter()) { - let ord = compare_path_segments(segment.0, segment.1); - if ord != Ordering::Equal { - return ord; - } - } - a.segments.len().cmp(&b.segments.len()) -} - -fn compare_use_trees(a: &ast::UseTree, b: &ast::UseTree, nested: bool) -> Ordering { - use ast::UseTreeKind::*; - - // `use_nested_groups` is not yet supported, remove the `if !nested` when support will be - // fully added - if !nested { - let paths_cmp = compare_paths(&a.prefix, &b.prefix); - if paths_cmp != Ordering::Equal { - return paths_cmp; - } - } - - match (&a.kind, &b.kind) { - (&Simple(ident_a), &Simple(ident_b)) => { - let name_a = &*path_to_imported_ident(&a.prefix).name.as_str(); - let name_b = &*path_to_imported_ident(&b.prefix).name.as_str(); - let name_ordering = if name_a == "self" { - if name_b == "self" { - Ordering::Equal - } else { - Ordering::Less - } - } else if name_b == "self" { - Ordering::Greater - } else { - name_a.cmp(name_b) - }; - if name_ordering == Ordering::Equal { - if ident_a.name.as_str() != name_a { - if ident_b.name.as_str() != name_b { - ident_a.name.as_str().cmp(&ident_b.name.as_str()) - } else { - Ordering::Greater - } - } else { - Ordering::Less - } - } else { - name_ordering - } - } - (&Glob, &Glob) => Ordering::Equal, - (&Simple(_), _) | (&Glob, &Nested(_)) => Ordering::Less, - (&Nested(ref a_items), &Nested(ref b_items)) => { - let mut a = a_items - .iter() - .map(|&(ref tree, _)| tree.clone()) - .collect::>(); - let mut b = b_items - .iter() - .map(|&(ref tree, _)| tree.clone()) - .collect::>(); - a.sort_by(|a, b| compare_use_trees(a, b, true)); - b.sort_by(|a, b| compare_use_trees(a, b, true)); - for comparison_pair in a.iter().zip(b.iter()) { - let ord = compare_use_trees(comparison_pair.0, comparison_pair.1, true); - if ord != Ordering::Equal { - return ord; - } - } - a.len().cmp(&b.len()) - } - (&Glob, &Simple(_)) | (&Nested(_), _) => Ordering::Greater, - } +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) } /// Choose the ordering between the given two items. @@ -120,7 +47,7 @@ fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { 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, false) + compare_use_trees(a_tree, b_tree) } (&ast::ItemKind::ExternCrate(ref a_name), &ast::ItemKind::ExternCrate(ref b_name)) => { // `extern crate foo as bar;` @@ -149,8 +76,6 @@ 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`. -// TODO (some day) remove unused imports, expand globs, compress many single -// imports into a list import. fn rewrite_reorderable_items( context: &RewriteContext, reorderable_items: &[&ast::Item], @@ -196,6 +121,7 @@ fn rewrite_reorderable_items( 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(); @@ -329,3 +255,399 @@ 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(), + } + } +} + +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::Ident( + (*p.identifier.name.as_str()).to_owned(), + None, + )); + } + 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 = if &name == &*rename.name.as_str() { + None + } else { + Some((&*rename.name.as_str()).to_owned()) + }; + + 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() + ); + } +} diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index f9919157620..db72a775f41 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -12,8 +12,8 @@ use config::Color; use diff; use std::collections::VecDeque; use std::io; -use term; use std::io::Write; +use term; use utils::use_colored_tty; #[derive(Debug, PartialEq)] @@ -211,8 +211,8 @@ where #[cfg(test)] mod test { - use super::{make_diff, Mismatch}; use super::DiffLine::*; + use super::{make_diff, Mismatch}; #[test] fn diff_simple() { diff --git a/src/utils.rs b/src/utils.rs index f6d2ff23c34..d1d526efe36 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -10,10 +10,10 @@ use std::borrow::Cow; -use syntax::{abi, ptr}; use syntax::ast::{self, Attribute, CrateSugar, MetaItem, MetaItemKind, NestedMetaItem, NestedMetaItemKind, Path, Visibility, VisibilityKind}; use syntax::codemap::{BytePos, Span, NO_EXPANSION}; +use syntax::{abi, ptr}; use config::Color; use rewrite::RewriteContext; diff --git a/src/visitor.rs b/src/visitor.rs index 81f35d787ab..54a85062d43 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use syntax::{ast, visit}; use syntax::attr::HasAttrs; use syntax::codemap::{self, BytePos, CodeMap, Pos, Span}; use syntax::parse::ParseSess; +use syntax::{ast, visit}; use attr::*; use codemap::{LineRangeUtils, SpanUtils}; diff --git a/tests/lib.rs b/tests/lib.rs index e98de537b0c..8c4d67575dc 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -23,11 +23,11 @@ use std::iter::{Enumerate, Peekable}; use std::path::{Path, PathBuf}; use std::str::Chars; -use rustfmt::*; -use rustfmt::config::{Color, Config, ReportTactic}; use rustfmt::config::summary::Summary; +use rustfmt::config::{Color, Config, ReportTactic}; use rustfmt::filemap::write_system_newlines; use rustfmt::rustfmt_diff::*; +use rustfmt::*; const DIFF_CONTEXT_SIZE: usize = 3; const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; diff --git a/tests/source/imports.rs b/tests/source/imports.rs index 73d1419f47d..cbe5f4c7bb2 100644 --- a/tests/source/imports.rs +++ b/tests/source/imports.rs @@ -21,6 +21,7 @@ use {/* Pre-comment! */ Foo, Bar /* comment */}; use Foo::{Bar, Baz}; pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath}; + use syntax::some::{}; use self; diff --git a/tests/target/imports-reorder-lines-and-items.rs b/tests/target/imports-reorder-lines-and-items.rs index f395710b186..e31819be2c0 100644 --- a/tests/target/imports-reorder-lines-and-items.rs +++ b/tests/target/imports-reorder-lines-and-items.rs @@ -2,9 +2,9 @@ // rustfmt-reorder_imported_names: true use std::cmp::{a, b, c, d}; -use std::ddd::{a, b, c as g, d as p}; use std::ddd::aaa; -// This comment should stay with `use std::ddd:bbb;` -use std::ddd::bbb; +use std::ddd::{a, b, c as g, d as p}; /// This comment should stay with `use std::str;` use std::str; +// This comment should stay with `use std::ddd:bbb;` +use std::ddd::bbb; diff --git a/tests/target/imports-reorder-lines.rs b/tests/target/imports-reorder-lines.rs index 3695d6b4913..2aeb8fadd2c 100644 --- a/tests/target/imports-reorder-lines.rs +++ b/tests/target/imports-reorder-lines.rs @@ -2,29 +2,29 @@ use std::cmp::{a, b, c, d}; use std::cmp::{b, e, f, g}; +use std::ddd::aaa; +use std::str; // This comment should stay with `use std::ddd;` use std::ddd; -use std::ddd::aaa; use std::ddd::bbb; -use std::str; mod test {} use aaa; -use aaa::*; use aaa::bbb; +use aaa::*; mod test {} // If item names are equal, order by rename -use test::{a as aa, c}; use test::{a as bb, b}; +use test::{a as aa, c}; mod test {} // If item names are equal, order by rename - no rename comes before a rename -use test::{a, c}; use test::{a as bb, b}; +use test::{a, c}; mod test {} // `self` always comes first diff --git a/tests/target/imports.rs b/tests/target/imports.rs index 2b825526199..af5f1ee7bcf 100644 --- a/tests/target/imports.rs +++ b/tests/target/imports.rs @@ -4,11 +4,11 @@ // Imports. // Long import. -use syntax::ast::{ItemDefaultImpl, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic}; -use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, - ItemB}; use exceedingly::loooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, ItemB}; +use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, + ItemB}; +use syntax::ast::{ItemDefaultImpl, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic}; use list::{// Another item AnotherItem, // Another Comment @@ -19,10 +19,10 @@ use list::{// Another item use test::{/* A */ self /* B */, Other /* C */}; -use syntax; -use {Bar /* comment */, /* Pre-comment! */ Foo}; use Foo::{Bar, Baz}; +use syntax; pub use syntax::ast::{Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath, Expr_}; +use {Bar /* comment */, /* Pre-comment! */ Foo}; use self; use std::io; @@ -43,21 +43,21 @@ fn test() { } // Simple imports -use foo::bar::baz; use bar::quux as kaas; use foo; +use foo::bar::baz; // With aliases. -use foo::{self as bar, baz}; use foo as bar; use foo::qux as bar; +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 @@ -83,6 +83,6 @@ use fooo::{bar, x, y, z, bar::*}; // nested imports with a single sub-tree. -use a::b::c::*; use a::b::c::d; +use a::b::c::*; use a::b::c::{xxx, yyy, zzz}; diff --git a/tests/target/issue-1124.rs b/tests/target/issue-1124.rs index d34870b2a28..ca77a761fb4 100644 --- a/tests/target/issue-1124.rs +++ b/tests/target/issue-1124.rs @@ -14,7 +14,9 @@ mod a { use d; } +use z; + +use y; + use a; use x; -use y; -use z; diff --git a/tests/target/issue-2256.rs b/tests/target/issue-2256.rs index 4b546223f94..d4e594515b3 100644 --- a/tests/target/issue-2256.rs +++ b/tests/target/issue-2256.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; /* comment */ + /* comment */ /* comment */ diff --git a/tests/target/skip.rs b/tests/target/skip.rs index 28897ade56f..11d1d69e91a 100644 --- a/tests/target/skip.rs +++ b/tests/target/skip.rs @@ -59,7 +59,7 @@ fn skip_on_statements() { // Item #[cfg_attr(rustfmt, rustfmt_skip)] - use foobar ; + use foobar; // Mac #[cfg_attr(rustfmt, rustfmt_skip)]