refactor and document the chain reformatting code

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
+/// Formatting of chained expressions, i.e. expressions which are chained by
-// dots: struct and enum field access and method calls.
+/// dots: struct and enum field access and method calls.
-//
+///
-// Instead of walking these subexpressions one-by-one, as is our usual strategy
+/// Instead of walking these subexpressions one-by-one, as is our usual strategy
-// for expression formatting, we collect maximal sequences of these expressions
+/// for expression formatting, we collect maximal sequences of these expressions
-// and handle them simultaneously.
+/// and handle them simultaneously.
-//
+///
-// Whenever possible, the entire chain is put on a single line. If that fails,
+/// 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
+/// we put each subexpression on a separate, much like the (default) function
-// argument function argument strategy.
+/// 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<String> {
     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) {
+    // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`.
-        subexpr_list.push(subexpr);
+    let parent_block_indent = chain_base_indent(context, offset);
-        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),
-    };
     let parent_context = &RewriteContext { block_indent: parent_block_indent, ..*context };
-    let parent = subexpr_list.pop().unwrap();
+    let parent_rewrite = try_opt!(parent.rewrite(parent_context, width, offset));
-    let parent_rewrite = try_opt!(expr.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 {
+        (chain_indent(context, offset), false)
-            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),
-        }
     };
 
     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,
+                                                rewrite_chain_subexpr(e,
-                                                                   total_span,
+                                                                      total_span,
-                                                                   context,
+                                                                      context,
-                                                                   max_width,
+                                                                      max_width,
-                                                                   indent)
+                                                                      indent)
                                             })
                                             .collect::<Option<Vec<_>>>());
 
@@ -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 &&
+    let mut fits_single_line = !veto_single_line && total_width <= width;
-                           match subexpr_list[0].node {
+    if fits_single_line {
-        ast::ExprKind::MethodCall(ref method_name, ref types, ref expressions)
+        let len = rewrites.len();
-            if context.config.chains_overflow_last => {
+        let (init, last) = rewrites.split_at_mut(len - 1);
-            let len = rewrites.len();
+        fits_single_line = init.iter().all(|s| !s.contains('\n'));
-            let (init, last) = rewrites.split_at_mut(len - 1);
-            let last = &mut last[0];
 
-            if init.iter().all(|s| !s.contains('\n')) && total_width <= width {
+        if fits_single_line {
-                let last_rewrite = width.checked_sub(almost_total)
+            fits_single_line = match expr.node {
-                                        .and_then(|inner_width| {
+                ref e @ ast::ExprKind::MethodCall(..) if context.config.chains_overflow_last => {
-                                            rewrite_method_call(method_name.node,
+                    rewrite_method_call_with_overflow(e,
-                                                                types,
+                                                      &mut last[0],
-                                                                expressions,
+                                                      almost_total,
-                                                                total_span,
+                                                      width,
-                                                                context,
+                                                      total_span,
-                                                                inner_width,
+                                                      context,
-                                                                offset + almost_total)
+                                                      offset)
-                                        });
-                match last_rewrite {
-                    Some(mut string) => {
-                        ::std::mem::swap(&mut string, last);
-                        true
-                    }
-                    None => false,
                 }
-            } else {
+                _ => !last[0].contains('\n'),
-                false
             }
         }
-        _ => 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> {
+// Returns the root of the chain and a Vec of the prefixes of the rest of the chain.
-    match expr.node {
+// E.g., for input `a.b.c` we return (`a`, [`a.b.c`, `a.b`])
-        ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]),
+fn make_subexpr_list(mut expr: &ast::Expr) -> (&ast::Expr, Vec<&ast::Expr>) {
-        ast::ExprKind::TupField(ref subexpr, _) |
+    fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> {
-        ast::ExprKind::Field(ref subexpr, _) => Some(subexpr),
+        match expr.node {
-        _ => None,
+            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,
+fn chain_indent(context: &RewriteContext, offset: Indent) -> Indent {
-                      span: Span,
+    match context.config.chain_indent {
-                      context: &RewriteContext,
+        BlockIndentStyle::Inherit => context.block_indent,
-                      width: usize,
+        BlockIndentStyle::Tabbed => context.block_indent.block_indent(context.config),
-                      offset: Indent)
+        BlockIndentStyle::Visual => offset + Indent::new(context.config.tab_spaces, 0),
-                      -> Option<String> {
+    }
+}
+
+// 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<String> {
     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,

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";