From 6e1f77664df56e3ebf21c35a5a8abb77e68d079c Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Sat, 14 Nov 2015 21:57:31 +0100 Subject: [PATCH] Unwrap match arms that are simple blocks --- src/bin/rustfmt.rs | 4 +- src/chains.rs | 16 ++----- src/expr.rs | 108 ++++++++++++++++-------------------------- src/imports.rs | 8 +--- src/items.rs | 4 +- src/lib.rs | 12 ++--- src/lists.rs | 4 +- src/patterns.rs | 8 +--- src/types.rs | 8 +--- src/utils.rs | 4 +- tests/target/match.rs | 20 ++------ 11 files changed, 63 insertions(+), 133 deletions(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index d5b34d98b2d..04251d09c74 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -118,9 +118,7 @@ fn execute() -> i32 { Operation::Stdin(input, write_mode) => { // try to read config from local directory let config = match lookup_and_read_project_file(&Path::new(".")) { - Ok((_, toml)) => { - Config::from_toml(&toml) - } + Ok((_, toml)) => Config::from_toml(&toml), Err(_) => Default::default(), }; diff --git a/src/chains.rs b/src/chains.rs index 31cd9624016..a011ad2ffef 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -171,13 +171,9 @@ fn is_block_expr(expr: &ast::Expr, repr: &str) -> bool { fn pop_expr_chain<'a>(expr: &'a ast::Expr) -> Option<&'a ast::Expr> { match expr.node { - ast::Expr_::ExprMethodCall(_, _, ref expressions) => { - Some(&expressions[0]) - } + ast::Expr_::ExprMethodCall(_, _, ref expressions) => Some(&expressions[0]), ast::Expr_::ExprTupField(ref subexpr, _) | - ast::Expr_::ExprField(ref subexpr, _) => { - Some(subexpr) - } + ast::Expr_::ExprField(ref subexpr, _) => Some(subexpr), _ => None, } } @@ -199,12 +195,8 @@ fn rewrite_chain_expr(expr: &ast::Expr, width, offset) } - ast::Expr_::ExprField(_, ref field) => { - Some(format!(".{}", field.node)) - } - ast::Expr_::ExprTupField(_, ref field) => { - Some(format!(".{}", field.node)) - } + ast::Expr_::ExprField(_, ref field) => Some(format!(".{}", field.node)), + ast::Expr_::ExprTupField(_, ref field) => Some(format!(".{}", field.node)), _ => unreachable!(), } } diff --git a/src/expr.rs b/src/expr.rs index 3247e98c13a..a0d7e08878f 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -58,9 +58,7 @@ impl Rewrite for ast::Expr { let inner_span = mk_sp(callee.span.hi, self.span.hi); rewrite_call(context, &**callee, args, inner_span, width, offset) } - ast::Expr_::ExprParen(ref subexpr) => { - rewrite_paren(context, subexpr, width, offset) - } + ast::Expr_::ExprParen(ref subexpr) => rewrite_paren(context, subexpr, width, offset), ast::Expr_::ExprBinary(ref op, ref lhs, ref rhs) => { rewrite_binary_op(context, op, lhs, rhs, width, offset) } @@ -91,9 +89,7 @@ impl Rewrite for ast::Expr { ast::Expr_::ExprLoop(ref block, label) => { Loop::new_loop(block, label).rewrite(context, width, offset) } - ast::Expr_::ExprBlock(ref block) => { - block.rewrite(context, width, offset) - } + ast::Expr_::ExprBlock(ref block) => block.rewrite(context, width, offset), ast::Expr_::ExprIf(ref cond, ref if_block, ref else_block) => { rewrite_if_else(context, cond, @@ -145,9 +141,7 @@ impl Rewrite for ast::Expr { } ast::Expr_::ExprField(..) | ast::Expr_::ExprTupField(..) | - ast::Expr_::ExprMethodCall(..) => { - rewrite_chain(self, context, width, offset) - } + ast::Expr_::ExprMethodCall(..) => rewrite_chain(self, context, width, offset), ast::Expr_::ExprMac(ref mac) => { // Failure to rewrite a marco should not imply failure to // rewrite the expression. @@ -730,6 +724,14 @@ pub fn is_empty_block(block: &ast::Block, codemap: &CodeMap) -> bool { block.stmts.is_empty() && block.expr.is_none() && !block_contains_comment(block, codemap) } +fn is_unsafe_block(block: &ast::Block) -> bool { + if let ast::BlockCheckMode::UnsafeBlock(..) = block.rules { + true + } else { + false + } +} + // inter-match-arm-comment-rules: // - all comments following a match arm before the start of the next arm // are about the second arm @@ -911,7 +913,7 @@ impl Rewrite for ast::Arm { } let pats_width = if vertical { - pat_strs[pat_strs.len() - 1].len() + pat_strs.last().unwrap().len() } else { total_width }; @@ -938,79 +940,53 @@ impl Rewrite for ast::Arm { line_start += offset.width(); } - let comma = arm_comma(body); + let body = match **body { + ast::Expr { node: ast::ExprBlock(ref b), .. } if !is_unsafe_block(b) && + is_simple_block(b, + context.codemap) => { + b.expr.as_ref().map(|e| &**e).unwrap() + } + ref x => x, + }; - // let body = match *body { - // ast::ExprBlock(ref b) if is_simple_block(b, context.codemap) => b.expr, - // ref x => x, - // }; + let comma = arm_comma(body); // Let's try and get the arm body on the same line as the condition. // 4 = ` => `.len() - let same_line_body = if context.config.max_width > line_start + comma.len() + 4 { + if context.config.max_width > line_start + comma.len() + 4 { let budget = context.config.max_width - line_start - comma.len() - 4; let offset = Indent::new(offset.block_indent, line_start + 4 - offset.block_indent); let rewrite = nop_block_collapse(body.rewrite(context, budget, offset), budget); match rewrite { - Some(ref body_str) if body_str.len() <= budget || comma.is_empty() => { + Some(ref body_str) if !body_str.contains('\n') || comma.is_empty() => { return Some(format!("{}{} => {}{}", attr_str.trim_left(), pats_str, body_str, comma)); } - _ => rewrite, + _ => {} } - } else { - None - }; - - if let ast::ExprBlock(_) = body.node { - // We're trying to fit a block in, but it still failed, give up. - return None; } - let mut result = format!("{}{} =>", attr_str.trim_left(), pats_str); + // FIXME: we're doing a second rewrite of the expr -- this may not be + // necessary. + let body_budget = try_opt!(width.checked_sub(context.config.tab_spaces)); + let indent = context.block_indent.block_indent(context.config); + let inner_context = &RewriteContext { block_indent: indent, ..*context }; + let next_line_body = try_opt!(nop_block_collapse(body.rewrite(inner_context, + body_budget, + indent), + body_budget)); + let indent_str = offset.block_indent(context.config).to_string(context.config); - match same_line_body { - // FIXME: also take this branch is expr is block - Some(ref body) if !body.contains('\n') => { - result.push(' '); - result.push_str(&body); - } - _ => { - let body_budget = try_opt!(width.checked_sub(context.config.tab_spaces)); - let indent = context.block_indent.block_indent(context.config); - let inner_context = &RewriteContext { block_indent: indent, ..*context }; - let next_line_body = try_opt!(nop_block_collapse(body.rewrite(inner_context, - body_budget, - indent), - body_budget)); - - result.push_str(" {\n"); - let indent_str = offset.block_indent(context.config).to_string(context.config); - result.push_str(&indent_str); - result.push_str(&next_line_body); - result.push('\n'); - result.push_str(&offset.to_string(context.config)); - result.push('}'); - } - }; - - Some(result) - } -} - -// Takes two possible rewrites for the match arm body and chooses the "nicest". -fn match_arm_heuristic<'a>(former: Option<&'a str>, latter: Option<&'a str>) -> Option<&'a str> { - match (former, latter) { - (f @ Some(..), None) => f, - (Some(f), Some(l)) if f.chars().filter(|&c| c == '\n').count() <= - l.chars().filter(|&c| c == '\n').count() => { - Some(f) - } - (_, l) => l, + Some(format!("{}{} => {{\n{}{}\n{}}}", + attr_str.trim_left(), + pats_str, + indent_str, + next_line_body, + offset.to_string(context.config))) } } @@ -1303,9 +1279,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, // Foo { a: Foo } - indent is +3, width is -5. let h_budget = width.checked_sub(path_str.len() + 5).unwrap_or(0); let (indent, v_budget) = match context.config.struct_lit_style { - StructLitStyle::Visual => { - (offset + path_str.len() + 3, h_budget) - } + StructLitStyle::Visual => (offset + path_str.len() + 3, h_budget), StructLitStyle::Block => { // If we are all on one line, then we'll ignore the indent, and we // have a smaller budget. diff --git a/src/imports.rs b/src/imports.rs index 6917b1ff403..42d152fc6ac 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -74,12 +74,8 @@ fn rewrite_single_use_list(path_str: String, vpi: &ast::PathListItem) -> String fn rewrite_path_item(vpi: &&ast::PathListItem) -> Option { let path_item_str = match vpi.node { - ast::PathListItem_::PathListIdent{ name, .. } => { - name.to_string() - } - ast::PathListItem_::PathListMod{ .. } => { - "self".to_owned() - } + ast::PathListItem_::PathListIdent{ name, .. } => name.to_string(), + ast::PathListItem_::PathListMod{ .. } => "self".to_owned(), }; Some(append_alias(path_item_str, vpi)) diff --git a/src/items.rs b/src/items.rs index 60d08743a95..4f3c82ee0ab 100644 --- a/src/items.rs +++ b/src/items.rs @@ -824,9 +824,7 @@ impl<'a> FmtVisitor<'a> { offset: Indent) -> Option { match *struct_def { - ast::VariantData::Unit(..) => { - self.format_unit_struct(item_name, ident, vis) - } + ast::VariantData::Unit(..) => self.format_unit_struct(item_name, ident, vis), ast::VariantData::Tuple(ref fields, _) => { self.format_tuple_struct(item_name, ident, vis, fields, generics, span, offset) } diff --git a/src/lib.rs b/src/lib.rs index bf1a0164f68..3923f6b742d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -225,15 +225,9 @@ pub enum ErrorKind { impl fmt::Display for ErrorKind { fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { match *self { - ErrorKind::LineOverflow => { - write!(fmt, "line exceeded maximum length") - } - ErrorKind::TrailingWhitespace => { - write!(fmt, "left behind trailing whitespace") - } - ErrorKind::BadIssue(issue) => { - write!(fmt, "found {}", issue) - } + ErrorKind::LineOverflow => write!(fmt, "line exceeded maximum length"), + ErrorKind::TrailingWhitespace => write!(fmt, "left behind trailing whitespace"), + ErrorKind::BadIssue(issue) => write!(fmt, "found {}", issue), } } } diff --git a/src/lists.rs b/src/lists.rs index b05e4d46066..ecde4f19fe4 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -392,9 +392,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3> 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 + 1 - } + (Some(i), None) if i > separator_index => separator_index + 1, // Block-style post-comment before the separator. (Some(i), None) => { cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, diff --git a/src/patterns.rs b/src/patterns.rs index 3e7a11bad41..af0bd827751 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -21,9 +21,7 @@ use syntax::ast::{BindingMode, Pat, Pat_}; impl Rewrite for Pat { fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option { match self.node { - Pat_::PatBox(ref pat) => { - rewrite_unary_prefix(context, "box ", &**pat, width, offset) - } + Pat_::PatBox(ref pat) => rewrite_unary_prefix(context, "box ", &**pat, width, offset), Pat_::PatIdent(binding_mode, ident, None) => { let (prefix, mutability) = match binding_mode { BindingMode::BindByRef(mutability) => ("ref ", mutability), @@ -50,9 +48,7 @@ impl Rewrite for Pat { let prefix = format!("&{}", format_mutability(mutability)); rewrite_unary_prefix(context, &prefix, &**pat, width, offset) } - Pat_::PatTup(ref items) => { - rewrite_tuple(context, items, self.span, width, offset) - } + Pat_::PatTup(ref items) => rewrite_tuple(context, items, self.span, width, offset), Pat_::PatEnum(ref path, Some(ref pat_vec)) => { let path_str = try_opt!(::types::rewrite_path(context, true, diff --git a/src/types.rs b/src/types.rs index 9b50333e043..9e693e9cff6 100644 --- a/src/types.rs +++ b/src/types.rs @@ -137,9 +137,7 @@ impl<'a> Rewrite for SegmentParam<'a> { width, offset) } - SegmentParam::Type(ref ty) => { - ty.rewrite(context, width, offset) - } + SegmentParam::Type(ref ty) => ty.rewrite(context, width, offset), SegmentParam::Binding(ref binding) => { let mut result = format!("{} = ", binding.ident); let budget = try_opt!(width.checked_sub(result.len())); @@ -479,9 +477,7 @@ impl Rewrite for ast::Ty { let budget = try_opt!(width.checked_sub(2)); ty.rewrite(context, budget, offset + 1).map(|ty_str| format!("[{}]", ty_str)) } - ast::TyTup(ref items) => { - rewrite_tuple(context, items, self.span, width, offset) - } + ast::TyTup(ref items) => rewrite_tuple(context, items, self.span, width, offset), ast::TyPolyTraitRef(ref trait_ref) => trait_ref.rewrite(context, width, offset), ast::TyPath(ref q_self, ref path) => { rewrite_path(context, false, q_self.as_ref(), path, width, offset) diff --git a/src/utils.rs b/src/utils.rs index 3965fc93c57..37dfb756972 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -75,9 +75,7 @@ pub fn last_line_width(s: &str) -> usize { fn is_skip(meta_item: &MetaItem) -> bool { match meta_item.node { MetaItem_::MetaWord(ref s) => *s == SKIP_ANNOTATION, - MetaItem_::MetaList(ref s, ref l) => { - *s == "cfg_attr" && l.len() == 2 && is_skip(&l[1]) - } + MetaItem_::MetaList(ref s, ref l) => *s == "cfg_attr" && l.len() == 2 && is_skip(&l[1]), _ => false, } } diff --git a/tests/target/match.rs b/tests/target/match.rs index 28a5ce2d98c..b7f41c1dd6b 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -18,9 +18,7 @@ fn foo() { } Pattern1 | Pattern2 | Pattern3 => false, Paternnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn | - Paternnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn => { - blah - } + Paternnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn => blah, Patternnnnnnnnnnnnnnnnnnn | Patternnnnnnnnnnnnnnnnnnn | Patternnnnnnnnnnnnnnnnnnn | @@ -164,15 +162,9 @@ fn issue355() { a => println!("a", b), b => vec![1, 2], c => vec!(3; 4), - d => { - println!("a", b) - } - e => { - vec![1, 2] - } - f => { - vec!(3; 4) - } + d => println!("a", b), + e => vec![1, 2], + f => vec!(3; 4), h => println!("a", b), // h comment i => vec![1, 2], // i comment j => vec!(3; 4), // j comment @@ -272,9 +264,7 @@ fn issue496() { match def { def::DefConst(def_id) | def::DefAssociatedConst(def_id) => { match const_eval::lookup_const_by_id(cx.tcx, def_id, Some(self.pat.id)) { - Some(const_expr) => { - x - } + Some(const_expr) => x, } } }