Merge pull request #1550 from topecongiro/issue-1547

Prevent rewriting closure block to expr inside macro

src/chains.rs

@@ -440,5 +440,5 @@ fn rewrite_method_call(method_name: ast::Ident,
     let callee_str = format!(".{}{}", method_name, type_str);
     let span = mk_sp(lo, span.hi);
 
-    rewrite_call(context, &callee_str, &args[1..], span, shape, false)
+    rewrite_call(context, &callee_str, &args[1..], span, shape)
 }

src/expr.rs

@@ -70,7 +70,7 @@ fn format_expr(expr: &ast::Expr,
         }
         ast::ExprKind::Call(ref callee, ref args) => {
             let inner_span = mk_sp(callee.span.hi, expr.span.hi);
-            rewrite_call(context, &**callee, args, inner_span, shape, false)
+            rewrite_call(context, &**callee, args, inner_span, shape)
         }
         ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape),
         ast::ExprKind::Binary(ref op, ref lhs, ref rhs) => {
@@ -512,7 +512,8 @@ fn rewrite_closure(capture: ast::CaptureBy,
         }
 
         // Figure out if the block is necessary.
-        let needs_block = block.rules != ast::BlockCheckMode::Default || block.stmts.len() > 1 ||
+        let needs_block = block.rules != ast::BlockCheckMode::Default ||
+                          block.stmts.len() > 1 || context.inside_macro ||
                           block_contains_comment(block, context.codemap) ||
                           prefix.contains('\n');
 
@@ -1599,20 +1600,12 @@ pub fn rewrite_call<R>(context: &RewriteContext,
                        callee: &R,
                        args: &[ptr::P<ast::Expr>],
                        span: Span,
-                       shape: Shape,
+                       shape: Shape)
-                       force_no_trailing_comma: bool)
                        -> Option<String>
     where R: Rewrite
 {
-    let closure = |callee_max_width| {
+    let closure =
-        rewrite_call_inner(context,
+        |callee_max_width| rewrite_call_inner(context, callee, callee_max_width, args, span, shape);
-                           callee,
-                           callee_max_width,
-                           args,
-                           span,
-                           shape,
-                           force_no_trailing_comma)
-    };
 
     // 2 is for parens
     let max_width = try_opt!(shape.width.checked_sub(2));
@@ -1624,8 +1617,7 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
                          max_callee_width: usize,
                          args: &[ptr::P<ast::Expr>],
                          span: Span,
-                         shape: Shape,
+                         shape: Shape)
-                         force_no_trailing_comma: bool)
                          -> Result<String, Ordering>
     where R: Rewrite
 {
@@ -1665,13 +1657,8 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
     let span_lo = context.codemap.span_after(span, "(");
     let span = mk_sp(span_lo, span.hi);
 
-    let list_str = rewrite_call_args(context,
+    let list_str = rewrite_call_args(context, args, span, nested_shape, one_line_width)
-                                     args,
+        .ok_or(Ordering::Less)?;
-                                     span,
-                                     nested_shape,
-                                     one_line_width,
-                                     force_no_trailing_comma)
-            .ok_or(Ordering::Less)?;
 
     let result = if context.config.fn_call_style == IndentStyle::Visual ||
                     (!list_str.contains('\n') && list_str.chars().last().unwrap_or(' ') != ',') {
@@ -1695,8 +1682,7 @@ fn rewrite_call_args(context: &RewriteContext,
                      args: &[ptr::P<ast::Expr>],
                      span: Span,
                      shape: Shape,
-                     one_line_width: usize,
+                     one_line_width: usize)
-                     force_no_trailing_comma: bool)
                      -> Option<String> {
     let arg_count = args.len();
 
@@ -1766,7 +1752,7 @@ fn rewrite_call_args(context: &RewriteContext,
     let mut fmt = ListFormatting {
         tactic: tactic,
         separator: ",",
-        trailing_separator: if force_no_trailing_comma ||
+        trailing_separator: if context.inside_macro ||
                                context.config.fn_call_style == IndentStyle::Visual ||
                                arg_count <= 1 {
             SeparatorTactic::Never
@@ -1783,7 +1769,7 @@ fn rewrite_call_args(context: &RewriteContext,
         // try to put it on the next line. Try this only when we are in block mode
         // and not rewriting macro.
         Some(ref s) if context.config.fn_call_style == IndentStyle::Block &&
-                       !force_no_trailing_comma &&
+                       !context.inside_macro &&
                        (!s.contains('\n') &&
                         (s.len() > one_line_width || s.len() > context.config.fn_call_width)) => {
             fmt.trailing_separator = SeparatorTactic::Vertical;

src/macros.rs

@@ -65,6 +65,8 @@ pub fn rewrite_macro(mac: &ast::Mac,
                      shape: Shape,
                      position: MacroPosition)
                      -> Option<String> {
+    let mut context = &mut context.clone();
+    context.inside_macro = true;
     if context.config.use_try_shorthand {
         if let Some(expr) = convert_try_mac(mac, context) {
             return expr.rewrite(context, shape);
@@ -146,11 +148,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, true)
+            rewrite_call(context, &macro_name, &expr_vec, mac.span, shape).map(|rw| {
-                .map(|rw| match position {
+                match position {
-                         MacroPosition::Item => format!("{};", rw),
+                    MacroPosition::Item => format!("{};", rw),
-                         _ => rw,
+                    _ => rw,
-                     })
+                }
+            })
         }
         MacroStyle::Brackets => {
             // Format macro invocation as array literal.

src/rewrite.rs

@@ -26,6 +26,7 @@ pub struct RewriteContext<'a> {
     pub parse_session: &'a ParseSess,
     pub codemap: &'a CodeMap,
     pub config: &'a Config,
+    pub inside_macro: bool,
 }
 
 impl<'a> RewriteContext<'a> {

src/visitor.rs

@@ -574,6 +574,7 @@ pub fn get_context(&self) -> RewriteContext {
             parse_session: self.parse_session,
             codemap: self.codemap,
             config: self.config,
+            inside_macro: false,
         }
     }
 }

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

@@ -0,0 +1,15 @@
+// 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..]);
+        }
+    }
+);