Skip to content

Commit

Permalink
Honor #[rustfmt::skip::attributes(derive)] attribute
Browse files Browse the repository at this point in the history
Fixes 5270

Previously, rustfmt only checked the `merge_derives` configuration value
to determine if it should merge_derives. This lead to derives being
merged even when annotated with the `rustfmt::skip` attribute.

Now, rustfmt also checks if derives are explicitly being skipped in the
current context via the `rustfmt::skip` attribute.
  • Loading branch information
ytmimi authored and calebcartwright committed Mar 21, 2022
1 parent 0dba01a commit 8984438
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ impl Rewrite for [ast::Attribute] {
let mut attrs = self;
let mut result = String::new();

// Determine if the source text is annotated with `#[rustfmt::skip::attributes(derive)]`
// or `#![rustfmt::skip::attributes(derive)]`
let skip_derives = context.skip_context.skip_attribute("derive");

// This is not just a simple map because we need to handle doc comments
// (where we take as many doc comment attributes as possible) and possibly
// merging derives into a single attribute.
Expand Down Expand Up @@ -431,7 +435,7 @@ impl Rewrite for [ast::Attribute] {
}

// Handle derives if we will merge them.
if context.config.merge_derives() && is_derive(&attrs[0]) {
if !skip_derives && context.config.merge_derives() && is_derive(&attrs[0]) {
let derives = take_while_with_pred(context, attrs, is_derive);
let derive_str = format_derive(derives, shape, context)?;
result.push_str(&derive_str);
Expand Down
62 changes: 62 additions & 0 deletions tests/source/issue-5270/merge_derives_true.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// rustfmt-merge_derives:true

#[rustfmt::skip::attributes(derive)]
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[rustfmt::skip::attributes(derive)]
#[derive(Clone)]
struct DoNotMergeDerivesSkipInMiddle {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
#[rustfmt::skip::attributes(derive)]
struct DoNotMergeDerivesSkipAtEnd {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct MergeDerives {
field: String,
}

mod inner_attribute_derive_skip {
#![rustfmt::skip::attributes(derive)]

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

#[rustfmt::skip::attributes(derive)]
mod outer_attribute_derive_skip {
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

mod no_derive_skip {
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct MergeDerives {
field: String,
}
}
62 changes: 62 additions & 0 deletions tests/target/issue-5270/merge_derives_false.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// rustfmt-merge_derives:false

#[rustfmt::skip::attributes(derive)]
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[rustfmt::skip::attributes(derive)]
#[derive(Clone)]
struct DoNotMergeDerivesSkipInMiddle {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
#[rustfmt::skip::attributes(derive)]
struct DoNotMergeDerivesSkipAtEnd {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct MergeDerives {
field: String,
}

mod inner_attribute_derive_skip {
#![rustfmt::skip::attributes(derive)]

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

#[rustfmt::skip::attributes(derive)]
mod outer_attribute_derive_skip {
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

mod no_derive_skip {
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct MergeDerives {
field: String,
}
}
60 changes: 60 additions & 0 deletions tests/target/issue-5270/merge_derives_true.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// rustfmt-merge_derives:true

#[rustfmt::skip::attributes(derive)]
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[rustfmt::skip::attributes(derive)]
#[derive(Clone)]
struct DoNotMergeDerivesSkipInMiddle {
field: String,
}

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
#[rustfmt::skip::attributes(derive)]
struct DoNotMergeDerivesSkipAtEnd {
field: String,
}

#[allow(dead_code)]
#[derive(StructField, Clone)]
struct MergeDerives {
field: String,
}

mod inner_attribute_derive_skip {
#![rustfmt::skip::attributes(derive)]

#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

#[rustfmt::skip::attributes(derive)]
mod outer_attribute_derive_skip {
#[allow(dead_code)]
#[derive(StructField)]
#[derive(Clone)]
struct DoNotMergeDerives {
field: String,
}
}

mod no_derive_skip {
#[allow(dead_code)]
#[derive(StructField, Clone)]
struct MergeDerives {
field: String,
}
}

0 comments on commit 8984438

Please sign in to comment.