Merge pull request #1584 from topecongiro/poor/chain-trailing-try

Enhance chain

Configurations.md

@@ -112,6 +112,28 @@ let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elit();
 #### Lines longer than `chain_one_line_max`:
 See [`chain_indent`](#chain_indent).
 
+## `chain_split_single_child`
+
+Split a chain with a single child if its length exceeds [`chain_one_line_max`](#chain_one_line_max).
+
+- **Default value**: `false`
+- **Possible values**: `false`, `true`
+
+#### `false`
+
+```rust
+let files = fs::read_dir("tests/coverage/source").expect("Couldn't read source dir");
+```
+
+#### `true`
+
+```rust
+let files = fs::read_dir("tests/coverage/source")
+    .expect("Couldn't read source dir");
+```
+
+See also [`chain_one_line_max`](#chain_one_line_max).
+
 ## `closure_block_indent_threshold`
 
 How many lines a closure must have before it is block indented. -1 means never use block indent.

src/chains.rs

@@ -83,6 +83,7 @@ use expr::rewrite_call;
 use config::IndentStyle;
 use macros::convert_try_mac;
 
+use std::cmp::min;
 use std::iter;
 use syntax::{ast, ptr};
 use syntax::codemap::{mk_sp, Span};
@@ -97,6 +98,13 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
     if chain_only_try(&subexpr_list) {
         return rewrite_try(&parent, subexpr_list.len(), context, shape);
     }
+    let trailing_try_num = subexpr_list
+        .iter()
+        .take_while(|e| match e.node {
+                        ast::ExprKind::Try(..) => true,
+                        _ => false,
+                    })
+        .count();
 
     // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`.
     let parent_shape = if is_block_expr(context, &parent, "\n") {
@@ -155,10 +163,10 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
            first_child_shape,
            other_child_shape);
 
-    let child_shape_iter = Some(first_child_shape)
-        .into_iter()
-        .chain(::std::iter::repeat(other_child_shape)
-                   .take(subexpr_list.len() - 1));
+    let child_shape_iter =
+        Some(first_child_shape)
+            .into_iter()
+            .chain(::std::iter::repeat(other_child_shape).take(subexpr_list.len() - 1));
     let iter = subexpr_list.iter().rev().zip(child_shape_iter);
     let mut rewrites = try_opt!(iter.map(|(e, shape)| {
                                              rewrite_chain_subexpr(e, total_span, context, shape)
@@ -166,17 +174,19 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
                                     .collect::<Option<Vec<_>>>());
 
     // Total of all items excluding the last.
-    let almost_total = rewrites[..rewrites.len() - 1]
+    let last_non_try_index = rewrites.len() - (1 + trailing_try_num);
+    let almost_total = rewrites[..last_non_try_index]
         .iter()
         .fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len();
     let one_line_len = rewrites.iter().fold(0, |a, r| a + first_line_width(r)) +
                        parent_rewrite.len();
 
-    let veto_single_line = if one_line_len > context.config.chain_one_line_max() {
+    let one_line_budget = min(shape.width, context.config.chain_one_line_max());
+    let veto_single_line = if one_line_len > one_line_budget {
         if rewrites.len() > 1 {
             true
         } else if rewrites.len() == 1 {
-            parent_rewrite.len() > context.config.chain_one_line_max() / 2
+            context.config.chain_split_single_child() || one_line_len > shape.width
         } else {
             false
         }
@@ -195,7 +205,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
     let mut fits_single_line = !veto_single_line && almost_total <= shape.width;
     if fits_single_line {
         let len = rewrites.len();
-        let (init, last) = rewrites.split_at_mut(len - 1);
+        let (init, last) = rewrites.split_at_mut(len - (1 + trailing_try_num));
         fits_single_line = init.iter().all(|s| !s.contains('\n'));
 
         if fits_single_line {
@@ -219,6 +229,25 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
         }
     }
 
+    // Try overflowing the last element if we are using block indent.
+    if !fits_single_line && context.config.fn_call_style() == IndentStyle::Block {
+        let (init, last) = rewrites.split_at_mut(last_non_try_index);
+        let almost_single_line = init.iter().all(|s| !s.contains('\n'));
+        if almost_single_line {
+            let overflow_shape = Shape {
+                width: one_line_budget,
+                ..parent_shape
+            };
+            fits_single_line = rewrite_last_child_with_overflow(context,
+                                                                &subexpr_list[trailing_try_num],
+                                                                overflow_shape,
+                                                                total_span,
+                                                                almost_total,
+                                                                one_line_budget,
+                                                                &mut last[0]);
+        }
+    }
+
     let connector = if fits_single_line && !parent_rewrite_contains_newline {
         // Yay, we can put everything on one line.
         String::new()
@@ -258,6 +287,27 @@ fn chain_only_try(exprs: &[ast::Expr]) -> bool {
                      })
 }
 
+// Try to rewrite and replace the last non-try child. Return `true` if
+// replacing succeeds.
+fn rewrite_last_child_with_overflow(context: &RewriteContext,
+                                    expr: &ast::Expr,
+                                    shape: Shape,
+                                    span: Span,
+                                    almost_total: usize,
+                                    one_line_budget: usize,
+                                    last_child: &mut String)
+                                    -> bool {
+    if let Some(shape) = shape.shrink_left(almost_total) {
+        if let Some(ref mut rw) = rewrite_chain_subexpr(expr, span, context, shape) {
+            if almost_total + first_line_width(rw) <= one_line_budget {
+                ::std::mem::swap(last_child, rw);
+                return true;
+            }
+        }
+    }
+    false
+}
+
 pub fn rewrite_try(expr: &ast::Expr,
                    try_count: usize,
                    context: &RewriteContext,

src/config.rs

@@ -448,6 +448,8 @@ create_config! {
         "Report all, none or unnumbered occurrences of FIXME in source file comments";
     chain_indent: IndentStyle, IndentStyle::Block, "Indentation of chain";
     chain_one_line_max: usize, 60, "Maximum length of a chain to fit on a single line";
+    chain_split_single_child: bool, false, "Split a chain with a single child if its length \
+                                            exceeds `chain_one_line_max`";
     reorder_imports: bool, false, "Reorder import statements alphabetically";
     reorder_imports_in_group: bool, false, "Reorder import statements in group";
     reorder_imported_names: bool, false,

src/expr.rs

@@ -98,20 +98,16 @@ fn format_expr(expr: &ast::Expr,
         }
         ast::ExprKind::Tup(ref items) => rewrite_tuple(context, items, expr.span, shape),
         ast::ExprKind::While(ref cond, ref block, label) => {
-            ControlFlow::new_while(None, cond, block, label, expr.span)
-                .rewrite(context, shape)
+            ControlFlow::new_while(None, cond, block, label, expr.span).rewrite(context, shape)
         }
         ast::ExprKind::WhileLet(ref pat, ref cond, ref block, label) => {
-            ControlFlow::new_while(Some(pat), cond, block, label, expr.span)
-                .rewrite(context, shape)
+            ControlFlow::new_while(Some(pat), cond, block, label, expr.span).rewrite(context, shape)
         }
         ast::ExprKind::ForLoop(ref pat, ref cond, ref block, label) => {
-            ControlFlow::new_for(pat, cond, block, label, expr.span)
-                .rewrite(context, shape)
+            ControlFlow::new_for(pat, cond, block, label, expr.span).rewrite(context, shape)
         }
         ast::ExprKind::Loop(ref block, label) => {
-            ControlFlow::new_loop(block, label, expr.span)
-                .rewrite(context, shape)
+            ControlFlow::new_loop(block, label, expr.span).rewrite(context, shape)
         }
         ast::ExprKind::Block(ref block) => block.rewrite(context, shape),
         ast::ExprKind::If(ref cond, ref if_block, ref else_block) => {
@@ -179,12 +175,11 @@ fn format_expr(expr: &ast::Expr,
         ast::ExprKind::Mac(ref mac) => {
             // Failure to rewrite a marco should not imply failure to
             // rewrite the expression.
-            rewrite_macro(mac, None, context, shape, MacroPosition::Expression)
-                .or_else(|| {
-                             wrap_str(context.snippet(expr.span),
-                                      context.config.max_width(),
-                                      shape)
-                         })
+            rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|| {
+                wrap_str(context.snippet(expr.span),
+                         context.config.max_width(),
+                         shape)
+            })
         }
         ast::ExprKind::Ret(None) => {
             wrap_str("return".to_owned(), context.config.max_width(), shape)
@@ -324,8 +319,7 @@ pub fn rewrite_pair<LHS, RHS>(lhs: &LHS,
                                   .checked_sub(shape.used_width() + prefix.len() + infix.len()));
     let rhs_shape = match context.config.control_style() {
         Style::Default => {
-            try_opt!(shape.sub_width(suffix.len() + prefix.len()))
-                .visual_indent(prefix.len())
+            try_opt!(shape.sub_width(suffix.len() + prefix.len())).visual_indent(prefix.len())
         }
         Style::Rfc => try_opt!(shape.block_left(context.config.tab_spaces())),
     };
@@ -516,8 +510,7 @@ fn rewrite_closure(capture: ast::CaptureBy,
 
     // 1 = space between `|...|` and body.
     let extra_offset = extra_offset(&prefix, shape) + 1;
-    let body_shape = try_opt!(shape.sub_width(extra_offset))
-        .add_offset(extra_offset);
+    let body_shape = try_opt!(shape.sub_width(extra_offset)).add_offset(extra_offset);
 
     if let ast::ExprKind::Block(ref block) = body.node {
         // The body of the closure is an empty block.
@@ -859,8 +852,8 @@ impl<'a> ControlFlow<'a> {
 
             let new_width = try_opt!(new_width.checked_sub(if_str.len()));
             let else_expr = &else_node.stmts[0];
-            let else_str = try_opt!(else_expr.rewrite(context,
-                                                      Shape::legacy(new_width, Indent::empty())));
+            let else_str =
+                try_opt!(else_expr.rewrite(context, Shape::legacy(new_width, Indent::empty())));
 
             if if_str.contains('\n') || else_str.contains('\n') {
                 return None;
@@ -1765,6 +1758,7 @@ fn rewrite_call_args(context: &RewriteContext,
         config: context.config,
     };
 
+    let one_line_budget = min(one_line_width, context.config.fn_call_width());
     let almost_no_newline =
         item_vec
             .iter()
@@ -1774,7 +1768,7 @@ fn rewrite_call_args(context: &RewriteContext,
     let extendable = almost_no_newline &&
                      item_vec.iter().fold(0, |acc, item| {
         acc + item.item.as_ref().map_or(0, |s| 2 + first_line_width(s))
-    }) <= min(one_line_width, context.config.fn_call_width()) + 2;
+    }) <= one_line_budget + 2;
 
     match write_list(&item_vec, &fmt) {
         // If arguments do not fit in a single line and do not contain newline,
@@ -1782,9 +1776,9 @@ fn rewrite_call_args(context: &RewriteContext,
         // and not rewriting macro.
         Some(ref s) if context.config.fn_call_style() == IndentStyle::Block &&
                        !context.inside_macro &&
-                       (!can_be_overflowed(context, args) && args.len() == 1 && s.contains('\n') ||
-                        first_line_width(s) > one_line_width ||
-                        first_line_width(s) > context.config.fn_call_width()) => {
+                       ((!can_be_overflowed(context, args) && args.len() == 1 &&
+                         s.contains('\n')) ||
+                        first_line_width(s) > one_line_budget) => {
             fmt.trailing_separator = SeparatorTactic::Vertical;
             fmt.tactic = DefinitiveListTactic::Vertical;
             write_list(&item_vec, &fmt).map(|rw| (false, rw))

src/file_lines.rs

@@ -135,8 +135,7 @@ impl FileLines {
             Some(ref map) => map,
         };
 
-        match canonicalize_path_string(file_name)
-                  .and_then(|file| map.get(&file)) {
+        match canonicalize_path_string(file_name).and_then(|file| map.get(&file)) {
             Some(ranges) => ranges.iter().any(f),
             None => false,
         }

src/items.rs

@@ -1125,8 +1125,7 @@ pub fn rewrite_type_alias(context: &RewriteContext,
 
     let generics_indent = indent + result.len();
     let generics_span = mk_sp(context.codemap.span_after(span, "type"), ty.span.lo);
-    let shape = try_opt!(Shape::indented(generics_indent, context.config)
-                             .sub_width(" =".len()));
+    let shape = try_opt!(Shape::indented(generics_indent, context.config).sub_width(" =".len()));
     let generics_str = try_opt!(rewrite_generics(context, generics, shape, generics_span));
 
     result.push_str(&generics_str);

src/lists.rs

@@ -430,8 +430,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
             let post_snippet_trimmed = if post_snippet.starts_with(',') {
                 post_snippet[1..].trim_matches(white_space)
             } else if post_snippet.ends_with(',') {
-                post_snippet[..(post_snippet.len() - 1)]
-                    .trim_matches(white_space)
+                post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space)
             } else {
                 post_snippet
             };
@@ -529,8 +528,7 @@ pub fn struct_lit_shape(shape: Shape,
                         -> Option<(Option<Shape>, Shape)> {
     let v_shape = match context.config.struct_lit_style() {
         IndentStyle::Visual => {
-            try_opt!(try_opt!(shape.shrink_left(prefix_width))
-                         .sub_width(suffix_width))
+            try_opt!(try_opt!(shape.shrink_left(prefix_width)).sub_width(suffix_width))
         }
         IndentStyle::Block => {
             let shape = shape.block_indent(context.config.tab_spaces());

src/macros.rs

@@ -172,11 +172,12 @@ pub fn rewrite_macro(mac: &ast::Mac,
         MacroStyle::Parens => {
             // Format macro invocation as function call, forcing no trailing
             // comma because not all macros support them.
-            rewrite_call(context, &macro_name, &expr_vec, mac.span, shape)
-                .map(|rw| match position {
-                         MacroPosition::Item => format!("{};", rw),
-                         _ => rw,
-                     })
+            rewrite_call(context, &macro_name, &expr_vec, mac.span, shape).map(|rw| {
+                match position {
+                    MacroPosition::Item => format!("{};", rw),
+                    _ => rw,
+                }
+            })
         }
         MacroStyle::Brackets => {
             let mac_shape = try_opt!(shape.shrink_left(macro_name.len()));

src/modules.rs

@@ -68,8 +68,7 @@ fn module_file(id: ast::Ident,
         return path;
     }
 
-    match parser::Parser::default_submod_path(id, dir_path, codemap)
-              .result {
+    match parser::Parser::default_submod_path(id, dir_path, codemap).result {
         Ok(parser::ModulePathSuccess { path, .. }) => path,
         Err(_) => panic!("Couldn't find module {}", id),
     }

src/patterns.rs

@@ -39,10 +39,10 @@ impl Rewrite for Pat {
                 let sub_pat = match *sub_pat {
                     Some(ref p) => {
                         // 3 - ` @ `.
-                        let width = try_opt!(shape.width.checked_sub(prefix.len() +
-                                                                     mut_infix.len() +
-                                                                     id_str.len() +
-                                                                     3));
+                        let width =
+                            try_opt!(shape.width.checked_sub(prefix.len() + mut_infix.len() +
+                                                             id_str.len() +
+                                                             3));
                         format!(" @ {}",
                                 try_opt!(p.rewrite(context, Shape::legacy(width, shape.indent))))
                     }

src/string.rs

@@ -35,8 +35,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
     let re = Regex::new(r"([^\\](\\\\)*)\\[\n\r][[:space:]]*").unwrap();
     let stripped_str = re.replace_all(orig, "$1");
 
-    let graphemes = UnicodeSegmentation::graphemes(&*stripped_str, false)
-        .collect::<Vec<&str>>();
+    let graphemes = UnicodeSegmentation::graphemes(&*stripped_str, false).collect::<Vec<&str>>();
     let shape = fmt.shape.visual_indent(0);
     let indent = shape.indent.to_string(fmt.config);
     let punctuation = ":,;.";

src/utils.rs

@@ -332,8 +332,7 @@ pub fn wrap_str<S: AsRef<str>>(s: S, max_width: usize, shape: Shape) -> Option<S
 
 impl Rewrite for String {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
-        wrap_str(self, context.config.max_width(), shape)
-            .map(ToOwned::to_owned)
+        wrap_str(self, context.config.max_width(), shape).map(ToOwned::to_owned)
     }
 }
 

tests/source/configs-fn_call_style-block.rs

@@ -5,6 +5,17 @@ fn main() {
     lorem("lorem", "ipsum", "dolor", "sit", "amet", "consectetur", "adipiscing", "elit");
     // #1501
     let hyper = Arc::new(Client::with_connector(HttpsConnector::new(TlsClient::new())));
+
+    // chain
+    let x = yooooooooooooo.fooooooooooooooo.baaaaaaaaaaaaar(hello, world);
+
+    // #1380
+    {
+        {
+            let creds = self.client
+                .client_credentials(&self.config.auth.oauth2.id, &self.config.auth.oauth2.secret)?;
+        }
+    }
 }
 
 // #1521

tests/system.rs

@@ -55,8 +55,7 @@ fn system_tests() {
 // the only difference is the coverage mode
 #[test]
 fn coverage_tests() {
-    let files = fs::read_dir("tests/coverage/source")
-        .expect("Couldn't read source dir");
+    let files = fs::read_dir("tests/coverage/source").expect("Couldn't read source dir");
     let files = files.map(get_path_string);
     let (_reports, count, fails) = check_files(files);
 
@@ -83,8 +82,7 @@ fn assert_output(source: &str, expected_filename: &str) {
     let _ = filemap::write_all_files(&file_map, &mut out, &config);
     let output = String::from_utf8(out).unwrap();
 
-    let mut expected_file = fs::File::open(&expected_filename)
-        .expect("Couldn't open target");
+    let mut expected_file = fs::File::open(&expected_filename).expect("Couldn't open target");
     let mut expected_text = String::new();
     expected_file
         .read_to_string(&mut expected_text)
@@ -279,8 +277,7 @@ fn get_config(config_file: Option<&str>) -> Config {
         }
     };
 
-    let mut def_config_file = fs::File::open(config_file_name)
-        .expect("Couldn't open config");
+    let mut def_config_file = fs::File::open(config_file_name).expect("Couldn't open config");
     let mut def_config = String::new();
     def_config_file
         .read_to_string(&mut def_config)

tests/target/closure.rs

@@ -144,9 +144,9 @@ fn issue470() {
 impl Foo {
     pub fn bar(&self) {
         Some(SomeType {
-                 push_closure_out_to_100_chars: iter(otherwise_it_works_ok.into_iter().map(|f| {
-                                                                                               Ok(f)
-                                                                                           })),
+                 push_closure_out_to_100_chars: iter(otherwise_it_works_ok
+                                                         .into_iter()
+                                                         .map(|f| Ok(f))),
              })
     }
 }

tests/target/configs-chain_split_single_child-false.rs

@@ -0,0 +1,5 @@
+// rustfmt-chain_split_single_child: false
+
+fn main() {
+    let files = fs::read_dir("tests/source").expect("Couldn't read source dir");
+}

tests/target/configs-chain_split_single_child-true.rs

@@ -0,0 +1,6 @@
+// rustfmt-chain_split_single_child: true
+
+fn main() {
+    let files = fs::read_dir("tests/source")
+        .expect("Couldn't read source dir");
+}

tests/target/configs-fn_call_style-block.rs

@@ -16,6 +16,22 @@ fn main() {
     let hyper = Arc::new(Client::with_connector(
         HttpsConnector::new(TlsClient::new()),
     ));
+
+    // chain
+    let x = yooooooooooooo.fooooooooooooooo.baaaaaaaaaaaaar(
+        hello,
+        world,
+    );
+
+    // #1380
+    {
+        {
+            let creds = self.client.client_credentials(
+                &self.config.auth.oauth2.id,
+                &self.config.auth.oauth2.secret,
+            )?;
+        }
+    }
 }
 
 // #1521