Better attribute handling

src/missed_spans.rs

@@ -44,6 +44,8 @@ impl<'a> FmtVisitor<'a> {
                self.codemap.lookup_char_pos(end));
 
         if start == end {
+            let file_name = &self.codemap.lookup_char_pos(start).file.name;
+            process_last_snippet(self, "", file_name, "");
             return;
         }
 

src/mod.rs

@@ -262,8 +262,8 @@ fn run(args: Vec<String>, write_mode: WriteMode) {
 
 fn main() {
     let args: Vec<_> = std::env::args().collect();
-    //run(args, WriteMode::Display);
-    run(args, WriteMode::Overwrite);
+    run(args, WriteMode::Display);
+    //run(args, WriteMode::Overwrite);
     std::env::set_exit_status(0);
 
     // TODO unit tests

src/visitor.rs

@@ -9,9 +9,11 @@
 // except according to those terms.
 
 use syntax::ast;
-use syntax::codemap::{CodeMap, Span, BytePos};
+use syntax::codemap::{self, CodeMap, Span, BytePos};
 use syntax::visit;
 
+use utils;
+
 use {MAX_WIDTH, TAB_SPACES, SKIP_ANNOTATION};
 use changes::ChangeSet;
 
@@ -35,6 +37,26 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
         self.last_pos = ex.span.hi;
     }
 
+    fn visit_stmt(&mut self, stmt: &'v ast::Stmt) {
+        // If the stmt is actually an item, then we'll handle any missing spans
+        // there. This is important because of annotations.
+        // Although it might make more sense for the statement span to include
+        // any annotations on the item.
+        let skip_missing = match stmt.node {
+            ast::Stmt_::StmtDecl(ref decl, _) => {
+                match decl.node {
+                    ast::Decl_::DeclItem(_) => true,
+                    _ => false,
+                }
+            }
+            _ => false,
+        };
+        if !skip_missing {
+            self.format_missing_with_indent(stmt.span.lo);
+        }
+        visit::walk_stmt(self, stmt);
+    }
+
     fn visit_block(&mut self, b: &'v ast::Block) {
         debug!("visit_block: {:?} {:?}",
                self.codemap.lookup_char_pos(b.span.lo),
@@ -46,7 +68,6 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
         self.block_indent += TAB_SPACES;
 
         for stmt in &b.stmts {
-            self.format_missing_with_indent(stmt.span.lo);
             self.visit_stmt(&stmt)
         }
         match b.expr {
@@ -148,6 +169,12 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                 visit::walk_item(self, item);
                 self.block_indent -= TAB_SPACES;
             }
+            ast::Item_::ItemExternCrate(_) => {
+                self.format_missing_with_indent(item.span.lo);
+                let new_str = self.snippet(item.span);
+                self.changes.push_str_span(item.span, &new_str);
+                self.last_pos = item.span.hi;
+            }
             _ => {
                 visit::walk_item(self, item);
             }
@@ -206,18 +233,50 @@ impl<'a> FmtVisitor<'a> {
 
     // Returns true if we should skip the following item.
     fn visit_attrs(&mut self, attrs: &[ast::Attribute]) -> bool {
-        for a in attrs {
-            self.format_missing_with_indent(a.span.lo);
-            if is_skip(&a.node.value) {
-                return true;
-            }
-
-            let attr_str = self.snippet(a.span);
-            self.changes.push_str_span(a.span, &attr_str);
-            self.last_pos = a.span.hi;
+        if attrs.len() == 0 {
+            return false;
         }
 
-        false
+        let first = &attrs[0];
+        self.format_missing_with_indent(first.span.lo);
+
+        match self.rewrite_attrs(attrs, self.block_indent) {
+            Some(s) => {
+                self.changes.push_str_span(first.span, &s);
+                let last = attrs.last().unwrap();
+                self.last_pos = last.span.hi;
+                false
+            }
+            None => true
+        }
+    }
+
+    fn rewrite_attrs(&self, attrs: &[ast::Attribute], indent: usize) -> Option<String> {
+        let mut result = String::new();
+        let indent = utils::make_indent(indent);
+
+        for (i, a) in attrs.iter().enumerate() {
+            if is_skip(&a.node.value) {
+                return None;
+            }
+
+            result.push_str(&self.snippet(a.span));
+
+            if i < attrs.len() - 1 {
+                result.push('\n');
+                result.push_str(&indent);
+
+                let comment = self.snippet(codemap::mk_sp(a.span.hi, attrs[i+1].span.lo));
+                let comment = comment.trim();
+                if comment.len() > 0 {
+                    result.push_str(&self.snippet(a.span));
+                    result.push('\n');
+                    result.push_str(comment);
+                }
+            }
+        }
+
+        Some(result)
     }
 }
 

tests/idem/attrib-extern-crate.rs

@@ -0,0 +1,17 @@
+// Attributes on extern crate.
+
+extern crate Foo;
+#[Attr1]
+extern crate Bar;
+#[Attr2]
+#[Attr2]
+extern crate Baz;
+
+fn foo() {
+    extern crate Foo;
+    #[Attr1]
+    extern crate Bar;
+    #[Attr2]
+    #[Attr2]
+    extern crate Baz;
+}

tests/idem/attrib.rs

@@ -1,17 +1,11 @@
 // Test attributes and doc comments are preserved.
 
-extern crate Foo;
-#[Attr1]
-extern crate Bar;
-#[Attr2]
-#[Attr2]
-extern crate Baz;
-
 /// Blah blah blah.
 impl Bar {
     /// Blah blah blooo.
     #[an_attribute]
-    fn foo(&mut self) -> isize {}
+    fn foo(&mut self) -> isize {
+    }
 
     /// Blah blah bing.
     pub fn f2(self) {
@@ -22,8 +16,3 @@ impl Bar {
     fn f3(self) -> Dog {
     }
 }
-
-/// Blah
-fn main() {
-    println!("Hello world!");
-}

tests/idem/comments-fn.rs

@@ -16,7 +16,8 @@ fn foo<F, G>(a: aaaaaaaaaaaaa, // A comment
 
 }
 
-fn bar<F /* comment on F */, G /* comment on G */>() {}
+fn bar<F /* comment on F */, G /* comment on G */>() {
+}
 
 fn baz() -> Baz /* Comment after return type */ {
 }

tests/idem/imports.rs

@@ -0,0 +1,8 @@
+// Imports.
+
+// Long import.
+use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic,
+                  ItemDefaultImpl};
+
+use {Foo, Bar};
+use Foo::{Bar, Baz};