Merge pull request #3454 from rchaser53/issue-3434

add new attribute rustfmt::skip::macros

README.md

@@ -179,6 +179,18 @@ 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)]`
+
+  Example:
+
+    ```rust
+    #[rustfmt::skip::macros(html)]
+    fn main() {
+        let macro_result1 = html! { <div>
+    Hello</div>
+        }.to_string();
+    ```
 * When you run rustfmt, place a file named `rustfmt.toml` or `.rustfmt.toml` in
   target file directory or its parents to override the default settings of
   rustfmt. You can generate a file containing the default configuration with

src/formatting.rs

@@ -15,6 +15,7 @@ use syntax::source_map::{FilePathMapping, SourceMap, Span, DUMMY_SP};
 use crate::comment::{CharClasses, FullCodeCharKind};
 use crate::config::{Config, FileName, Verbosity};
 use crate::issues::BadIssueSeeker;
+use crate::utils::{count_newlines, get_skip_macro_names};
 use crate::visitor::{FmtVisitor, SnippetProvider};
 use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session};
 
@@ -153,6 +154,10 @@ 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));
 
         // Format inner attributes if available.
         if !self.krate.attrs.is_empty() && is_root {
@@ -168,10 +173,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
             visitor.format_separate_mod(module, &*source_file);
         };
 
-        debug_assert_eq!(
-            visitor.line_number,
-            crate::utils::count_newlines(&visitor.buffer)
-        );
+        debug_assert_eq!(visitor.line_number, count_newlines(&visitor.buffer));
 
         // For some reason, the source_map does not include terminating
         // newlines so we must add one on for each file. This is sad.

src/lib.rs

@@ -94,7 +94,7 @@ pub enum ErrorKind {
     /// Used deprecated skip attribute.
     #[fail(display = "`rustfmt_skip` is deprecated; use `rustfmt::skip`")]
     DeprecatedAttr,
-    /// Used a rustfmt:: attribute other than skip.
+    /// Used a rustfmt:: attribute other than skip or skip::macros.
     #[fail(display = "invalid attribute")]
     BadAttr,
     /// An io error during reading or writing.

src/macros.rs

@@ -208,12 +208,21 @@ pub fn rewrite_macro(
     shape: Shape,
     position: MacroPosition,
 ) -> Option<String> {
-    let guard = InsideMacroGuard::inside_macro_context(context);
-    let result = rewrite_macro_inner(mac, extra_ident, context, shape, position, guard.is_nested);
-    if result.is_none() {
-        context.macro_rewrite_failure.replace(true);
+    let should_skip = context
+        .skip_macro_names
+        .borrow()
+        .contains(&context.snippet(mac.node.path.span).to_owned());
+    if should_skip {
+        None
+    } else {
+        let guard = InsideMacroGuard::inside_macro_context(context);
+        let result =
+            rewrite_macro_inner(mac, extra_ident, context, shape, position, guard.is_nested);
+        if result.is_none() {
+            context.macro_rewrite_failure.replace(true);
+        }
+        result
     }
-    result
 }
 
 fn check_keyword<'a, 'b: 'a>(parser: &'a mut Parser<'b>) -> Option<MacroArg> {

src/rewrite.rs

@@ -39,6 +39,7 @@ pub struct RewriteContext<'a> {
     // Used for `format_snippet`
     pub(crate) macro_rewrite_failure: RefCell<bool>,
     pub(crate) report: FormatReport,
+    pub skip_macro_names: RefCell<Vec<String>>,
 }
 
 impl<'a> RewriteContext<'a> {

src/test/mod.rs

@@ -22,6 +22,7 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[
     // We want to make sure that the `skip_children` is correctly working,
     // so we do not want to test this file directly.
     "configs/skip_children/foo/mod.rs",
+    "issue-3434/no_entry.rs",
 ];
 
 fn is_file_skip(path: &Path) -> bool {

src/utils.rs

@@ -603,6 +603,26 @@ pub(crate) fn unicode_str_width(s: &str) -> usize {
     s.width()
 }
 
+pub 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 spanned in list {
+                if let Some(name) = spanned.name() {
+                    skip_macro_names.push(name.to_string());
+                }
+            }
+        }
+    }
+    skip_macro_names
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

src/visitor.rs

@@ -20,8 +20,8 @@ use crate::shape::{Indent, Shape};
 use crate::source_map::{LineRangeUtils, SpanUtils};
 use crate::spanned::Spanned;
 use crate::utils::{
-    self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec,
-    rewrite_ident, stmt_expr, DEPR_SKIP_ANNOTATION,
+    self, contains_skip, count_newlines, get_skip_macro_names, inner_attributes, mk_sp,
+    ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, DEPR_SKIP_ANNOTATION,
 };
 use crate::{ErrorKind, FormatReport, FormattingError};
 
@@ -66,6 +66,7 @@ pub struct FmtVisitor<'a> {
     pub skipped_range: Vec<(usize, usize)>,
     pub macro_rewrite_failure: bool,
     pub(crate) report: FormatReport,
+    pub skip_macro_names: RefCell<Vec<String>>,
 }
 
 impl<'a> Drop for FmtVisitor<'a> {
@@ -296,25 +297,32 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         // the AST lumps them all together.
         let filtered_attrs;
         let mut attrs = &item.attrs;
-        match item.node {
+        let temp_skip_macro_names = self.skip_macro_names.clone();
+        self.skip_macro_names
+            .borrow_mut()
+            .append(&mut get_skip_macro_names(&attrs));
+
+        let should_visit_node_again = match item.node {
             // For use items, skip rewriting attributes. Just check for a skip attribute.
             ast::ItemKind::Use(..) => {
                 if contains_skip(attrs) {
                     self.push_skipped_with_span(attrs.as_slice(), item.span(), item.span());
-                    return;
+                    false
+                } else {
+                    true
                 }
             }
             // Module is inline, in this case we treat it like any other item.
             _ if !is_mod_decl(item) => {
                 if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
                     self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
-                    return;
+                    false
+                } else {
+                    true
                 }
             }
             // Module is not inline, but should be skipped.
-            ast::ItemKind::Mod(..) if contains_skip(&item.attrs) => {
-                return;
-            }
+            ast::ItemKind::Mod(..) if contains_skip(&item.attrs) => false,
             // Module is not inline and should not be skipped. We want
             // to process only the attributes in the current file.
             ast::ItemKind::Mod(..) => {
@@ -323,121 +331,127 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 // the above case.
                 assert!(!self.visit_attrs(&filtered_attrs, ast::AttrStyle::Outer));
                 attrs = &filtered_attrs;
+                true
             }
             _ => {
                 if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
                     self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
-                    return;
+                    false
+                } else {
+                    true
                 }
             }
-        }
+        };
 
-        match item.node {
-            ast::ItemKind::Use(ref tree) => self.format_import(item, tree),
-            ast::ItemKind::Impl(..) => {
-                let snippet = self.snippet(item.span);
-                let where_span_end = snippet
-                    .find_uncommented("{")
-                    .map(|x| BytePos(x as u32) + source!(self, item.span).lo());
-                let block_indent = self.block_indent;
-                let rw =
-                    self.with_context(|ctx| format_impl(&ctx, item, block_indent, where_span_end));
-                self.push_rewrite(item.span, rw);
-            }
-            ast::ItemKind::Trait(..) => {
-                let block_indent = self.block_indent;
-                let rw = self.with_context(|ctx| format_trait(&ctx, item, block_indent));
-                self.push_rewrite(item.span, rw);
-            }
-            ast::ItemKind::TraitAlias(ref generics, ref generic_bounds) => {
-                let shape = Shape::indented(self.block_indent, self.config);
-                let rw = format_trait_alias(
-                    &self.get_context(),
-                    item.ident,
-                    &item.vis,
-                    generics,
-                    generic_bounds,
-                    shape,
-                );
-                self.push_rewrite(item.span, rw);
-            }
-            ast::ItemKind::ExternCrate(_) => {
-                let rw = rewrite_extern_crate(&self.get_context(), item);
-                self.push_rewrite(item.span, rw);
-            }
-            ast::ItemKind::Struct(..) | ast::ItemKind::Union(..) => {
-                self.visit_struct(&StructParts::from_item(item));
-            }
-            ast::ItemKind::Enum(ref def, ref generics) => {
-                self.format_missing_with_indent(source!(self, item.span).lo());
-                self.visit_enum(item.ident, &item.vis, def, generics, item.span);
-                self.last_pos = source!(self, item.span).hi();
-            }
-            ast::ItemKind::Mod(ref module) => {
-                let is_inline = !is_mod_decl(item);
-                self.format_missing_with_indent(source!(self, item.span).lo());
-                self.format_mod(module, &item.vis, item.span, item.ident, attrs, is_inline);
-            }
-            ast::ItemKind::Mac(ref mac) => {
-                self.visit_mac(mac, Some(item.ident), MacroPosition::Item);
-            }
-            ast::ItemKind::ForeignMod(ref foreign_mod) => {
-                self.format_missing_with_indent(source!(self, item.span).lo());
-                self.format_foreign_mod(foreign_mod, item.span);
-            }
-            ast::ItemKind::Static(..) | ast::ItemKind::Const(..) => {
-                self.visit_static(&StaticParts::from_item(item));
-            }
-            ast::ItemKind::Fn(ref decl, ref fn_header, ref generics, ref body) => {
-                let inner_attrs = inner_attributes(&item.attrs);
-                self.visit_fn(
-                    visit::FnKind::ItemFn(item.ident, fn_header, &item.vis, body),
-                    generics,
-                    decl,
-                    item.span,
-                    ast::Defaultness::Final,
-                    Some(&inner_attrs),
-                )
-            }
-            ast::ItemKind::Ty(ref ty, ref generics) => {
-                let rewrite = rewrite_type_alias(
-                    &self.get_context(),
-                    self.block_indent,
-                    item.ident,
-                    ty,
-                    generics,
-                    &item.vis,
-                );
-                self.push_rewrite(item.span, rewrite);
-            }
-            ast::ItemKind::Existential(ref generic_bounds, ref generics) => {
-                let rewrite = rewrite_existential_type(
-                    &self.get_context(),
-                    self.block_indent,
-                    item.ident,
-                    generic_bounds,
-                    generics,
-                    &item.vis,
-                );
-                self.push_rewrite(item.span, rewrite);
-            }
-            ast::ItemKind::GlobalAsm(..) => {
-                let snippet = Some(self.snippet(item.span).to_owned());
-                self.push_rewrite(item.span, snippet);
-            }
-            ast::ItemKind::MacroDef(ref def) => {
-                let rewrite = rewrite_macro_def(
-                    &self.get_context(),
-                    self.shape(),
-                    self.block_indent,
-                    def,
-                    item.ident,
-                    &item.vis,
-                    item.span,
-                );
-                self.push_rewrite(item.span, rewrite);
-            }
+        if should_visit_node_again {
+            match item.node {
+                ast::ItemKind::Use(ref tree) => self.format_import(item, tree),
+                ast::ItemKind::Impl(..) => {
+                    let snippet = self.snippet(item.span);
+                    let where_span_end = snippet
+                        .find_uncommented("{")
+                        .map(|x| BytePos(x as u32) + source!(self, item.span).lo());
+                    let block_indent = self.block_indent;
+                    let rw = self
+                        .with_context(|ctx| format_impl(&ctx, item, block_indent, where_span_end));
+                    self.push_rewrite(item.span, rw);
+                }
+                ast::ItemKind::Trait(..) => {
+                    let block_indent = self.block_indent;
+                    let rw = self.with_context(|ctx| format_trait(&ctx, item, block_indent));
+                    self.push_rewrite(item.span, rw);
+                }
+                ast::ItemKind::TraitAlias(ref generics, ref generic_bounds) => {
+                    let shape = Shape::indented(self.block_indent, self.config);
+                    let rw = format_trait_alias(
+                        &self.get_context(),
+                        item.ident,
+                        &item.vis,
+                        generics,
+                        generic_bounds,
+                        shape,
+                    );
+                    self.push_rewrite(item.span, rw);
+                }
+                ast::ItemKind::ExternCrate(_) => {
+                    let rw = rewrite_extern_crate(&self.get_context(), item);
+                    self.push_rewrite(item.span, rw);
+                }
+                ast::ItemKind::Struct(..) | ast::ItemKind::Union(..) => {
+                    self.visit_struct(&StructParts::from_item(item));
+                }
+                ast::ItemKind::Enum(ref def, ref generics) => {
+                    self.format_missing_with_indent(source!(self, item.span).lo());
+                    self.visit_enum(item.ident, &item.vis, def, generics, item.span);
+                    self.last_pos = source!(self, item.span).hi();
+                }
+                ast::ItemKind::Mod(ref module) => {
+                    let is_inline = !is_mod_decl(item);
+                    self.format_missing_with_indent(source!(self, item.span).lo());
+                    self.format_mod(module, &item.vis, item.span, item.ident, attrs, is_inline);
+                }
+                ast::ItemKind::Mac(ref mac) => {
+                    self.visit_mac(mac, Some(item.ident), MacroPosition::Item);
+                }
+                ast::ItemKind::ForeignMod(ref foreign_mod) => {
+                    self.format_missing_with_indent(source!(self, item.span).lo());
+                    self.format_foreign_mod(foreign_mod, item.span);
+                }
+                ast::ItemKind::Static(..) | ast::ItemKind::Const(..) => {
+                    self.visit_static(&StaticParts::from_item(item));
+                }
+                ast::ItemKind::Fn(ref decl, ref fn_header, ref generics, ref body) => {
+                    let inner_attrs = inner_attributes(&item.attrs);
+                    self.visit_fn(
+                        visit::FnKind::ItemFn(item.ident, fn_header, &item.vis, body),
+                        generics,
+                        decl,
+                        item.span,
+                        ast::Defaultness::Final,
+                        Some(&inner_attrs),
+                    )
+                }
+                ast::ItemKind::Ty(ref ty, ref generics) => {
+                    let rewrite = rewrite_type_alias(
+                        &self.get_context(),
+                        self.block_indent,
+                        item.ident,
+                        ty,
+                        generics,
+                        &item.vis,
+                    );
+                    self.push_rewrite(item.span, rewrite);
+                }
+                ast::ItemKind::Existential(ref generic_bounds, ref generics) => {
+                    let rewrite = rewrite_existential_type(
+                        &self.get_context(),
+                        self.block_indent,
+                        item.ident,
+                        generic_bounds,
+                        generics,
+                        &item.vis,
+                    );
+                    self.push_rewrite(item.span, rewrite);
+                }
+                ast::ItemKind::GlobalAsm(..) => {
+                    let snippet = Some(self.snippet(item.span).to_owned());
+                    self.push_rewrite(item.span, snippet);
+                }
+                ast::ItemKind::MacroDef(ref def) => {
+                    let rewrite = rewrite_macro_def(
+                        &self.get_context(),
+                        self.shape(),
+                        self.block_indent,
+                        def,
+                        item.ident,
+                        &item.vis,
+                        item.span,
+                    );
+                    self.push_rewrite(item.span, rewrite);
+                }
+            };
         }
+        self.skip_macro_names = temp_skip_macro_names;
     }
 
     pub fn visit_trait_item(&mut self, ti: &ast::TraitItem) {
@@ -592,6 +606,10 @@ 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.set_parent_context(ctx);
         visitor
     }
@@ -616,6 +634,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             skipped_range: vec![],
             macro_rewrite_failure: false,
             report,
+            skip_macro_names: RefCell::new(vec![]),
         }
     }
 
@@ -640,10 +659,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                         ErrorKind::DeprecatedAttr,
                     )],
                 );
-            } else if attr.path.segments[0].ident.to_string() == "rustfmt"
-                && (attr.path.segments.len() == 1
-                    || attr.path.segments[1].ident.to_string() != "skip")
-            {
+            } else if self.is_unknown_rustfmt_attr(&attr.path.segments) {
                 let file_name = self.source_map.span_to_filename(attr.span).into();
                 self.report.append(
                     file_name,
@@ -671,6 +687,20 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         false
     }
 
+    fn is_unknown_rustfmt_attr(&self, segments: &[ast::PathSegment]) -> bool {
+        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,
+        }
+    }
+
     fn walk_mod_items(&mut self, m: &ast::Mod) {
         self.visit_items_with_reordering(&ptr_vec_to_ref_vec(&m.items));
     }
@@ -817,6 +847,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(),
         }
     }
 }

tests/source/issue-3434/lib.rs

@@ -0,0 +1,57 @@
+#![rustfmt::skip::macros(skip_macro_mod)]
+
+mod no_entry;
+
+#[rustfmt::skip::macros(html, skip_macro)]
+fn main() {
+    let macro_result1 = html! { <div>
+this should be skipped</div>
+    }
+    .to_string();
+
+    let macro_result2 = not_skip_macro! { <div>
+this should be mangled</div>
+        }
+    .to_string();
+
+    skip_macro! {
+this should be skipped
+};
+
+    foo();
+}
+
+fn foo() {
+    let macro_result1 = html! { <div>
+this should be mangled</div>
+            }
+    .to_string();
+}
+
+fn bar() {
+    let macro_result1 = skip_macro_mod! { <div>
+this should be skipped</div>
+        }
+    .to_string();
+}
+
+fn visitor_made_from_same_context() {
+    let pair = (
+        || {
+            foo!(<div>
+this should be mangled</div>
+            );
+            skip_macro_mod!(<div>
+this should be skipped</div>
+            );
+        },
+        || {
+            foo!(<div>
+this should be mangled</div>
+            );
+            skip_macro_mod!(<div>
+this should be skipped</div>
+            );
+        },
+    );
+}

tests/source/issue-3434/no_entry.rs

@@ -0,0 +1,18 @@
+#[rustfmt::skip::macros(another_macro)]
+fn foo() {
+    another_macro!(
+This should be skipped.
+        );
+}
+
+fn bar() {
+    skip_macro_mod!(
+This should be skipped.
+        );
+}
+
+fn baz() {
+    let macro_result1 = no_skip_macro! { <div>
+this should be mangled</div>
+    }.to_string();
+}

tests/source/issue-3434/not_skip_macro.rs

@@ -0,0 +1,8 @@
+#[this::is::not::skip::macros(ouch)]
+
+fn main() {
+    let macro_result1 = ouch! { <div>
+this should be mangled</div>
+    }
+    .to_string();
+}

tests/target/issue-3434/lib.rs

@@ -0,0 +1,57 @@
+#![rustfmt::skip::macros(skip_macro_mod)]
+
+mod no_entry;
+
+#[rustfmt::skip::macros(html, skip_macro)]
+fn main() {
+    let macro_result1 = html! { <div>
+this should be skipped</div>
+    }
+    .to_string();
+
+    let macro_result2 = not_skip_macro! { <div>
+    this should be mangled</div>
+            }
+    .to_string();
+
+    skip_macro! {
+this should be skipped
+};
+
+    foo();
+}
+
+fn foo() {
+    let macro_result1 = html! { <div>
+    this should be mangled</div>
+                }
+    .to_string();
+}
+
+fn bar() {
+    let macro_result1 = skip_macro_mod! { <div>
+this should be skipped</div>
+        }
+    .to_string();
+}
+
+fn visitor_made_from_same_context() {
+    let pair = (
+        || {
+            foo!(<div>
+            this should be mangled</div>
+                        );
+            skip_macro_mod!(<div>
+this should be skipped</div>
+            );
+        },
+        || {
+            foo!(<div>
+            this should be mangled</div>
+                        );
+            skip_macro_mod!(<div>
+this should be skipped</div>
+            );
+        },
+    );
+}

tests/target/issue-3434/no_entry.rs

@@ -0,0 +1,19 @@
+#[rustfmt::skip::macros(another_macro)]
+fn foo() {
+    another_macro!(
+This should be skipped.
+        );
+}
+
+fn bar() {
+    skip_macro_mod!(
+This should be skipped.
+        );
+}
+
+fn baz() {
+    let macro_result1 = no_skip_macro! { <div>
+    this should be mangled</div>
+        }
+    .to_string();
+}

tests/target/issue-3434/not_skip_macro.rs

@@ -0,0 +1,8 @@
+#[this::is::not::skip::macros(ouch)]
+
+fn main() {
+    let macro_result1 = ouch! { <div>
+    this should be mangled</div>
+        }
+    .to_string();
+}