From cdd08da27bfb76d24dc44848e4b086df7d25d4f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Tue, 19 Mar 2019 10:19:45 +0100 Subject: [PATCH] fix line numbering in missed spans and handle file_lines in edge cases - a leading/trailing newline character in missed spans was throwing off the start/end of ranges used to compare against file_lines - fix handling of file_lines when closing a block Close #3442 --- src/missed_spans.rs | 47 ++++++++++++++++++++++------ src/source_map.rs | 8 +++-- src/visitor.rs | 64 ++++++++++++++++++++++---------------- tests/target/issue-3442.rs | 10 ++++++ 4 files changed, 92 insertions(+), 37 deletions(-) create mode 100644 tests/target/issue-3442.rs diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 27b94a6c8ae..70ca898f2af 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use syntax::source_map::{BytePos, Pos, Span}; use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices}; +use crate::config::file_lines::FileLines; use crate::config::{EmitMode, FileName}; use crate::shape::{Indent, Shape}; use crate::source_map::LineRangeUtils; @@ -156,7 +157,7 @@ impl<'a> FmtVisitor<'a> { fn write_snippet_inner( &mut self, big_snippet: &str, - big_diff: usize, + mut big_diff: usize, old_snippet: &str, span: Span, process_last_snippet: F, @@ -175,16 +176,36 @@ impl<'a> FmtVisitor<'a> { _ => Cow::from(old_snippet), }; + // if the snippet starts with a new line, then information about the lines needs to be + // adjusted since it is off by 1. + let snippet = if snippet.starts_with('\n') { + // this takes into account the blank_lines_* options + self.push_vertical_spaces(1); + // include the newline character into the big_diff + big_diff += 1; + status.cur_line += 1; + &snippet[1..] + } else { + snippet + }; + + let slice_within_file_lines_range = |file_lines: FileLines, cur_line, s| -> (usize, bool) { + let newline_count = count_newlines(s); + let within_file_lines_range = file_lines.contains_range( + file_name, + cur_line, + // if a newline character is at the end of the slice, then the number of newlines + // needs to be decreased by 1 so that the range checked against the file_lines is + // the visual range one would expect. + cur_line + newline_count - if s.ends_with('\n') { 1 } else { 0 }, + ); + (newline_count, within_file_lines_range) + }; for (kind, offset, subslice) in CommentCodeSlices::new(snippet) { debug!("{:?}: {:?}", kind, subslice); - let newline_count = count_newlines(subslice); - let within_file_lines_range = self.config.file_lines().contains_range( - file_name, - status.cur_line, - status.cur_line + newline_count, - ); - + let (newline_count, within_file_lines_range) = + slice_within_file_lines_range(self.config.file_lines(), status.cur_line, subslice); if CodeCharKind::Comment == kind && within_file_lines_range { // 1: comment. self.process_comment( @@ -205,7 +226,15 @@ impl<'a> FmtVisitor<'a> { } } - process_last_snippet(self, &snippet[status.line_start..], snippet); + let last_snippet = &snippet[status.line_start..]; + let (_, within_file_lines_range) = + slice_within_file_lines_range(self.config.file_lines(), status.cur_line, last_snippet); + if within_file_lines_range { + process_last_snippet(self, last_snippet, snippet); + } else { + // just append what's left + self.push_str(last_snippet); + } } fn process_comment( diff --git a/src/source_map.rs b/src/source_map.rs index c98b7596383..096a7ce5713 100644 --- a/src/source_map.rs +++ b/src/source_map.rs @@ -71,6 +71,7 @@ impl<'a> SpanUtils for SnippetProvider<'a> { impl LineRangeUtils for SourceMap { fn lookup_line_range(&self, span: Span) -> LineRange { + let snippet = self.span_to_snippet(span).unwrap_or(String::new()); let lo = self.lookup_line(span.lo()).unwrap(); let hi = self.lookup_line(span.hi()).unwrap(); @@ -80,11 +81,14 @@ impl LineRangeUtils for SourceMap { lo, hi ); + // in case the span starts with a newline, the line range is off by 1 without the + // adjustment below + let offset = 1 + if snippet.starts_with('\n') { 1 } else { 0 }; // Line numbers start at 1 LineRange { file: lo.sf.clone(), - lo: lo.line + 1, - hi: hi.line + 1, + lo: lo.line + offset, + hi: hi.line + offset, } } } diff --git a/src/visitor.rs b/src/visitor.rs index 3bd5af3112d..2c603fbebf7 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -6,6 +6,7 @@ use syntax::{ast, visit}; use crate::attr::*; use crate::comment::{CodeCharKind, CommentCodeSlices, FindUncommented}; +use crate::config::file_lines::FileName; use crate::config::{BraceStyle, Config, Version}; use crate::expr::{format_expr, ExprType}; use crate::items::{ @@ -170,7 +171,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if skip_rewrite { self.push_rewrite(b.span, None); - self.close_block(false); + self.close_block(false, b.span); self.last_pos = source!(self, b.span).hi(); return; } @@ -187,21 +188,25 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let mut remove_len = BytePos(0); if let Some(stmt) = b.stmts.last() { - let snippet = self.snippet(mk_sp( + let span_after_last_stmt = mk_sp( stmt.span.hi(), source!(self, b.span).hi() - brace_compensation, - )); - let len = CommentCodeSlices::new(snippet) - .last() - .and_then(|(kind, _, s)| { - if kind == CodeCharKind::Normal && s.trim().is_empty() { - Some(s.len()) - } else { - None - } - }); - if let Some(len) = len { - remove_len = BytePos::from_usize(len); + ); + // if the span is outside of a file_lines range, then do not try to remove anything + if !out_of_file_lines_range!(self, span_after_last_stmt) { + let snippet = self.snippet(span_after_last_stmt); + let len = CommentCodeSlices::new(snippet) + .last() + .and_then(|(kind, _, s)| { + if kind == CodeCharKind::Normal && s.trim().is_empty() { + Some(s.len()) + } else { + None + } + }); + if let Some(len) = len { + remove_len = BytePos::from_usize(len); + } } } @@ -220,7 +225,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if unindent_comment { self.block_indent = self.block_indent.block_indent(self.config); } - self.close_block(unindent_comment); + self.close_block(unindent_comment, b.span); self.last_pos = source!(self, b.span).hi(); } @@ -228,16 +233,23 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // item in the block and the closing brace to the block's level. // The closing brace itself, however, should be indented at a shallower // level. - fn close_block(&mut self, unindent_comment: bool) { - let total_len = self.buffer.len(); - let chars_too_many = if unindent_comment { - 0 - } else if self.config.hard_tabs() { - 1 - } else { - self.config.tab_spaces() - }; - self.buffer.truncate(total_len - chars_too_many); + fn close_block(&mut self, unindent_comment: bool, span: Span) { + let file_name: FileName = self.source_map.span_to_filename(span).into(); + let skip_this_line = !self + .config + .file_lines() + .contains_line(&file_name, self.line_number); + if !skip_this_line { + let total_len = self.buffer.len(); + let chars_too_many = if unindent_comment { + 0 + } else if self.config.hard_tabs() { + 1 + } else { + self.config.tab_spaces() + }; + self.buffer.truncate(total_len - chars_too_many); + } self.push_str("}"); self.block_indent = self.block_indent.block_unindent(self.config); } @@ -759,7 +771,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.visit_attrs(attrs, ast::AttrStyle::Inner); self.walk_mod_items(m); self.format_missing_with_indent(source!(self, m.inner).hi() - BytePos(1)); - self.close_block(false); + self.close_block(false, m.inner); } self.last_pos = source!(self, m.inner).hi(); } else { diff --git a/tests/target/issue-3442.rs b/tests/target/issue-3442.rs new file mode 100644 index 00000000000..3664c50ee7a --- /dev/null +++ b/tests/target/issue-3442.rs @@ -0,0 +1,10 @@ +// rustfmt-file_lines: [{"file":"tests/target/issue-3442.rs","range":[5,5]},{"file":"tests/target/issue-3442.rs","range":[8,8]}] + +extern crate alpha; // comment 1 +extern crate beta; // comment 2 +#[allow(aaa)] // comment 3 +#[macro_use] +extern crate gamma; +#[allow(bbb)] // comment 4 +#[macro_use] +extern crate lazy_static;