From 5e6bb3edb0a627dd2104a5314232a15ad342b7d5 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 8 Dec 2017 16:59:04 +0900 Subject: [PATCH 1/3] Keep track of line number in visitor --- src/imports.rs | 2 +- src/items.rs | 23 +++++++++++------------ src/lib.rs | 5 +++++ src/missed_spans.rs | 36 +++++++++++++++++------------------- src/visitor.rs | 37 +++++++++++++++++++++++-------------- 5 files changed, 57 insertions(+), 46 deletions(-) diff --git a/src/imports.rs b/src/imports.rs index 9fbf814919a..1d38328d45c 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -285,7 +285,7 @@ impl<'a> FmtVisitor<'a> { } Some(ref s) => { self.format_missing_with_indent(source!(self, span).lo()); - self.buffer.push_str(s); + self.push_str(s); self.last_pos = source!(self, span).hi(); } None => { diff --git a/src/items.rs b/src/items.rs index b3647f26f53..44c03ebf95b 100644 --- a/src/items.rs +++ b/src/items.rs @@ -240,12 +240,12 @@ impl<'a> FnSig<'a> { impl<'a> FmtVisitor<'a> { fn format_item(&mut self, item: Item) { - self.buffer.push_str(&item.abi); + self.push_str(&item.abi); let snippet = self.snippet(item.span); let brace_pos = snippet.find_uncommented("{").unwrap(); - self.buffer.push_str("{"); + self.push_str("{"); if !item.body.is_empty() || contains_comment(&snippet[brace_pos..]) { // FIXME: this skips comments between the extern keyword and the opening // brace. @@ -255,9 +255,8 @@ impl<'a> FmtVisitor<'a> { if item.body.is_empty() { self.format_missing_no_indent(item.span.hi() - BytePos(1)); self.block_indent = self.block_indent.block_unindent(self.config); - - self.buffer - .push_str(&self.block_indent.to_string(self.config)); + let indent_str = self.block_indent.to_string(self.config); + self.push_str(&indent_str); } else { for item in &item.body { self.format_body_element(item); @@ -268,7 +267,7 @@ impl<'a> FmtVisitor<'a> { } } - self.buffer.push_str("}"); + self.push_str("}"); self.last_pos = item.span.hi(); } @@ -423,7 +422,7 @@ impl<'a> FmtVisitor<'a> { span: Span, ) { let enum_header = format_header("enum ", ident, vis); - self.buffer.push_str(&enum_header); + self.push_str(&enum_header); let enum_snippet = self.snippet(span); let brace_pos = enum_snippet.find_uncommented("{").unwrap(); @@ -441,23 +440,23 @@ impl<'a> FmtVisitor<'a> { mk_sp(span.lo(), body_start), last_line_width(&enum_header), ).unwrap(); - self.buffer.push_str(&generics_str); + self.push_str(&generics_str); self.last_pos = body_start; self.block_indent = self.block_indent.block_indent(self.config); let variant_list = self.format_variant_list(enum_def, body_start, span.hi() - BytePos(1)); match variant_list { - Some(ref body_str) => self.buffer.push_str(body_str), + Some(ref body_str) => self.push_str(body_str), None => self.format_missing_no_indent(span.hi() - BytePos(1)), } self.block_indent = self.block_indent.block_unindent(self.config); if variant_list.is_some() || contains_comment(&enum_snippet[brace_pos..]) { - self.buffer - .push_str(&self.block_indent.to_string(self.config)); + let indent_str = self.block_indent.to_string(self.config); + self.push_str(&indent_str); } - self.buffer.push_str("}"); + self.push_str("}"); self.last_pos = span.hi(); } diff --git a/src/lib.rs b/src/lib.rs index f6902db969c..3acba4a4cf1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -338,6 +338,11 @@ where visitor.format_separate_mod(module, &*filemap); }; + assert_eq!( + visitor.line_number, + ::utils::count_newlines(&format!("{}", visitor.buffer)) + ); + has_diff |= match after_file(path_str, &mut visitor.buffer) { Ok(result) => result, Err(e) => { diff --git a/src/missed_spans.rs b/src/missed_spans.rs index a3cad47063e..2ed1f50182b 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -28,27 +28,25 @@ impl<'a> FmtVisitor<'a> { // TODO these format_missing methods are ugly. Refactor and add unit tests // for the central whitespace stripping loop. pub fn format_missing(&mut self, end: BytePos) { - self.format_missing_inner(end, |this, last_snippet, _| { - this.buffer.push_str(last_snippet) - }) + self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet)) } pub fn format_missing_with_indent(&mut self, end: BytePos) { let config = self.config; self.format_missing_inner(end, |this, last_snippet, snippet| { - this.buffer.push_str(last_snippet.trim_right()); + this.push_str(last_snippet.trim_right()); if last_snippet == snippet && !this.output_at_start() { // No new lines in the snippet. - this.buffer.push_str("\n"); + this.push_str("\n"); } let indent = this.block_indent.to_string(config); - this.buffer.push_str(&indent); + this.push_str(&indent); }) } pub fn format_missing_no_indent(&mut self, end: BytePos) { self.format_missing_inner(end, |this, last_snippet, _| { - this.buffer.push_str(last_snippet.trim_right()); + this.push_str(last_snippet.trim_right()); }) } @@ -97,7 +95,7 @@ impl<'a> FmtVisitor<'a> { newline_count = newline_lower_bound; } let blank_lines: String = repeat('\n').take(newline_count).collect(); - self.buffer.push_str(&blank_lines); + self.push_str(&blank_lines); } fn write_snippet(&mut self, span: Span, process_last_snippet: F) @@ -154,12 +152,12 @@ impl<'a> FmtVisitor<'a> { if status.rewrite_next_comment { if fix_indent { if let Some('{') = last_char { - self.buffer.push_str("\n"); + self.push_str("\n"); } - self.buffer - .push_str(&self.block_indent.to_string(self.config)); + let indent_str = self.block_indent.to_string(self.config); + self.push_str(&indent_str); } else { - self.buffer.push_str(" "); + self.push_str(" "); } let comment_width = ::std::cmp::min( @@ -170,7 +168,7 @@ impl<'a> FmtVisitor<'a> { let comment_shape = Shape::legacy(comment_width, comment_indent); let comment_str = rewrite_comment(subslice, false, comment_shape, self.config) .unwrap_or_else(|| String::from(subslice)); - self.buffer.push_str(&comment_str); + self.push_str(&comment_str); status.last_wspace = None; status.line_start = offset + subslice.len(); @@ -183,13 +181,13 @@ impl<'a> FmtVisitor<'a> { .any(|s| s.len() >= 2 && &s[0..2] == "/*") { // Add a newline after line comments - self.buffer.push_str("\n"); + self.push_str("\n"); } } else if status.line_start <= snippet.len() { // For other comments add a newline if there isn't one at the end already match snippet[status.line_start..].chars().next() { Some('\n') | Some('\r') => (), - _ => self.buffer.push_str("\n"), + _ => self.push_str("\n"), } } @@ -277,10 +275,10 @@ impl<'a> FmtVisitor<'a> { } if let Some(lw) = status.last_wspace { - self.buffer.push_str(&snippet[status.line_start..lw]); - self.buffer.push_str("\n"); + self.push_str(&snippet[status.line_start..lw]); + self.push_str("\n"); } else { - self.buffer.push_str(&snippet[status.line_start..i + 1]); + self.push_str(&snippet[status.line_start..i + 1]); } status.cur_line += 1; @@ -306,7 +304,7 @@ impl<'a> FmtVisitor<'a> { let remaining = snippet[status.line_start..subslice.len() + offset].trim(); if !remaining.is_empty() { - self.buffer.push_str(remaining); + self.push_str(remaining); status.line_start = subslice.len() + offset; status.rewrite_next_comment = true; } diff --git a/src/visitor.rs b/src/visitor.rs index 2ba0b744a58..dc11dcd8938 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -81,6 +81,7 @@ pub struct FmtVisitor<'a> { pub config: &'a Config, pub is_if_else_block: bool, pub snippet_provider: &'a SnippetProvider<'a>, + pub line_number: usize, } impl<'b, 'a: 'b> FmtVisitor<'a> { @@ -132,7 +133,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.last_pos = self.last_pos + brace_compensation; self.block_indent = self.block_indent.block_indent(self.config); - self.buffer.push_str("{"); + self.push_str("{"); if self.config.remove_blank_lines_at_start_or_end_of_block() { if let Some(first_stmt) = b.stmts.first() { @@ -195,7 +196,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if !b.stmts.is_empty() { if let Some(expr) = utils::stmt_expr(&b.stmts[b.stmts.len() - 1]) { if utils::semicolon_for_expr(&self.get_context(), expr) { - self.buffer.push_str(";"); + self.push_str(";"); } } } @@ -255,7 +256,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.config.tab_spaces() }; self.buffer.truncate(total_len - chars_too_many); - self.buffer.push_str("}"); + self.push_str("}"); self.block_indent = self.block_indent.block_unindent(self.config); } @@ -288,7 +289,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if let Some(fn_str) = rewrite { self.format_missing_with_indent(source!(self, s).lo()); - self.buffer.push_str(&fn_str); + self.push_str(&fn_str); if let Some(c) = fn_str.chars().last() { if c == '}' { self.last_pos = source!(self, block.span).hi(); @@ -519,13 +520,18 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_rewrite(mac.span, rewrite); } + pub fn push_str(&mut self, s: &str) { + self.line_number += count_newlines(s); + self.buffer.push_str(s); + } + pub fn push_rewrite(&mut self, span: Span, rewrite: Option) { self.format_missing_with_indent(source!(self, span).lo()); if let Some(ref s) = rewrite { - self.buffer.push_str(s); + self.push_str(s); } else { let snippet = self.snippet(span); - self.buffer.push_str(snippet); + self.push_str(snippet); } self.last_pos = source!(self, span).hi(); } @@ -548,6 +554,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { config: config, is_if_else_block: false, snippet_provider: snippet_provider, + line_number: 0, } } @@ -692,15 +699,17 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let is_internal = !(inner_span.lo().0 == 0 && inner_span.hi().0 == 0) && local_file_name == self.codemap.span_to_filename(inner_span); - self.buffer.push_str(&*utils::format_visibility(vis)); - self.buffer.push_str("mod "); - self.buffer.push_str(&ident.to_string()); + self.push_str(&*utils::format_visibility(vis)); + self.push_str("mod "); + self.push_str(&ident.to_string()); if is_internal { match self.config.brace_style() { - BraceStyle::AlwaysNextLine => self.buffer - .push_str(&format!("\n{}{{", self.block_indent.to_string(self.config))), - _ => self.buffer.push_str(" {"), + BraceStyle::AlwaysNextLine => { + let sep_str = format!("\n{}{{", self.block_indent.to_string(self.config)); + self.push_str(&sep_str); + } + _ => self.push_str(" {"), } // Hackery to account for the closing }. let mod_lo = self.codemap.span_after(source!(self, s), "{"); @@ -708,7 +717,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.snippet(mk_sp(mod_lo, source!(self, m.inner).hi() - BytePos(1))); let body_snippet = body_snippet.trim(); if body_snippet.is_empty() { - self.buffer.push_str("}"); + self.push_str("}"); } else { self.last_pos = mod_lo; self.block_indent = self.block_indent.block_indent(self.config); @@ -719,7 +728,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } self.last_pos = source!(self, m.inner).hi(); } else { - self.buffer.push_str(";"); + self.push_str(";"); self.last_pos = source!(self, s).hi(); } } From 821d04b2a4a11ae000aed4c30b1d838fe276890f Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 8 Dec 2017 17:46:43 +0900 Subject: [PATCH 2/3] Do not report errors on skipped items or statements --- src/lib.rs | 39 ++++++++++++++++++++++++++++----------- src/spanned.rs | 2 ++ src/visitor.rs | 45 ++++++++++++++++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3acba4a4cf1..229e25bb65b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -300,7 +300,7 @@ fn format_ast( mut after_file: F, ) -> Result<(FileMap, bool), io::Error> where - F: FnMut(&str, &mut StringBuffer) -> Result, + F: FnMut(&str, &mut StringBuffer, &[(usize, usize)]) -> Result, { let mut result = FileMap::new(); // diff mode: check if any files are differing @@ -343,7 +343,7 @@ where ::utils::count_newlines(&format!("{}", visitor.buffer)) ); - has_diff |= match after_file(path_str, &mut visitor.buffer) { + has_diff |= match after_file(path_str, &mut visitor.buffer, &visitor.skipped_range) { Ok(result) => result, Err(e) => { // Create a new error with path_str to help users see which files failed @@ -358,10 +358,24 @@ where Ok((result, has_diff)) } +/// Returns true if the line with the given line number was skipped by `#[rustfmt_skip]`. +fn is_skipped_line(line_number: usize, skipped_range: &[(usize, usize)]) -> bool { + skipped_range + .iter() + .any(|&(lo, hi)| lo <= line_number && line_number <= hi) +} + // Formatting done on a char by char or line by line basis. // FIXME(#209) warn on bad license // FIXME(#20) other stuff for parity with make tidy -fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &mut FormatReport) { +fn format_lines( + text: &mut StringBuffer, + name: &str, + skipped_range: &[(usize, usize)], + config: &Config, + report: &mut FormatReport, +) { + println!("skipped_range: {:?}", skipped_range); // Iterate over the chars in the file map. let mut trims = vec![]; let mut last_wspace: Option = None; @@ -403,6 +417,7 @@ fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &m // Check for any line width errors we couldn't correct. let report_error_on_line_overflow = config.error_on_line_overflow() + && !is_skipped_line(cur_line, skipped_range) && (config.error_on_line_overflow_comments() || !is_comment); if report_error_on_line_overflow && line_len > config.max_width() { errors.push(FormattingError { @@ -448,12 +463,14 @@ fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &m } for &(l, _, _, ref b) in &trims { - errors.push(FormattingError { - line: l, - kind: ErrorKind::TrailingWhitespace, - is_comment: false, - line_buffer: b.clone(), - }); + if !is_skipped_line(l, skipped_range) { + errors.push(FormattingError { + line: l, + kind: ErrorKind::TrailingWhitespace, + is_comment: false, + line_buffer: b.clone(), + }); + } } report.file_error_map.insert(name.to_owned(), errors); @@ -546,12 +563,12 @@ pub fn format_input( &mut parse_session, &main_file, config, - |file_name, file| { + |file_name, file, skipped_range| { // For some reason, the codemap does not include terminating // newlines so we must add one on for each file. This is sad. filemap::append_newline(file); - format_lines(file, file_name, config, &mut report); + format_lines(file, file_name, skipped_range, config, &mut report); if let Some(ref mut out) = out { return filemap::write_file(file, file_name, out, config); diff --git a/src/spanned.rs b/src/spanned.rs index 6978f2812e6..7181512273e 100644 --- a/src/spanned.rs +++ b/src/spanned.rs @@ -54,6 +54,8 @@ implement_spanned!(ast::Field); implement_spanned!(ast::ForeignItem); implement_spanned!(ast::Item); implement_spanned!(ast::Local); +implement_spanned!(ast::TraitItem); +implement_spanned!(ast::ImplItem); impl Spanned for ast::Stmt { fn span(&self) -> Span { diff --git a/src/visitor.rs b/src/visitor.rs index dc11dcd8938..ac5501cae4c 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -82,6 +82,7 @@ pub struct FmtVisitor<'a> { pub is_if_else_block: bool, pub snippet_provider: &'a SnippetProvider<'a>, pub line_number: usize, + pub skipped_range: Vec<(usize, usize)>, } impl<'b, 'a: 'b> FmtVisitor<'a> { @@ -101,13 +102,17 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.visit_item(item); } ast::StmtKind::Local(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => { - let rewrite = stmt.rewrite(&self.get_context(), self.shape()); - self.push_rewrite(stmt.span(), rewrite) + if contains_skip(get_attrs_from_stmt(stmt)) { + self.push_skipped_with_span(stmt.span()); + } else { + let rewrite = stmt.rewrite(&self.get_context(), self.shape()); + self.push_rewrite(stmt.span(), rewrite) + } } ast::StmtKind::Mac(ref mac) => { let (ref mac, _macro_style, ref attrs) = **mac; if self.visit_attrs(attrs, ast::AttrStyle::Outer) { - self.push_rewrite(stmt.span(), None); + self.push_skipped_with_span(stmt.span()); } else { self.visit_mac(mac, None, MacroPosition::Statement); } @@ -321,7 +326,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // Module is inline, in this case we treat modules like any // other item. if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) { - self.push_rewrite(item.span, None); + self.push_skipped_with_span(item.span()); return; } } else if contains_skip(&item.attrs) { @@ -349,7 +354,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } _ => { if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) { - self.push_rewrite(item.span, None); + self.push_skipped_with_span(item.span()); return; } } @@ -436,7 +441,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skip_out_of_file_lines_range_visitor!(self, ti.span); if self.visit_attrs(&ti.attrs, ast::AttrStyle::Outer) { - self.push_rewrite(ti.span, None); + self.push_skipped_with_span(ti.span()); return; } @@ -478,7 +483,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skip_out_of_file_lines_range_visitor!(self, ii.span); if self.visit_attrs(&ii.attrs, ast::AttrStyle::Outer) { - self.push_rewrite(ii.span, None); + self.push_skipped_with_span(ii.span()); return; } @@ -525,8 +530,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.buffer.push_str(s); } - pub fn push_rewrite(&mut self, span: Span, rewrite: Option) { - self.format_missing_with_indent(source!(self, span).lo()); + fn push_rewrite_inner(&mut self, span: Span, rewrite: Option) { if let Some(ref s) = rewrite { self.push_str(s); } else { @@ -536,6 +540,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.last_pos = source!(self, span).hi(); } + pub fn push_rewrite(&mut self, span: Span, rewrite: Option) { + self.format_missing_with_indent(source!(self, span).lo()); + self.push_rewrite_inner(span, rewrite); + } + + pub fn push_skipped_with_span(&mut self, span: Span) { + self.format_missing_with_indent(source!(self, span).lo()); + let lo = self.line_number + 1; + self.push_rewrite_inner(span, None); + let hi = self.line_number + 1; + self.skipped_range.push((lo, hi)); + } + pub fn from_context(ctx: &'a RewriteContext) -> FmtVisitor<'a> { FmtVisitor::from_codemap(ctx.parse_session, ctx.config, ctx.snippet_provider) } @@ -555,6 +572,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { is_if_else_block: false, snippet_provider: snippet_provider, line_number: 0, + skipped_range: vec![], } } @@ -1058,3 +1076,12 @@ pub fn rewrite_extern_crate(context: &RewriteContext, item: &ast::Item) -> Optio String::from(&*Regex::new(r"\s;").unwrap().replace(no_whitespace, ";")) }) } + +fn get_attrs_from_stmt(stmt: &ast::Stmt) -> &[ast::Attribute] { + match stmt.node { + ast::StmtKind::Local(ref local) => &local.attrs, + ast::StmtKind::Item(ref item) => &item.attrs, + ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => &expr.attrs, + ast::StmtKind::Mac(ref mac) => &mac.2, + } +} From adc3b12ad496a014cd1b3cdbeabafbcd3f206796 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 8 Dec 2017 17:48:49 +0900 Subject: [PATCH 3/3] Remove println! debug :( --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 229e25bb65b..18e802b1d2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -375,7 +375,6 @@ fn format_lines( config: &Config, report: &mut FormatReport, ) { - println!("skipped_range: {:?}", skipped_range); // Iterate over the chars in the file map. let mut trims = vec![]; let mut last_wspace: Option = None;