Comments in function decls and annotations/doc comments

src/functions.rs

@@ -80,9 +80,6 @@ impl<'a> FmtVisitor<'a> {
                                            span_for_return(&fd.output)));
         result.push(')');
 
-        // Where clause.
-        result.push_str(&self.rewrite_where_clause(where_clause, indent, next_span));
-
         // Return type.
         if ret_str.len() > 0 {
             // If we've already gone multi-line, or the return type would push
@@ -91,10 +88,13 @@ impl<'a> FmtVisitor<'a> {
                result.len() + indent + ret_str.len() > MAX_WIDTH {
                 let indent = match FN_RETURN_INDENT {
                     ReturnIndent::WithWhereClause => indent + 4,
+                    ReturnIndent::WithWhereClauseOrArgs if where_clause.predicates.len() > 0 => {
+                        indent + 4
+                    }
                     // TODO we might want to check that using the arg indent doesn't
                     // blow our budget, and if it does, then fallback to the where
                     // clause indent.
-                    ReturnIndent::WithArgs => arg_indent,
+                    _ => arg_indent,
                 };
 
                 result.push('\n');
@@ -103,9 +103,27 @@ impl<'a> FmtVisitor<'a> {
                 result.push(' ');
             }
             result.push_str(&ret_str);
+
+            // Comment between return type and the end of the decl.
+            let snippet_lo = fd.output.span().hi;
+            if where_clause.predicates.len() == 0 {
+                let snippet_hi = next_span.lo;
+                let snippet = self.snippet(codemap::mk_sp(snippet_lo, snippet_hi));
+                let snippet = snippet.trim();
+                if snippet.len() > 0 {
+                    println!("found comment {}", snippet);
+                    result.push(' ');
+                    result.push_str(snippet);
+                }
+            } else {
+                // FIXME it would be nice to catch comments between the return type
+                // and the where clause, but we don't have a span for the where
+                // clause.
+            }
         }
 
-        // TODO any comments here?
+        // Where clause.
+        result.push_str(&self.rewrite_where_clause(where_clause, indent, next_span));
 
         // Prepare for the function body by possibly adding a newline and indent.
         // FIXME we'll miss anything between the end of the signature and the start
@@ -236,7 +254,7 @@ impl<'a> FmtVisitor<'a> {
         // The fix is comments in the AST or a span for the closing paren.
         let snippet = self.snippet(codemap::mk_sp(prev_end, next_span_start));
         let snippet = snippet.trim();
-        let snippet = &snippet[..snippet.find(terminator).unwrap()];
+        let snippet = &snippet[..snippet.find(terminator).unwrap_or(snippet.len())];
         let snippet = snippet.trim();
         result.push(snippet.to_string());
 
@@ -377,16 +395,13 @@ impl<'a> FmtVisitor<'a> {
         result.push_str(&make_indent(indent + 4));
         result.push_str("where ");
 
-        let comments = vec![String::new(); where_clause.predicates.len()];
-        // TODO uncomment when spans are fixed
-        // println!("{:?} {:?}", where_clause.predicates.iter().map(|p| self.snippet(span_for_where_pred(p))).collect::<Vec<_>>(), next_span.lo);
-        // let comments = self.make_comments_for_list(Vec::new(),
-        //                                            where_clause.predicates.iter(),
-        //                                            ",",
-        //                                            "{",
-        //                                            |pred| span_for_where_pred(pred).lo,
-        //                                            |pred| span_for_where_pred(pred).hi,
-        //                                            next_span.lo);
+        let comments = self.make_comments_for_list(Vec::new(),
+                                                   where_clause.predicates.iter(),
+                                                   ",",
+                                                   "{",
+                                                   |pred| span_for_where_pred(pred).lo,
+                                                   |pred| span_for_where_pred(pred).hi,
+                                                   next_span.lo);
         let where_strs: Vec<_> = where_clause.predicates.iter()
                                                         .map(|p| (self.rewrite_pred(p)))
                                                         .zip(comments.into_iter())

src/mod.rs

@@ -20,8 +20,9 @@
 // TODO for lint violations of names, emit a refactor script
 
 // TODO priorities
+// annotations/doc comments
 // Fix fns and methods properly
-//   dead spans (comments) - in where clause (wait for fixed spans, test)
+//   dead spans (comments) - in where clause - test
 //
 // Smoke testing till we can use it
 // take config options from a file
@@ -67,7 +68,7 @@ const MAX_WIDTH: usize = 100;
 const MIN_STRING: usize = 10;
 const TAB_SPACES: usize = 4;
 const FN_BRACE_STYLE: BraceStyle = BraceStyle::SameLineWhere;
-const FN_RETURN_INDENT: ReturnIndent = ReturnIndent::WithArgs;
+const FN_RETURN_INDENT: ReturnIndent = ReturnIndent::WithWhereClauseOrArgs;
 // When we get scoped annotations, we should have rustfmt::skip.
 const SKIP_ANNOTATION: &'static str = "rustfmt_skip";
 
@@ -98,6 +99,8 @@ enum ReturnIndent {
     WithArgs,
     // Aligned with the where clause
     WithWhereClause,
+    // Aligned with the where clause if there is one, otherwise the args.
+    WithWhereClauseOrArgs,
 }
 
 // Formatting which depends on the AST.
@@ -354,6 +357,7 @@ mod test {
             if result[file_name] != text {
                 fails += 1;
                 println!("Mismatch in {}.", file_name);
+                println!("{}", result[file_name]);
             }
         }
 

src/visitor.rs

@@ -75,8 +75,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
         self.format_missing(s.lo);
         self.last_pos = s.lo;
 
-        // TODO need to check against expected indent
-        let indent = self.codemap.lookup_char_pos(s.lo).col.0;
+        let indent = self.block_indent;
         match fk {
             visit::FkItemFn(ident, ref generics, ref unsafety, ref abi, vis) => {
                 let new_fn = self.rewrite_fn(indent,
@@ -110,7 +109,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
     }
 
     fn visit_item(&mut self, item: &'v ast::Item) {
-        if item.attrs.iter().any(|a| is_skip(&a.node.value)) {
+        if self.visit_attrs(&item.attrs) {
             return;
         }
 
@@ -147,14 +146,14 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
     }
 
     fn visit_trait_item(&mut self, ti: &'v ast::TraitItem) {
-        if ti.attrs.iter().any(|a| is_skip(&a.node.value)) {
+        if self.visit_attrs(&ti.attrs) {
             return;
         }
         visit::walk_trait_item(self, ti)
     }
 
     fn visit_impl_item(&mut self, ii: &'v ast::ImplItem) {
-        if ii.attrs.iter().any(|a| is_skip(&a.node.value)) {
+        if self.visit_attrs(&ii.attrs) {
             return;
         }
         visit::walk_impl_item(self, ii)
@@ -195,6 +194,22 @@ 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;
+        }
+
+        false
+    }
 }
 
 fn is_skip(meta_item: &ast::MetaItem) -> bool {

tests/idem/attrib.rs

@@ -0,0 +1,22 @@
+// Test attributes and doc comments are preserved.
+
+/// Blah blah blah.
+impl Bar {
+    /// Blah blah blooo.
+    #[an_attribute]
+    fn foo(&mut self) -> isize {}
+
+    /// Blah blah bing.
+    pub fn f2(self) {
+        (foo, bar)
+    }
+
+    #[another_attribute]
+    fn f3(self) -> Dog {
+    }
+}
+
+/// Blah
+fn main() {
+    println!("Hello world!");
+}

tests/idem/comments-fn.rs

@@ -0,0 +1,19 @@
+// Test comments on functions are preserved.
+
+// Comment on foo.
+fn foo<F, G>(a: aaaaaaaaaaaaa, // A comment
+             b: bbbbbbbbbbbbb, /* a second comment */
+             c: ccccccccccccc,
+             d: ddddddddddddd,
+             e: eeeeeeeeeeeee /* comment before paren*/)
+    -> bar
+    where F: Foo, // COmment after where clause
+          G: Goo /* final comment */
+{
+
+}
+
+fn bar<F /* comment on F */, G /* comment on G */>() {}
+
+fn baz() -> Baz /* Comment after return type */ {
+}