diff --git a/src/comment.rs b/src/comment.rs index a1338d5b369..1a16d349208 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -46,9 +46,16 @@ pub fn rewrite_comment(orig: &str, block_style: bool, width: usize, offset: usiz line = &line[..(line.len() - 2)]; } - line.trim_right_matches(' ') + line.trim_right() }) .map(left_trim_comment_line) + .map(|line| { + if line_breaks == 0 { + line.trim_left() + } else { + line + } + }) .fold((true, opener.to_owned()), |(first, mut acc), line| { if !first { acc.push('\n'); @@ -98,6 +105,8 @@ fn format_comments() { * men\n \ * t */"; assert_eq!(expected_output, rewrite_comment(input, true, 9, 69)); + + assert_eq!("/* trimmed */", rewrite_comment("/* trimmed */", true, 100, 100)); } @@ -156,6 +165,7 @@ fn test_find_uncommented() { check("/*sup yo? \n sup*/ sup", "p", Some(20)); check("hel/*lohello*/lo", "hello", None); check("acb", "ab", None); + check(",/*A*/ ", ",", Some(0)); } // Returns the first byte position after the first comment. The given string diff --git a/src/config.rs b/src/config.rs index 8bcf12854a8..0ea13768457 100644 --- a/src/config.rs +++ b/src/config.rs @@ -29,6 +29,7 @@ pub struct Config { pub enum_trailing_comma: bool, pub report_todo: ReportTactic, pub report_fixme: ReportTactic, + pub reorder_imports: bool, // Alphabetically, case sensitive. } impl Config { diff --git a/src/default.toml b/src/default.toml index 2d5ba9c03ee..951fde9d665 100644 --- a/src/default.toml +++ b/src/default.toml @@ -11,3 +11,4 @@ struct_lit_trailing_comma = "Vertical" enum_trailing_comma = true report_todo = "Always" report_fixme = "Never" +reorder_imports = false diff --git a/src/imports.rs b/src/imports.rs index de33be849ba..535e409288b 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -9,12 +9,13 @@ // except according to those terms. use visitor::FmtVisitor; -use lists::{write_list, ListItem, ListFormatting, SeparatorTactic, ListTactic}; -use utils::format_visibility; +use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic}; +use utils::{span_after, format_visibility}; use syntax::ast; use syntax::parse::token; use syntax::print::pprust; +use syntax::codemap::Span; // TODO (some day) remove unused imports, expand globs, compress many single imports into a list import @@ -40,13 +41,14 @@ fn rewrite_single_use_list(path_str: String, vpi: ast::PathListItem, vis: &str) impl<'a> FmtVisitor<'a> { // Basically just pretty prints a multi-item import. // Returns None when the import can be removed. - pub fn rewrite_use_list(&mut self, + pub fn rewrite_use_list(&self, block_indent: usize, one_line_budget: usize, // excluding indentation multi_line_budget: usize, path: &ast::Path, path_list: &[ast::PathListItem], - visibility: ast::Visibility) -> Option { + visibility: ast::Visibility, + span: Span) -> Option { let path_str = pprust::path_to_string(path); let vis = format_visibility(visibility); @@ -78,34 +80,53 @@ impl<'a> FmtVisitor<'a> { ends_with_newline: true, }; - // TODO handle any comments inbetween items. - // If `self` is in the list, put it first. - let head = if path_list.iter().any(|vpi| - if let ast::PathListItem_::PathListMod{ .. } = vpi.node { - true - } else { - false - } - ) { - Some(ListItem::from_str("self")) - } else { - None - }; + let mut items = itemize_list(self.codemap, + vec![ListItem::from_str("")], // Dummy value, explanation below + path_list.iter(), + ",", + "}", + |vpi| vpi.span.lo, + |vpi| vpi.span.hi, + |vpi| match vpi.node { + ast::PathListItem_::PathListIdent{ name, .. } => { + token::get_ident(name).to_string() + } + ast::PathListItem_::PathListMod{ .. } => { + "self".to_owned() + } + }, + span_after(span, "{", self.codemap), + span.hi); - let items: Vec<_> = head.into_iter().chain(path_list.iter().filter_map(|vpi| { - match vpi.node { - ast::PathListItem_::PathListIdent{ name, .. } => { - Some(ListItem::from_str(token::get_ident(name).to_string())) - } - // Skip `self`, because we added it above. - ast::PathListItem_::PathListMod{ .. } => None, - } - })).collect(); + // 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. + // FIXME: Make more efficient by using a linked list? That would + // require changes to the signatures of itemize_list and write_list. + let has_self = move_self_to_front(&mut items); + let first_index = if has_self { 0 } else { 1 }; + + if self.config.reorder_imports { + items.tail_mut().sort_by(|a, b| a.item.cmp(&b.item)); + } + + let list = write_list(&items[first_index..], &fmt); Some(if path_str.len() == 0 { - format!("{}use {{{}}};", vis, write_list(&items, &fmt)) + format!("{}use {{{}}};", vis, list) } else { - format!("{}use {}::{{{}}};", vis, path_str, write_list(&items, &fmt)) + format!("{}use {}::{{{}}};", vis, path_str, list) }) } } + +// Returns true when self item was found. +fn move_self_to_front(items: &mut Vec) -> bool { + match items.iter().position(|item| item.item == "self") { + Some(pos) => { + items[0] = items.remove(pos); + true + }, + None => false + } +} diff --git a/src/lib.rs b/src/lib.rs index 5ee68e70cf5..e0e72f90e6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ #![feature(rustc_private)] #![feature(str_escape)] #![feature(str_char)] +#![feature(slice_extras)] // TODO we're going to allocate a whole bunch of temp Strings, is it worth // keeping some scratch mem for this and running our own StrPool? diff --git a/src/lists.rs b/src/lists.rs index adabb00566c..87a71578e96 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -14,7 +14,6 @@ use syntax::codemap::{self, CodeMap, BytePos}; use utils::{round_up_to_power_of_two, make_indent}; use comment::{FindUncommented, rewrite_comment, find_comment_end}; -use string::before; #[derive(Eq, PartialEq, Debug, Copy, Clone)] pub enum ListTactic { @@ -66,6 +65,10 @@ impl ListItem { self.post_comment.as_ref().map(|s| s.contains('\n')).unwrap_or(false) } + pub fn has_line_pre_comment(&self) -> bool { + self.pre_comment.as_ref().map_or(false, |comment| comment.starts_with("//")) + } + pub fn from_str>(s: S) -> ListItem { ListItem { pre_comment: None, @@ -115,7 +118,7 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St } // Switch to vertical mode if we find non-block comments. - if items.iter().any(has_line_pre_comment) { + if items.iter().any(ListItem::has_line_pre_comment) { tactic = ListTactic::Vertical; } @@ -223,13 +226,6 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St result } -fn has_line_pre_comment(item: &ListItem) -> bool { - match item.pre_comment { - Some(ref comment) => comment.starts_with("//"), - None => false - } -} - // Turns a list into a vector of items with associated comments. // TODO: we probably do not want to take a terminator any more. Instead, we // should demand a proper span end. @@ -250,6 +246,8 @@ pub fn itemize_list(codemap: &CodeMap, F3: Fn(&T) -> String { let mut result = prefix; + result.reserve(it.size_hint().0); + let mut new_it = it.peekable(); let white_space: &[_] = &[' ', '\t']; @@ -276,14 +274,27 @@ pub fn itemize_list(codemap: &CodeMap, let comment_end = match new_it.peek() { Some(..) => { - if let Some(start) = before(&post_snippet, "/*", "\n") { + let block_open_index = post_snippet.find("/*"); + let newline_index = post_snippet.find('\n'); + let separator_index = post_snippet.find_uncommented(separator).unwrap(); + + match (block_open_index, newline_index) { + // Separator before comment, with the next item on same line. + // Comment belongs to next item. + (Some(i), None) if i > separator_index => { separator_index + separator.len() } + // Block-style post-comment before the separator. + (Some(i), None) => { + cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, + separator_index + separator.len()) + } // Block-style post-comment. Either before or after the separator. - cmp::max(find_comment_end(&post_snippet[start..]).unwrap() + start, - post_snippet.find_uncommented(separator).unwrap() + separator.len()) - } else if let Some(idx) = post_snippet.find('\n') { - idx + 1 - } else { - post_snippet.len() + (Some(i), Some(j)) if i < j => { + cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, + separator_index + separator.len()) + } + // Potential *single* line comment. + (_, Some(j)) => { j + 1 } + _ => post_snippet.len() } }, None => { @@ -292,6 +303,7 @@ pub fn itemize_list(codemap: &CodeMap, } }; + // Cleanup post-comment: strip separators and whitespace. prev_span_end = get_hi(&item) + BytePos(comment_end as u32); let mut post_snippet = post_snippet[..comment_end].trim(); diff --git a/src/string.rs b/src/string.rs index d474fdb468e..413237e182c 100644 --- a/src/string.rs +++ b/src/string.rs @@ -88,16 +88,3 @@ pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> String { result } - -#[inline] -// Checks if a appears before b in given string and, if so, returns the index of -// a. -// FIXME: could be more generic -pub fn before<'x>(s: &'x str, a: &str, b: &str) -> Option { - s.find(a).and_then(|i| { - match s.find(b) { - Some(j) if j <= i => None, - _ => Some(i) - } - }) -} diff --git a/src/visitor.rs b/src/visitor.rs index ebd59f0cf20..df50f6cda27 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -168,7 +168,8 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { multi_line_budget, path, path_list, - item.vis); + item.vis, + item.span); if let Some(new_str) = formatted { self.format_missing_with_indent(item.span.lo); @@ -186,9 +187,12 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { self.last_pos = item.span.hi; } ast::ViewPath_::ViewPathGlob(_) => { + self.format_missing_with_indent(item.span.lo); // FIXME convert to list? } - ast::ViewPath_::ViewPathSimple(_,_) => {} + ast::ViewPath_::ViewPathSimple(_,_) => { + self.format_missing_with_indent(item.span.lo); + } } visit::walk_item(self, item); } diff --git a/tests/config/reorder_imports.toml b/tests/config/reorder_imports.toml new file mode 100644 index 00000000000..31b0e2c6217 --- /dev/null +++ b/tests/config/reorder_imports.toml @@ -0,0 +1,14 @@ +max_width = 100 +ideal_width = 80 +leeway = 5 +tab_spaces = 4 +newline_style = "Unix" +fn_brace_style = "SameLineWhere" +fn_return_indent = "WithArgs" +fn_args_paren_newline = true +struct_trailing_comma = "Vertical" +struct_lit_trailing_comma = "Vertical" +enum_trailing_comma = true +report_todo = "Always" +report_fixme = "Never" +reorder_imports = true diff --git a/tests/config/small_tabs.toml b/tests/config/small_tabs.toml index da39ac1a8f4..35477559e43 100644 --- a/tests/config/small_tabs.toml +++ b/tests/config/small_tabs.toml @@ -11,3 +11,4 @@ struct_lit_trailing_comma = "Vertical" enum_trailing_comma = true report_todo = "Always" report_fixme = "Never" +reorder_imports = false diff --git a/tests/source/imports-reorder.rs b/tests/source/imports-reorder.rs new file mode 100644 index 00000000000..2a90c212148 --- /dev/null +++ b/tests/source/imports-reorder.rs @@ -0,0 +1,5 @@ +// rustfmt-config: reorder_imports.toml + +use path::{C,/*A*/ A, B /* B */, self /* self */}; + +use {ab, ac, aa, Z, b}; diff --git a/tests/source/imports.rs b/tests/source/imports.rs new file mode 100644 index 00000000000..3590ecae61f --- /dev/null +++ b/tests/source/imports.rs @@ -0,0 +1,41 @@ +// Imports. + +// Long import. +use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl}; +use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, + ItemB}; + +use list::{ + // Some item + SomeItem /* Comment */, /* Another item */ AnotherItem /* Another Comment */, // Last Item + LastItem +}; + +use test::{ Other /* C */ , /* A */ self /* B */ }; + +use syntax::{self}; +use {/* Pre-comment! */ + Foo, Bar /* comment */}; +use Foo::{Bar, Baz}; +pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath}; +use syntax::some::{}; + +mod Foo { + pub use syntax::ast::{ + ItemForeignMod, + ItemImpl, + ItemMac, + ItemMod, + ItemStatic, + ItemDefaultImpl + }; + + mod Foo2 { + pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, self, ItemDefaultImpl}; + } +} + +fn test() { +use Baz::*; + use Qux; +} diff --git a/tests/target/imports-reorder.rs b/tests/target/imports-reorder.rs new file mode 100644 index 00000000000..27d394238e8 --- /dev/null +++ b/tests/target/imports-reorder.rs @@ -0,0 +1,5 @@ +// rustfmt-config: reorder_imports.toml + +use path::{self /* self */, /* A */ A, B /* B */, C}; + +use {Z, aa, ab, ac, b}; diff --git a/tests/target/imports.rs b/tests/target/imports.rs index 9644c62dde7..372b4f2051f 100644 --- a/tests/target/imports.rs +++ b/tests/target/imports.rs @@ -5,7 +5,17 @@ use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDe use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, ItemB}; -use {Foo, Bar}; +use list::{// Some item + SomeItem, // Comment + // Another item + AnotherItem, // Another Comment + // Last Item + LastItem}; + +use test::{/* A */ self /* B */, Other /* C */}; + +use syntax; +use {/* Pre-comment! */ Foo, Bar /* comment */}; use Foo::{Bar, Baz}; pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath}; @@ -13,11 +23,12 @@ mod Foo { pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl}; mod Foo2 { - pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, + pub use syntax::ast::{self, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl}; } } fn test() { use Baz::*; + use Qux; }