Fix bugs related to file-lines (rust-lang#3684)

Author: topecongiro

src/bin/main.rs

@@ -167,7 +167,7 @@ fn make_opts() -> Options {
 }
 
 fn is_nightly() -> bool {
-    option_env!("CFG_RELEASE_CHANNEL").map_or(false, |c| c == "nightly" || c == "dev")
+    option_env!("CFG_RELEASE_CHANNEL").map_or(true, |c| c == "nightly" || c == "dev")
 }
 
 // Returned i32 is an exit code

src/config/file_lines.rs

@@ -9,7 +9,7 @@ use std::{cmp, fmt, iter, str};
 use serde::{ser, Deserialize, Deserializer, Serialize, Serializer};
 use serde_json as json;
 
-use syntax::source_map::{self, SourceFile};
+use syntax::source_map::{self, SourceFile, SourceMap, Span};
 
 /// A range of lines in a file, inclusive of both ends.
 pub struct LineRange {
@@ -78,6 +78,17 @@ impl LineRange {
     pub fn file_name(&self) -> FileName {
         self.file.name.clone().into()
     }
+
+    pub(crate) fn from_span(source_map: &SourceMap, span: Span) -> LineRange {
+        let lo_char_pos = source_map.lookup_char_pos(span.lo());
+        let hi_char_pos = source_map.lookup_char_pos(span.hi());
+        debug_assert!(lo_char_pos.file.name == hi_char_pos.file.name);
+        LineRange {
+            file: lo_char_pos.file.clone(),
+            lo: lo_char_pos.line,
+            hi: hi_char_pos.line,
+        }
+    }
 }
 
 /// A range that is inclusive of both ends.

src/missed_spans.rs

@@ -7,7 +7,7 @@ use crate::config::file_lines::FileLines;
 use crate::config::{EmitMode, FileName};
 use crate::shape::{Indent, Shape};
 use crate::source_map::LineRangeUtils;
-use crate::utils::{count_newlines, last_line_width, mk_sp};
+use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp};
 use crate::visitor::FmtVisitor;
 
 struct SnippetStatus {
@@ -157,7 +157,7 @@ impl<'a> FmtVisitor<'a> {
     fn write_snippet_inner<F>(
         &mut self,
         big_snippet: &str,
-        mut big_diff: usize,
+        big_diff: usize,
         old_snippet: &str,
         span: Span,
         process_last_snippet: F,
@@ -176,36 +176,26 @@ 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)
-        };
+        let slice_within_file_lines_range =
+            |file_lines: FileLines, cur_line, s| -> (usize, usize, bool) {
+                let (lf_count, crlf_count) = count_lf_crlf(s);
+                let newline_count = lf_count + crlf_count;
+                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 },
+                );
+                (lf_count, crlf_count, within_file_lines_range)
+            };
         for (kind, offset, subslice) in CommentCodeSlices::new(snippet) {
             debug!("{:?}: {:?}", kind, subslice);
 
-            let (newline_count, within_file_lines_range) =
+            let (lf_count, crlf_count, within_file_lines_range) =
                 slice_within_file_lines_range(self.config.file_lines(), status.cur_line, subslice);
+            let newline_count = lf_count + crlf_count;
             if CodeCharKind::Comment == kind && within_file_lines_range {
                 // 1: comment.
                 self.process_comment(
@@ -219,15 +209,15 @@ impl<'a> FmtVisitor<'a> {
                 // 2: blank lines.
                 self.push_vertical_spaces(newline_count);
                 status.cur_line += newline_count;
-                status.line_start = offset + newline_count;
+                status.line_start = offset + lf_count + crlf_count * 2;
             } else {
                 // 3: code which we failed to format or which is not within file-lines range.
                 self.process_missing_code(&mut status, snippet, subslice, offset, file_name);
             }
         }
 
         let last_snippet = &snippet[status.line_start..];
-        let (_, within_file_lines_range) =
+        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);

src/source_map.rs

@@ -5,6 +5,7 @@ use syntax::source_map::{BytePos, SourceMap, Span};
 
 use crate::comment::FindUncommented;
 use crate::config::file_lines::LineRange;
+use crate::utils::starts_with_newline;
 use crate::visitor::SnippetProvider;
 
 pub(crate) trait SpanUtils {
@@ -95,7 +96,7 @@ impl LineRangeUtils for SourceMap {
 
         // 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 };
+        let offset = 1 + if starts_with_newline(&snippet) { 1 } else { 0 };
         // Line numbers start at 1
         LineRange {
             file: lo.sf.clone(),

src/utils.rs

@@ -306,7 +306,22 @@ pub(crate) fn stmt_expr(stmt: &ast::Stmt) -> Option<&ast::Expr> {
     }
 }
 
-#[inline]
+/// Returns the number of LF and CRLF respectively.
+pub(crate) fn count_lf_crlf(input: &str) -> (usize, usize) {
+    let mut lf = 0;
+    let mut crlf = 0;
+    let mut is_crlf = false;
+    for c in input.as_bytes() {
+        match c {
+            b'\r' => is_crlf = true,
+            b'\n' if is_crlf => crlf += 1,
+            b'\n' => lf += 1,
+            _ => is_crlf = false,
+        }
+    }
+    (lf, crlf)
+}
+
 pub(crate) fn count_newlines(input: &str) -> usize {
     // Using bytes to omit UTF-8 decoding
     bytecount::count(input.as_bytes(), b'\n')

src/visitor.rs

@@ -6,7 +6,7 @@ use syntax::{ast, visit};
 
 use crate::attr::*;
 use crate::comment::{CodeCharKind, CommentCodeSlices};
-use crate::config::file_lines::FileName;
+use crate::config::file_lines::LineRange;
 use crate::config::{BraceStyle, Config};
 use crate::items::{
     format_impl, format_trait, format_trait_alias, is_mod_decl, is_use_item,
@@ -89,6 +89,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         Shape::indented(self.block_indent, self.config)
     }
 
+    fn next_span(&self, hi: BytePos) -> Span {
+        mk_sp(self.last_pos, hi)
+    }
+
     fn visit_stmt(&mut self, stmt: &Stmt<'_>) {
         debug!(
             "visit_stmt: {:?} {:?}",
@@ -132,31 +136,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         }
     }
 
-    pub(crate) fn visit_block(
+    /// Remove spaces between the opening brace and the first statement or the inner attribute
+    /// of the block.
+    fn trim_spaces_after_opening_brace(
         &mut self,
         b: &ast::Block,
         inner_attrs: Option<&[ast::Attribute]>,
-        has_braces: bool,
     ) {
-        debug!(
-            "visit_block: {:?} {:?}",
-            self.source_map.lookup_char_pos(b.span.lo()),
-            self.source_map.lookup_char_pos(b.span.hi())
-        );
-
-        // Check if this block has braces.
-        let brace_compensation = BytePos(if has_braces { 1 } else { 0 });
-
-        self.last_pos = self.last_pos + brace_compensation;
-        self.block_indent = self.block_indent.block_indent(self.config);
-        self.push_str("{");
-
         if let Some(first_stmt) = b.stmts.first() {
             let hi = inner_attrs
                 .and_then(|attrs| inner_attributes(attrs).first().map(|attr| attr.span.lo()))
                 .unwrap_or_else(|| first_stmt.span().lo());
-
-            let snippet = self.snippet(mk_sp(self.last_pos, hi));
+            let missing_span = self.next_span(hi);
+            let snippet = self.snippet(missing_span);
             let len = CommentCodeSlices::new(snippet)
                 .nth(0)
                 .and_then(|(kind, _, s)| {
@@ -170,19 +162,57 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 self.last_pos = self.last_pos + BytePos::from_usize(len);
             }
         }
+    }
 
-        // Format inner attributes if available.
-        let skip_rewrite = if let Some(attrs) = inner_attrs {
-            self.visit_attrs(attrs, ast::AttrStyle::Inner)
-        } else {
-            false
-        };
+    /// Returns the total length of the spaces which should be trimmed between the last statement
+    /// and the closing brace of the block.
+    fn trimmed_spaces_width_before_closing_brace(
+        &mut self,
+        b: &ast::Block,
+        brace_compensation: BytePos,
+    ) -> usize {
+        match b.stmts.last() {
+            None => 0,
+            Some(..) => {
+                let span_after_last_stmt = self.next_span(b.span.hi() - brace_compensation);
+                let missing_snippet = self.snippet(span_after_last_stmt);
+                CommentCodeSlices::new(missing_snippet)
+                    .last()
+                    .and_then(|(kind, _, s)| {
+                        if kind == CodeCharKind::Normal && s.trim().is_empty() {
+                            Some(s.len())
+                        } else {
+                            None
+                        }
+                    })
+                    .unwrap_or(0)
+            }
+        }
+    }
 
-        if skip_rewrite {
-            self.push_rewrite(b.span, None);
-            self.close_block(false, b.span);
-            self.last_pos = source!(self, b.span).hi();
-            return;
+    pub(crate) fn visit_block(
+        &mut self,
+        b: &ast::Block,
+        inner_attrs: Option<&[ast::Attribute]>,
+        has_braces: bool,
+    ) {
+        debug!(
+            "visit_block: {:?} {:?}",
+            self.source_map.lookup_char_pos(b.span.lo()),
+            self.source_map.lookup_char_pos(b.span.hi())
+        );
+
+        // Check if this block has braces.
+        let brace_compensation = BytePos(if has_braces { 1 } else { 0 });
+
+        self.last_pos = self.last_pos + brace_compensation;
+        self.block_indent = self.block_indent.block_indent(self.config);
+        self.push_str("{");
+        self.trim_spaces_after_opening_brace(b, inner_attrs);
+
+        // Format inner attributes if available.
+        if let Some(attrs) = inner_attrs {
+            self.visit_attrs(attrs, ast::AttrStyle::Inner);
         }
 
         self.walk_block_stmts(b);
@@ -195,36 +225,22 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             }
         }
 
-        let mut remove_len = BytePos(0);
-        if let Some(stmt) = b.stmts.last() {
-            let span_after_last_stmt = mk_sp(
-                stmt.span.hi(),
-                source!(self, b.span).hi() - brace_compensation,
-            );
-            // 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);
-                }
-            }
+        let missing_span = self.next_span(b.span.hi());
+        if out_of_file_lines_range!(self, missing_span) {
+            self.push_str(self.snippet(missing_span));
+            self.block_indent = self.block_indent.block_unindent(self.config);
+            self.last_pos = source!(self, b.span).hi();
+            return;
         }
 
+        let remove_len = BytePos::from_usize(
+            self.trimmed_spaces_width_before_closing_brace(b, brace_compensation),
+        );
         let unindent_comment = self.is_if_else_block && !b.stmts.is_empty() && {
             let end_pos = source!(self, b.span).hi() - brace_compensation - remove_len;
             let snippet = self.snippet(mk_sp(self.last_pos, end_pos));
             snippet.contains("//") || snippet.contains("/*")
         };
-        // FIXME: we should compress any newlines here to just one
         if unindent_comment {
             self.block_indent = self.block_indent.block_unindent(self.config);
         }
@@ -234,7 +250,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, b.span);
+        self.close_block(unindent_comment, self.next_span(b.span.hi()));
         self.last_pos = source!(self, b.span).hi();
     }
 
@@ -243,12 +259,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
     // The closing brace itself, however, should be indented at a shallower
     // level.
     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 {
+            .contains(&LineRange::from_span(self.source_map, span));
+        if skip_this_line {
+            self.push_str(self.snippet(span));
+        } else {
             let total_len = self.buffer.len();
             let chars_too_many = if unindent_comment {
                 0
@@ -271,8 +288,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                     self.buffer.push_str("\n");
                 }
             }
+            self.push_str("}");
         }
-        self.push_str("}");
         self.block_indent = self.block_indent.block_unindent(self.config);
     }
 
@@ -800,8 +817,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 self.block_indent = self.block_indent.block_indent(self.config);
                 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, m.inner);
+                let missing_span = mk_sp(source!(self, m.inner).hi() - BytePos(1), m.inner.hi());
+                self.format_missing_with_indent(missing_span.lo());
+                self.close_block(false, missing_span);
             }
             self.last_pos = source!(self, m.inner).hi();
         } else {
@@ -823,9 +841,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
     pub(crate) fn skip_empty_lines(&mut self, end_pos: BytePos) {
         while let Some(pos) = self
             .snippet_provider
-            .opt_span_after(mk_sp(self.last_pos, end_pos), "\n")
+            .opt_span_after(self.next_span(end_pos), "\n")
         {
-            if let Some(snippet) = self.opt_snippet(mk_sp(self.last_pos, pos)) {
+            if let Some(snippet) = self.opt_snippet(self.next_span(pos)) {
                 if snippet.trim().is_empty() {
                     self.last_pos = pos;
                 } else {