diff --git a/CHANGELOG.md b/CHANGELOG.md index b15c6948fbc..c157c6f092e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Added +- Add new attribute `rustfmt::skip::attributes` to prevent rustfmt +from formatting an attribute #3665 - Add `--manifest-path` support to `cargo fmt` (#3683). ### Fixed diff --git a/README.md b/README.md index 381dc73209e..292fcf644d2 100644 --- 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! {
diff --git a/src/attr.rs b/src/attr.rs index 8497917ab06..1c9092ea629 100644 --- 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 index 0faeb5102f2..879ddc61b13 100644 --- 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 index 1b4ce663313..a672e2cc8aa 100644 --- 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 index ac00e4452a7..7e5ce9deea9 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -211,9 +211,8 @@ pub(crate) fn rewrite_macro( position: MacroPosition, ) -> Option { 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 index 9a955dbd91b..606aafc9ebd 100644 --- 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, pub(crate) report: FormatReport, - pub(crate) skip_macro_names: RefCell>, + pub(crate) skip_context: SkipContext, } impl<'a> RewriteContext<'a> { diff --git a/src/skip.rs b/src/skip.rs new file mode 100644 index 00000000000..6b4e04a7173 --- /dev/null +++ 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, + attributes: Vec, +} + +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 { + 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 index 10aad268a8d..a7063c76f28 100644 --- 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 index 26ee7317df5..7a432c0f7d8 100644 --- 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 { - 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 index d145693b9aa..35291cecfb5 100644 --- 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>, + 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 new file mode 100644 index 00000000000..e049fbc5680 --- /dev/null +++ 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 new file mode 100644 index 00000000000..14985259a50 --- /dev/null +++ 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 new file mode 100644 index 00000000000..75fb24b4a65 --- /dev/null +++ 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 new file mode 100644 index 00000000000..c313f320368 --- /dev/null +++ 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 new file mode 100644 index 00000000000..a4e8b948736 --- /dev/null +++ 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() {} diff --git a/tests/target/issue-3665/sub_mod.rs b/tests/target/issue-3665/sub_mod.rs new file mode 100644 index 00000000000..30a2b0fd9d9 --- /dev/null +++ b/tests/target/issue-3665/sub_mod.rs @@ -0,0 +1,13 @@ +#[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() {}