rust-lang#3665: Implemented (rust-lang#3689)

rust-lang-ci · Jul 17, 2019 · 76e2ba2 · 76e2ba2
1 parent 66c27c9
commit 76e2ba2
Show file tree

Hide file tree

Showing 17 changed files with 207 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+### Added
+
+- Add new attribute `rustfmt::skip::attributes` to prevent rustfmt 
+from formatting an attribute #3665
+
 ## [1.3.3] 2019-07-15
 
 ### Added

diff --git a/README.md b/README.md
@@ -179,12 +179,16 @@ needs to be specified in `rustfmt.toml`, e.g., with `edition = "2018"`.
 ## Tips
 
 * For things you do not want rustfmt to mangle, use `#[rustfmt::skip]`
-* To prevent rustfmt from formatting a macro,
-  use `#[rustfmt::skip::macros(target_macro_name)]`
+* To prevent rustfmt from formatting a macro or an attribute,
+  use `#[rustfmt::skip::macros(target_macro_name)]` or 
+  `#[rustfmt::skip::attributes(target_attribute_name)]`
 
   Example:
 
     ```rust
+    #![rustfmt::skip::attributes(custom_attribute)]   
+
+    #[custom_attribute(formatting , here , should , be , Skipped)]
     #[rustfmt::skip::macros(html)]
     fn main() {
         let macro_result1 = html! { <div>

diff --git a/src/attr.rs b/src/attr.rs
@@ -319,9 +319,13 @@ impl Rewrite for ast::Attribute {
         if self.is_sugared_doc {
             rewrite_doc_comment(snippet, shape.comment(context.config), context.config)
         } else {
+            let should_skip = self
+                .ident()
+                .map(|s| context.skip_context.skip_attribute(&s.name.as_str()))
+                .unwrap_or(false);
             let prefix = attr_prefix(self);
 
-            if contains_comment(snippet) {
+            if should_skip || contains_comment(snippet) {
                 return Some(snippet.to_owned());
             }
 

diff --git a/src/formatting.rs b/src/formatting.rs
@@ -17,7 +17,7 @@ use crate::comment::{CharClasses, FullCodeCharKind};
 use crate::config::{Config, FileName, Verbosity};
 use crate::ignore_path::IgnorePathSet;
 use crate::issues::BadIssueSeeker;
-use crate::utils::{count_newlines, get_skip_macro_names};
+use crate::utils::count_newlines;
 use crate::visitor::{FmtVisitor, SnippetProvider};
 use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session};
 
@@ -158,10 +158,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
             &snippet_provider,
             self.report.clone(),
         );
-        visitor
-            .skip_macro_names
-            .borrow_mut()
-            .append(&mut get_skip_macro_names(&self.krate.attrs));
+        visitor.skip_context.update_with_attrs(&self.krate.attrs);
 
         // Format inner attributes if available.
         if !self.krate.attrs.is_empty() && is_root {

diff --git a/src/lib.rs b/src/lib.rs
@@ -69,6 +69,7 @@ mod reorder;
 mod rewrite;
 pub(crate) mod rustfmt_diff;
 mod shape;
+mod skip;
 pub(crate) mod source_file;
 pub(crate) mod source_map;
 mod spanned;

diff --git a/src/macros.rs b/src/macros.rs
@@ -211,9 +211,8 @@ pub(crate) fn rewrite_macro(
     position: MacroPosition,
 ) -> Option<String> {
     let should_skip = context
-        .skip_macro_names
-        .borrow()
-        .contains(&context.snippet(mac.node.path.span).to_owned());
+        .skip_context
+        .skip_macro(&context.snippet(mac.node.path.span).to_owned());
     if should_skip {
         None
     } else {

diff --git a/src/rewrite.rs b/src/rewrite.rs
@@ -8,6 +8,7 @@ use syntax::source_map::{SourceMap, Span};
 
 use crate::config::{Config, IndentStyle};
 use crate::shape::Shape;
+use crate::skip::SkipContext;
 use crate::visitor::SnippetProvider;
 use crate::FormatReport;
 
@@ -39,7 +40,7 @@ pub(crate) struct RewriteContext<'a> {
     // Used for `format_snippet`
     pub(crate) macro_rewrite_failure: RefCell<bool>,
     pub(crate) report: FormatReport,
-    pub(crate) skip_macro_names: RefCell<Vec<String>>,
+    pub(crate) skip_context: SkipContext,
 }
 
 impl<'a> RewriteContext<'a> {

diff --git a/src/skip.rs b/src/skip.rs
@@ -0,0 +1,73 @@
+//! Module that contains skip related stuffs.
+
+use syntax::ast;
+
+/// Take care of skip name stack. You can update it by attributes slice or
+/// by other context. Query this context to know if you need skip a block.
+#[derive(Default, Clone)]
+pub(crate) struct SkipContext {
+    macros: Vec<String>,
+    attributes: Vec<String>,
+}
+
+impl SkipContext {
+    pub(crate) fn update_with_attrs(&mut self, attrs: &[ast::Attribute]) {
+        self.macros.append(&mut get_skip_names("macros", attrs));
+        self.attributes
+            .append(&mut get_skip_names("attributes", attrs));
+    }
+
+    pub(crate) fn update(&mut self, mut other: SkipContext) {
+        self.macros.append(&mut other.macros);
+        self.attributes.append(&mut other.attributes);
+    }
+
+    pub(crate) fn skip_macro(&self, name: &str) -> bool {
+        self.macros.iter().any(|n| n == name)
+    }
+
+    pub(crate) fn skip_attribute(&self, name: &str) -> bool {
+        self.attributes.iter().any(|n| n == name)
+    }
+}
+
+static RUSTFMT: &'static str = "rustfmt";
+static SKIP: &'static str = "skip";
+
+/// Say if you're playing with `rustfmt`'s skip attribute
+pub(crate) fn is_skip_attr(segments: &[ast::PathSegment]) -> bool {
+    if segments.len() < 2 || segments[0].ident.to_string() != RUSTFMT {
+        return false;
+    }
+    match segments.len() {
+        2 => segments[1].ident.to_string() == SKIP,
+        3 => {
+            segments[1].ident.to_string() == SKIP
+                && ["macros", "attributes"]
+                    .iter()
+                    .any(|&n| n == &segments[2].ident.name.as_str())
+        }
+        _ => false,
+    }
+}
+
+fn get_skip_names(kind: &str, attrs: &[ast::Attribute]) -> Vec<String> {
+    let mut skip_names = vec![];
+    let path = format!("{}::{}::{}", RUSTFMT, SKIP, kind);
+    for attr in attrs {
+        // syntax::ast::Path is implemented partialEq
+        // but it is designed for segments.len() == 1
+        if format!("{}", attr.path) != path {
+            continue;
+        }
+
+        if let Some(list) = attr.meta_item_list() {
+            for nested_meta_item in list {
+                if let Some(name) = nested_meta_item.ident() {
+                    skip_names.push(name.to_string());
+                }
+            }
+        }
+    }
+    skip_names
+}
diff --git a/src/test/mod.rs b/src/test/mod.rs
@@ -26,6 +26,7 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[
     // so we do not want to test this file directly.
     "configs/skip_children/foo/mod.rs",
     "issue-3434/no_entry.rs",
+    "issue-3665/sub_mod.rs",
     // These files and directory are a part of modules defined inside `cfg_if!`.
     "cfg_if/mod.rs",
     "cfg_if/detect",

diff --git a/src/utils.rs b/src/utils.rs
@@ -638,26 +638,6 @@ pub(crate) fn unicode_str_width(s: &str) -> usize {
     s.width()
 }
 
-pub(crate) fn get_skip_macro_names(attrs: &[ast::Attribute]) -> Vec<String> {
-    let mut skip_macro_names = vec![];
-    for attr in attrs {
-        // syntax::ast::Path is implemented partialEq
-        // but it is designed for segments.len() == 1
-        if format!("{}", attr.path) != "rustfmt::skip::macros" {
-            continue;
-        }
-
-        if let Some(list) = attr.meta_item_list() {
-            for nested_meta_item in list {
-                if let Some(name) = nested_meta_item.ident() {
-                    skip_macro_names.push(name.to_string());
-                }
-            }
-        }
-    }
-    skip_macro_names
-}
-
 #[cfg(test)]
 mod test {
     use super::*;

diff --git a/src/visitor.rs b/src/visitor.rs
@@ -17,12 +17,13 @@ use crate::items::{
 use crate::macros::{rewrite_macro, rewrite_macro_def, MacroPosition};
 use crate::rewrite::{Rewrite, RewriteContext};
 use crate::shape::{Indent, Shape};
+use crate::skip::{is_skip_attr, SkipContext};
 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, get_skip_macro_names,
-    inner_attributes, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, stmt_expr,
+    self, contains_skip, count_newlines, depr_skip_annotation, inner_attributes, mk_sp,
+    ptr_vec_to_ref_vec, rewrite_ident, stmt_expr,
 };
 use crate::{ErrorKind, FormatReport, FormattingError};
 
@@ -67,7 +68,7 @@ pub(crate) struct FmtVisitor<'a> {
     pub(crate) skipped_range: Vec<(usize, usize)>,
     pub(crate) macro_rewrite_failure: bool,
     pub(crate) report: FormatReport,
-    pub(crate) skip_macro_names: RefCell<Vec<String>>,
+    pub(crate) skip_context: SkipContext,
 }
 
 impl<'a> Drop for FmtVisitor<'a> {
@@ -347,10 +348,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         // the AST lumps them all together.
         let filtered_attrs;
         let mut attrs = &item.attrs;
-        let temp_skip_macro_names = self.skip_macro_names.clone();
-        self.skip_macro_names
-            .borrow_mut()
-            .append(&mut get_skip_macro_names(&attrs));
+        let skip_context_saved = self.skip_context.clone();
+        self.skip_context.update_with_attrs(&attrs);
 
         let should_visit_node_again = match item.node {
             // For use/extern crate items, skip rewriting attributes but check for a skip attribute.
@@ -501,7 +500,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 }
             };
         }
-        self.skip_macro_names = temp_skip_macro_names;
+        self.skip_context = skip_context_saved;
     }
 
     pub(crate) fn visit_trait_item(&mut self, ti: &ast::TraitItem) {
@@ -656,10 +655,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             ctx.snippet_provider,
             ctx.report.clone(),
         );
-        visitor
-            .skip_macro_names
-            .borrow_mut()
-            .append(&mut ctx.skip_macro_names.borrow().clone());
+        visitor.skip_context.update(ctx.skip_context.clone());
         visitor.set_parent_context(ctx);
         visitor
     }
@@ -684,7 +680,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             skipped_range: vec![],
             macro_rewrite_failure: false,
             report,
-            skip_macro_names: RefCell::new(vec![]),
+            skip_context: Default::default(),
         }
     }
 
@@ -741,14 +737,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         if segments[0].ident.to_string() != "rustfmt" {
             return false;
         }
-
-        match segments.len() {
-            2 => segments[1].ident.to_string() != "skip",
-            3 => {
-                segments[1].ident.to_string() != "skip" || segments[2].ident.to_string() != "macros"
-            }
-            _ => false,
-        }
+        !is_skip_attr(segments)
     }
 
     fn walk_mod_items(&mut self, m: &ast::Mod) {
@@ -881,7 +870,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             snippet_provider: self.snippet_provider,
             macro_rewrite_failure: RefCell::new(false),
             report: self.report.clone(),
-            skip_macro_names: self.skip_macro_names.clone(),
+            skip_context: self.skip_context.clone(),
         }
     }
 }
diff --git a/tests/source/issue-3665/lib.rs b/tests/source/issue-3665/lib.rs
@@ -0,0 +1,33 @@
+#![rustfmt::skip::attributes(skip_mod_attr)]
+
+mod sub_mod;
+
+#[rustfmt::skip::attributes(other, skip_attr)]
+fn main() {
+    #[other(should,
+skip,
+        this,                               format)]
+    struct S {}
+
+    #[skip_attr(should, skip,
+this,                               format,too)]
+    fn doesnt_mater() {}
+
+    #[skip_mod_attr(should, skip,
+this,                               format,
+         enerywhere)]
+    fn more() {}
+
+    #[not_skip(not,
+skip,    me)]
+    struct B {}
+}
+
+#[other(should, not, skip,
+this,                               format, here)]
+fn foo() {}
+
+#[skip_mod_attr(should, skip,
+this,                               format,in,                    master,
+                    and, sub, module)]
+fn bar() {}
diff --git a/tests/source/issue-3665/not_skip_attribute.rs b/tests/source/issue-3665/not_skip_attribute.rs
@@ -0,0 +1,4 @@
+#![this::is::not::skip::attribute(ouch)]
+
+#[ouch(not,      skip,  me)]
+fn main() {}
diff --git a/tests/source/issue-3665/sub_mod.rs b/tests/source/issue-3665/sub_mod.rs
@@ -0,0 +1,14 @@
+#[rustfmt::skip::attributes(more_skip)]
+#[more_skip(should,
+      skip,
+this,                               format)]
+fn foo() {}
+
+#[skip_mod_attr(should, skip,
+this,                               format,in,                    master,
+                    and, sub, module)]
+fn bar() {}
+
+#[skip_attr(should,  not,
+  skip,      this, attribute,    here)]
+fn baz() {}
diff --git a/tests/target/issue-3665/lib.rs b/tests/target/issue-3665/lib.rs
@@ -0,0 +1,31 @@
+#![rustfmt::skip::attributes(skip_mod_attr)]
+
+mod sub_mod;
+
+#[rustfmt::skip::attributes(other, skip_attr)]
+fn main() {
+    #[other(should,
+skip,
+        this,                               format)]
+    struct S {}
+
+    #[skip_attr(should, skip,
+this,                               format,too)]
+    fn doesnt_mater() {}
+
+    #[skip_mod_attr(should, skip,
+this,                               format,
+         enerywhere)]
+    fn more() {}
+
+    #[not_skip(not, skip, me)]
+    struct B {}
+}
+
+#[other(should, not, skip, this, format, here)]
+fn foo() {}
+
+#[skip_mod_attr(should, skip,
+this,                               format,in,                    master,
+                    and, sub, module)]
+fn bar() {}
diff --git a/tests/target/issue-3665/not_skip_attribute.rs b/tests/target/issue-3665/not_skip_attribute.rs
@@ -0,0 +1,4 @@
+#![this::is::not::skip::attribute(ouch)]
+
+#[ouch(not, skip, me)]
+fn main() {}