From 3d46532e72a693b0ca9100b7ba9f721e4daaff31 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Fri, 22 Apr 2016 13:36:27 +1200 Subject: [PATCH] refactor and document the chain reformatting code --- src/chains.rs | 276 ++++++++++++++++++++++++++++++++++++-------------- src/config.rs | 4 +- 2 files changed, 201 insertions(+), 79 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index c535287df243..5c097c83cb1d 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -8,16 +8,88 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// Formatting of chained expressions, i.e. expressions which are chained by -// dots: struct and enum field access and method calls. -// -// Instead of walking these subexpressions one-by-one, as is our usual strategy -// for expression formatting, we collect maximal sequences of these expressions -// and handle them simultaneously. -// -// Whenever possible, the entire chain is put on a single line. If that fails, -// we put each subexpression on a separate, much like the (default) function -// argument function argument strategy. +/// Formatting of chained expressions, i.e. expressions which are chained by +/// dots: struct and enum field access and method calls. +/// +/// Instead of walking these subexpressions one-by-one, as is our usual strategy +/// for expression formatting, we collect maximal sequences of these expressions +/// and handle them simultaneously. +/// +/// Whenever possible, the entire chain is put on a single line. If that fails, +/// we put each subexpression on a separate, much like the (default) function +/// argument function argument strategy. +/// +/// Depends on config options: `chain_base_indent` is the indent to use for +/// blocks in the parent/root/base of the chain. +/// E.g., `let foo = { aaaa; bbb; ccc }.bar.baz();`, we would layout for the +/// following values of `chain_base_indent`: +/// Visual: +/// ``` +/// let foo = { +/// aaaa; +/// bbb; +/// ccc +/// } +/// .bar +/// .baz(); +/// ``` +/// Inherit: +/// ``` +/// let foo = { +/// aaaa; +/// bbb; +/// ccc +/// } +/// .bar +/// .baz(); +/// ``` +/// Tabbed: +/// ``` +/// let foo = { +/// aaaa; +/// bbb; +/// ccc +/// } +/// .bar +/// .baz(); +/// ``` +/// +/// `chain_indent` dictates how the rest of the chain is aligned. This only seems +/// to have an effect if the first non-root part of the chain is put on a +/// newline, otherwise we align the dots: +/// ``` +/// foo.bar +/// .baz() +/// ``` +/// If the first item in the chain is a block expression, we align the dots with +/// the braces. +/// +/// Otherwise: +/// Visual: +/// ``` +/// let a = foo(aaa, bbb) +/// .bar +/// .baz() +/// ``` +/// Visual seems to be a tab indented from the indent of the whole expression. +/// Inherit: +/// ``` +/// let a = foo(aaa, bbb) +/// .bar +/// .baz() +/// ``` +/// Tabbed: +/// ``` +/// let a = foo(aaa, bbb) +/// .bar +/// .baz() +/// ``` +/// `chains_overflow_last` applies only to chains where the last item is a +/// method call. Usually, any line break in a chain sub-expression causes the +/// whole chain to be split with newlines at each `.`. With `chains_overflow_last` +/// true, then we allow the last method call to spill over multiple lines without +/// forcing the rest of the chain to be split. + use Indent; use rewrite::{Rewrite, RewriteContext}; @@ -28,49 +100,43 @@ use config::BlockIndentStyle; use syntax::{ast, ptr}; use syntax::codemap::{mk_sp, Span}; -pub fn rewrite_chain(mut expr: &ast::Expr, + +pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, width: usize, offset: Indent) -> Option { let total_span = expr.span; - let mut subexpr_list = vec![expr]; + let (parent, subexpr_list) = make_subexpr_list(expr); - while let Some(subexpr) = pop_expr_chain(expr) { - subexpr_list.push(subexpr); - expr = subexpr; - } - - let parent_block_indent = match context.config.chain_base_indent { - BlockIndentStyle::Visual => offset, - BlockIndentStyle::Inherit => context.block_indent, - BlockIndentStyle::Tabbed => context.block_indent.block_indent(context.config), - }; + // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`. + let parent_block_indent = chain_base_indent(context, offset); let parent_context = &RewriteContext { block_indent: parent_block_indent, ..*context }; - let parent = subexpr_list.pop().unwrap(); - let parent_rewrite = try_opt!(expr.rewrite(parent_context, width, offset)); + let parent_rewrite = try_opt!(parent.rewrite(parent_context, width, offset)); + + // Decide how to layout the rest of the chain. `extend` is true if we can + // put the first non-parent item on the same line as the parent. let (indent, extend) = if !parent_rewrite.contains('\n') && is_continuable(parent) || parent_rewrite.len() <= context.config.tab_spaces { + // Try and put the whole chain on one line. (offset + Indent::new(0, parent_rewrite.len()), true) } else if is_block_expr(parent, &parent_rewrite) { + // The parent is a block, so align the rest of the chain with the closing + // brace. (parent_block_indent, false) } else { - match context.config.chain_indent { - BlockIndentStyle::Inherit => (context.block_indent, false), - BlockIndentStyle::Tabbed => (context.block_indent.block_indent(context.config), false), - BlockIndentStyle::Visual => (offset + Indent::new(context.config.tab_spaces, 0), false), - } + (chain_indent(context, offset), false) }; let max_width = try_opt!((width + offset.width()).checked_sub(indent.width())); let mut rewrites = try_opt!(subexpr_list.iter() .rev() .map(|e| { - rewrite_chain_expr(e, - total_span, - context, - max_width, - indent) + rewrite_chain_subexpr(e, + total_span, + context, + max_width, + indent) }) .collect::>>()); @@ -80,6 +146,7 @@ pub fn rewrite_chain(mut expr: &ast::Expr, .fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len(); let total_width = almost_total + first_line_width(rewrites.last().unwrap()); + let veto_single_line = if context.config.take_source_hints && subexpr_list.len() > 1 { // Look at the source code. Unless all chain elements start on the same // line, we won't consider putting them on a single line either. @@ -92,49 +159,40 @@ pub fn rewrite_chain(mut expr: &ast::Expr, false }; - let fits_single_line = !veto_single_line && - match subexpr_list[0].node { - ast::ExprKind::MethodCall(ref method_name, ref types, ref expressions) - if context.config.chains_overflow_last => { - let len = rewrites.len(); - let (init, last) = rewrites.split_at_mut(len - 1); - let last = &mut last[0]; + let mut fits_single_line = !veto_single_line && total_width <= width; + if fits_single_line { + let len = rewrites.len(); + let (init, last) = rewrites.split_at_mut(len - 1); + fits_single_line = init.iter().all(|s| !s.contains('\n')); - if init.iter().all(|s| !s.contains('\n')) && total_width <= width { - let last_rewrite = width.checked_sub(almost_total) - .and_then(|inner_width| { - rewrite_method_call(method_name.node, - types, - expressions, - total_span, - context, - inner_width, - offset + almost_total) - }); - match last_rewrite { - Some(mut string) => { - ::std::mem::swap(&mut string, last); - true - } - None => false, + if fits_single_line { + fits_single_line = match expr.node { + ref e @ ast::ExprKind::MethodCall(..) if context.config.chains_overflow_last => { + rewrite_method_call_with_overflow(e, + &mut last[0], + almost_total, + width, + total_span, + context, + offset) } - } else { - false + _ => !last[0].contains('\n'), } } - _ => total_width <= width && rewrites.iter().all(|s| !s.contains('\n')), - }; + } let connector = if fits_single_line && !parent_rewrite.contains('\n') { + // Yay, we can put everything on one line. String::new() } else { + // Use new lines. format!("\n{}", indent.to_string(context.config)) }; let first_connector = if extend { "" } else { - &connector[..] + &connector }; wrap_str(format!("{}{}{}", @@ -146,8 +204,40 @@ pub fn rewrite_chain(mut expr: &ast::Expr, offset) } +fn rewrite_method_call_with_overflow(expr_kind: &ast::ExprKind, + last: &mut String, + almost_total: usize, + width: usize, + total_span: Span, + context: &RewriteContext, + offset: Indent) + -> bool { + if let &ast::ExprKind::MethodCall(ref method_name, ref types, ref expressions) = expr_kind { + let budget = match width.checked_sub(almost_total) { + Some(b) => b, + None => return false, + }; + let mut last_rewrite = rewrite_method_call(method_name.node, + types, + expressions, + total_span, + context, + budget, + offset + almost_total); + + if let Some(ref mut s) = last_rewrite { + ::std::mem::swap(s, last); + true + } else { + false + } + } else { + unreachable!(); + } +} + // States whether an expression's last line exclusively consists of closing -// parens, braces and brackets in its idiomatic formatting. +// parens, braces, and brackets in its idiomatic formatting. fn is_block_expr(expr: &ast::Expr, repr: &str) -> bool { match expr.node { ast::ExprKind::Struct(..) | @@ -167,21 +257,53 @@ fn is_block_expr(expr: &ast::Expr, repr: &str) -> bool { } } -fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> { - match expr.node { - ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]), - ast::ExprKind::TupField(ref subexpr, _) | - ast::ExprKind::Field(ref subexpr, _) => Some(subexpr), - _ => None, +// Returns the root of the chain and a Vec of the prefixes of the rest of the chain. +// E.g., for input `a.b.c` we return (`a`, [`a.b.c`, `a.b`]) +fn make_subexpr_list(mut expr: &ast::Expr) -> (&ast::Expr, Vec<&ast::Expr>) { + fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> { + match expr.node { + ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]), + ast::ExprKind::TupField(ref subexpr, _) | + ast::ExprKind::Field(ref subexpr, _) => Some(subexpr), + _ => None, + } + } + + let mut subexpr_list = vec![expr]; + + while let Some(subexpr) = pop_expr_chain(expr) { + subexpr_list.push(subexpr); + expr = subexpr; + } + + let parent = subexpr_list.pop().unwrap(); + (parent, subexpr_list) +} + +fn chain_base_indent(context: &RewriteContext, offset: Indent) -> Indent { + match context.config.chain_base_indent { + BlockIndentStyle::Visual => offset, + BlockIndentStyle::Inherit => context.block_indent, + BlockIndentStyle::Tabbed => context.block_indent.block_indent(context.config), } } -fn rewrite_chain_expr(expr: &ast::Expr, - span: Span, - context: &RewriteContext, - width: usize, - offset: Indent) - -> Option { +fn chain_indent(context: &RewriteContext, offset: Indent) -> Indent { + match context.config.chain_indent { + BlockIndentStyle::Inherit => context.block_indent, + BlockIndentStyle::Tabbed => context.block_indent.block_indent(context.config), + BlockIndentStyle::Visual => offset + Indent::new(context.config.tab_spaces, 0), + } +} + +// Rewrite the last element in the chain `expr`. E.g., given `a.b.c` we rewrite +// `.c`. +fn rewrite_chain_subexpr(expr: &ast::Expr, + span: Span, + context: &RewriteContext, + width: usize, + offset: Indent) + -> Option { match expr.node { ast::ExprKind::MethodCall(ref method_name, ref types, ref expressions) => { let inner = &RewriteContext { block_indent: offset, ..*context }; @@ -213,7 +335,7 @@ fn rewrite_chain_expr(expr: &ast::Expr, } } -// Determines we can continue formatting a given expression on the same line. +// Determines if we can continue formatting a given expression on the same line. fn is_continuable(expr: &ast::Expr) -> bool { match expr.node { ast::ExprKind::Path(..) => true, diff --git a/src/config.rs b/src/config.rs index 846b8a391354..71edfe955bd6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -377,11 +377,11 @@ create_config! { "Report all, none or unnumbered occurrences of FIXME in source file comments"; chain_base_indent: BlockIndentStyle, BlockIndentStyle::Visual, "Indent on chain base"; chain_indent: BlockIndentStyle, BlockIndentStyle::Visual, "Indentation of chain"; + chains_overflow_last: bool, true, "Allow last call in method chain to break the line"; reorder_imports: bool, false, "Reorder import statements alphabetically"; single_line_if_else: bool, false, "Put else on same line as closing brace for if statements"; format_strings: bool, true, "Format string literals where necessary"; force_format_strings: bool, false, "Always format string literals"; - chains_overflow_last: bool, true, "Allow last call in method chain to break the line"; take_source_hints: bool, true, "Retain some formatting characteristics from the source code"; hard_tabs: bool, false, "Use tab characters for indentation, spaces for alignment"; wrap_comments: bool, false, "Break comments to fit on the line"; @@ -390,7 +390,7 @@ create_config! { match_block_trailing_comma: bool, false, "Put a trailing comma after a block based match arm (non-block arms are not affected)"; match_wildcard_trailing_comma: bool, true, "Put a trailing comma after a wildcard arm"; - closure_block_indent_threshold: isize, 4, "How many lines a closure must have before it is \ + closure_block_indent_threshold: isize, -1, "How many lines a closure must have before it is \ block indented. -1 means never use block indent."; write_mode: WriteMode, WriteMode::Replace, "What Write Mode to use when none is supplied: Replace, Overwrite, Display, Diff, Coverage";