Merge pull request #1669 from topecongiro/issue-1668

Use correct spans when rewrting attributes

src/expr.rs

@@ -47,16 +47,45 @@ enum ExprType {
     SubExpression,
 }
 
+fn combine_attr_and_expr(
+    context: &RewriteContext,
+    shape: Shape,
+    attr_str: &str,
+    expr_str: &str,
+) -> String {
+    let separator = if attr_str.is_empty() {
+        String::new()
+    } else {
+        if expr_str.contains('\n') || attr_str.contains('\n') ||
+            attr_str.len() + expr_str.len() > shape.width
+        {
+            format!("\n{}", shape.indent.to_string(context.config))
+        } else {
+            String::from(" ")
+        }
+    };
+    format!("{}{}{}", attr_str, separator, expr_str)
+}
+
 fn format_expr(
     expr: &ast::Expr,
     expr_type: ExprType,
     context: &RewriteContext,
     shape: Shape,
 ) -> Option<String> {
-    if contains_skip(&*expr.attrs) {
-        return Some(context.snippet(expr.span));
-    }
     let attr_rw = (&*expr.attrs).rewrite(context, shape);
+    if contains_skip(&*expr.attrs) {
+        if let Some(attr_str) = attr_rw {
+            return Some(combine_attr_and_expr(
+                context,
+                shape,
+                &attr_str,
+                &context.snippet(expr.span),
+            ));
+        } else {
+            return Some(context.snippet(expr.span));
+        }
+    }
     let expr_rw = match expr.node {
         ast::ExprKind::Array(ref expr_vec) => {
             rewrite_array(
@@ -289,9 +318,8 @@ fn format_expr(
     };
     match (attr_rw, expr_rw) {
         (Some(attr_str), Some(expr_str)) => {
-            let space = if attr_str.is_empty() { "" } else { " " };
             recover_comment_removed(
-                format!("{}{}{}", attr_str, space, expr_str),
+                combine_attr_and_expr(context, shape, &attr_str, &expr_str),
                 expr.span,
                 context,
                 shape,

src/items.rs

@@ -1684,7 +1684,7 @@ fn rewrite_explicit_self(
                 Some(ref l) => {
                     let lifetime_str = try_opt!(l.rewrite(
                         context,
-                        Shape::legacy(usize::max_value(), Indent::empty()),
+                        Shape::legacy(context.config.max_width(), Indent::empty()),
                     ));
                     Some(format!("&{} {}self", lifetime_str, mut_str))
                 }

src/lib.rs

@@ -148,8 +148,12 @@ pub fn block_indent(mut self, config: &Config) -> Indent {
     }
 
     pub fn block_unindent(mut self, config: &Config) -> Indent {
-        self.block_indent -= config.tab_spaces();
-        self
+        if self.block_indent < config.tab_spaces() {
+            Indent::new(self.block_indent, 0)
+        } else {
+            self.block_indent -= config.tab_spaces();
+            self
+        }
     }
 
     pub fn width(&self) -> usize {

src/visitor.rs

@@ -82,15 +82,26 @@ fn visit_stmt(&mut self, stmt: &ast::Stmt) {
             ast::StmtKind::Item(ref item) => {
                 self.visit_item(item);
             }
-            ast::StmtKind::Local(..) |
-            ast::StmtKind::Expr(..) |
-            ast::StmtKind::Semi(..) => {
+            ast::StmtKind::Local(..) => {
                 let rewrite = stmt.rewrite(
                     &self.get_context(),
                     Shape::indented(self.block_indent, self.config),
                 );
                 self.push_rewrite(stmt.span, rewrite);
             }
+            ast::StmtKind::Expr(ref expr) |
+            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.attrs[0].span.lo, stmt.span.hi)
+                };
+                self.push_rewrite(span, rewrite)
+            }
             ast::StmtKind::Mac(ref mac) => {
                 let (ref mac, _macro_style, _) = **mac;
                 self.visit_mac(mac, None, MacroPosition::Statement);
@@ -702,6 +713,12 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
     }
 }
 
+fn count_missing_closing_parens(s: &str) -> u32 {
+    let op_parens = s.chars().filter(|c| *c == '(').count();
+    let cl_parens = s.chars().filter(|c| *c == ')').count();
+    op_parens.checked_sub(cl_parens).unwrap_or(0) as u32
+}
+
 impl Rewrite for ast::MetaItem {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
         Some(match self.node {
@@ -712,15 +729,21 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
                 let item_shape = try_opt!(shape.shrink_left(name.len() + 3).and_then(
                     |s| s.sub_width(2),
                 ));
+                let hi = self.span.hi +
+                    BytePos(count_missing_closing_parens(&context.snippet(self.span)));
                 let items = itemize_list(
                     context.codemap,
                     list.iter(),
                     ")",
                     |nested_meta_item| nested_meta_item.span.lo,
-                    |nested_meta_item| nested_meta_item.span.hi,
+                    // FIXME: Span from MetaItem is missing closing parens.
+                    |nested_meta_item| {
+                        let snippet = context.snippet(nested_meta_item.span);
+                        nested_meta_item.span.hi + BytePos(count_missing_closing_parens(&snippet))
+                    },
                     |nested_meta_item| nested_meta_item.rewrite(context, item_shape),
                     self.span.lo,
-                    self.span.hi,
+                    hi,
                 );
                 let item_vec = items.collect::<Vec<_>>();
                 let fmt = ListFormatting {

tests/source/attrib.rs

@@ -52,3 +52,19 @@ struct Foo {
     # [ derive ( Clone , PartialEq , Debug , Deserialize , Serialize ) ]
     foo: usize,
 }
+
+// #1668
+
+/// Default path (*nix)
+#[cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))]
+fn foo() {
+    #[cfg(target_os = "freertos")]
+    match port_id {
+        'a' | 'A' => GpioPort { port_address: GPIO_A },
+        'b' | 'B' => GpioPort { port_address: GPIO_B },
+        _ => panic!(),
+    }
+
+    #[cfg_attr(not(target_os = "freertos"), allow(unused_variables))]
+    let x = 3;
+}

tests/target/attrib.rs

@@ -48,3 +48,19 @@ struct Foo {
     #[derive(Clone, PartialEq, Debug, Deserialize, Serialize)]
     foo: usize,
 }
+
+// #1668
+
+/// Default path (*nix)
+#[cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))]
+fn foo() {
+    #[cfg(target_os = "freertos")]
+    match port_id {
+        'a' | 'A' => GpioPort { port_address: GPIO_A },
+        'b' | 'B' => GpioPort { port_address: GPIO_B },
+        _ => panic!(),
+    }
+
+    #[cfg_attr(not(target_os = "freertos"), allow(unused_variables))]
+    let x = 3;
+}