Auto merge of #12555 - GuillaumeGomez:duplicated_attribute, r=blyxyas

Don't emit `duplicated_attribute` lint on "complex" `cfg`s Part of #12537. changelog: Don't emit `duplicated_attribute` lint on "complex" `cfg`s
rust-lang · Mar 26, 2024 · b8b9b27 · b8b9b27
2 parents 13ef845 + e3f3a4b
commit b8b9b27
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 88 deletions.
diff --git a/clippy_lints/src/attrs/duplicated_attributes.rs b/clippy_lints/src/attrs/duplicated_attributes.rs
@@ -25,20 +25,31 @@ fn emit_if_duplicated(
     }
 }
 
+#[allow(clippy::needless_return)]
 fn check_duplicated_attr(
     cx: &EarlyContext<'_>,
     attr: &MetaItem,
     attr_paths: &mut FxHashMap<String, Span>,
     parent: &mut Vec<String>,
 ) {
+    if attr.span.from_expansion() {
+        return;
+    }
     let Some(ident) = attr.ident() else { return };
     let name = ident.name;
     if name == sym::doc || name == sym::cfg_attr {
         // FIXME: Would be nice to handle `cfg_attr` as well. Only problem is to check that cfg
         // conditions are the same.
         return;
     }
-    if let Some(value) = attr.value_str() {
+    if let Some(direct_parent) = parent.last()
+        && ["cfg", "cfg_attr"].contains(&direct_parent.as_str())
+        && [sym::all, sym::not, sym::any].contains(&name)
+    {
+        // FIXME: We don't correctly check `cfg`s for now, so if it's more complex than just a one
+        // level `cfg`, we leave.
+        return;
+    } else if let Some(value) = attr.value_str() {
         emit_if_duplicated(cx, attr, attr_paths, format!("{}:{name}={value}", parent.join(":")));
     } else if let Some(sub_attrs) = attr.meta_item_list() {
         parent.push(name.as_str().to_string());

diff --git a/tests/ui/duplicated_attributes.rs b/tests/ui/duplicated_attributes.rs
@@ -2,16 +2,17 @@
 #![cfg(any(unix, windows))]
 #![allow(dead_code)]
 #![allow(dead_code)] //~ ERROR: duplicated attribute
-#![cfg(any(unix, windows))]
-//~^ ERROR: duplicated attribute
-//~| ERROR: duplicated attribute
+#![cfg(any(unix, windows))] // Should not warn!
 
 #[cfg(any(unix, windows, target_os = "linux"))]
 #[allow(dead_code)]
 #[allow(dead_code)] //~ ERROR: duplicated attribute
-#[cfg(any(unix, windows, target_os = "linux"))]
-//~^ ERROR: duplicated attribute
-//~| ERROR: duplicated attribute
+#[cfg(any(unix, windows, target_os = "linux"))] // Should not warn!
 fn foo() {}
 
+#[cfg(unix)]
+#[cfg(windows)]
+#[cfg(unix)] //~ ERROR: duplicated attribute
+fn bar() {}
+
 fn main() {}
diff --git a/tests/ui/duplicated_attributes.stderr b/tests/ui/duplicated_attributes.stderr
@@ -18,106 +18,38 @@ LL | #![allow(dead_code)]
    = help: to override `-D warnings` add `#[allow(clippy::duplicated_attributes)]`
 
 error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:5:12
-   |
-LL | #![cfg(any(unix, windows))]
-   |            ^^^^
-   |
-note: first defined here
-  --> tests/ui/duplicated_attributes.rs:2:12
-   |
-LL | #![cfg(any(unix, windows))]
-   |            ^^^^
-help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:5:12
-   |
-LL | #![cfg(any(unix, windows))]
-   |            ^^^^
-
-error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:5:18
-   |
-LL | #![cfg(any(unix, windows))]
-   |                  ^^^^^^^
-   |
-note: first defined here
-  --> tests/ui/duplicated_attributes.rs:2:18
-   |
-LL | #![cfg(any(unix, windows))]
-   |                  ^^^^^^^
-help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:5:18
-   |
-LL | #![cfg(any(unix, windows))]
-   |                  ^^^^^^^
-
-error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:11:9
+  --> tests/ui/duplicated_attributes.rs:9:9
    |
 LL | #[allow(dead_code)]
    |         ^^^^^^^^^
    |
 note: first defined here
-  --> tests/ui/duplicated_attributes.rs:10:9
+  --> tests/ui/duplicated_attributes.rs:8:9
    |
 LL | #[allow(dead_code)]
    |         ^^^^^^^^^
 help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:11:9
+  --> tests/ui/duplicated_attributes.rs:9:9
    |
 LL | #[allow(dead_code)]
    |         ^^^^^^^^^
 
 error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:12:11
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |           ^^^^
-   |
-note: first defined here
-  --> tests/ui/duplicated_attributes.rs:9:11
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |           ^^^^
-help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:12:11
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |           ^^^^
-
-error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:12:17
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                 ^^^^^^^
-   |
-note: first defined here
-  --> tests/ui/duplicated_attributes.rs:9:17
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                 ^^^^^^^
-help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:12:17
-   |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                 ^^^^^^^
-
-error: duplicated attribute
-  --> tests/ui/duplicated_attributes.rs:12:26
+  --> tests/ui/duplicated_attributes.rs:15:7
    |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                          ^^^^^^^^^^^^^^^^^^^
+LL | #[cfg(unix)]
+   |       ^^^^
    |
 note: first defined here
-  --> tests/ui/duplicated_attributes.rs:9:26
+  --> tests/ui/duplicated_attributes.rs:13:7
    |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                          ^^^^^^^^^^^^^^^^^^^
+LL | #[cfg(unix)]
+   |       ^^^^
 help: remove this attribute
-  --> tests/ui/duplicated_attributes.rs:12:26
+  --> tests/ui/duplicated_attributes.rs:15:7
    |
-LL | #[cfg(any(unix, windows, target_os = "linux"))]
-   |                          ^^^^^^^^^^^^^^^^^^^
+LL | #[cfg(unix)]
+   |       ^^^^
 
-error: aborting due to 7 previous errors
+error: aborting due to 3 previous errors