Merge branch 'matthew-mcallister-attrib-block-expr'

topecongiro · topecongiro · commit 0dca70f29004 · 2018-03-14T01:16:57.000+09:00
diff --git a/src/closures.rs b/src/closures.rs
@@ -50,7 +50,8 @@ pub fn rewrite_closure(
     if let ast::ExprKind::Block(ref block) = body.node {
         // The body of the closure is an empty block.
         if block.stmts.is_empty() && !block_contains_comment(block, context.codemap) {
-            return Some(format!("{} {{}}", prefix));
+            return body.rewrite(context, shape)
+                .map(|s| format!("{} {}", prefix, s));
         }
 
         let result = match fn_decl.output {
@@ -138,7 +139,7 @@ fn rewrite_closure_with_block(
         span: body.span,
         recovered: false,
     };
-    let block = ::expr::rewrite_block_with_visitor(context, "", &block, shape, false)?;
+    let block = ::expr::rewrite_block_with_visitor(context, "", &block, None, shape, false)?;
     Some(format!("{} {}", prefix, block))
 }
 
@@ -291,7 +292,8 @@ pub fn rewrite_last_closure(
     if let ast::ExprKind::Closure(capture, movability, ref fn_decl, ref body, _) = expr.node {
         let body = match body.node {
             ast::ExprKind::Block(ref block)
-                if !is_unsafe_block(block) && is_simple_block(block, context.codemap) =>
+                if !is_unsafe_block(block)
+                    && is_simple_block(block, Some(&body.attrs), context.codemap) =>
             {
                 stmt_expr(&block.stmts[0]).unwrap_or(body)
             }
diff --git a/src/expr.rs b/src/expr.rs
@@ -115,16 +115,25 @@ pub fn format_expr(
             match expr_type {
                 ExprType::Statement => {
                     if is_unsafe_block(block) {
-                        block.rewrite(context, shape)
-                    } else if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
+                        rewrite_block(block, Some(&expr.attrs), context, shape)
+                    } else if let rw @ Some(_) =
+                        rewrite_empty_block(context, block, Some(&expr.attrs), "", shape)
+                    {
                         // Rewrite block without trying to put it in a single line.
                         rw
                     } else {
                         let prefix = block_prefix(context, block, shape)?;
-                        rewrite_block_with_visitor(context, &prefix, block, shape, true)
+                        rewrite_block_with_visitor(
+                            context,
+                            &prefix,
+                            block,
+                            Some(&expr.attrs),
+                            shape,
+                            true,
+                        )
                     }
                 }
-                ExprType::SubExpression => block.rewrite(context, shape),
+                ExprType::SubExpression => rewrite_block(block, Some(&expr.attrs), context, shape),
             }
         }
         ast::ExprKind::Match(ref cond, ref arms) => {
@@ -290,15 +299,22 @@ pub fn format_expr(
             Some(context.snippet(expr.span).to_owned())
         }
         ast::ExprKind::Catch(ref block) => {
-            if let rw @ Some(_) = rewrite_single_line_block(context, "do catch ", block, shape) {
+            if let rw @ Some(_) =
+                rewrite_single_line_block(context, "do catch ", block, Some(&expr.attrs), shape)
+            {
                 rw
             } else {
                 // 9 = `do catch `
                 let budget = shape.width.checked_sub(9).unwrap_or(0);
                 Some(format!(
                     "{}{}",
                     "do catch ",
-                    block.rewrite(context, Shape::legacy(budget, shape.indent))?
+                    rewrite_block(
+                        block,
+                        Some(&expr.attrs),
+                        context,
+                        Shape::legacy(budget, shape.indent)
+                    )?
                 ))
             }
         }
@@ -347,7 +363,7 @@ where
     let lhs_result = lhs.rewrite(context, lhs_shape)
         .map(|lhs_str| format!("{}{}", pp.prefix, lhs_str))?;
 
-    // Try to the both lhs and rhs on the same line.
+    // Try to put both lhs and rhs on the same line.
     let rhs_orig_result = shape
         .offset_left(last_line_width(&lhs_result) + pp.infix.len())
         .and_then(|s| s.sub_width(pp.suffix.len()))
@@ -564,11 +580,17 @@ fn nop_block_collapse(block_str: Option<String>, budget: usize) -> Option<String
 fn rewrite_empty_block(
     context: &RewriteContext,
     block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
+    prefix: &str,
     shape: Shape,
 ) -> Option<String> {
+    if attrs.map_or(false, |a| !inner_attributes(a).is_empty()) {
+        return None;
+    }
+
     if block.stmts.is_empty() && !block_contains_comment(block, context.codemap) && shape.width >= 2
     {
-        return Some("{}".to_owned());
+        return Some(format!("{}{{}}", prefix));
     }
 
     // If a block contains only a single-line comment, then leave it on one line.
@@ -579,7 +601,7 @@ fn rewrite_empty_block(
         if block.stmts.is_empty() && !comment_str.contains('\n') && !comment_str.starts_with("//")
             && comment_str.len() + 4 <= shape.width
         {
-            return Some(format!("{{ {} }}", comment_str));
+            return Some(format!("{}{{ {} }}", prefix, comment_str));
         }
     }
 
@@ -618,9 +640,10 @@ fn rewrite_single_line_block(
     context: &RewriteContext,
     prefix: &str,
     block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
     shape: Shape,
 ) -> Option<String> {
-    if is_simple_block(block, context.codemap) {
+    if is_simple_block(block, attrs, context.codemap) {
         let expr_shape = shape.offset_left(last_line_width(prefix))?;
         let expr_str = block.stmts[0].rewrite(context, expr_shape)?;
         let result = format!("{}{{ {} }}", prefix, expr_str);
@@ -635,10 +658,11 @@ pub fn rewrite_block_with_visitor(
     context: &RewriteContext,
     prefix: &str,
     block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
     shape: Shape,
     has_braces: bool,
 ) -> Option<String> {
-    if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
+    if let rw @ Some(_) = rewrite_empty_block(context, block, attrs, prefix, shape) {
         return rw;
     }
 
@@ -654,31 +678,41 @@ pub fn rewrite_block_with_visitor(
         ast::BlockCheckMode::Default => visitor.last_pos = block.span.lo(),
     }
 
-    visitor.visit_block(block, None, has_braces);
+    let inner_attrs = attrs.map(inner_attributes);
+    visitor.visit_block(block, inner_attrs.as_ref().map(|a| &**a), has_braces);
     Some(format!("{}{}", prefix, visitor.buffer))
 }
 
 impl Rewrite for ast::Block {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
-        // shape.width is used only for the single line case: either the empty block `{}`,
-        // or an unsafe expression `unsafe { e }`.
-        if let rw @ Some(_) = rewrite_empty_block(context, self, shape) {
-            return rw;
-        }
+        rewrite_block(self, None, context, shape)
+    }
+}
 
-        let prefix = block_prefix(context, self, shape)?;
+fn rewrite_block(
+    block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
+    context: &RewriteContext,
+    shape: Shape,
+) -> Option<String> {
+    let prefix = block_prefix(context, block, shape)?;
 
-        let result = rewrite_block_with_visitor(context, &prefix, self, shape, true);
-        if let Some(ref result_str) = result {
-            if result_str.lines().count() <= 3 {
-                if let rw @ Some(_) = rewrite_single_line_block(context, &prefix, self, shape) {
-                    return rw;
-                }
+    // shape.width is used only for the single line case: either the empty block `{}`,
+    // or an unsafe expression `unsafe { e }`.
+    if let rw @ Some(_) = rewrite_empty_block(context, block, attrs, &prefix, shape) {
+        return rw;
+    }
+
+    let result = rewrite_block_with_visitor(context, &prefix, block, attrs, shape, true);
+    if let Some(ref result_str) = result {
+        if result_str.lines().count() <= 3 {
+            if let rw @ Some(_) = rewrite_single_line_block(context, &prefix, block, attrs, shape) {
+                return rw;
             }
         }
-
-        result
     }
+
+    result
 }
 
 impl Rewrite for ast::Stmt {
@@ -889,8 +923,8 @@ impl<'a> ControlFlow<'a> {
         let fixed_cost = self.keyword.len() + "  {  } else {  }".len();
 
         if let ast::ExprKind::Block(ref else_node) = else_block.node {
-            if !is_simple_block(self.block, context.codemap)
-                || !is_simple_block(else_node, context.codemap)
+            if !is_simple_block(self.block, None, context.codemap)
+                || !is_simple_block(else_node, None, context.codemap)
                 || pat_expr_str.contains('\n')
             {
                 return None;
@@ -1123,7 +1157,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
         let mut block_context = context.clone();
         block_context.is_if_else_block = self.else_block.is_some();
         let block_str =
-            rewrite_block_with_visitor(&block_context, "", self.block, block_shape, true)?;
+            rewrite_block_with_visitor(&block_context, "", self.block, None, block_shape, true)?;
 
         let mut result = format!("{}{}", cond_str, block_str);
 
@@ -1234,22 +1268,39 @@ pub fn block_contains_comment(block: &ast::Block, codemap: &CodeMap) -> bool {
     contains_comment(&snippet)
 }
 
-// Checks that a block contains no statements, an expression and no comments.
+// Checks that a block contains no statements, an expression and no comments or
+// attributes.
 // FIXME: incorrectly returns false when comment is contained completely within
 // the expression.
-pub fn is_simple_block(block: &ast::Block, codemap: &CodeMap) -> bool {
+pub fn is_simple_block(
+    block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
+    codemap: &CodeMap,
+) -> bool {
     (block.stmts.len() == 1 && stmt_is_expr(&block.stmts[0])
-        && !block_contains_comment(block, codemap))
+        && !block_contains_comment(block, codemap) && attrs.map_or(true, |a| a.is_empty()))
 }
 
-/// Checks whether a block contains at most one statement or expression, and no comments.
-pub fn is_simple_block_stmt(block: &ast::Block, codemap: &CodeMap) -> bool {
+/// Checks whether a block contains at most one statement or expression, and no
+/// comments or attributes.
+pub fn is_simple_block_stmt(
+    block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
+    codemap: &CodeMap,
+) -> bool {
     block.stmts.len() <= 1 && !block_contains_comment(block, codemap)
+        && attrs.map_or(true, |a| a.is_empty())
 }
 
-/// Checks whether a block contains no statements, expressions, or comments.
-pub fn is_empty_block(block: &ast::Block, codemap: &CodeMap) -> bool {
+/// Checks whether a block contains no statements, expressions, comments, or
+/// inner attributes.
+pub fn is_empty_block(
+    block: &ast::Block,
+    attrs: Option<&[ast::Attribute]>,
+    codemap: &CodeMap,
+) -> bool {
     block.stmts.is_empty() && !block_contains_comment(block, codemap)
+        && attrs.map_or(true, |a| inner_attributes(a).is_empty())
 }
 
 pub fn stmt_is_expr(stmt: &ast::Stmt) -> bool {
@@ -1577,7 +1628,8 @@ fn rewrite_match_pattern(
 fn flatten_arm_body<'a>(context: &'a RewriteContext, body: &'a ast::Expr) -> (bool, &'a ast::Expr) {
     match body.node {
         ast::ExprKind::Block(ref block)
-            if !is_unsafe_block(block) && is_simple_block(block, context.codemap) =>
+            if !is_unsafe_block(block)
+                && is_simple_block(block, Some(&body.attrs), context.codemap) =>
         {
             if let ast::StmtKind::Expr(ref expr) = block.stmts[0].node {
                 (
@@ -1605,7 +1657,10 @@ fn rewrite_match_body(
 ) -> Option<String> {
     let (extend, body) = flatten_arm_body(context, body);
     let (is_block, is_empty_block) = if let ast::ExprKind::Block(ref block) = body.node {
-        (true, is_empty_block(block, context.codemap))
+        (
+            true,
+            is_empty_block(block, Some(&body.attrs), context.codemap),
+        )
     } else {
         (false, false)
     };
diff --git a/src/items.rs b/src/items.rs
@@ -301,6 +301,7 @@ impl<'a> FmtVisitor<'a> {
         fn_sig: &FnSig,
         span: Span,
         block: &ast::Block,
+        inner_attrs: Option<&[ast::Attribute]>,
     ) -> Option<String> {
         let context = self.get_context();
 
@@ -329,7 +330,8 @@ impl<'a> FmtVisitor<'a> {
             result.push(' ');
         }
 
-        self.single_line_fn(&result, block).or_else(|| Some(result))
+        self.single_line_fn(&result, block, inner_attrs)
+            .or_else(|| Some(result))
     }
 
     pub fn rewrite_required_fn(
@@ -360,20 +362,25 @@ impl<'a> FmtVisitor<'a> {
         Some(result)
     }
 
-    fn single_line_fn(&self, fn_str: &str, block: &ast::Block) -> Option<String> {
-        if fn_str.contains('\n') {
+    fn single_line_fn(
+        &self,
+        fn_str: &str,
+        block: &ast::Block,
+        inner_attrs: Option<&[ast::Attribute]>,
+    ) -> Option<String> {
+        if fn_str.contains('\n') || inner_attrs.map_or(false, |a| !a.is_empty()) {
             return None;
         }
 
         let codemap = self.get_context().codemap;
 
-        if self.config.empty_item_single_line() && is_empty_block(block, codemap)
+        if self.config.empty_item_single_line() && is_empty_block(block, None, codemap)
             && self.block_indent.width() + fn_str.len() + 2 <= self.config.max_width()
         {
             return Some(format!("{}{{}}", fn_str));
         }
 
-        if self.config.fn_single_line() && is_simple_block_stmt(block, codemap) {
+        if self.config.fn_single_line() && is_simple_block_stmt(block, None, codemap) {
             let rewrite = {
                 if let Some(stmt) = block.stmts.first() {
                     match stmt_expr(stmt) {
diff --git a/src/visitor.rs b/src/visitor.rs
@@ -268,6 +268,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                     &FnSig::from_fn_kind(&fk, generics, fd, defaultness),
                     mk_sp(s.lo(), b.span.lo()),
                     b,
+                    inner_attrs,
                 )
             }
             visit::FnKind::Closure(_) => unreachable!(),
@@ -381,13 +382,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 self.visit_static(&StaticParts::from_item(item));
             }
             ast::ItemKind::Fn(ref decl, unsafety, constness, abi, ref generics, ref body) => {
+                let inner_attrs = inner_attributes(&item.attrs);
                 self.visit_fn(
                     visit::FnKind::ItemFn(item.ident, unsafety, constness, abi, &item.vis, body),
                     generics,
                     decl,
                     item.span,
                     ast::Defaultness::Final,
-                    Some(&item.attrs),
+                    Some(&inner_attrs),
                 )
             }
             ast::ItemKind::Ty(ref ty, ref generics) => {
@@ -438,13 +440,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 self.push_rewrite(ti.span, rewrite);
             }
             ast::TraitItemKind::Method(ref sig, Some(ref body)) => {
+                let inner_attrs = inner_attributes(&ti.attrs);
                 self.visit_fn(
                     visit::FnKind::Method(ti.ident, sig, None, body),
                     &ti.generics,
                     &sig.decl,
                     ti.span,
                     ast::Defaultness::Final,
-                    Some(&ti.attrs),
+                    Some(&inner_attrs),
                 );
             }
             ast::TraitItemKind::Type(ref type_param_bounds, ref type_default) => {
@@ -473,13 +476,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
         match ii.node {
             ast::ImplItemKind::Method(ref sig, ref body) => {
+                let inner_attrs = inner_attributes(&ii.attrs);
                 self.visit_fn(
                     visit::FnKind::Method(ii.ident, sig, Some(&ii.vis), body),
                     &ii.generics,
                     &sig.decl,
                     ii.span,
                     ii.defaultness,
-                    Some(&ii.attrs),
+                    Some(&inner_attrs),
                 );
             }
             ast::ImplItemKind::Const(..) => self.visit_static(&StaticParts::from_impl_item(ii)),
diff --git a/tests/target/attrib-block-expr.rs b/tests/target/attrib-block-expr.rs
