diff --git a/src/config.rs b/src/config.rs index 51daa7a505a..9cd15bc0e33 100644 --- a/src/config.rs +++ b/src/config.rs @@ -391,6 +391,8 @@ create_config! { chain_indent: BlockIndentStyle, BlockIndentStyle::Tabbed, "Indentation of chain"; chains_overflow_last: bool, true, "Allow last call in method chain to break the line"; reorder_imports: bool, false, "Reorder import statements alphabetically"; + reorder_imported_names: bool, false, + "Reorder lists of names in import statements alphabetically"; single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else \ expressions. A value of zero means always break \ if-else expressions."; diff --git a/src/imports.rs b/src/imports.rs index 89030392774..36953463d38 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -9,13 +9,120 @@ // except according to those terms. use Indent; +use utils; +use syntax::codemap::{self, BytePos, Span}; use codemap::SpanUtils; use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, definitive_tactic}; use types::rewrite_path; use rewrite::{Rewrite, RewriteContext}; +use visitor::FmtVisitor; +use std::cmp::Ordering; -use syntax::ast; -use syntax::codemap::Span; +use syntax::{ast, ptr}; + +fn path_of(a: &ast::ViewPath_) -> &ast::Path { + match a { + &ast::ViewPath_::ViewPathSimple(_, ref p) => p, + &ast::ViewPath_::ViewPathGlob(ref p) => p, + &ast::ViewPath_::ViewPathList(ref p, _) => p, + } +} + +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_path_list_items(a: &ast::PathListItem, b: &ast::PathListItem) -> Ordering { + let name_ordering = match a.node.name() { + Some(a_name) => { + match b.node.name() { + Some(b_name) => a_name.name.as_str().cmp(&b_name.name.as_str()), + None => Ordering::Greater, + } + } + None => { + match b.node.name() { + Some(_) => Ordering::Less, + None => Ordering::Equal, + } + } + }; + if name_ordering == Ordering::Equal { + match a.node.rename() { + Some(a_rename) => { + match b.node.rename() { + Some(b_rename) => a_rename.name.as_str().cmp(&b_rename.name.as_str()), + None => Ordering::Greater, + } + } + None => { + match b.node.name() { + Some(_) => Ordering::Less, + None => Ordering::Equal, + } + } + } + } else { + name_ordering + } +} + +fn compare_path_list_item_lists(a_items: &Vec, + b_items: &Vec) + -> Ordering { + let mut a = a_items.clone(); + let mut b = b_items.clone(); + a.sort_by(|a, b| compare_path_list_items(a, b)); + b.sort_by(|a, b| compare_path_list_items(a, b)); + for comparison_pair in a.iter().zip(b.iter()) { + let ord = compare_path_list_items(comparison_pair.0, comparison_pair.1); + if ord != Ordering::Equal { + return ord; + } + } + a.len().cmp(&b.len()) +} + +fn compare_view_path_types(a: &ast::ViewPath_, b: &ast::ViewPath_) -> Ordering { + use syntax::ast::ViewPath_::*; + match (a, b) { + (&ViewPathSimple(..), &ViewPathSimple(..)) => Ordering::Equal, + (&ViewPathSimple(..), _) => Ordering::Less, + (&ViewPathGlob(_), &ViewPathSimple(..)) => Ordering::Greater, + (&ViewPathGlob(_), &ViewPathGlob(_)) => Ordering::Equal, + (&ViewPathGlob(_), &ViewPathList(..)) => Ordering::Less, + (&ViewPathList(_, ref a_items), &ViewPathList(_, ref b_items)) => { + compare_path_list_item_lists(a_items, b_items) + } + (&ViewPathList(..), _) => Ordering::Greater, + } +} + +fn compare_view_paths(a: &ast::ViewPath_, b: &ast::ViewPath_) -> Ordering { + match compare_paths(path_of(a), path_of(b)) { + Ordering::Equal => compare_view_path_types(a, b), + cmp => cmp, + } +} + +fn compare_use_items(a: &ast::Item, b: &ast::Item) -> Option { + match (&a.node, &b.node) { + (&ast::ItemKind::Use(ref a_vp), &ast::ItemKind::Use(ref b_vp)) => { + Some(compare_view_paths(&a_vp.node, &b_vp.node)) + } + _ => None, + } +} // TODO (some day) remove unused imports, expand globs, compress many single // imports into a list import. @@ -50,6 +157,63 @@ impl Rewrite for ast::ViewPath { } } +impl<'a> FmtVisitor<'a> { + pub fn format_imports(&mut self, use_items: &[ptr::P]) { + let mut last_pos = + use_items.first().map(|p_i| p_i.span.lo - BytePos(1)).unwrap_or(self.last_pos); + let prefix = codemap::mk_sp(self.last_pos, last_pos); + let mut ordered_use_items = use_items.iter() + .map(|p_i| { + let new_item = (&*p_i, last_pos); + last_pos = p_i.span.hi; + new_item + }) + .collect::>(); + // Order the imports by view-path & other import path properties + ordered_use_items.sort_by(|a, b| compare_use_items(a.0, b.0).unwrap()); + // First, output the span before the first import + self.format_missing(prefix.hi); + for ordered in ordered_use_items { + // Fake out the formatter by setting `self.last_pos` to the appropriate location before + // each item before visiting it. + self.last_pos = ordered.1; + self.visit_item(&ordered.0); + } + self.last_pos = last_pos; + } + + pub fn format_import(&mut self, vis: &ast::Visibility, vp: &ast::ViewPath, span: Span) { + let vis = utils::format_visibility(vis); + let mut offset = self.block_indent; + offset.alignment += vis.len() + "use ".len(); + // 1 = ";" + match vp.rewrite(&self.get_context(), + self.config.max_width - offset.width() - 1, + offset) { + Some(ref s) if s.is_empty() => { + // Format up to last newline + let prev_span = codemap::mk_sp(self.last_pos, source!(self, span).lo); + let span_end = match self.snippet(prev_span).rfind('\n') { + Some(offset) => self.last_pos + BytePos(offset as u32), + None => source!(self, span).lo, + }; + self.format_missing(span_end); + self.last_pos = source!(self, span).hi; + } + Some(ref s) => { + let s = format!("{}use {};", vis, s); + self.format_missing_with_indent(source!(self, span).lo); + self.buffer.push_str(&s); + self.last_pos = source!(self, span).hi; + } + None => { + self.format_missing_with_indent(source!(self, span).lo); + self.format_missing(source!(self, span).hi); + } + } + } +} + fn rewrite_single_use_list(path_str: String, vpi: &ast::PathListItem) -> String { let path_item_str = if let ast::PathListItemKind::Ident { name, .. } = vpi.node { // A name. @@ -138,7 +302,7 @@ pub fn rewrite_use_list(width: usize, let has_self = move_self_to_front(&mut items); let first_index = if has_self { 0 } else { 1 }; - if context.config.reorder_imports { + if context.config.reorder_imported_names { items[1..].sort_by(|a, b| a.item.cmp(&b.item)); } diff --git a/src/utils.rs b/src/utils.rs index 5807d4c158c..0978e552837 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -224,6 +224,13 @@ macro_rules! msg { ) } +// For format_missing and last_pos, need to use the source callsite (if applicable). +// Required as generated code spans aren't guaranteed to follow on from the last span. +macro_rules! source { + ($this:ident, $sp: expr) => { + $this.codemap.source_callsite($sp) + } +} // Wraps string-like values in an Option. Returns Some when the string adheres // to the Rewrite constraints defined for the Rewrite trait and else otherwise. diff --git a/src/visitor.rs b/src/visitor.rs index c5962e986b5..b60c5156680 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use syntax::{ast, visit}; +use syntax::{ast, ptr, visit}; use syntax::codemap::{self, CodeMap, Span, BytePos}; use syntax::parse::ParseSess; @@ -23,11 +23,10 @@ use comment::rewrite_comment; use macros::rewrite_macro; use items::{rewrite_static, rewrite_associated_type, rewrite_type_alias, format_impl, format_trait}; -// For format_missing and last_pos, need to use the source callsite (if applicable). -// Required as generated code spans aren't guaranteed to follow on from the last span. -macro_rules! source { - ($this:ident, $sp: expr) => { - $this.codemap.source_callsite($sp) +fn is_use_item(item: &ast::Item) -> bool { + match item.node { + ast::ItemKind::Use(_) => true, + _ => false, } } @@ -191,7 +190,7 @@ impl<'a> FmtVisitor<'a> { self.visit_block(b) } - fn visit_item(&mut self, item: &ast::Item) { + pub fn visit_item(&mut self, item: &ast::Item) { // This is where we bail out if there is a skip attribute. This is only // complex in the module case. It is complex because the module could be // in a seperate file and there might be attributes in both files, but @@ -502,8 +501,24 @@ impl<'a> FmtVisitor<'a> { } fn walk_mod_items(&mut self, m: &ast::Mod) { - for item in &m.items { - self.visit_item(&item); + let mut items_left: &[ptr::P] = &m.items; + while !items_left.is_empty() { + // If the next item is a `use` declaration, then extract it and any subsequent `use`s + // to be potentially reordered within `format_imports`. Otherwise, just format the + // next item for output. + if self.config.reorder_imports && is_use_item(&*items_left[0]) { + let use_item_length = + items_left.iter().take_while(|ppi| is_use_item(&***ppi)).count(); + let (use_items, rest) = items_left.split_at(use_item_length); + self.format_imports(use_items); + items_left = rest; + } else { + // `unwrap()` is safe here because we know `items_left` + // has elements from the loop condition + let (item, rest) = items_left.split_first().unwrap(); + self.visit_item(&item); + items_left = rest; + } } } @@ -547,37 +562,6 @@ impl<'a> FmtVisitor<'a> { self.format_missing(filemap.end_pos); } - fn format_import(&mut self, vis: &ast::Visibility, vp: &ast::ViewPath, span: Span) { - let vis = utils::format_visibility(vis); - let mut offset = self.block_indent; - offset.alignment += vis.len() + "use ".len(); - // 1 = ";" - match vp.rewrite(&self.get_context(), - self.config.max_width - offset.width() - 1, - offset) { - Some(ref s) if s.is_empty() => { - // Format up to last newline - let prev_span = codemap::mk_sp(self.last_pos, source!(self, span).lo); - let span_end = match self.snippet(prev_span).rfind('\n') { - Some(offset) => self.last_pos + BytePos(offset as u32), - None => source!(self, span).lo, - }; - self.format_missing(span_end); - self.last_pos = source!(self, span).hi; - } - Some(ref s) => { - let s = format!("{}use {};", vis, s); - self.format_missing_with_indent(source!(self, span).lo); - self.buffer.push_str(&s); - self.last_pos = source!(self, span).hi; - } - None => { - self.format_missing_with_indent(source!(self, span).lo); - self.format_missing(source!(self, span).hi); - } - } - } - pub fn get_context(&self) -> RewriteContext { RewriteContext { parse_session: self.parse_session, diff --git a/tests/source/imports-reorder-lines-and-items.rs b/tests/source/imports-reorder-lines-and-items.rs new file mode 100644 index 00000000000..b61f26771f6 --- /dev/null +++ b/tests/source/imports-reorder-lines-and-items.rs @@ -0,0 +1,9 @@ +// rustfmt-reorder_imports: true +// rustfmt-reorder_imported_names: true + +use std::str; +use std::cmp::{d, c, b, a}; +use std::ddd::aaa; +use std::ddd::{d as p, c as g, b, a}; +// This comment should stay with `use std::ddd:bbb;` +use std::ddd::bbb; diff --git a/tests/source/imports-reorder-lines.rs b/tests/source/imports-reorder-lines.rs new file mode 100644 index 00000000000..a855a964235 --- /dev/null +++ b/tests/source/imports-reorder-lines.rs @@ -0,0 +1,34 @@ +// rustfmt-reorder_imports: true + +use std::str; +use std::cmp::{d, c, b, a}; +use std::cmp::{b, e, g, f}; +use std::ddd::aaa; +// This comment should stay with `use std::ddd;` +use std::ddd; +use std::ddd::bbb; + +mod test { +} + +use aaa::bbb; +use aaa; +use aaa::*; + +mod test {} +// If item names are equal, order by rename + +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 as bb, b}; +use test::{a, c}; + +mod test {} +// `self` always comes first + +use test::{a as aa, c}; +use test::{self as bb, b}; diff --git a/tests/source/imports-reorder.rs b/tests/source/imports-reorder.rs index 4feb8e90561..4ad9e4b08d3 100644 --- a/tests/source/imports-reorder.rs +++ b/tests/source/imports-reorder.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imported_names: true use path::{C,/*A*/ A, B /* B */, self /* self */}; diff --git a/tests/target/imports-reorder-lines-and-items.rs b/tests/target/imports-reorder-lines-and-items.rs new file mode 100644 index 00000000000..fb2e0347aac --- /dev/null +++ b/tests/target/imports-reorder-lines-and-items.rs @@ -0,0 +1,9 @@ +// rustfmt-reorder_imports: true +// 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::str; diff --git a/tests/target/imports-reorder-lines.rs b/tests/target/imports-reorder-lines.rs new file mode 100644 index 00000000000..7c8735c2dd5 --- /dev/null +++ b/tests/target/imports-reorder-lines.rs @@ -0,0 +1,33 @@ +// rustfmt-reorder_imports: true + +use std::cmp::{d, c, b, a}; +use std::cmp::{b, e, g, f}; +// 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; + +mod test {} +// If item names are equal, order by rename + +use test::{a as aa, c}; +use test::{a as bb, b}; + +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}; + +mod test {} +// `self` always comes first + +use test::{self as bb, b}; +use test::{a as aa, c}; diff --git a/tests/target/imports-reorder.rs b/tests/target/imports-reorder.rs index 63ebbf1ec7b..32b5ee156cc 100644 --- a/tests/target/imports-reorder.rs +++ b/tests/target/imports-reorder.rs @@ -1,4 +1,4 @@ -// rustfmt-reorder_imports: true +// rustfmt-reorder_imported_names: true use path::{self /* self */, /* A */ A, B /* B */, C};