Use scoped attributes for skip attribute

src/lib.rs

@@ -37,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};
@@ -47,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};
@@ -125,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,
@@ -137,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:",
         }
     }
 
@@ -158,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())
@@ -174,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()
@@ -201,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);
@@ -247,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()?;
@@ -271,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);
@@ -310,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.",
@@ -336,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
@@ -357,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);
@@ -377,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
@@ -387,7 +421,7 @@ where
             }
         };
 
-        result.push((filename, visitor.buffer));
+        result.push((path.clone(), visitor.buffer));
     }
 
     Ok((result, has_diff))
@@ -426,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;
@@ -540,7 +574,7 @@ fn format_lines(
         }
     }
 
-    report.file_error_map.insert(name.clone(), errors);
+    report.append(name.clone(), errors);
 }
 
 fn parse_input<'sess>(
@@ -757,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);

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

src/utils.rs

@@ -22,8 +22,8 @@ use config::Color;
 use rewrite::RewriteContext;
 use shape::Shape;
 
-const DEPR_SKIP_ANNOTATION: &str = "rustfmt_skip";
-const SKIP_ANNOTATION: &str = "rustfmt::skip";
+pub const DEPR_SKIP_ANNOTATION: &str = "rustfmt_skip";
+pub const SKIP_ANNOTATION: &str = "rustfmt::skip";
 
 // Computes the length of a string's last line, minus offset.
 pub fn extra_offset(text: &str, shape: Shape) -> usize {

src/visitor.rs

@@ -26,7 +26,11 @@ use macros::{rewrite_macro, rewrite_macro_def, MacroPosition};
 use rewrite::{Rewrite, RewriteContext};
 use shape::{Indent, Shape};
 use spanned::Spanned;
-use utils::{self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec};
+use utils::{
+    self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec,
+    DEPR_SKIP_ANNOTATION,
+};
+use {ErrorKind, FormatReport, FormattingError};
 
 use std::cell::RefCell;
 
@@ -66,6 +70,7 @@ pub struct FmtVisitor<'a> {
     pub snippet_provider: &'a SnippetProvider<'a>,
     pub line_number: usize,
     pub skipped_range: Vec<(usize, usize)>,
+    pub report: FormatReport,
 }
 
 impl<'b, 'a: 'b> FmtVisitor<'a> {
@@ -552,13 +557,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
     }
 
     pub fn from_context(ctx: &'a RewriteContext) -> FmtVisitor<'a> {
-        FmtVisitor::from_codemap(ctx.parse_session, ctx.config, ctx.snippet_provider)
+        FmtVisitor::from_codemap(
+            ctx.parse_session,
+            ctx.config,
+            ctx.snippet_provider,
+            ctx.report.clone(),
+        )
     }
 
     pub fn from_codemap(
         parse_session: &'a ParseSess,
         config: &'a Config,
         snippet_provider: &'a SnippetProvider,
+        report: FormatReport,
     ) -> FmtVisitor<'a> {
         FmtVisitor {
             parse_session,
@@ -571,6 +582,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             snippet_provider,
             line_number: 0,
             skipped_range: vec![],
+            report,
         }
     }
 
@@ -584,6 +596,33 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
     // Returns true if we should skip the following item.
     pub fn visit_attrs(&mut self, attrs: &[ast::Attribute], style: ast::AttrStyle) -> bool {
+        for attr in attrs {
+            if attr.name() == DEPR_SKIP_ANNOTATION {
+                let file_name = self.codemap.span_to_filename(attr.span);
+                self.report.append(
+                    file_name,
+                    vec![FormattingError::from_span(
+                        &attr.span,
+                        &self.codemap,
+                        ErrorKind::DeprecatedAttr,
+                    )],
+                );
+            } else if attr.path.segments[0].ident.to_string() == "rustfmt" {
+                if attr.path.segments.len() == 1
+                    || attr.path.segments[1].ident.to_string() != "skip"
+                {
+                    let file_name = self.codemap.span_to_filename(attr.span);
+                    self.report.append(
+                        file_name,
+                        vec![FormattingError::from_span(
+                            &attr.span,
+                            &self.codemap,
+                            ErrorKind::BadAttr,
+                        )],
+                    );
+                }
+            }
+        }
         if contains_skip(attrs) {
             return true;
         }
@@ -711,6 +750,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             is_if_else_block: RefCell::new(false),
             force_one_line_chain: RefCell::new(false),
             snippet_provider: self.snippet_provider,
+            report: self.report.clone(),
         }
     }
 }