Force to use block for closure body with a single control flow expression

src/bin/cargo-fmt.rs

@@ -133,8 +133,10 @@ fn format_crate(
     let files: Vec<_> = targets
         .into_iter()
         .filter(|t| t.kind.should_format())
-        .inspect(|t| if verbosity == Verbosity::Verbose {
-            println!("[{:?}] {:?}", t.kind, t.path)
+        .inspect(|t| {
+            if verbosity == Verbosity::Verbose {
+                println!("[{:?}] {:?}", t.kind, t.path)
+            }
         })
         .map(|t| t.path)
         .collect();

src/chains.rs

@@ -297,10 +297,12 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
 
 // True if the chain is only `?`s.
 fn chain_only_try(exprs: &[ast::Expr]) -> bool {
-    exprs.iter().all(|e| if let ast::ExprKind::Try(_) = e.node {
-        true
-    } else {
-        false
+    exprs.iter().all(|e| {
+        if let ast::ExprKind::Try(_) = e.node {
+            true
+        } else {
+            false
+        }
     })
 }
 

src/comment.rs

@@ -311,10 +311,12 @@ fn rewrite_comment_inner(
             line
         })
         .map(|s| left_trim_comment_line(s, &style))
-        .map(|line| if orig.starts_with("/*") && line_breaks == 0 {
-            line.trim_left()
-        } else {
-            line
+        .map(|line| {
+            if orig.starts_with("/*") && line_breaks == 0 {
+                line.trim_left()
+            } else {
+                line
+            }
         });
 
     let mut result = opener.to_owned();

src/expr.rs

@@ -630,8 +630,7 @@ fn rewrite_closure(
         }
 
         // Figure out if the block is necessary.
-        let needs_block = block.rules != ast::BlockCheckMode::Default || block.stmts.len() > 1
-            || context.inside_macro
+        let needs_block = is_unsafe_block(block) || block.stmts.len() > 1 || context.inside_macro
             || block_contains_comment(block, context.codemap)
             || prefix.contains('\n');
 
@@ -642,8 +641,12 @@ fn rewrite_closure(
         };
         if no_return_type && !needs_block {
             // block.stmts.len() == 1
-            if let Some(expr) = stmt_expr(&block.stmts[0]) {
-                if let Some(rw) = rewrite_closure_expr(expr, &prefix, context, body_shape) {
+            if let Some(ref expr) = stmt_expr(&block.stmts[0]) {
+                if let Some(rw) = if is_block_closure_forced(expr) {
+                    rewrite_closure_with_block(context, shape, &prefix, expr)
+                } else {
+                    rewrite_closure_expr(expr, &prefix, context, body_shape)
+                } {
                     return Some(rw);
                 }
             }
@@ -1195,12 +1198,13 @@ impl<'a> ControlFlow<'a> {
             context
                 .codemap
                 .span_after(mk_sp(lo, self.span.hi()), self.keyword.trim()),
-            self.pat
-                .map_or(cond_span.lo(), |p| if self.matcher.is_empty() {
+            self.pat.map_or(cond_span.lo(), |p| {
+                if self.matcher.is_empty() {
                     p.span.lo()
                 } else {
                     context.codemap.span_before(self.span, self.matcher.trim())
-                }),
+                }
+            }),
         );
 
         let between_kwd_cond_comment = extract_comment(between_kwd_cond, context, shape);
@@ -2248,7 +2252,34 @@ fn rewrite_last_closure(
         if prefix.contains('\n') {
             return None;
         }
+        // If we are inside macro, we do not want to add or remove block from closure body.
+        if context.inside_macro {
+            return expr.rewrite(context, shape);
+        }
+
         let body_shape = shape.offset_left(extra_offset)?;
+
+        // We force to use block for the body of the closure for certain kinds of expressions.
+        if is_block_closure_forced(body) {
+            return rewrite_closure_with_block(context, body_shape, &prefix, body).and_then(
+                |body_str| {
+                    // If the expression can fit in a single line, we need not force block closure.
+                    if body_str.lines().count() <= 7 {
+                        match rewrite_closure_expr(body, &prefix, context, shape) {
+                            Some(ref single_line_body_str)
+                                if !single_line_body_str.contains('\n') =>
+                            {
+                                Some(single_line_body_str.clone())
+                            }
+                            _ => Some(body_str),
+                        }
+                    } else {
+                        Some(body_str)
+                    }
+                },
+            );
+        }
+
         // When overflowing the closure which consists of a single control flow expression,
         // force to use block if its condition uses multi line.
         let is_multi_lined_cond = rewrite_cond(context, body, body_shape)
@@ -2264,6 +2295,23 @@ fn rewrite_last_closure(
     None
 }
 
+fn is_block_closure_forced(expr: &ast::Expr) -> bool {
+    match expr.node {
+        ast::ExprKind::If(..) |
+        ast::ExprKind::IfLet(..) |
+        ast::ExprKind::Loop(..) |
+        ast::ExprKind::While(..) |
+        ast::ExprKind::WhileLet(..) |
+        ast::ExprKind::ForLoop(..) => true,
+        ast::ExprKind::AddrOf(_, ref expr) |
+        ast::ExprKind::Box(ref expr) |
+        ast::ExprKind::Try(ref expr) |
+        ast::ExprKind::Unary(_, ref expr) |
+        ast::ExprKind::Cast(ref expr, _) => is_block_closure_forced(expr),
+        _ => false,
+    }
+}
+
 fn rewrite_last_arg_with_overflow<'a, T>(
     context: &RewriteContext,
     args: &[&T],
@@ -2281,15 +2329,7 @@ where
             ast::ExprKind::Closure(..) => {
                 // If the argument consists of multiple closures, we do not overflow
                 // the last closure.
-                if args.len() > 1
-                    && args.iter()
-                        .rev()
-                        .skip(1)
-                        .filter_map(|arg| arg.to_expr())
-                        .any(|expr| match expr.node {
-                            ast::ExprKind::Closure(..) => true,
-                            _ => false,
-                        }) {
+                if args_have_many_closure(args) {
                     None
                 } else {
                     rewrite_last_closure(context, expr, shape)
@@ -2310,6 +2350,23 @@ where
     }
 }
 
+/// Returns true if the given vector of arguments has more than one `ast::ExprKind::Closure`.
+fn args_have_many_closure<T>(args: &[&T]) -> bool
+where
+    T: ToExpr,
+{
+    args.iter()
+        .filter(|arg| {
+            arg.to_expr()
+                .map(|e| match e.node {
+                    ast::ExprKind::Closure(..) => true,
+                    _ => false,
+                })
+                .unwrap_or(false)
+        })
+        .count() > 1
+}
+
 fn can_be_overflowed<'a, T>(context: &RewriteContext, args: &[&T]) -> bool
 where
     T: Rewrite + Spanned + ToExpr + 'a,
@@ -2698,13 +2755,17 @@ where
     if items.len() == 1 {
         // 3 = "(" + ",)"
         let nested_shape = shape.sub_width(3)?.visual_indent(1);
-        return items.next().unwrap().rewrite(context, nested_shape).map(
-            |s| if context.config.spaces_within_parens() {
-                format!("( {}, )", s)
-            } else {
-                format!("({},)", s)
-            },
-        );
+        return items
+            .next()
+            .unwrap()
+            .rewrite(context, nested_shape)
+            .map(|s| {
+                if context.config.spaces_within_parens() {
+                    format!("( {}, )", s)
+                } else {
+                    format!("({},)", s)
+                }
+            });
     }
 
     let list_lo = context.codemap.span_after(span, "(");

src/types.rs

@@ -670,10 +670,12 @@ impl Rewrite for ast::Ty {
             ast::TyKind::Paren(ref ty) => {
                 let budget = shape.width.checked_sub(2)?;
                 ty.rewrite(context, Shape::legacy(budget, shape.indent + 1))
-                    .map(|ty_str| if context.config.spaces_within_parens() {
-                        format!("( {} )", ty_str)
-                    } else {
-                        format!("({})", ty_str)
+                    .map(|ty_str| {
+                        if context.config.spaces_within_parens() {
+                            format!("( {} )", ty_str)
+                        } else {
+                            format!("({})", ty_str)
+                        }
                     })
             }
             ast::TyKind::Slice(ref ty) => {
@@ -683,10 +685,12 @@ impl Rewrite for ast::Ty {
                     shape.width.checked_sub(2)?
                 };
                 ty.rewrite(context, Shape::legacy(budget, shape.indent + 1))
-                    .map(|ty_str| if context.config.spaces_within_square_brackets() {
-                        format!("[ {} ]", ty_str)
-                    } else {
-                        format!("[{}]", ty_str)
+                    .map(|ty_str| {
+                        if context.config.spaces_within_square_brackets() {
+                            format!("[ {} ]", ty_str)
+                        } else {
+                            format!("[{}]", ty_str)
+                        }
                     })
             }
             ast::TyKind::Tup(ref items) => rewrite_tuple(

src/vertical.rs

@@ -191,13 +191,13 @@ fn struct_field_prefix_max_min_width<T: AlignedItem>(
     fields
         .iter()
         .map(|field| {
-            field
-                .rewrite_prefix(context, shape)
-                .and_then(|field_str| if field_str.contains('\n') {
+            field.rewrite_prefix(context, shape).and_then(|field_str| {
+                if field_str.contains('\n') {
                     None
                 } else {
                     Some(field_str.len())
-                })
+                }
+            })
         })
         .fold(Some((0, ::std::usize::MAX)), |acc, len| match (acc, len) {
             (Some((max_len, min_len)), Some(len)) => {

src/visitor.rs

@@ -136,13 +136,15 @@ impl<'a> FmtVisitor<'a> {
                     self.last_pos,
                     attr_lo.unwrap_or(first_stmt.span.lo()),
                 ));
-                let len = CommentCodeSlices::new(&snippet).nth(0).and_then(
-                    |(kind, _, s)| if kind == CodeCharKind::Normal {
-                        s.rfind('\n')
-                    } else {
-                        None
-                    },
-                );
+                let len = CommentCodeSlices::new(&snippet)
+                    .nth(0)
+                    .and_then(|(kind, _, s)| {
+                        if kind == CodeCharKind::Normal {
+                            s.rfind('\n')
+                        } else {
+                            None
+                        }
+                    });
                 if let Some(len) = len {
                     self.last_pos = self.last_pos + BytePos::from_usize(len);
                 }

tests/source/closure-block-inside-macro.rs

@@ -0,0 +1,11 @@
+// rustfmt-fn_call_style: Block
+
+// #1547
+fuzz_target!(|data: &[u8]| if let Some(first) = data.first() {
+    let index = *first as usize;
+    if index >= ENCODINGS.len() {
+        return;
+    }
+    let encoding = ENCODINGS[index];
+    dispatch_test(encoding, &data[1..]);
+});

tests/system.rs

@@ -446,15 +446,17 @@ struct CharsIgnoreNewlineRepr<'a>(Peekable<Chars<'a>>);
 impl<'a> Iterator for CharsIgnoreNewlineRepr<'a> {
     type Item = char;
     fn next(&mut self) -> Option<char> {
-        self.0.next().map(|c| if c == '\r' {
-            if *self.0.peek().unwrap_or(&'\0') == '\n' {
-                self.0.next();
-                '\n'
+        self.0.next().map(|c| {
+            if c == '\r' {
+                if *self.0.peek().unwrap_or(&'\0') == '\n' {
+                    self.0.next();
+                    '\n'
+                } else {
+                    '\r'
+                }
             } else {
-                '\r'
+                c
             }
-        } else {
-            c
         })
     }
 }

tests/target/chains-visual.rs

@@ -21,10 +21,12 @@ fn main() {
         false => (),
     });
 
-    loong_func().quux(move || if true {
-        1
-    } else {
-        2
+    loong_func().quux(move || {
+        if true {
+            1
+        } else {
+            2
+        }
     });
 
     some_fuuuuuuuuunction().method_call_a(aaaaa, bbbbb, |c| {

tests/target/chains.rs

@@ -23,10 +23,12 @@ fn main() {
         false => (),
     });
 
-    loong_func().quux(move || if true {
-        1
-    } else {
-        2
+    loong_func().quux(move || {
+        if true {
+            1
+        } else {
+            2
+        }
     });
 
     some_fuuuuuuuuunction().method_call_a(aaaaa, bbbbb, |c| {

tests/target/hard-tabs.rs

@@ -74,10 +74,12 @@ fn main() {
 		arg(a, b, c, d, e)
 	}
 
-	loong_func().quux(move || if true {
-		1
-	} else {
-		2
+	loong_func().quux(move || {
+		if true {
+			1
+		} else {
+			2
+		}
 	});
 
 	fffffffffffffffffffffffffffffffffff(a, {