Skip to content

fix line numbering in missed spans and handle file_lines in edge cases #3459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions src/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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(
Expand Down
8 changes: 6 additions & 2 deletions src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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,
}
}
}
64 changes: 38 additions & 26 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
}
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions tests/target/issue-3442.rs
Original file line number Diff line number Diff line change
@@ -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;