Merge pull request #2703 from nrc/attrs

Use scoped attributes for skip attribute

Author: nrc

.travis.yml

@@ -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:
@@ -47,6 +45,8 @@ matrix:
     - env: INTEGRATION=futures-rs
     - env: INTEGRATION=log
     - env: INTEGRATION=rand
+    - env: INTEGRATION=glob
+    - env: INTEGRATION=tempdir
 
 before_script:
 - |

Configurations.md

@@ -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");
@@ -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");

README.md

@@ -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

rustfmt.toml

@@ -0,0 +1,2 @@
+error_on_line_overflow = true
+error_on_unformatted = true

src/attr.rs

@@ -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.
@@ -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(
@@ -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),
@@ -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)
             }
         })
     }

src/comment.rs

@@ -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);

src/lib.rs

@@ -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)]
 
@@ -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};
@@ -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};
@@ -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,
@@ -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:",
         }
     }
 
@@ -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())
@@ -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()
@@ -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);
@@ -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()?;
@@ -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);
@@ -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.",
@@ -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
@@ -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);
@@ -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
@@ -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()
@@ -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;
@@ -543,7 +574,7 @@ fn format_lines(
         }
     }
 
-    report.file_error_map.insert(name.clone(), errors);
+    report.append(name.clone(), errors);
 }
 
 fn parse_input<'sess>(
@@ -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);
@@ -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());
     }

src/rewrite.rs

@@ -16,6 +16,7 @@ use syntax::parse::ParseSess;
 use config::{Config, IndentStyle};
 use shape::Shape;
 use visitor::SnippetProvider;
+use FormatReport;
 
 use std::cell::RefCell;
 
@@ -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> {