Merge pull request #1855 from topecongiro/inner-outer-attrs

Format inner attributes in match expr

src/expr.rs

@@ -31,8 +31,9 @@ use rewrite::{Rewrite, RewriteContext};
 use string::{rewrite_string, StringFormat};
 use types::{can_be_overflowed_type, rewrite_path, PathContext};
 use utils::{binary_search, colon_spaces, contains_skip, extra_offset, first_line_width,
-            last_line_extendable, last_line_width, left_most_sub_expr, mk_sp, paren_overhead,
-            semicolon_for_stmt, stmt_expr, trimmed_last_line_width, wrap_str};
+            inner_attributes, last_line_extendable, last_line_width, left_most_sub_expr, mk_sp,
+            outer_attributes, paren_overhead, semicolon_for_stmt, stmt_expr,
+            trimmed_last_line_width, wrap_str};
 use vertical::rewrite_with_alignment;
 use visitor::FmtVisitor;
 
@@ -54,15 +55,13 @@ fn combine_attr_and_expr(
     expr: &ast::Expr,
     expr_str: &str,
 ) -> Option<String> {
-    let attr_str = try_opt!((&*expr.attrs).rewrite(context, shape));
+    let attrs = outer_attributes(&expr.attrs);
+    let attr_str = try_opt!(attrs.rewrite(context, shape));
     let separator = if attr_str.is_empty() {
         String::new()
     } else {
         // Try to recover comments between the attributes and the expression if available.
-        let missing_snippet = context.snippet(mk_sp(
-            expr.attrs[expr.attrs.len() - 1].span.hi,
-            expr.span.lo,
-        ));
+        let missing_snippet = context.snippet(mk_sp(attrs[attrs.len() - 1].span.hi, expr.span.lo));
         let comment_opening_pos = missing_snippet.chars().position(|c| c == '/');
         let prefer_same_line = if let Some(pos) = comment_opening_pos {
             !missing_snippet[..pos].contains('\n')
@@ -198,7 +197,7 @@ pub fn format_expr(
             }
         }
         ast::ExprKind::Match(ref cond, ref arms) => {
-            rewrite_match(context, cond, arms, shape, expr.span)
+            rewrite_match(context, cond, arms, shape, expr.span, &expr.attrs)
         }
         ast::ExprKind::Path(ref qself, ref path) => {
             rewrite_path(context, PathContext::Expr, qself.as_ref(), path, shape)
@@ -1531,6 +1530,7 @@ fn rewrite_match(
     arms: &[ast::Arm],
     shape: Shape,
     span: Span,
+    attrs: &[ast::Attribute],
 ) -> Option<String> {
     if arms.is_empty() {
         return None;
@@ -1558,11 +1558,42 @@ fn rewrite_match(
         _ => " ",
     };
 
+    let nested_indent_str = shape
+        .indent
+        .block_indent(context.config)
+        .to_string(context.config);
+    // Inner attributes.
+    let inner_attrs = &inner_attributes(attrs);
+    let inner_attrs_str = if inner_attrs.is_empty() {
+        String::new()
+    } else {
+        try_opt!(
+            inner_attrs
+                .rewrite(context, shape)
+                .map(|s| format!("\n{}{}", nested_indent_str, s))
+        )
+    };
+
+    let open_brace_pos = if inner_attrs.is_empty() {
+        context
+            .codemap
+            .span_after(mk_sp(cond.span.hi, arms[0].span().lo), "{")
+    } else {
+        inner_attrs[inner_attrs.len() - 1].span().hi
+    };
+
     Some(format!(
-        "match {}{}{{{}\n{}}}",
+        "match {}{}{{{}{}\n{}}}",
         cond_str,
         block_sep,
-        try_opt!(rewrite_match_arms(context, arms, shape, span, cond.span.hi)),
+        inner_attrs_str,
+        try_opt!(rewrite_match_arms(
+            context,
+            arms,
+            shape,
+            span,
+            open_brace_pos,
+        )),
         shape.indent.to_string(context.config),
     ))
 }
@@ -1586,7 +1617,7 @@ fn rewrite_match_arms(
     arms: &[ast::Arm],
     shape: Shape,
     span: Span,
-    cond_end_pos: BytePos,
+    open_brace_pos: BytePos,
 ) -> Option<String> {
     let mut result = String::new();
 
@@ -1597,10 +1628,6 @@ fn rewrite_match_arms(
     }.with_max_width(context.config);
     let arm_indent_str = arm_shape.indent.to_string(context.config);
 
-    let open_brace_pos = context
-        .codemap
-        .span_after(mk_sp(cond_end_pos, arms[0].span().lo), "{");
-
     let arm_num = arms.len();
     for (i, arm) in arms.iter().enumerate() {
         // Make sure we get the stuff between arms.
@@ -1615,7 +1642,9 @@ fn rewrite_match_arms(
             arm_shape,
             &arm_indent_str,
         ));
-        result.push_str(&comment);
+        if !comment.chars().all(|c| c == ' ') {
+            result.push_str(&comment);
+        }
         result.push('\n');
         result.push_str(&arm_indent_str);
 

src/lib.rs

@@ -46,7 +46,7 @@ use checkstyle::{output_footer, output_header};
 use config::Config;
 use filemap::FileMap;
 use issues::{BadIssueSeeker, Issue};
-use utils::mk_sp;
+use utils::{mk_sp, outer_attributes};
 use visitor::FmtVisitor;
 
 pub use self::summary::Summary;
@@ -81,13 +81,33 @@ pub trait Spanned {
     fn span(&self) -> Span;
 }
 
+macro_rules! span_with_attrs_lo_hi {
+    ($this:ident, $lo:expr, $hi:expr) => {
+        {
+            let attrs = outer_attributes(&$this.attrs);
+            if attrs.is_empty() {
+                mk_sp($lo, $hi)
+            } else {
+                mk_sp(attrs[0].span.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 {
-        if self.attrs.is_empty() {
-            self.span
-        } else {
-            mk_sp(self.attrs[0].span.lo, self.span.hi)
-        }
+        span_with_attrs!(self)
+    }
+}
+
+impl Spanned for ast::Item {
+    fn span(&self) -> Span {
+        span_with_attrs!(self)
     }
 }
 
@@ -117,12 +137,7 @@ impl Spanned for ast::Ty {
 
 impl Spanned for ast::Arm {
     fn span(&self) -> Span {
-        let hi = self.body.span.hi;
-        if self.attrs.is_empty() {
-            mk_sp(self.pats[0].span.lo, hi)
-        } else {
-            mk_sp(self.attrs[0].span.lo, hi)
-        }
+        span_with_attrs_lo_hi!(self, self.pats[0].span.lo, self.body.span.hi)
     }
 }
 
@@ -138,23 +153,13 @@ impl Spanned for ast::Arg {
 
 impl Spanned for ast::StructField {
     fn span(&self) -> Span {
-        if self.attrs.is_empty() {
-            mk_sp(self.span.lo, self.ty.span.hi)
-        } else {
-            // Include attributes and doc comments, if present
-            mk_sp(self.attrs[0].span.lo, self.ty.span.hi)
-        }
+        span_with_attrs_lo_hi!(self, self.span.lo, self.ty.span.hi)
     }
 }
 
 impl Spanned for ast::Field {
     fn span(&self) -> Span {
-        let lo = if self.attrs.is_empty() {
-            self.span.lo
-        } else {
-            self.attrs[0].span.lo
-        };
-        mk_sp(lo, self.span.hi)
+        span_with_attrs!(self)
     }
 }
 

src/utils.rs

@@ -97,6 +97,25 @@ pub fn format_abi(abi: abi::Abi, explicit_abi: bool) -> String {
     }
 }
 
+#[inline]
+pub fn filter_attributes(attrs: &[ast::Attribute], style: ast::AttrStyle) -> Vec<ast::Attribute> {
+    attrs
+        .iter()
+        .filter(|a| a.style == style)
+        .cloned()
+        .collect::<Vec<_>>()
+}
+
+#[inline]
+pub fn inner_attributes(attrs: &[ast::Attribute]) -> Vec<ast::Attribute> {
+    filter_attributes(attrs, ast::AttrStyle::Inner)
+}
+
+#[inline]
+pub fn outer_attributes(attrs: &[ast::Attribute]) -> Vec<ast::Attribute> {
+    filter_attributes(attrs, ast::AttrStyle::Outer)
+}
+
 // The width of the first line in s.
 #[inline]
 pub fn first_line_width(s: &str) -> usize {

src/visitor.rs

@@ -15,7 +15,7 @@ use syntax::{ast, ptr, visit};
 use syntax::codemap::{self, BytePos, CodeMap, Span};
 use syntax::parse::ParseSess;
 
-use {Indent, Shape};
+use {Indent, Shape, Spanned};
 use codemap::{LineRangeUtils, SpanUtils};
 use comment::{contains_comment, FindUncommented};
 use comment::rewrite_comment;
@@ -36,19 +36,6 @@ fn is_use_item(item: &ast::Item) -> bool {
     }
 }
 
-fn item_bound(item: &ast::Item) -> Span {
-    item.attrs.iter().map(|attr| attr.span).fold(
-        item.span,
-        |bound, span| {
-            Span {
-                lo: cmp::min(bound.lo, span.lo),
-                hi: cmp::max(bound.hi, span.hi),
-                ctxt: span.ctxt,
-            }
-        },
-    )
-}
-
 pub struct FmtVisitor<'a> {
     pub parse_session: &'a ParseSess,
     pub codemap: &'a CodeMap,
@@ -93,7 +80,7 @@ impl<'a> FmtVisitor<'a> {
                 let span = if expr.attrs.is_empty() {
                     stmt.span
                 } else {
-                    mk_sp(expr.attrs[0].span.lo, stmt.span.hi)
+                    mk_sp(expr.span().lo, stmt.span.hi)
                 };
                 self.push_rewrite(span, rewrite)
             }
@@ -105,7 +92,7 @@ impl<'a> FmtVisitor<'a> {
                 let span = if expr.attrs.is_empty() {
                     stmt.span
                 } else {
-                    mk_sp(expr.attrs[0].span.lo, stmt.span.hi)
+                    mk_sp(expr.span().lo, stmt.span.hi)
                 };
                 self.push_rewrite(span, rewrite)
             }
@@ -648,12 +635,12 @@ impl<'a> FmtVisitor<'a> {
             // next item for output.
             if self.config.reorder_imports() && is_use_item(&*items_left[0]) {
                 let reorder_imports_in_group = self.config.reorder_imports_in_group();
-                let mut last = self.codemap.lookup_line_range(item_bound(&items_left[0]));
+                let mut last = self.codemap.lookup_line_range(items_left[0].span());
                 let use_item_length = items_left
                     .iter()
                     .take_while(|ppi| {
                         is_use_item(&***ppi) && (!reorder_imports_in_group || {
-                            let current = self.codemap.lookup_line_range(item_bound(&ppi));
+                            let current = self.codemap.lookup_line_range(ppi.span());
                             let in_same_group = current.lo < last.hi + 2;
                             last = current;
                             in_same_group

tests/source/match.rs

@@ -408,3 +408,11 @@ fn match_with_near_max_width() {
         {}
     }
 }
+
+fn match_with_trailing_spaces() {
+    match x {    
+        #![allow(simple_match)]
+        Some(..) => 0,
+        None => 1,
+    }
+}

tests/target/match.rs

@@ -450,3 +450,11 @@ fn match_with_near_max_width() {
         Variant::Tag6 => {}
     }
 }
+
+fn match_with_trailing_spaces() {
+    match x {
+        #![allow(simple_match)]
+        Some(..) => 0,
+        None => 1,
+    }
+}