From 2fa6220f57228d149f99272a4b3f97b560359738 Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Sun, 19 Jul 2015 23:39:48 +0200 Subject: [PATCH] Format all the loops! --- src/expr.rs | 176 ++++++++++++++++++++++++++++++++++--------- src/lib.rs | 7 +- src/lists.rs | 9 ++- src/visitor.rs | 2 +- tests/source/loop.rs | 9 +++ tests/target/loop.rs | 9 +++ 6 files changed, 171 insertions(+), 41 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 68b780b414a..35bd90f5f32 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -56,21 +56,45 @@ impl Rewrite for ast::Expr { ast::Expr_::ExprTup(ref items) => { rewrite_tuple_lit(context, items, self.span, width, offset) } - ast::Expr_::ExprWhile(ref subexpr, ref block, label) => { - let label_string = rewrite_label(label); - // 6 = "while " - // 2 = " {" - let expr_width = width - 6 - 2 - label_string.len(); - let expr_offset = offset + 6 + label_string.len(); - - subexpr.rewrite(context, expr_width, expr_offset).and_then(|expr_string| { - // FIXME: this drops any comment between "loop" and the block. - block.rewrite(context, width, offset).map(|result| { - format!("{}while {} {}", rewrite_label(label), expr_string, result) - }) - }) + ast::Expr_::ExprWhile(ref cond, ref block, label) => { + rewrite_loop(context, + cond, + block, + label, + None, + "while ", + "let ", + " = ", + width, + offset) + } + ast::Expr_::ExprWhileLet(ref pat, ref cond, ref block, label) => { + rewrite_loop(context, + cond, + block, + label, + Some(pat), + "while ", + "let ", + " = ", + width, + offset) + } + ast::Expr_::ExprForLoop(ref pat, ref cond, ref block, label) => { + rewrite_loop(context, + cond, + block, + label, + Some(pat), + "for ", + "", + " in ", + width, + offset) } ast::Expr_::ExprLoop(ref block, label) => { + // Of all the loops, this is the only one that does not use + // rewrite_loop! // FIXME: this drops any comment between "loop" and the block. block.rewrite(context, width, offset).map(|result| { format!("{}loop {}", rewrite_label(label), result) @@ -97,6 +121,14 @@ impl Rewrite for ast::Expr { width, offset) } + // We reformat it ourselves because rustc gives us a bad span for ranges + ast::Expr_::ExprRange(ref left, ref right) => { + rewrite_range(context, + left.as_ref().map(|e| &**e), + right.as_ref().map(|e| &**e), + width, + offset) + } _ => context.codemap.span_to_snippet(self.span).ok() } } @@ -136,6 +168,61 @@ fn rewrite_label(label: Option) -> String { } } +// FIXME: this doesn't play well with line breaks +fn rewrite_range(context: &RewriteContext, + left: Option<&ast::Expr>, + right: Option<&ast::Expr>, + width: usize, + offset: usize) + -> Option { + let left_string = match left { + // 2 = .. + Some(expr) => try_opt!(expr.rewrite(context, width - 2, offset)), + None => String::new() + }; + + let right_string = match right { + Some(expr) => { + // 2 = .. + let max_width = (width - 2).checked_sub(left_string.len()).unwrap_or(0); + try_opt!(expr.rewrite(context, max_width, offset + 2 + left_string.len())) + } + None => String::new() + }; + + Some(format!("{}..{}", left_string, right_string)) +} + +fn rewrite_loop(context: &RewriteContext, + cond: &ast::Expr, + block: &ast::Block, + label: Option, + pat: Option<&ast::Pat>, + keyword: &str, + matcher: &str, // FIXME: think of better identifiers + connector: &str, + width: usize, + offset: usize) + -> Option { + let label_string = rewrite_label(label); + // 2 = " {" + let inner_width = width - keyword.len() - 2 - label_string.len(); + let inner_offset = offset + keyword.len() + label_string.len(); + + let pat_expr_string = try_opt!(rewrite_pat_expr(context, + pat, + cond, + matcher, + connector, + inner_width, + inner_offset)); + + // FIXME: this drops any comment between "loop" and the block. + block.rewrite(context, width, offset).map(|result| { + format!("{}{}{} {}", label_string, keyword, pat_expr_string, result) + }) +} + fn rewrite_if_else(context: &RewriteContext, cond: &ast::Expr, if_block: &ast::Block, @@ -144,30 +231,17 @@ fn rewrite_if_else(context: &RewriteContext, width: usize, offset: usize) -> Option { - // FIXME: missing comments between control statements and blocks // 3 = "if ", 2 = " {" - let pat_string = match pat { - Some(pat) => { - // 7 = "let ".len() + " = ".len() - // 4 = "let ".len() - let pat_string = try_opt!(pat.rewrite(context, width - 3 - 2 - 7, offset + 3 + 4)); - format!("let {} = ", pat_string) - } - None => String::new() - }; + let pat_expr_string = try_opt!(rewrite_pat_expr(context, + pat, + cond, + "let ", + " = ", + width - 3 - 2, + offset + 3)); - // Consider only the last line of the pat string - let extra_offset = match pat_string.rfind('\n') { - // 1 for newline character - Some(idx) => pat_string.len() - idx - 1 - offset, - None => 3 + pat_string.len() - }; - - let cond_string = try_opt!(cond.rewrite(context, - width - extra_offset - 2, - offset + extra_offset)); let if_block_string = try_opt!(if_block.rewrite(context, width, offset)); - let mut result = format!("if {}{} {}", pat_string, cond_string, if_block_string); + let mut result = format!("if {} {}", pat_expr_string, if_block_string); if let Some(else_block) = else_block { let else_block_string = try_opt!(else_block.rewrite(context, width, offset)); @@ -179,6 +253,40 @@ fn rewrite_if_else(context: &RewriteContext, Some(result) } +fn rewrite_pat_expr(context: &RewriteContext, + pat: Option<&ast::Pat>, + expr: &ast::Expr, + matcher: &str, + connector: &str, + width: usize, + offset: usize) + -> Option { + let mut result = match pat { + Some(pat) => { + let pat_string = try_opt!(pat.rewrite(context, + width - connector.len() - matcher.len(), + offset + matcher.len())); + format!("{}{}{}", matcher, pat_string, connector) + } + None => String::new() + }; + + // Consider only the last line of the pat string + let extra_offset = match result.rfind('\n') { + // 1 for newline character + Some(idx) => result.len() - idx - 1 - offset, + None => result.len() + }; + + let expr_string = try_opt!(expr.rewrite(context, + width - extra_offset, + offset + extra_offset)); + + result.push_str(&expr_string); + + Some(result) +} + fn rewrite_string_lit(context: &RewriteContext, s: &str, span: Span, diff --git a/src/lib.rs b/src/lib.rs index a3a0659c292..5ea4812e144 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -218,11 +218,12 @@ fn fmt_lines(changes: &mut ChangeSet, config: &Config) -> FormatReport { let mut cur_line = 1; let mut newline_count = 0; let mut errors = vec![]; - let mut issue_seeker = BadIssueSeeker::new(config.report_todo, - config.report_fixme); + let mut issue_seeker = BadIssueSeeker::new(config.report_todo, config.report_fixme); for (c, b) in text.chars() { - if c == '\r' { continue; } + if c == '\r' { + continue; + } // Add warnings for bad todos/ fixmes if let Some(issue) = issue_seeker.inspect(c) { diff --git a/src/lists.rs b/src/lists.rs index 155c8ab08cb..13c0cf763e3 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -136,7 +136,11 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St let first = i == 0; let last = i == items.len() - 1; let separate = !last || trailing_separator; - let item_sep_len = if separate { sep_len } else { 0 }; + let item_sep_len = if separate { + sep_len + } else { + 0 + }; let item_width = item.item.len() + item_sep_len; match tactic { @@ -208,8 +212,7 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St let comment = item.post_comment.as_ref().unwrap(); // Use block-style only for the last item or multiline comments. let block_style = formatting.ends_with_newline && last || - comment.trim().contains('\n') || - comment.trim().len() > width; + comment.trim().contains('\n') || comment.trim().len() > width; let formatted_comment = rewrite_comment(comment, block_style, width, offset); diff --git a/src/visitor.rs b/src/visitor.rs index ae5b2083ccc..ee5688c14a1 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -353,7 +353,7 @@ impl<'a> FmtVisitor<'a> { result.push_str(&a_str); - if i < attrs.len() -1 { + if i < attrs.len() - 1 { result.push('\n'); } } diff --git a/tests/source/loop.rs b/tests/source/loop.rs index 143a3a23760..36e6ab43de8 100644 --- a/tests/source/loop.rs +++ b/tests/source/loop.rs @@ -13,4 +13,13 @@ let x = loop { do_forever(); }; while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb { } + + 'b: for xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx in some_iter(arg1, arg2) { + // do smth + } + + while let Some(i) = x.find('s') + { + x.update(); + } } diff --git a/tests/target/loop.rs b/tests/target/loop.rs index cec8b2bff61..e1f2ccc91a4 100644 --- a/tests/target/loop.rs +++ b/tests/target/loop.rs @@ -18,4 +18,13 @@ fn main() { while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb { } + + 'b: for xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx in some_iter(arg1, + arg2) { + // do smth + } + + while let Some(i) = x.find('s') { + x.update(); + } }