From ccd404ac6f5f8cd9ac1cbb11494b65e336294432 Mon Sep 17 00:00:00 2001
From: Marcus Klaas <mail@marcusklaas.nl>
Date: Thu, 8 Oct 2015 23:07:19 +0200
Subject: [PATCH] Try "overflowing" the final function argument when it's a
 closure

This means that we try formatting the last argument of a function call with block
indentation instead of visual indentation when it is a closure and its first line
fits on the same line as the first arguments.
---
 src/expr.rs               | 103 +++++++++++++++++++++++++++++---------
 src/missed_spans.rs       |  19 ++++---
 tests/source/chains.rs    |  14 ++++++
 tests/target/chains.rs    |  26 +++++++---
 tests/target/hard-tabs.rs |  11 ++--
 5 files changed, 128 insertions(+), 45 deletions(-)

diff --git a/src/expr.rs b/src/expr.rs
index 1d0621539e0..325841fbfd1 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -10,13 +10,14 @@
 
 use std::cmp::Ordering;
 use std::borrow::Borrow;
+use std::mem::swap;
 
 use Indent;
 use rewrite::{Rewrite, RewriteContext};
 use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
             DefinitiveListTactic, definitive_tactic, ListItem, format_fn_args};
 use string::{StringFormat, rewrite_string};
-use utils::{span_after, extra_offset, last_line_width, wrap_str, binary_search};
+use utils::{span_after, extra_offset, last_line_width, wrap_str, binary_search, first_line_width};
 use visitor::FmtVisitor;
 use config::{StructLitStyle, MultilineStyle};
 use comment::{FindUncommented, rewrite_comment, contains_comment};
@@ -223,29 +224,27 @@ fn rewrite_pair<LHS, RHS>(lhs: &LHS,
 {
     let max_width = try_opt!(width.checked_sub(prefix.len() + infix.len() + suffix.len()));
 
-    binary_search(1,
-                  max_width,
-                  |lhs_budget| {
-                      let lhs_offset = offset + prefix.len();
-                      let lhs_str = match lhs.rewrite(context, lhs_budget, lhs_offset) {
-                          Some(result) => result,
-                          None => return Err(Ordering::Greater),
-                      };
+    binary_search(1, max_width, |lhs_budget| {
+        let lhs_offset = offset + prefix.len();
+        let lhs_str = match lhs.rewrite(context, lhs_budget, lhs_offset) {
+            Some(result) => result,
+            None => return Err(Ordering::Greater),
+        };
 
-                      let last_line_width = last_line_width(&lhs_str);
-                      let rhs_budget = match max_width.checked_sub(last_line_width) {
-                          Some(b) => b,
-                          None => return Err(Ordering::Less),
-                      };
-                      let rhs_indent = offset + last_line_width + prefix.len() + infix.len();
+        let last_line_width = last_line_width(&lhs_str);
+        let rhs_budget = match max_width.checked_sub(last_line_width) {
+            Some(b) => b,
+            None => return Err(Ordering::Less),
+        };
+        let rhs_indent = offset + last_line_width + prefix.len() + infix.len();
 
-                      let rhs_str = match rhs.rewrite(context, rhs_budget, rhs_indent) {
-                          Some(result) => result,
-                          None => return Err(Ordering::Less),
-                      };
+        let rhs_str = match rhs.rewrite(context, rhs_budget, rhs_indent) {
+            Some(result) => result,
+            None => return Err(Ordering::Less),
+        };
 
-                      Ok(format!("{}{}{}{}{}", prefix, lhs_str, infix, rhs_str, suffix))
-                  })
+        Ok(format!("{}{}{}{}{}", prefix, lhs_str, infix, rhs_str, suffix))
+    })
 }
 
 pub fn rewrite_array<'a, I>(expr_iter: I,
@@ -1135,7 +1134,8 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
         None => return Err(Ordering::Greater),
     };
     let offset = offset + extra_offset + 1;
-    let block_indent = if args.len() == 1 {
+    let arg_count = args.len();
+    let block_indent = if arg_count == 1 {
         context.block_indent
     } else {
         offset
@@ -1150,8 +1150,65 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
                              |item| item.rewrite(&inner_context, remaining_width, offset),
                              span.lo,
                              span.hi);
+    let mut item_vec: Vec<_> = items.collect();
 
-    let list_str = match format_fn_args(items, remaining_width, offset, context.config) {
+    // Try letting the last argument overflow to the next line with block
+    // indentation. If its first line fits on one line with the other arguments,
+    // we format the function arguments horizontally.
+    let overflow_last = match args.last().map(|x| &x.node) {
+        Some(&ast::Expr_::ExprClosure(..)) |
+        Some(&ast::Expr_::ExprBlock(..)) if arg_count > 1 => true,
+        _ => false,
+    } && context.config.chains_overflow_last;
+
+    let mut orig_last = None;
+    let mut placeholder = None;
+
+    // Replace the last item with its first line to see if it fits with
+    // first arguments.
+    if overflow_last {
+        let inner_context = &RewriteContext { block_indent: context.block_indent, ..*context };
+        let rewrite = args.last().unwrap().rewrite(&inner_context, remaining_width, offset);
+
+        if let Some(rewrite) = rewrite {
+            let rewrite_first_line = Some(rewrite[..first_line_width(&rewrite)].to_owned());
+            placeholder = Some(rewrite);
+
+            swap(&mut item_vec[arg_count - 1].item, &mut orig_last);
+            item_vec[arg_count - 1].item = rewrite_first_line;
+        }
+    }
+
+    let tactic = definitive_tactic(&item_vec,
+                                   ListTactic::LimitedHorizontalVertical(context.config
+                                                                                .fn_call_width),
+                                   remaining_width);
+
+    // Replace the stub with the full overflowing last argument if the rewrite
+    // succeeded and its first line fits with the other arguments.
+    match (overflow_last, tactic, placeholder) {
+        (true, DefinitiveListTactic::Horizontal, placeholder @ Some(..)) => {
+            item_vec[arg_count - 1].item = placeholder;
+        }
+        (true, _, _) => {
+            item_vec[arg_count - 1].item = orig_last;
+        }
+        (false, _, _) => {}
+    }
+
+    let fmt = ListFormatting {
+        tactic: tactic,
+        separator: ",",
+        trailing_separator: SeparatorTactic::Never,
+        indent: offset,
+        width: width,
+        ends_with_newline: false,
+        config: context.config,
+    };
+
+    // format_fn_args(items, remaining_width, offset, context.config)
+
+    let list_str = match write_list(&item_vec, &fmt) {
         Some(str) => str,
         None => return Err(Ordering::Less),
     };
diff --git a/src/missed_spans.rs b/src/missed_spans.rs
index 5c5565c6596..f33b7a3014f 100644
--- a/src/missed_spans.rs
+++ b/src/missed_spans.rs
@@ -22,16 +22,15 @@ impl<'a> FmtVisitor<'a> {
 
     pub fn format_missing_with_indent(&mut self, end: BytePos) {
         let config = self.config;
-        self.format_missing_inner(end,
-                                  |this, last_snippet, snippet| {
-                                      this.buffer.push_str(last_snippet.trim_right());
-                                      if last_snippet == snippet {
-                                          // No new lines in the snippet.
-                                          this.buffer.push_str("\n");
-                                      }
-                                      let indent = this.block_indent.to_string(config);
-                                      this.buffer.push_str(&indent);
-                                  })
+        self.format_missing_inner(end, |this, last_snippet, snippet| {
+            this.buffer.push_str(last_snippet.trim_right());
+            if last_snippet == snippet {
+                // No new lines in the snippet.
+                this.buffer.push_str("\n");
+            }
+            let indent = this.block_indent.to_string(config);
+            this.buffer.push_str(&indent);
+        })
     }
 
     fn format_missing_inner<F: Fn(&mut FmtVisitor, &str, &str)>(&mut self,
diff --git a/tests/source/chains.rs b/tests/source/chains.rs
index f53afaa0c28..2a400b306d2 100644
--- a/tests/source/chains.rs
+++ b/tests/source/chains.rs
@@ -23,6 +23,20 @@ fn main() {
             2
         });
 
+    some_fuuuuuuuuunction()
+        .method_call_a(aaaaa, bbbbb, |c| {
+            let x = c;
+            x
+        });
+
+    some_fuuuuuuuuunction().method_call_a(aaaaa, bbbbb, |c| {
+        let x = c;
+        x
+    }).method_call_b(aaaaa, bbbbb, |c| {
+        let x = c;
+        x
+    });
+
     fffffffffffffffffffffffffffffffffff(a,
                                         {
                                             SCRIPT_TASK_ROOT
diff --git a/tests/target/chains.rs b/tests/target/chains.rs
index 590f0a5b156..ea9b6867efe 100644
--- a/tests/target/chains.rs
+++ b/tests/target/chains.rs
@@ -30,12 +30,26 @@ fn main() {
         }
     });
 
-    fffffffffffffffffffffffffffffffffff(a,
-                                        {
-                                            SCRIPT_TASK_ROOT.with(|root| {
-                                                *root.borrow_mut() = Some(&script_task);
-                                            });
-                                        });
+    some_fuuuuuuuuunction().method_call_a(aaaaa, bbbbb, |c| {
+        let x = c;
+        x
+    });
+
+    some_fuuuuuuuuunction()
+        .method_call_a(aaaaa, bbbbb, |c| {
+            let x = c;
+            x
+        })
+        .method_call_b(aaaaa, bbbbb, |c| {
+            let x = c;
+            x
+        });
+
+    fffffffffffffffffffffffffffffffffff(a, {
+        SCRIPT_TASK_ROOT.with(|root| {
+            *root.borrow_mut() = Some(&script_task);
+        });
+    });
 
     let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum = xxxxxxx.map(|x| x + 5)
                                                                           .map(|x| x / 2)
diff --git a/tests/target/hard-tabs.rs b/tests/target/hard-tabs.rs
index d8df0c2936a..e3160285c4d 100644
--- a/tests/target/hard-tabs.rs
+++ b/tests/target/hard-tabs.rs
@@ -76,12 +76,11 @@ fn main() {
 		}
 	});
 
-	fffffffffffffffffffffffffffffffffff(a,
-	                                    {
-		                                    SCRIPT_TASK_ROOT.with(|root| {
-			                                    *root.borrow_mut() = Some(&script_task);
-		                                    });
-	                                    });
+	fffffffffffffffffffffffffffffffffff(a, {
+		SCRIPT_TASK_ROOT.with(|root| {
+			*root.borrow_mut() = Some(&script_task);
+		});
+	});
 	a.b
 	 .c
 	 .d();