From f0fe9c3c4acb4eafba5a552a2823e6997e75a3f4 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 11 Jul 2018 21:01:40 +1200 Subject: [PATCH] chains: prefer to use the next line for an expression, if using the same line would introduce an open block or similar This problem came to light due to the chains changes, but effects other code too. It only happens rarely, e.g., before this fix: ``` match foo { MacroArgKind::Delimited(ref delim_tok, ref args) => rewrite_delimited_inner( delim_tok, args, ).map(|(lhs, inner, rhs)| format!("{}{}{}", lhs, inner, rhs)), }; ``` after: ``` match foo { MacroArgKind::Delimited(ref delim_tok, ref args) => { rewrite_delimited_inner(delim_tok, args) .map(|(lhs, inner, rhs)| format!("{}{}{}", lhs, inner, rhs)) } } ``` --- src/chains.rs | 5 +++-- src/expr.rs | 9 ++++++--- src/utils.rs | 7 +++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 81be8d541287..d7126b2f5e79 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -398,8 +398,8 @@ impl <'a> ChainFormatterShared<'a> { if all_in_one_line || extendable { // First we try to 'overflow' the last child and see if it looks better than using // vertical layout. - if let Some(shape) = last_shape.offset_left(almost_total) { - if let Some(rw) = last.rewrite_postfix(context, shape) { + if let Some(one_line_shape) = last_shape.offset_left(almost_total) { + if let Some(rw) = last.rewrite_postfix(context, one_line_shape) { // We allow overflowing here only if both of the following conditions match: // 1. The entire chain fits in a single line except the last child. // 2. `last_child_str.lines().count() >= 5`. @@ -413,6 +413,7 @@ impl <'a> ChainFormatterShared<'a> { // layout, just by looking at the overflowed rewrite. Now we rewrite the // last child on its own line, and compare two rewrites to choose which is // better. + let last_shape = child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries)?; match last.rewrite_postfix(context, last_shape) { Some(ref new_rw) if !could_fit_single_line => { last_subexpr_str = Some(new_rw.clone()); diff --git a/src/expr.rs b/src/expr.rs index 6e1af266e9bf..8aa5d9bd2864 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -39,9 +39,9 @@ use spanned::Spanned; use string::{rewrite_string, StringFormat}; use types::{can_be_overflowed_type, rewrite_path, PathContext}; use utils::{ - colon_spaces, contains_skip, count_newlines, first_line_width, inner_attributes, - last_line_extendable, last_line_width, mk_sp, outer_attributes, ptr_vec_to_ref_vec, - semicolon_for_stmt, wrap_str, + colon_spaces, contains_skip, count_newlines, first_line_ends_with, first_line_width, + inner_attributes, last_line_extendable, last_line_width, mk_sp, outer_attributes, + ptr_vec_to_ref_vec, semicolon_for_stmt, wrap_str, }; use vertical::rewrite_with_alignment; use visitor::FmtVisitor; @@ -1953,6 +1953,9 @@ pub fn prefer_next_line(orig_rhs: &str, next_line_rhs: &str, rhs_tactics: RhsTac rhs_tactics == RhsTactics::ForceNextLineWithoutIndent || !next_line_rhs.contains('\n') || count_newlines(orig_rhs) > count_newlines(next_line_rhs) + 1 + || first_line_ends_with(orig_rhs, '(') && !first_line_ends_with(next_line_rhs, '(') + || first_line_ends_with(orig_rhs, '{') && !first_line_ends_with(next_line_rhs, '{') + || first_line_ends_with(orig_rhs, '[') && !first_line_ends_with(next_line_rhs, '[') } fn rewrite_expr_addrof( diff --git a/src/utils.rs b/src/utils.rs index f1b0582b1200..b8474792cd30 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -382,6 +382,7 @@ pub fn colon_spaces(before: bool, after: bool) -> &'static str { } } +#[inline] pub fn left_most_sub_expr(e: &ast::Expr) -> &ast::Expr { match e.node { ast::ExprKind::Call(ref e, _) @@ -398,6 +399,12 @@ pub fn left_most_sub_expr(e: &ast::Expr) -> &ast::Expr { } } +#[inline] pub fn starts_with_newline(s: &str) -> bool { s.starts_with('\n') || s.starts_with("\r\n") } + +#[inline] +pub fn first_line_ends_with(s: &str, c: char) -> bool { + s.lines().next().map_or(false, |l| l.ends_with(c)) +}