Merge pull request #2576 from topecongiro/merge-imports

Use normalized form to format use items
This commit is contained in:
Nick Cameron 2018-04-05 17:39:22 +12:00 committed by GitHub
commit b7ba6f70b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 770 additions and 783 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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)]

File diff suppressed because it is too large Load Diff

View File

@ -53,8 +53,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;

View File

@ -56,7 +56,7 @@ impl AsRef<ListItem> 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<String>,
@ -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);
}
@ -617,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 {

View File

@ -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<String> {
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,90 @@ 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<String> {
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<String> {
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 +281,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<String>),
Slf(Option<String>),
Super(Option<String>),
Glob,
List(Vec<UseTree>),
}
#[derive(Debug, Clone, Eq, PartialEq)]
struct UseTree {
path: Vec<UseSegment>,
}
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<Ordering> {
Some(self.cmp(other))
}
}
impl PartialOrd for UseTree {
fn partial_cmp(&self, other: &UseTree) -> Option<Ordering> {
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<Chars<'a>>,
}
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<UseSegment>,
buf: &mut String,
alias_buf: &mut Option<String>,
) {
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<UseTree> {
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()
);
}
}

View File

@ -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::{};

View File

@ -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 foo::{a,
b,
bar::{baz,
foo::{a, b, cxxxxxxxxxxxxx, yyyyyyyyyyyyyy, zzzzzzzzzzzzzzzz},
qux,
xxxxxxxxxxx,
yyyyyyyyyyyyy,
zzzzzzzzzzzzzzzz},
boo,
c};
use fooo::{bar, x, y, z,
baar::foobar::{xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy,
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;

View File

@ -1,8 +1,7 @@
// こんにちは
use std::borrow::Cow;
/* comment */
/* comment 1 */
/* comment 2 */
/* comment */
/* comment */
/* comment 3 */