Skip to content

Implement closing-block procedure without relying on missed_span module #3691

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 8 commits into from
Jul 17, 2019
13 changes: 1 addition & 12 deletions 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, SourceMap, Span};
use syntax::source_map::{self, SourceFile};

/// A range of lines in a file, inclusive of both ends.
pub struct LineRange {
Expand Down Expand Up @@ -78,17 +78,6 @@ 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
15 changes: 15 additions & 0 deletions src/coverage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use crate::{Config, EmitMode};
use std::borrow::Cow;

pub(crate) fn transform_missing_snippet<'a>(config: &Config, string: &'a str) -> Cow<'a, str> {
match config.emit_mode() {
EmitMode::Coverage => Cow::from(replace_chars(string)),
_ => Cow::from(string),
}
}

fn replace_chars(s: &str) -> String {
s.chars()
.map(|ch| if ch.is_whitespace() { ch } else { 'X' })
.collect()
}
1 change: 0 additions & 1 deletion src/format-diff/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,5 +292,4 @@ mod cmd_line_tests {
.is_err()
);
}

}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ mod chains;
mod closures;
mod comment;
pub(crate) mod config;
mod coverage;
mod emitter;
mod expr;
mod format_report_formatter;
Expand Down
17 changes: 3 additions & 14 deletions src/missed_spans.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::borrow::Cow;

use syntax::source_map::{BytePos, Pos, Span};

use crate::comment::{is_last_comment_block, rewrite_comment, CodeCharKind, CommentCodeSlices};
use crate::config::file_lines::FileLines;
use crate::config::{EmitMode, FileName};
use crate::config::FileName;
use crate::coverage::transform_missing_snippet;
use crate::shape::{Indent, Shape};
use crate::source_map::LineRangeUtils;
use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp};
Expand Down Expand Up @@ -171,10 +170,7 @@ impl<'a> FmtVisitor<'a> {
let file_name = &char_pos.file.name.clone().into();
let mut status = SnippetStatus::new(char_pos.line);

let snippet = &*match self.config.emit_mode() {
EmitMode::Coverage => Cow::from(replace_chars(old_snippet)),
_ => Cow::from(old_snippet),
};
let snippet = &*transform_missing_snippet(self.config, old_snippet);

let slice_within_file_lines_range =
|file_lines: FileLines, cur_line, s| -> (usize, usize, bool) {
Expand Down Expand Up @@ -333,10 +329,3 @@ impl<'a> FmtVisitor<'a> {
}
}
}

fn replace_chars(string: &str) -> String {
string
.chars()
.map(|ch| if ch.is_whitespace() { ch } else { 'X' })
.collect()
}
178 changes: 89 additions & 89 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use syntax::source_map::{self, BytePos, Pos, SourceMap, Span};
use syntax::{ast, visit};

use crate::attr::*;
use crate::comment::{CodeCharKind, CommentCodeSlices};
use crate::config::file_lines::LineRange;
use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices};
use crate::config::{BraceStyle, Config};
use crate::coverage::transform_missing_snippet;
use crate::items::{
format_impl, format_trait, format_trait_alias, is_mod_decl, is_use_item,
rewrite_associated_impl_type, rewrite_associated_type, rewrite_existential_impl_type,
Expand All @@ -22,8 +22,8 @@ use crate::source_map::{LineRangeUtils, SpanUtils};
use crate::spanned::Spanned;
use crate::stmt::Stmt;
use crate::utils::{
self, contains_skip, count_newlines, depr_skip_annotation, inner_attributes, mk_sp,
ptr_vec_to_ref_vec, rewrite_ident, stmt_expr,
self, contains_skip, count_newlines, depr_skip_annotation, inner_attributes, last_line_width,
mk_sp, ptr_vec_to_ref_vec, rewrite_ident, stmt_expr,
};
use crate::{ErrorKind, FormatReport, FormattingError};

Expand Down Expand Up @@ -165,32 +165,6 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
}

/// 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)
}
}
}

pub(crate) fn visit_block(
&mut self,
b: &ast::Block,
Expand Down Expand Up @@ -226,72 +200,99 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
}

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("/*")
};
if unindent_comment {
let rest_span = self.next_span(b.span.hi());
if out_of_file_lines_range!(self, rest_span) {
self.push_str(self.snippet(rest_span));
self.block_indent = self.block_indent.block_unindent(self.config);
} else {
// Ignore the closing brace.
let missing_span = self.next_span(b.span.hi() - brace_compensation);
self.close_block(missing_span, self.unindent_comment_on_closing_brace(b));
}
self.format_missing_with_indent(
source!(self, b.span).hi() - brace_compensation - remove_len,
);
if unindent_comment {
self.block_indent = self.block_indent.block_indent(self.config);
}
self.close_block(unindent_comment, self.next_span(b.span.hi()));
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, span: Span) {
let skip_this_line = !self
.config
.file_lines()
.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
} else if self.config.hard_tabs() {
1
} else {
self.config.tab_spaces()
};
fn close_block(&mut self, span: Span, unindent_comment: bool) {
let config = self.config;

// FIXME this is a temporaly fix
// should be remove truncate logic in close_block
// avoid not to truncate necessary chars
let truncate_start = total_len - chars_too_many;
let target_str = &self.buffer[truncate_start..total_len];
let truncate_length = target_str.len() - target_str.trim().len();

if let Some(last_char) = target_str.chars().last() {
self.buffer.truncate(total_len - truncate_length);
if last_char == '\n' {
self.buffer.push_str("\n");
let mut last_hi = span.lo();
let mut unindented = false;
let mut prev_ends_with_newline = false;
let mut extra_newline = false;

let skip_normal = |s: &str| {
let trimmed = s.trim();
trimmed.is_empty() || trimmed.chars().all(|c| c == ';')
};

for (kind, offset, sub_slice) in CommentCodeSlices::new(self.snippet(span)) {
let sub_slice = transform_missing_snippet(config, sub_slice);

debug!("close_block: {:?} {:?} {:?}", kind, offset, sub_slice);

match kind {
CodeCharKind::Comment => {
if !unindented && unindent_comment {
unindented = true;
self.block_indent = self.block_indent.block_unindent(config);
}
let span_in_between = mk_sp(last_hi, span.lo() + BytePos::from_usize(offset));
let snippet_in_between = self.snippet(span_in_between);
let mut comment_on_same_line = !snippet_in_between.contains("\n");

let mut comment_shape =
Shape::indented(self.block_indent, config).comment(config);
if comment_on_same_line {
// 1 = a space before `//`
let offset_len = 1 + last_line_width(&self.buffer)
.saturating_sub(self.block_indent.width());
match comment_shape
.visual_indent(offset_len)
.sub_width(offset_len)
{
Some(shp) => comment_shape = shp,
None => comment_on_same_line = false,
}
};

if comment_on_same_line {
self.push_str(" ");
} else {
if count_newlines(snippet_in_between) >= 2 || extra_newline {
self.push_str("\n");
}
self.push_str(&self.block_indent.to_string_with_newline(config));
}

let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config);
match comment_str {
Some(ref s) => self.push_str(s),
None => self.push_str(&sub_slice),
}
}
CodeCharKind::Normal if skip_normal(&sub_slice) => {
extra_newline = prev_ends_with_newline && sub_slice.contains('\n');
continue;
}
CodeCharKind::Normal => {
self.push_str(&self.block_indent.to_string_with_newline(config));
self.push_str(sub_slice.trim());
}
}
self.push_str("}");
prev_ends_with_newline = sub_slice.ends_with('\n');
extra_newline = false;
last_hi = span.lo() + BytePos::from_usize(offset + sub_slice.len());
}
if unindented {
self.block_indent = self.block_indent.block_indent(self.config);
}
self.block_indent = self.block_indent.block_unindent(self.config);
self.push_str(&self.block_indent.to_string_with_newline(config));
self.push_str("}");
}

fn unindent_comment_on_closing_brace(&self, b: &ast::Block) -> bool {
self.is_if_else_block && !b.stmts.is_empty()
}

// Note that this only gets called for function definitions. Required methods
Expand Down Expand Up @@ -806,9 +807,8 @@ 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);
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);
let missing_span = self.next_span(m.inner.hi() - BytePos(1));
self.close_block(missing_span, false);
}
self.last_pos = source!(self, m.inner).hi();
} else {
Expand Down
1 change: 0 additions & 1 deletion tests/source/configs/empty_item_single_line/false.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ fn lorem() {
}

fn lorem() {

}
1 change: 0 additions & 1 deletion tests/source/configs/empty_item_single_line/true.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ fn lorem() {
}

fn lorem() {

}
1 change: 0 additions & 1 deletion tests/source/issue-977.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// rustfmt-normalize_comments: true
// FIXME(#919)

trait NameC { /* comment */ }
struct FooC { /* comment */ }
Expand Down
2 changes: 1 addition & 1 deletion tests/target/assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn main() {
single_lit_fit >>= 10;

// #2791
let x = 2;;;;
let x = 2;
}

fn break_meee() {
Expand Down
3 changes: 1 addition & 2 deletions tests/target/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ fn issue_1086() {

// random comment

fn main() {
// Test
fn main() { // Test
}

// #1643
Expand Down
1 change: 0 additions & 1 deletion tests/target/comments-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ where
F: Foo, // COmment after where-clause
G: Goo, // final comment
{

}

fn bar<F /* comment on F */, G /* comment on G */>() {}
Expand Down
1 change: 0 additions & 1 deletion tests/target/configs/empty_item_single_line/false.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ fn lorem() {
}

fn lorem() {

}
2 changes: 0 additions & 2 deletions tests/target/fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ fn foo(
where
T: Blah,
{

}

fn foo<U, T>(
Expand All @@ -32,7 +31,6 @@ where
T: Blah,
U: dsfasdfasdfasd,
{

}

fn foo<U: Fn(A) -> B /* paren inside generics */>() {}
Expand Down
4 changes: 1 addition & 3 deletions tests/target/issue-977.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// rustfmt-normalize_comments: true
// FIXME(#919)

trait NameC {
// comment
Expand All @@ -10,8 +9,7 @@ struct FooC {
enum MooC {
// comment
}
mod BarC {
// comment
mod BarC { // comment
}
extern "C" {
// comment
Expand Down
Loading