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 rust-lang#3442

Author: scampi

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<F>(
         &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),
         };
 
-        for (kind, offset, subslice) in CommentCodeSlices::new(snippet) {
-            debug!("{:?}: {:?}", kind, subslice);
+        // 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 newline_count = count_newlines(subslice);
-            let within_file_lines_range = self.config.file_lines().contains_range(
+        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,
-                status.cur_line,
-                status.cur_line + newline_count,
+                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, 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(

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,
         }
     }
 }

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,24 +225,31 @@ 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();
     }
 
     // FIXME: this is a terrible hack to indent the comments between the last
     // 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 {

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;