Skip to content

Fix bugs related to file-lines #3684

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 6 commits into from
Jul 15, 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
2 changes: 1 addition & 1 deletion src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,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
Expand Down
13 changes: 12 additions & 1 deletion src/config/file_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
50 changes: 20 additions & 30 deletions src/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down
17 changes: 16 additions & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
140 changes: 79 additions & 61 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: {:?} {:?}",
Expand Down Expand Up @@ -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)| {
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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();
}

Expand All @@ -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
Expand All @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Loading