From 749a9689be022f2737820a87fe09d930907435a5 Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Fri, 11 Sep 2015 00:52:57 +0200 Subject: [PATCH] Break chains that don't start with path expressions --- src/bin/rustfmt.rs | 5 ++- src/chains.rs | 99 ++++++++++++++++++++++------------------------ src/comment.rs | 1 - src/expr.rs | 12 ++---- src/lists.rs | 2 +- tests/system.rs | 3 +- 6 files changed, 56 insertions(+), 66 deletions(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index 9f2387f5c2c..9913c68cdfa 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -82,7 +82,9 @@ fn main() { } fn print_usage>(reason: S) { - println!("{}\n\r usage: rustfmt [-h Help] [--write-mode=[replace|overwrite|display|diff]] ", reason.into()); + println!("{}\n\r usage: rustfmt [-h Help] [--write-mode=[replace|overwrite|display|diff]] \ + ", + reason.into()); } fn determine_params(args: I) -> Option<(Vec, WriteMode)> @@ -123,6 +125,5 @@ fn determine_params(args: I) -> Option<(Vec, WriteMode)> return None; } - Some((rustc_args, write_mode)) } diff --git a/src/chains.rs b/src/chains.rs index 23e669d08af..4f395df0487 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -9,7 +9,7 @@ // except according to those terms. use rewrite::{Rewrite, RewriteContext}; -use utils::{span_after, make_indent, extra_offset}; +use utils::{make_indent, extra_offset}; use expr::rewrite_call; use syntax::{ast, ptr}; @@ -26,66 +26,23 @@ pub fn rewrite_chain(orig_expr: &ast::Expr, let indent = offset + context.config.tab_spaces; let max_width = try_opt!(context.config.max_width.checked_sub(indent)); - loop { - match expr.node { - ast::Expr_::ExprMethodCall(ref method_name, ref types, ref expressions) => { - // FIXME: a lot of duplication between this and the - // rewrite_method_call in expr.rs. - let new_span = mk_sp(expressions[0].span.hi, expr.span.hi); - let lo = span_after(new_span, "(", context.codemap); - let new_span = mk_sp(lo, expr.span.hi); + while let Some(pair) = pop_expr_chain(expr, orig_expr.span, context, max_width, indent) { + let (rewrite, parent_expr) = pair; - let rewrite = rewrite_method_call(method_name.node, - types, - &expressions[1..], - new_span, - context, - max_width, - indent); - rewrites.push(try_opt!(rewrite)); - expr = &expressions[0]; - } - ast::Expr_::ExprField(ref subexpr, ref field) => { - expr = subexpr; - rewrites.push(format!(".{}", field.node)); - } - ast::Expr_::ExprTupField(ref subexpr, ref field) => { - expr = subexpr; - rewrites.push(format!(".{}", field.node)); - } - _ => break, - } + rewrites.push(try_opt!(rewrite)); + expr = parent_expr; } let parent_rewrite = try_opt!(expr.rewrite(context, width, offset)); - // TODO: add exception for when rewrites.len() == 1 if rewrites.len() == 1 { let extra_offset = extra_offset(&parent_rewrite, offset); + let offset = offset + extra_offset; let max_width = try_opt!(width.checked_sub(extra_offset)); - // FIXME: massive duplication - let rerewrite = match orig_expr.node { - ast::Expr_::ExprMethodCall(ref method_name, ref types, ref expressions) => { - let new_span = mk_sp(expressions[0].span.hi, orig_expr.span.hi); - let lo = span_after(new_span, "(", context.codemap); - let new_span = mk_sp(lo, orig_expr.span.hi); - rewrite_method_call(method_name.node, - types, - &expressions[1..], - new_span, - context, - max_width, - offset + extra_offset) - } - ast::Expr_::ExprField(_, ref field) => { - Some(format!(".{}", field.node)) - } - ast::Expr_::ExprTupField(_, ref field) => { - Some(format!(".{}", field.node)) - } - _ => unreachable!(), - }; + let rerewrite = pop_expr_chain(orig_expr, orig_expr.span, context, max_width, offset) + .unwrap() + .0; return Some(format!("{}{}", parent_rewrite, try_opt!(rerewrite))); } @@ -103,6 +60,7 @@ pub fn rewrite_chain(orig_expr: &ast::Expr, // Put the first link on the same line as parent, if it fits. let first_connector = if parent_rewrite.len() + rewrites[0].len() <= width && + is_continuable(expr) && !rewrites[0].contains('\n') || parent_rewrite.len() <= context.config.tab_spaces { "" @@ -113,6 +71,43 @@ pub fn rewrite_chain(orig_expr: &ast::Expr, Some(format!("{}{}{}", parent_rewrite, first_connector, rewrites.join(&connector))) } +// Returns None when the expression is not a chainable. Otherwise, rewrites the +// outermost chain element and returns the remaining chain. +fn pop_expr_chain<'a>(expr: &'a ast::Expr, + span: Span, + context: &RewriteContext, + width: usize, + offset: usize) + -> Option<(Option, &'a ast::Expr)> { + match expr.node { + ast::Expr_::ExprMethodCall(ref method_name, ref types, ref expressions) => { + Some((rewrite_method_call(method_name.node, + types, + expressions, + span, + context, + width, + offset), + &expressions[0])) + } + ast::Expr_::ExprField(ref subexpr, ref field) => { + Some((Some(format!(".{}", field.node)), subexpr)) + } + ast::Expr_::ExprTupField(ref subexpr, ref field) => { + Some((Some(format!(".{}", field.node)), subexpr)) + } + _ => None, + } +} + +// Determines we can continue formatting a given expression on the same line. +fn is_continuable(expr: &ast::Expr) -> bool { + match expr.node { + ast::Expr_::ExprPath(..) => true, + _ => false, + } +} + fn rewrite_method_call(method_name: ast::Ident, types: &[ptr::P], args: &[ptr::P], diff --git a/src/comment.rs b/src/comment.rs index ed06ff7e5b6..313510f02b7 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -44,7 +44,6 @@ pub fn rewrite_comment(orig: &str, block_style: bool, width: usize, offset: usiz .enumerate() .map(|(i, mut line)| { line = line.trim(); - // Drop old closer. if i == line_breaks && line.ends_with("*/") && !line.starts_with("//") { line = &line[..(line.len() - 2)]; diff --git a/src/expr.rs b/src/expr.rs index 353150197b1..69fea0e5927 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -40,15 +40,6 @@ impl Rewrite for ast::Expr { } } ast::Expr_::ExprCall(ref callee, ref args) => { - // // FIXME using byte lens instead of char lens (and probably all over the place too) - // // 2 is for parens - // let max_callee_width = try_opt!(width.checked_sub(2)); - // let callee_str = try_opt!(callee.rewrite(context, max_callee_width, offset)); - - // let new_span = mk_sp(callee.span.hi, self.span.hi); - // let lo = span_after(new_span, "(", context.codemap); - // let new_span = mk_sp(lo, self.span.hi); - rewrite_call(context, &**callee, args, self.span, width, offset) } ast::Expr_::ExprParen(ref subexpr) => { @@ -927,6 +918,9 @@ fn rewrite_call_inner(context: &RewriteContext, None => return Err(Ordering::Greater), }; + let span_lo = span_after(span, "(", context.codemap); + let span = mk_sp(span_lo, span.hi); + let extra_offset = extra_offset(&callee_str, offset); // 2 is for parens. let remaining_width = match width.checked_sub(extra_offset + 2) { diff --git a/src/lists.rs b/src/lists.rs index b07b81bd672..0afee8f79ac 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -282,7 +282,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3> None }; - // Post-comment + // Post-comment let next_start = match self.inner.peek() { Some(ref next_item) => (self.get_lo)(next_item), None => self.next_span_start, diff --git a/tests/system.rs b/tests/system.rs index e77257663db..9cfe9101986 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -152,7 +152,8 @@ fn read_significant_comments(file_name: &str) -> HashMap { let regex = regex::Regex::new(&pattern).ok().expect("Failed creating pattern 1."); // Matches lines containing significant comments or whitespace. - let line_regex = regex::Regex::new(r"(^\s*$)|(^\s*//\s*rustfmt-[^:]+:\s*\S+)").ok() + let line_regex = regex::Regex::new(r"(^\s*$)|(^\s*//\s*rustfmt-[^:]+:\s*\S+)") + .ok() .expect("Failed creating pattern 2."); reader.lines()