From e29fd7bebe0ba302ec8d326a02c86b7fe71c30b4 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 15 Oct 2018 14:06:52 +1300 Subject: [PATCH 1/2] Only put `{` on a newline in a match arm where necessary Fixes #3005 --- src/chains.rs | 36 ++-------------------------- src/matches.rs | 64 +++++++++++++++++++++----------------------------- src/utils.rs | 32 +++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 71 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index b7619c0a18a..eb9771a2356 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -74,7 +74,7 @@ use rewrite::{Rewrite, RewriteContext}; use shape::Shape; use source_map::SpanUtils; use utils::{ - first_line_width, last_line_extendable, last_line_width, mk_sp, rewrite_ident, + self, first_line_width, last_line_extendable, last_line_width, mk_sp, rewrite_ident, trimmed_last_line_width, wrap_str, }; @@ -130,7 +130,7 @@ enum ChainItemKind { impl ChainItemKind { fn is_block_like(&self, context: &RewriteContext, reps: &str) -> bool { match self { - ChainItemKind::Parent(ref expr) => is_block_expr(context, expr, reps), + ChainItemKind::Parent(ref expr) => utils::is_block_expr(context, expr, reps), ChainItemKind::MethodCall(..) | ChainItemKind::StructField(..) | ChainItemKind::TupleField(..) @@ -845,38 +845,6 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> { } } -// States whether an expression's last line exclusively consists of closing -// parens, braces, and brackets in its idiomatic formatting. -fn is_block_expr(context: &RewriteContext, expr: &ast::Expr, repr: &str) -> bool { - match expr.node { - ast::ExprKind::Mac(..) - | ast::ExprKind::Call(..) - | ast::ExprKind::MethodCall(..) - | ast::ExprKind::Array(..) - | ast::ExprKind::Struct(..) - | ast::ExprKind::While(..) - | ast::ExprKind::WhileLet(..) - | ast::ExprKind::If(..) - | ast::ExprKind::IfLet(..) - | ast::ExprKind::Block(..) - | ast::ExprKind::Loop(..) - | ast::ExprKind::ForLoop(..) - | ast::ExprKind::Match(..) => repr.contains('\n'), - ast::ExprKind::Paren(ref expr) - | ast::ExprKind::Binary(_, _, ref expr) - | ast::ExprKind::Index(_, ref expr) - | ast::ExprKind::Unary(_, ref expr) - | ast::ExprKind::Closure(_, _, _, _, ref expr, _) - | ast::ExprKind::Try(ref expr) - | ast::ExprKind::Yield(Some(ref expr)) => is_block_expr(context, expr, repr), - // This can only be a string lit - ast::ExprKind::Lit(_) => { - repr.contains('\n') && trimmed_last_line_width(repr) <= context.config.tab_spaces() - } - _ => false, - } -} - /// Remove try operators (`?`s) that appear in the given string. If removing /// them leaves an empty line, remove that line as well unless it is the first /// line (we need the first newline for detecting pre/post comment). diff --git a/src/matches.rs b/src/matches.rs index a143c20f4bf..24f55483972 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -247,54 +247,44 @@ fn rewrite_match_arm( } else { (mk_sp(arm.span().lo(), arm.span().lo()), String::new()) }; - let pats_str = - rewrite_match_pattern(context, &ptr_vec_to_ref_vec(&arm.pats), &arm.guard, shape) - .and_then(|pats_str| { - combine_strs_with_missing_comments( - context, - &attrs_str, - &pats_str, - missing_span, - shape, - false, - ) - })?; + + // Patterns + // 5 = ` => {` + let pat_shape = shape.sub_width(5)?; + let pats_str = rewrite_multiple_patterns(context, &ptr_vec_to_ref_vec(&arm.pats), pat_shape)?; + + // Guard + let block_like_pat = trimmed_last_line_width(&pats_str) <= context.config.tab_spaces(); + let new_line_guard = pats_str.contains('\n') && !block_like_pat; + let guard_str = rewrite_guard( + context, + &arm.guard, + shape, + trimmed_last_line_width(&pats_str), + new_line_guard, + )?; + + let lhs_str = combine_strs_with_missing_comments( + context, + &attrs_str, + &format!("{}{}", pats_str, guard_str), + missing_span, + shape, + false, + )?; let arrow_span = mk_sp(arm.pats.last().unwrap().span.hi(), arm.body.span.lo()); rewrite_match_body( context, &arm.body, - &pats_str, + &lhs_str, shape, - arm.guard.is_some(), + guard_str.contains('\n'), arrow_span, is_last, ) } -fn rewrite_match_pattern( - context: &RewriteContext, - pats: &[&ast::Pat], - guard: &Option, - shape: Shape, -) -> Option { - // Patterns - // 5 = ` => {` - let pat_shape = shape.sub_width(5)?; - let pats_str = rewrite_multiple_patterns(context, pats, pat_shape)?; - - // Guard - let guard_str = rewrite_guard( - context, - guard, - shape, - trimmed_last_line_width(&pats_str), - pats_str.contains('\n'), - )?; - - Some(format!("{}{}", pats_str, guard_str)) -} - fn block_can_be_flattened<'a>( context: &RewriteContext, expr: &'a ast::Expr, diff --git a/src/utils.rs b/src/utils.rs index 63b238a7817..7d9272fe7e3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -417,3 +417,35 @@ pub fn starts_with_newline(s: &str) -> bool { pub fn first_line_ends_with(s: &str, c: char) -> bool { s.lines().next().map_or(false, |l| l.ends_with(c)) } + +// States whether an expression's last line exclusively consists of closing +// parens, braces, and brackets in its idiomatic formatting. +pub fn is_block_expr(context: &RewriteContext, expr: &ast::Expr, repr: &str) -> bool { + match expr.node { + ast::ExprKind::Mac(..) + | ast::ExprKind::Call(..) + | ast::ExprKind::MethodCall(..) + | ast::ExprKind::Array(..) + | ast::ExprKind::Struct(..) + | ast::ExprKind::While(..) + | ast::ExprKind::WhileLet(..) + | ast::ExprKind::If(..) + | ast::ExprKind::IfLet(..) + | ast::ExprKind::Block(..) + | ast::ExprKind::Loop(..) + | ast::ExprKind::ForLoop(..) + | ast::ExprKind::Match(..) => repr.contains('\n'), + ast::ExprKind::Paren(ref expr) + | ast::ExprKind::Binary(_, _, ref expr) + | ast::ExprKind::Index(_, ref expr) + | ast::ExprKind::Unary(_, ref expr) + | ast::ExprKind::Closure(_, _, _, _, ref expr, _) + | ast::ExprKind::Try(ref expr) + | ast::ExprKind::Yield(Some(ref expr)) => is_block_expr(context, expr, repr), + // This can only be a string lit + ast::ExprKind::Lit(_) => { + repr.contains('\n') && trimmed_last_line_width(repr) <= context.config.tab_spaces() + } + _ => false, + } +} From e2be62c7a58331abefb5f8390f5c62b742d683b0 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 15 Oct 2018 14:09:31 +1300 Subject: [PATCH 2/2] Add test (issue 3005) --- tests/source/match.rs | 13 +++++++++++++ tests/target/match.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+) diff --git a/tests/source/match.rs b/tests/source/match.rs index 1643449773e..c5a5455cfc7 100644 --- a/tests/source/match.rs +++ b/tests/source/match.rs @@ -536,3 +536,16 @@ fn issue_3030() { } } } + +fn issue_3005() { + match *token { + Token::Dimension { + value, ref unit, .. + } if num_context.is_ok(context.parsing_mode, value) => + { + return NoCalcLength::parse_dimension(context, value, unit) + .map(LengthOrPercentage::Length) + .map_err(|()| location.new_unexpected_token_error(token.clone())) + }, + } +} diff --git a/tests/target/match.rs b/tests/target/match.rs index 130c9adfb3d..c8a4022d2ac 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -564,3 +564,15 @@ fn issue_3030() { ) => {} } } + +fn issue_3005() { + match *token { + Token::Dimension { + value, ref unit, .. + } if num_context.is_ok(context.parsing_mode, value) => { + return NoCalcLength::parse_dimension(context, value, unit) + .map(LengthOrPercentage::Length) + .map_err(|()| location.new_unexpected_token_error(token.clone())) + } + } +}