Format and preserve attributes on ast::Stmt

topecongiro · topecongiro · commit 979d131c5d83 · 2017-08-30T19:27:36.000+09:00
diff --git a/src/expr.rs b/src/expr.rs
@@ -130,11 +130,10 @@ pub fn format_expr(
                 ExprType::Statement => {
                     if is_unsafe_block(block) {
                         block.rewrite(context, shape)
-                    } else {
+                    } else if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
                         // Rewrite block without trying to put it in a single line.
-                        if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
-                            return rw;
-                        }
+                        rw
+                    } else {
                         let prefix = try_opt!(block_prefix(context, block, shape));
                         rewrite_block_with_visitor(context, &prefix, block, shape)
                     }
@@ -293,17 +292,17 @@ pub fn format_expr(
             shape,
         ),
         ast::ExprKind::Catch(ref block) => {
-            if let rewrite @ Some(_) = rewrite_single_line_block(context, "do catch ", block, shape)
-            {
-                return rewrite;
+            if let rw @ Some(_) = rewrite_single_line_block(context, "do catch ", block, shape) {
+                rw
+            } else {
+                // 9 = `do catch `
+                let budget = shape.width.checked_sub(9).unwrap_or(0);
+                Some(format!(
+                    "{}{}",
+                    "do catch ",
+                    try_opt!(block.rewrite(&context, Shape::legacy(budget, shape.indent)))
+                ))
             }
-            // 9 = `do catch `
-            let budget = shape.width.checked_sub(9).unwrap_or(0);
-            Some(format!(
-                "{}{}",
-                "do catch ",
-                try_opt!(block.rewrite(&context, Shape::legacy(budget, shape.indent)))
-            ))
         }
     };
 
@@ -883,16 +882,13 @@ impl Rewrite for ast::Stmt {
                     ""
                 };
 
-                format_expr(
-                    ex,
-                    match self.node {
-                        ast::StmtKind::Expr(_) => ExprType::SubExpression,
-                        ast::StmtKind::Semi(_) => ExprType::Statement,
-                        _ => unreachable!(),
-                    },
-                    context,
-                    try_opt!(shape.sub_width(suffix.len())),
-                ).map(|s| s + suffix)
+                let expr_type = match self.node {
+                    ast::StmtKind::Expr(_) => ExprType::SubExpression,
+                    ast::StmtKind::Semi(_) => ExprType::Statement,
+                    _ => unreachable!(),
+                };
+                let shape = try_opt!(shape.sub_width(suffix.len()));
+                format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
             }
             ast::StmtKind::Mac(..) | ast::StmtKind::Item(..) => None,
         };
diff --git a/src/items.rs b/src/items.rs
@@ -55,7 +55,23 @@ impl Rewrite for ast::Local {
 
         skip_out_of_file_lines_range!(context, self.span);
 
-        let mut result = "let ".to_owned();
+        if contains_skip(&self.attrs) {
+            return None;
+        }
+
+        let attrs_str = try_opt!(self.attrs.rewrite(context, shape));
+        let mut result = if attrs_str.is_empty() {
+            "let ".to_owned()
+        } else {
+            try_opt!(combine_strs_with_missing_comments(
+                context,
+                &attrs_str,
+                "let ",
+                mk_sp(self.attrs.last().map(|a| a.span.hi).unwrap(), self.span.lo),
+                shape,
+                false,
+            ))
+        };
 
         // 4 = "let ".len()
         let pat_shape = try_opt!(shape.offset_left(4));
diff --git a/src/lib.rs b/src/lib.rs
@@ -91,32 +91,46 @@ macro_rules! span_with_attrs_lo_hi {
         }
     }
 }
+
 macro_rules! span_with_attrs {
     ($this:ident) => {
         span_with_attrs_lo_hi!($this, $this.span.lo, $this.span.hi)
     }
 }
 
-impl Spanned for ast::Expr {
-    fn span(&self) -> Span {
-        span_with_attrs!(self)
+macro_rules! implement_spanned {
+    ($this:ty) => {
+        impl Spanned for $this {
+            fn span(&self) -> Span {
+                span_with_attrs!(self)
+            }
+        }
     }
 }
 
-impl Spanned for ast::Item {
-    fn span(&self) -> Span {
-        span_with_attrs!(self)
-    }
-}
+// Implement `Spanned` for structs with `attrs` field.
+implement_spanned!(ast::Expr);
+implement_spanned!(ast::Field);
+implement_spanned!(ast::ForeignItem);
+implement_spanned!(ast::Item);
+implement_spanned!(ast::Local);
 
 impl Spanned for ast::Stmt {
     fn span(&self) -> Span {
         match self.node {
-            // Cover attributes
+            ast::StmtKind::Local(ref local) => mk_sp(local.span().lo, self.span.hi),
+            ast::StmtKind::Item(ref item) => mk_sp(item.span().lo, self.span.hi),
             ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => {
                 mk_sp(expr.span().lo, self.span.hi)
             }
-            _ => self.span,
+            ast::StmtKind::Mac(ref mac) => {
+                let (_, _, ref attrs) = **mac;
+                if attrs.is_empty() {
+                    self.span
+                } else {
+                    mk_sp(attrs[0].span.lo, self.span.hi)
+                }
+            }
         }
     }
 }
@@ -155,12 +169,6 @@ impl Spanned for ast::StructField {
     }
 }
 
-impl Spanned for ast::Field {
-    fn span(&self) -> Span {
-        span_with_attrs!(self)
-    }
-}
-
 impl Spanned for ast::WherePredicate {
     fn span(&self) -> Span {
         match *self {
@@ -208,12 +216,6 @@ impl Spanned for ast::TyParamBound {
     }
 }
 
-impl Spanned for ast::ForeignItem {
-    fn span(&self) -> Span {
-        span_with_attrs!(self)
-    }
-}
-
 #[derive(Copy, Clone, Debug)]
 pub struct Indent {
     // Width of the block indent, in characters. Must be a multiple of
diff --git a/src/visitor.rs b/src/visitor.rs
@@ -30,7 +30,7 @@ use lists::{itemize_list, write_list, DefinitiveListTactic, ListFormatting, Sepa
 use macros::{rewrite_macro, MacroPosition};
 use regex::Regex;
 use rewrite::{Rewrite, RewriteContext};
-use utils::{self, contains_skip, mk_sp};
+use utils::{self, contains_skip, inner_attributes, mk_sp};
 
 fn is_use_item(item: &ast::Item) -> bool {
     match item.node {
@@ -73,47 +73,23 @@ impl<'a> FmtVisitor<'a> {
             ast::StmtKind::Item(ref item) => {
                 self.visit_item(item);
             }
-            ast::StmtKind::Local(ref local) => {
-                let rewrite = if contains_skip(&local.attrs) {
-                    None
-                } else {
-                    stmt.rewrite(
-                        &self.get_context(),
-                        Shape::indented(self.block_indent, self.config),
-                    )
-                };
-                self.push_rewrite(stmt.span, rewrite);
+            ast::StmtKind::Local(..) => {
+                let rewrite = stmt.rewrite(&self.get_context(), self.shape());
+                self.push_rewrite(stmt.span(), rewrite);
             }
             ast::StmtKind::Expr(ref expr) => {
-                let rewrite = format_expr(
-                    expr,
-                    ExprType::Statement,
-                    &self.get_context(),
-                    Shape::indented(self.block_indent, self.config),
-                );
-                let span = if expr.attrs.is_empty() {
-                    stmt.span
-                } else {
-                    mk_sp(expr.span().lo, stmt.span.hi)
-                };
-                self.push_rewrite(span, rewrite)
+                let rewrite =
+                    format_expr(expr, ExprType::Statement, &self.get_context(), self.shape());
+                self.push_rewrite(stmt.span(), rewrite)
             }
-            ast::StmtKind::Semi(ref expr) => {
-                let rewrite = stmt.rewrite(
-                    &self.get_context(),
-                    Shape::indented(self.block_indent, self.config),
-                );
-                let span = if expr.attrs.is_empty() {
-                    stmt.span
-                } else {
-                    mk_sp(expr.span().lo, stmt.span.hi)
-                };
-                self.push_rewrite(span, rewrite)
+            ast::StmtKind::Semi(..) => {
+                let rewrite = stmt.rewrite(&self.get_context(), self.shape());
+                self.push_rewrite(stmt.span(), rewrite)
             }
             ast::StmtKind::Mac(ref mac) => {
                 let (ref mac, _macro_style, ref attrs) = **mac;
-                if contains_skip(attrs) {
-                    self.push_rewrite(mac.span, None);
+                if self.visit_attrs(attrs, ast::AttrStyle::Outer) {
+                    self.push_rewrite(stmt.span(), None);
                 } else {
                     self.visit_mac(mac, None, MacroPosition::Statement);
                 }
diff --git a/tests/source/attrib.rs b/tests/source/attrib.rs
@@ -124,3 +124,25 @@ impl InnerAttributes() {
 mod InnerAttributes {
     #![ this_is_an_inner_attribute ( foo ) ]
 }
+
+fn attributes_on_statements() {
+    // Local
+    # [ attr ( on ( local ) ) ]
+    let x = 3;
+
+    // Item
+    # [ attr ( on ( item ) ) ]
+    use foo;
+
+    // Expr
+    # [ attr ( on ( expr ) ) ]
+    {}
+
+    // Semi
+    # [ attr ( on ( semi ) ) ]
+    foo();
+
+    // Mac
+    # [ attr ( on ( mac ) ) ]
+    foo!();
+}
diff --git a/tests/target/attrib.rs b/tests/target/attrib.rs
@@ -124,3 +124,25 @@ impl InnerAttributes() {
 mod InnerAttributes {
     #![this_is_an_inner_attribute(foo)]
 }
+
+fn attributes_on_statements() {
+    // Local
+    #[attr(on(local))]
+    let x = 3;
+
+    // Item
+    #[attr(on(item))]
+    use foo;
+
+    // Expr
+    #[attr(on(expr))]
+    {}
+
+    // Semi
+    #[attr(on(semi))]
+    foo();
+
+    // Mac
+    #[attr(on(mac))]
+    foo!();
+}

Original file line number	Diff line number	Diff line change
`@@ -91,32 +91,46 @@ macro_rules! span_with_attrs_lo_hi {`
`91`	`91`	`}`
`92`	`92`	`}`
`93`	`93`	`}`
	`94`	`+`
`94`	`95`	`macro_rules! span_with_attrs {`
`95`	`96`	`($this:ident) => {`
`96`	`97`	`span_with_attrs_lo_hi!($this, $this.span.lo, $this.span.hi)`
`97`	`98`	`}`
`98`	`99`	`}`
`99`	`100`
`100`		`-impl Spanned for ast::Expr {`
`101`		`- fn span(&self) -> Span {`
`102`		`- span_with_attrs!(self)`
	`101`	`+macro_rules! implement_spanned {`
	`102`	`+ ($this:ty) => {`
	`103`	`+ impl Spanned for $this {`
	`104`	`+ fn span(&self) -> Span {`
	`105`	`+ span_with_attrs!(self)`
	`106`	`+ }`
	`107`	`+ }`
`103`	`108`	`}`
`104`	`109`	`}`
`105`	`110`
`106`		`-impl Spanned for ast::Item {`
`107`		`- fn span(&self) -> Span {`
`108`		`- span_with_attrs!(self)`
`109`		`- }`
`110`		`-}`
	`111`	+// Implement `Spanned` for structs with `attrs` field.
	`112`	`+implement_spanned!(ast::Expr);`
	`113`	`+implement_spanned!(ast::Field);`
	`114`	`+implement_spanned!(ast::ForeignItem);`
	`115`	`+implement_spanned!(ast::Item);`
	`116`	`+implement_spanned!(ast::Local);`
`111`	`117`
`112`	`118`	`impl Spanned for ast::Stmt {`
`113`	`119`	`fn span(&self) -> Span {`
`114`	`120`	`match self.node {`
`115`		`- // Cover attributes`
	`121`	`+ ast::StmtKind::Local(ref local) => mk_sp(local.span().lo, self.span.hi),`
	`122`	`+ ast::StmtKind::Item(ref item) => mk_sp(item.span().lo, self.span.hi),`
`116`	`123`	`ast::StmtKind::Expr(ref expr) \| ast::StmtKind::Semi(ref expr) => {`
`117`	`124`	`mk_sp(expr.span().lo, self.span.hi)`
`118`	`125`	`}`
`119`		`- _ => self.span,`
	`126`	`+ ast::StmtKind::Mac(ref mac) => {`
	`127`	`+ let (_, _, ref attrs) = **mac;`
	`128`	`+ if attrs.is_empty() {`
	`129`	`+ self.span`
	`130`	`+ } else {`
	`131`	`+ mk_sp(attrs[0].span.lo, self.span.hi)`
	`132`	`+ }`
	`133`	`+ }`
`120`	`134`	`}`
`121`	`135`	`}`
`122`	`136`	`}`
`@@ -155,12 +169,6 @@ impl Spanned for ast::StructField {`
`155`	`169`	`}`
`156`	`170`	`}`
`157`	`171`
`158`		`-impl Spanned for ast::Field {`
`159`		`- fn span(&self) -> Span {`
`160`		`- span_with_attrs!(self)`
`161`		`- }`
`162`		`-}`
`163`		`-`
`164`	`172`	`impl Spanned for ast::WherePredicate {`
`165`	`173`	`fn span(&self) -> Span {`
`166`	`174`	`match *self {`
`@@ -208,12 +216,6 @@ impl Spanned for ast::TyParamBound {`
`208`	`216`	`}`
`209`	`217`	`}`
`210`	`218`
`211`		`-impl Spanned for ast::ForeignItem {`
`212`		`- fn span(&self) -> Span {`
`213`		`- span_with_attrs!(self)`
`214`		`- }`
`215`		`-}`
`216`		`-`
`217`	`219`	`#[derive(Copy, Clone, Debug)]`
`218`	`220`	`pub struct Indent {`
`219`	`221`	`// Width of the block indent, in characters. Must be a multiple of`