Skip to content
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.

Sign up for GitHub

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

Do not report errors on skipped items or statements #2250

Merged
merged 3 commits into from
Dec 10, 2017
Merged

Do not report errors on skipped items or statements #2250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e6bb3e
Keep track of line number in visitor
topecongiro Dec 8, 2017
821d04b
Do not report errors on skipped items or statements
topecongiro Dec 8, 2017
adc3b12
Remove println! debug :(
topecongiro Dec 8, 2017
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/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'a> FmtVisitor<'a> {
}
Some(ref s) => {
self.format_missing_with_indent(source!(self, span).lo());
self.buffer.push_str(s);
self.push_str(s);
self.last_pos = source!(self, span).hi();
}
None => {
Expand Down
23 changes: 11 additions & 12 deletions src/items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ impl<'a> FnSig<'a> {

impl<'a> FmtVisitor<'a> {
fn format_item(&mut self, item: Item) {
self.buffer.push_str(&item.abi);
self.push_str(&item.abi);

let snippet = self.snippet(item.span);
let brace_pos = snippet.find_uncommented("{").unwrap();

self.buffer.push_str("{");
self.push_str("{");
if !item.body.is_empty() || contains_comment(&snippet[brace_pos..]) {
// FIXME: this skips comments between the extern keyword and the opening
// brace.
Expand All @@ -255,9 +255,8 @@ impl<'a> FmtVisitor<'a> {
if item.body.is_empty() {
self.format_missing_no_indent(item.span.hi() - BytePos(1));
self.block_indent = self.block_indent.block_unindent(self.config);

self.buffer
.push_str(&self.block_indent.to_string(self.config));
let indent_str = self.block_indent.to_string(self.config);
self.push_str(&indent_str);
} else {
for item in &item.body {
self.format_body_element(item);
Expand All @@ -268,7 +267,7 @@ impl<'a> FmtVisitor<'a> {
}
}

self.buffer.push_str("}");
self.push_str("}");
self.last_pos = item.span.hi();
}

Expand Down Expand Up @@ -423,7 +422,7 @@ impl<'a> FmtVisitor<'a> {
span: Span,
) {
let enum_header = format_header("enum ", ident, vis);
self.buffer.push_str(&enum_header);
self.push_str(&enum_header);

let enum_snippet = self.snippet(span);
let brace_pos = enum_snippet.find_uncommented("{").unwrap();
Expand All @@ -441,23 +440,23 @@ impl<'a> FmtVisitor<'a> {
mk_sp(span.lo(), body_start),
last_line_width(&enum_header),
).unwrap();
self.buffer.push_str(&generics_str);
self.push_str(&generics_str);

self.last_pos = body_start;

self.block_indent = self.block_indent.block_indent(self.config);
let variant_list = self.format_variant_list(enum_def, body_start, span.hi() - BytePos(1));
match variant_list {
Some(ref body_str) => self.buffer.push_str(body_str),
Some(ref body_str) => self.push_str(body_str),
None => self.format_missing_no_indent(span.hi() - BytePos(1)),
}
self.block_indent = self.block_indent.block_unindent(self.config);

if variant_list.is_some() || contains_comment(&enum_snippet[brace_pos..]) {
self.buffer
.push_str(&self.block_indent.to_string(self.config));
let indent_str = self.block_indent.to_string(self.config);
self.push_str(&indent_str);
}
self.buffer.push_str("}");
self.push_str("}");
self.last_pos = span.hi();
}

Expand Down
43 changes: 32 additions & 11 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ fn format_ast<F>(
mut after_file: F,
) -> Result<(FileMap, bool), io::Error>
where
F: FnMut(&str, &mut StringBuffer) -> Result<bool, io::Error>,
F: FnMut(&str, &mut StringBuffer, &[(usize, usize)]) -> Result<bool, io::Error>,
{
let mut result = FileMap::new();
// diff mode: check if any files are differing
Expand Down Expand Up @@ -338,7 +338,12 @@ where
visitor.format_separate_mod(module, &*filemap);
};

has_diff |= match after_file(path_str, &mut visitor.buffer) {
assert_eq!(
visitor.line_number,
::utils::count_newlines(&format!("{}", visitor.buffer))
);

has_diff |= match after_file(path_str, &mut visitor.buffer, &visitor.skipped_range) {
Ok(result) => result,
Err(e) => {
// Create a new error with path_str to help users see which files failed
Expand All @@ -353,10 +358,23 @@ where
Ok((result, has_diff))
}

/// Returns true if the line with the given line number was skipped by `#[rustfmt_skip]`.
fn is_skipped_line(line_number: usize, skipped_range: &[(usize, usize)]) -> bool {
skipped_range
.iter()
.any(|&(lo, hi)| lo <= line_number && line_number <= hi)
}

// Formatting done on a char by char or line by line basis.
// FIXME(#209) warn on bad license
// FIXME(#20) other stuff for parity with make tidy
fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &mut FormatReport) {
fn format_lines(
text: &mut StringBuffer,
name: &str,
skipped_range: &[(usize, usize)],
config: &Config,
report: &mut FormatReport,
) {
// Iterate over the chars in the file map.
let mut trims = vec![];
let mut last_wspace: Option<usize> = None;
Expand Down Expand Up @@ -398,6 +416,7 @@ fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &m

// Check for any line width errors we couldn't correct.
let report_error_on_line_overflow = config.error_on_line_overflow()
&& !is_skipped_line(cur_line, skipped_range)
&& (config.error_on_line_overflow_comments() || !is_comment);
if report_error_on_line_overflow && line_len > config.max_width() {
errors.push(FormattingError {
Expand Down Expand Up @@ -443,12 +462,14 @@ fn format_lines(text: &mut StringBuffer, name: &str, config: &Config, report: &m
}

for &(l, _, _, ref b) in &trims {
errors.push(FormattingError {
line: l,
kind: ErrorKind::TrailingWhitespace,
is_comment: false,
line_buffer: b.clone(),
});
if !is_skipped_line(l, skipped_range) {
errors.push(FormattingError {
line: l,
kind: ErrorKind::TrailingWhitespace,
is_comment: false,
line_buffer: b.clone(),
});
}
}

report.file_error_map.insert(name.to_owned(), errors);
Expand Down Expand Up @@ -541,12 +562,12 @@ pub fn format_input<T: Write>(
&mut parse_session,
&main_file,
config,
|file_name, file| {
|file_name, file, skipped_range| {
// For some reason, the codemap does not include terminating
// newlines so we must add one on for each file. This is sad.
filemap::append_newline(file);

format_lines(file, file_name, config, &mut report);
format_lines(file, file_name, skipped_range, config, &mut report);

if let Some(ref mut out) = out {
return filemap::write_file(file, file_name, out, config);
Expand Down
36 changes: 17 additions & 19 deletions src/missed_spans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,25 @@ impl<'a> FmtVisitor<'a> {
// TODO these format_missing methods are ugly. Refactor and add unit tests
// for the central whitespace stripping loop.
pub fn format_missing(&mut self, end: BytePos) {
self.format_missing_inner(end, |this, last_snippet, _| {
this.buffer.push_str(last_snippet)
})
self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet))
}

pub fn format_missing_with_indent(&mut self, end: BytePos) {
let config = self.config;
self.format_missing_inner(end, |this, last_snippet, snippet| {
this.buffer.push_str(last_snippet.trim_right());
this.push_str(last_snippet.trim_right());
if last_snippet == snippet && !this.output_at_start() {
// No new lines in the snippet.
this.buffer.push_str("\n");
this.push_str("\n");
}
let indent = this.block_indent.to_string(config);
this.buffer.push_str(&indent);
this.push_str(&indent);
})
}

pub fn format_missing_no_indent(&mut self, end: BytePos) {
self.format_missing_inner(end, |this, last_snippet, _| {
this.buffer.push_str(last_snippet.trim_right());
this.push_str(last_snippet.trim_right());
})
}

Expand Down Expand Up @@ -97,7 +95,7 @@ impl<'a> FmtVisitor<'a> {
newline_count = newline_lower_bound;
}
let blank_lines: String = repeat('\n').take(newline_count).collect();
self.buffer.push_str(&blank_lines);
self.push_str(&blank_lines);
}

fn write_snippet<F>(&mut self, span: Span, process_last_snippet: F)
Expand Down Expand Up @@ -154,12 +152,12 @@ impl<'a> FmtVisitor<'a> {
if status.rewrite_next_comment {
if fix_indent {
if let Some('{') = last_char {
self.buffer.push_str("\n");
self.push_str("\n");
}
self.buffer
.push_str(&self.block_indent.to_string(self.config));
let indent_str = self.block_indent.to_string(self.config);
self.push_str(&indent_str);
} else {
self.buffer.push_str(" ");
self.push_str(" ");
}

let comment_width = ::std::cmp::min(
Expand All @@ -170,7 +168,7 @@ impl<'a> FmtVisitor<'a> {
let comment_shape = Shape::legacy(comment_width, comment_indent);
let comment_str = rewrite_comment(subslice, false, comment_shape, self.config)
.unwrap_or_else(|| String::from(subslice));
self.buffer.push_str(&comment_str);
self.push_str(&comment_str);

status.last_wspace = None;
status.line_start = offset + subslice.len();
Expand All @@ -183,13 +181,13 @@ impl<'a> FmtVisitor<'a> {
.any(|s| s.len() >= 2 && &s[0..2] == "/*")
{
// Add a newline after line comments
self.buffer.push_str("\n");
self.push_str("\n");
}
} else if status.line_start <= snippet.len() {
// For other comments add a newline if there isn't one at the end already
match snippet[status.line_start..].chars().next() {
Some('\n') | Some('\r') => (),
_ => self.buffer.push_str("\n"),
_ => self.push_str("\n"),
}
}

Expand Down Expand Up @@ -277,10 +275,10 @@ impl<'a> FmtVisitor<'a> {
}

if let Some(lw) = status.last_wspace {
self.buffer.push_str(&snippet[status.line_start..lw]);
self.buffer.push_str("\n");
self.push_str(&snippet[status.line_start..lw]);
self.push_str("\n");
} else {
self.buffer.push_str(&snippet[status.line_start..i + 1]);
self.push_str(&snippet[status.line_start..i + 1]);
}

status.cur_line += 1;
Expand All @@ -306,7 +304,7 @@ impl<'a> FmtVisitor<'a> {

let remaining = snippet[status.line_start..subslice.len() + offset].trim();
if !remaining.is_empty() {
self.buffer.push_str(remaining);
self.push_str(remaining);
status.line_start = subslice.len() + offset;
status.rewrite_next_comment = true;
}
Expand Down
2 changes: 2 additions & 0 deletions src/spanned.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ implement_spanned!(ast::Field);
implement_spanned!(ast::ForeignItem);
implement_spanned!(ast::Item);
implement_spanned!(ast::Local);
implement_spanned!(ast::TraitItem);
implement_spanned!(ast::ImplItem);

impl Spanned for ast::Stmt {
fn span(&self) -> Span {
Expand Down
Loading