From 2e485ea086990ed6a13c4f3116006635a6266ca1 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 29 Apr 2015 15:00:58 +1200 Subject: [PATCH] Better attribute handling --- src/missed_spans.rs | 2 + src/mod.rs | 4 +- src/visitor.rs | 83 ++++++++++++++++++++++++++----- tests/idem/attrib-extern-crate.rs | 17 +++++++ tests/idem/attrib.rs | 15 +----- tests/idem/comments-fn.rs | 3 +- tests/idem/imports.rs | 8 +++ 7 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 tests/idem/attrib-extern-crate.rs create mode 100644 tests/idem/imports.rs diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 7b352dc8a04..c861d631be8 100644 --- a/src/missed_spans.rs +++ b/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; } diff --git a/src/mod.rs b/src/mod.rs index 7b18664b672..076a826035b 100644 --- a/src/mod.rs +++ b/src/mod.rs @@ -262,8 +262,8 @@ fn run(args: Vec, 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 diff --git a/src/visitor.rs b/src/visitor.rs index 8825a094792..e4d48201dd6 100644 --- a/src/visitor.rs +++ b/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 { + 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) } } diff --git a/tests/idem/attrib-extern-crate.rs b/tests/idem/attrib-extern-crate.rs new file mode 100644 index 00000000000..fe06195b154 --- /dev/null +++ b/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; +} diff --git a/tests/idem/attrib.rs b/tests/idem/attrib.rs index aa2b1a69df0..e36f6425421 100644 --- a/tests/idem/attrib.rs +++ b/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!"); -} diff --git a/tests/idem/comments-fn.rs b/tests/idem/comments-fn.rs index 12c0e72193f..748a6edb6ca 100644 --- a/tests/idem/comments-fn.rs +++ b/tests/idem/comments-fn.rs @@ -16,7 +16,8 @@ fn foo(a: aaaaaaaaaaaaa, // A comment } -fn bar() {} +fn bar() { +} fn baz() -> Baz /* Comment after return type */ { } diff --git a/tests/idem/imports.rs b/tests/idem/imports.rs new file mode 100644 index 00000000000..9891994c66e --- /dev/null +++ b/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};