From a84287c6b06026ecc1eced2ef4d0351c11b130f0 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 28 Apr 2015 20:56:01 +1200 Subject: [PATCH] Comments in function decls and annotations/doc comments --- src/functions.rs | 47 ++++++++++++++++++++++++++------------- src/mod.rs | 8 +++++-- src/visitor.rs | 25 ++++++++++++++++----- tests/idem/attrib.rs | 22 ++++++++++++++++++ tests/idem/comments-fn.rs | 19 ++++++++++++++++ 5 files changed, 98 insertions(+), 23 deletions(-) create mode 100644 tests/idem/attrib.rs create mode 100644 tests/idem/comments-fn.rs diff --git a/src/functions.rs b/src/functions.rs index 777120f5b97d..0afcb66f64a1 100644 --- a/src/functions.rs +++ b/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::>(), 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()) diff --git a/src/mod.rs b/src/mod.rs index c5f5150b3bb0..537752d17a30 100644 --- a/src/mod.rs +++ b/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]); } } diff --git a/src/visitor.rs b/src/visitor.rs index 2a6dbc876f82..df974b7e8fa9 100644 --- a/src/visitor.rs +++ b/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 { diff --git a/tests/idem/attrib.rs b/tests/idem/attrib.rs new file mode 100644 index 000000000000..b66be08c7284 --- /dev/null +++ b/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!"); +} diff --git a/tests/idem/comments-fn.rs b/tests/idem/comments-fn.rs new file mode 100644 index 000000000000..ec1b87cde0d0 --- /dev/null +++ b/tests/idem/comments-fn.rs @@ -0,0 +1,19 @@ +// Test comments on functions are preserved. + +// Comment on foo. +fn foo(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() {} + +fn baz() -> Baz /* Comment after return type */ { +}