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.

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

Use scoped attributes for skip attribute #2703

Merged
merged 5 commits into from
May 15, 2018
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
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ matrix:
- env: INTEGRATION=futures-rs
- env: INTEGRATION=rand
- env: INTEGRATION=failure
- env: INTEGRATION=glob
- env: INTEGRATION=error-chain
- env: INTEGRATION=tempdir
- env: INTEGRATION=bitflags
- env: INTEGRATION=log
allow_failures:
Expand All @@ -47,6 +45,8 @@ matrix:
- env: INTEGRATION=futures-rs
- env: INTEGRATION=log
- env: INTEGRATION=rand
- env: INTEGRATION=glob
- env: INTEGRATION=tempdir

before_script:
- |
Expand Down
4 changes: 2 additions & 2 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1951,7 +1951,7 @@ lines are found, they are trimmed down to match this integer.
Original Code:

```rust
#![rustfmt_skip]
#![rustfmt::skip]

fn foo() {
println!("a");
Expand Down Expand Up @@ -2010,7 +2010,7 @@ them, additional blank lines are inserted.
Original Code (rustfmt will not change it with the default value of `0`):

```rust
#![rustfmt_skip]
#![rustfmt::skip]

fn foo() {
println!("a");
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ See [Configurations.md](Configurations.md) for details.
* For things you do not want rustfmt to mangle, use one of

```rust
#[rustfmt_skip] // requires nightly and #![feature(custom_attribute)] in crate root
#[rustfmt::skip] // requires nightly Rust and #![feature(tool_attributes)] in crate root
#[cfg_attr(rustfmt, rustfmt_skip)] // works in stable
```
* When you run rustfmt, place a file named `rustfmt.toml` or `.rustfmt.toml` in
Expand Down
2 changes: 2 additions & 0 deletions rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error_on_line_overflow = true
error_on_unformatted = true
21 changes: 12 additions & 9 deletions src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use expr::rewrite_literal;
use lists::{itemize_list, write_list, ListFormatting};
use rewrite::{Rewrite, RewriteContext};
use shape::Shape;
use types::{rewrite_path, PathContext};
use utils::{count_newlines, mk_sp};

/// Returns attributes on the given statement.
Expand Down Expand Up @@ -200,17 +201,19 @@ fn allow_mixed_tactic_for_nested_metaitem_list(list: &[ast::NestedMetaItem]) ->
impl Rewrite for ast::MetaItem {
fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
Some(match self.node {
ast::MetaItemKind::Word => String::from(&*self.name().as_str()),
ast::MetaItemKind::Word => {
rewrite_path(context, PathContext::Type, None, &self.ident, shape)?
}
ast::MetaItemKind::List(ref list) => {
let name = self.name().as_str();
let path = rewrite_path(context, PathContext::Type, None, &self.ident, shape)?;
let item_shape = match context.config.indent_style() {
IndentStyle::Block => shape
.block_indent(context.config.tab_spaces())
.with_max_width(context.config),
// 1 = `(`, 2 = `]` and `)`
IndentStyle::Visual => shape
.visual_indent(0)
.shrink_left(name.len() + 1)
.shrink_left(path.len() + 1)
.and_then(|s| s.sub_width(2))?,
};
let items = itemize_list(
Expand Down Expand Up @@ -248,21 +251,21 @@ impl Rewrite for ast::MetaItem {
};
let item_str = write_list(&item_vec, &fmt)?;
// 3 = "()" and "]"
let one_line_budget = shape.offset_left(name.len())?.sub_width(3)?.width;
let one_line_budget = shape.offset_left(path.len())?.sub_width(3)?.width;
if context.config.indent_style() == IndentStyle::Visual
|| (!item_str.contains('\n') && item_str.len() <= one_line_budget)
{
format!("{}({})", name, item_str)
format!("{}({})", path, item_str)
} else {
let indent = shape.indent.to_string_with_newline(context.config);
let nested_indent = item_shape.indent.to_string_with_newline(context.config);
format!("{}({}{}{})", name, nested_indent, item_str, indent)
format!("{}({}{}{})", path, nested_indent, item_str, indent)
}
}
ast::MetaItemKind::NameValue(ref literal) => {
let name = self.name().as_str();
let path = rewrite_path(context, PathContext::Type, None, &self.ident, shape)?;
// 3 = ` = `
let lit_shape = shape.shrink_left(name.len() + 3)?;
let lit_shape = shape.shrink_left(path.len() + 3)?;
// `rewrite_literal` returns `None` when `literal` exceeds max
// width. Since a literal is basically unformattable unless it
// is a string literal (and only if `format_strings` is set),
Expand All @@ -271,7 +274,7 @@ impl Rewrite for ast::MetaItem {
// See #2479 for example.
let value = rewrite_literal(context, literal, lit_shape)
.unwrap_or_else(|| context.snippet(literal.span).to_owned());
format!("{} = {}", name, value)
format!("{} = {}", path, value)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,7 @@ mod test {
}

#[test]
#[cfg_attr(rustfmt, rustfmt_skip)]
#[rustfmt::skip]
fn format_comments() {
let mut config: ::config::Config = Default::default();
config.set().wrap_comments(true);
Expand Down
86 changes: 59 additions & 27 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(custom_attribute)]
#![feature(tool_attributes)]
#![feature(decl_macro)]
// FIXME(cramertj) remove after match_default_bindings merges
#![allow(stable_features)]
#![allow(unused_attributes)]
#![feature(match_default_bindings)]
#![feature(type_ascription)]
#![feature(unicode_internals)]

Expand All @@ -40,6 +37,7 @@ extern crate term;
extern crate toml;
extern crate unicode_segmentation;

use std::cell::RefCell;
use std::collections::HashMap;
use std::fmt;
use std::io::{self, stdout, Write};
Expand All @@ -50,7 +48,7 @@ use std::time::Duration;

use syntax::ast;
pub use syntax::codemap::FileName;
use syntax::codemap::{CodeMap, FilePathMapping};
use syntax::codemap::{CodeMap, FilePathMapping, Span};
use syntax::errors::emitter::{ColorConfig, EmitterWriter};
use syntax::errors::{DiagnosticBuilder, Handler};
use syntax::parse::{self, ParseSess};
Expand Down Expand Up @@ -128,9 +126,14 @@ pub enum ErrorKind {
// License check has failed
#[fail(display = "license check failed")]
LicenseCheck,
// Used deprecated skip attribute
#[fail(display = "`rustfmt_skip` is deprecated; use `rustfmt::skip`")]
DeprecatedAttr,
// Used a rustfmt:: attribute other than skip
#[fail(display = "invalid attribute")]
BadAttr,
}

// Formatting errors that are identified *after* rustfmt has run.
struct FormattingError {
line: usize,
kind: ErrorKind,
Expand All @@ -140,11 +143,28 @@ struct FormattingError {
}

impl FormattingError {
fn from_span(span: &Span, codemap: &CodeMap, kind: ErrorKind) -> FormattingError {
FormattingError {
line: codemap.lookup_char_pos(span.lo()).line,
kind,
is_comment: false,
is_string: false,
line_buffer: codemap
.span_to_lines(*span)
.ok()
.and_then(|fl| {
fl.file
.get_line(fl.lines[0].line_index)
.map(|l| l.into_owned())
})
.unwrap_or_else(|| String::new()),
}
}
fn msg_prefix(&self) -> &str {
match self.kind {
ErrorKind::LineOverflow(..) | ErrorKind::TrailingWhitespace => "internal error:",
ErrorKind::LicenseCheck => "error:",
ErrorKind::BadIssue(_) => "warning:",
ErrorKind::LicenseCheck | ErrorKind::BadAttr => "error:",
ErrorKind::BadIssue(_) | ErrorKind::DeprecatedAttr => "warning:",
}
}

Expand All @@ -161,7 +181,7 @@ impl FormattingError {
fn format_len(&self) -> (usize, usize) {
match self.kind {
ErrorKind::LineOverflow(found, max) => (max, found - max),
ErrorKind::TrailingWhitespace => {
ErrorKind::TrailingWhitespace | ErrorKind::DeprecatedAttr | ErrorKind::BadAttr => {
let trailing_ws_start = self
.line_buffer
.rfind(|c: char| !c.is_whitespace())
Expand All @@ -177,20 +197,30 @@ impl FormattingError {
}
}

#[derive(Clone)]
pub struct FormatReport {
// Maps stringified file paths to their associated formatting errors.
file_error_map: HashMap<FileName, Vec<FormattingError>>,
file_error_map: Rc<RefCell<HashMap<FileName, Vec<FormattingError>>>>,
}

impl FormatReport {
fn new() -> FormatReport {
FormatReport {
file_error_map: HashMap::new(),
file_error_map: Rc::new(RefCell::new(HashMap::new())),
}
}

fn append(&self, f: FileName, mut v: Vec<FormattingError>) {
self.file_error_map
.borrow_mut()
.entry(f)
.and_modify(|fe| fe.append(&mut v))
.or_insert(v);
}

fn warning_count(&self) -> usize {
self.file_error_map
.borrow()
.iter()
.map(|(_, errors)| errors.len())
.sum()
Expand All @@ -204,7 +234,7 @@ impl FormatReport {
&self,
mut t: Box<term::Terminal<Output = io::Stderr>>,
) -> Result<(), term::Error> {
for (file, errors) in &self.file_error_map {
for (file, errors) in &*self.file_error_map.borrow() {
for error in errors {
let prefix_space_len = error.line.to_string().len();
let prefix_spaces = " ".repeat(1 + prefix_space_len);
Expand Down Expand Up @@ -250,7 +280,7 @@ impl FormatReport {
}
}

if !self.file_error_map.is_empty() {
if !self.file_error_map.borrow().is_empty() {
t.attr(term::Attr::Bold)?;
write!(t, "warning: ")?;
t.reset()?;
Expand All @@ -274,7 +304,7 @@ fn target_str(space_len: usize, target_len: usize) -> String {
impl fmt::Display for FormatReport {
// Prints all the formatting errors.
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
for (file, errors) in &self.file_error_map {
for (file, errors) in &*self.file_error_map.borrow() {
for error in errors {
let prefix_space_len = error.line.to_string().len();
let prefix_spaces = " ".repeat(1 + prefix_space_len);
Expand Down Expand Up @@ -313,7 +343,7 @@ impl fmt::Display for FormatReport {
)?;
}
}
if !self.file_error_map.is_empty() {
if !self.file_error_map.borrow().is_empty() {
writeln!(
fmt,
"warning: rustfmt may have failed to format. See previous {} errors.",
Expand All @@ -339,10 +369,11 @@ fn format_ast<F>(
parse_session: &mut ParseSess,
main_file: &FileName,
config: &Config,
report: FormatReport,
mut after_file: F,
) -> Result<(FileMap, bool), io::Error>
where
F: FnMut(&FileName, &mut String, &[(usize, usize)]) -> Result<bool, io::Error>,
F: FnMut(&FileName, &mut String, &[(usize, usize)], &FormatReport) -> Result<bool, io::Error>,
{
let mut result = FileMap::new();
// diff mode: check if any files are differing
Expand All @@ -360,7 +391,8 @@ where
.file;
let big_snippet = filemap.src.as_ref().unwrap();
let snippet_provider = SnippetProvider::new(filemap.start_pos, big_snippet);
let mut visitor = FmtVisitor::from_codemap(parse_session, config, &snippet_provider);
let mut visitor =
FmtVisitor::from_codemap(parse_session, config, &snippet_provider, report.clone());
// Format inner attributes if available.
if !krate.attrs.is_empty() && path == *main_file {
visitor.skip_empty_lines(filemap.end_pos);
Expand All @@ -380,8 +412,7 @@ where
::utils::count_newlines(&visitor.buffer)
);

let filename = path.clone();
has_diff |= match after_file(&filename, &mut visitor.buffer, &visitor.skipped_range) {
has_diff |= match after_file(&path, &mut visitor.buffer, &visitor.skipped_range, &report) {
Ok(result) => result,
Err(e) => {
// Create a new error with path_str to help users see which files failed
Expand All @@ -390,13 +421,13 @@ where
}
};

result.push((filename, visitor.buffer));
result.push((path.clone(), visitor.buffer));
}

Ok((result, has_diff))
}

/// Returns true if the line with the given line number was skipped by `#[rustfmt_skip]`.
/// 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()
Expand Down Expand Up @@ -429,7 +460,7 @@ fn format_lines(
name: &FileName,
skipped_range: &[(usize, usize)],
config: &Config,
report: &mut FormatReport,
report: &FormatReport,
) {
let mut trims = vec![];
let mut last_wspace: Option<usize> = None;
Expand Down Expand Up @@ -543,7 +574,7 @@ fn format_lines(
}
}

report.file_error_map.insert(name.clone(), errors);
report.append(name.clone(), errors);
}

fn parse_input<'sess>(
Expand Down Expand Up @@ -760,19 +791,20 @@ fn format_input_inner<T: Write>(
));
parse_session.span_diagnostic = Handler::with_emitter(true, false, silent_emitter);

let mut report = FormatReport::new();
let report = FormatReport::new();

let format_result = format_ast(
&krate,
&mut parse_session,
&main_file,
config,
|file_name, file, skipped_range| {
report.clone(),
|file_name, file, skipped_range, report| {
// 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, skipped_range, config, &mut report);
format_lines(file, file_name, skipped_range, config, report);

if let Some(ref mut out) = out {
return filemap::write_file(file, file_name, out, config);
Expand Down Expand Up @@ -975,7 +1007,7 @@ mod unit_tests {

#[test]
fn test_format_code_block_fail() {
#[rustfmt_skip]
#[rustfmt::skip]
let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);";
assert!(format_code_block(code_block, &Config::default()).is_none());
}
Expand Down
2 changes: 2 additions & 0 deletions src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use syntax::parse::ParseSess;
use config::{Config, IndentStyle};
use shape::Shape;
use visitor::SnippetProvider;
use FormatReport;

use std::cell::RefCell;

Expand All @@ -38,6 +39,7 @@ pub struct RewriteContext<'a> {
// When rewriting chain, veto going multi line except the last element
pub force_one_line_chain: RefCell<bool>,
pub snippet_provider: &'a SnippetProvider<'a>,
pub report: FormatReport,
}

impl<'a> RewriteContext<'a> {
Expand Down
Loading