Auto merge of #12292 - GuillaumeGomez:DEPRECATED_CLIPPY_CFG_ATTR, r=flip1995

Add new lint `DEPRECATED_CLIPPY_CFG_ATTR`

As discussed [on zulip](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Is.20.60--cfg.20feature.3Dcargo-clippy.60.20deprecated.20or.20can.20it.20be.3F).

This lint suggests to replace `feature = "cargo-clippy"` with `clippy`.

r? `@flip1995`

changelog:  Add new lint `DEPRECATED_CLIPPY_CFG_ATTR`

CHANGELOG.md

@@ -5125,6 +5125,7 @@ Released 2018-09-13
 [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
 [`default_union_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_union_representation
 [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
+[`deprecated_clippy_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_clippy_cfg_attr
 [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
 [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
 [`deref_by_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_by_slicing

book/src/configuration.md

@@ -113,17 +113,14 @@ found [here](https://rust-lang.github.io/rust-clippy/master/index.html#msrv)
 
 Very rarely, you may wish to prevent Clippy from evaluating certain sections of code entirely. You can do this with
 [conditional compilation](https://doc.rust-lang.org/reference/conditional-compilation.html) by checking that the
-`cargo-clippy` feature is not set. You may need to provide a stub so that the code compiles:
+`clippy` cfg is not set. You may need to provide a stub so that the code compiles:
 
 ```rust
-#[cfg(not(feature = "cargo-clippy"))]
+#[cfg(not(clippy)]
 include!(concat!(env!("OUT_DIR"), "/my_big_function-generated.rs"));
 
-#[cfg(feature = "cargo-clippy")]
+#[cfg(clippy)]
 fn my_big_function(_input: &str) -> Option<MyStruct> {
     None
 }
 ```
-
-This feature is not actually part of your crate, so specifying `--all-features` to other tools, e.g. `cargo test
---all-features`, will not disable it.

clippy_lints/src/attrs.rs

@@ -433,6 +433,32 @@
     "prevent from misusing the wrong attr name"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `#[cfg_attr(feature = "cargo-clippy", ...)]` and for
+    /// `#[cfg(feature = "cargo-clippy")]` and suggests to replace it with
+    /// `#[cfg_attr(clippy, ...)]` or `#[cfg(clippy)]`.
+    ///
+    /// ### Why is this bad?
+    /// This feature has been deprecated for years and shouldn't be used anymore.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// #[cfg(feature = "cargo-clippy")]
+    /// struct Bar;
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// #[cfg(clippy)]
+    /// struct Bar;
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub DEPRECATED_CLIPPY_CFG_ATTR,
+    suspicious,
+    "usage of `cfg(feature = \"cargo-clippy\")` instead of `cfg(clippy)`"
+}
+
 declare_lint_pass!(Attributes => [
     ALLOW_ATTRIBUTES_WITHOUT_REASON,
     INLINE_ALWAYS,
@@ -794,6 +820,7 @@ pub struct EarlyAttributes {
     EMPTY_LINE_AFTER_DOC_COMMENTS,
     NON_MINIMAL_CFG,
     MAYBE_MISUSED_CFG,
+    DEPRECATED_CLIPPY_CFG_ATTR,
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
@@ -803,6 +830,7 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
 
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
         check_deprecated_cfg_attr(cx, attr, &self.msrv);
+        check_deprecated_cfg(cx, attr);
         check_mismatched_target_os(cx, attr);
         check_minimal_cfg_condition(cx, attr);
         check_misused_cfg(cx, attr);
@@ -857,42 +885,83 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
     }
 }
 
-fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
+fn check_cargo_clippy_attr(cx: &EarlyContext<'_>, item: &rustc_ast::MetaItem) {
-    if msrv.meets(msrvs::TOOL_ATTRIBUTES)
+    if item.has_name(sym::feature) && item.value_str().is_some_and(|v| v.as_str() == "cargo-clippy") {
-        // check cfg_attr
-        && attr.has_name(sym::cfg_attr)
-        && let Some(items) = attr.meta_item_list()
-        && items.len() == 2
-        // check for `rustfmt`
-        && let Some(feature_item) = items[0].meta_item()
-        && feature_item.has_name(sym::rustfmt)
-        // check for `rustfmt_skip` and `rustfmt::skip`
-        && let Some(skip_item) = &items[1].meta_item()
-        && (skip_item.has_name(sym!(rustfmt_skip))
-            || skip_item
-                .path
-                .segments
-                .last()
-                .expect("empty path in attribute")
-                .ident
-                .name
-                == sym::skip)
-        // Only lint outer attributes, because custom inner attributes are unstable
-        // Tracking issue: https://github.com/rust-lang/rust/issues/54726
-        && attr.style == AttrStyle::Outer
-    {
         span_lint_and_sugg(
             cx,
-            DEPRECATED_CFG_ATTR,
+            DEPRECATED_CLIPPY_CFG_ATTR,
-            attr.span,
+            item.span,
-            "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
+            "`feature = \"cargo-clippy\"` was replaced by `clippy`",
-            "use",
+            "replace with",
-            "#[rustfmt::skip]".to_string(),
+            "clippy".to_string(),
             Applicability::MachineApplicable,
         );
     }
 }
 
+fn check_deprecated_cfg_recursively(cx: &EarlyContext<'_>, attr: &rustc_ast::MetaItem) {
+    if let Some(ident) = attr.ident() {
+        if ["any", "all", "not"].contains(&ident.name.as_str()) {
+            let Some(list) = attr.meta_item_list() else { return };
+            for item in list.iter().filter_map(|item| item.meta_item()) {
+                check_deprecated_cfg_recursively(cx, item);
+            }
+        } else {
+            check_cargo_clippy_attr(cx, attr);
+        }
+    }
+}
+
+fn check_deprecated_cfg(cx: &EarlyContext<'_>, attr: &Attribute) {
+    if attr.has_name(sym::cfg)
+        && let Some(list) = attr.meta_item_list()
+    {
+        for item in list.iter().filter_map(|item| item.meta_item()) {
+            check_deprecated_cfg_recursively(cx, item);
+        }
+    }
+}
+
+fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
+    // check cfg_attr
+    if attr.has_name(sym::cfg_attr)
+        && let Some(items) = attr.meta_item_list()
+        && items.len() == 2
+        && let Some(feature_item) = items[0].meta_item()
+    {
+        // check for `rustfmt`
+        if feature_item.has_name(sym::rustfmt)
+            && msrv.meets(msrvs::TOOL_ATTRIBUTES)
+            // check for `rustfmt_skip` and `rustfmt::skip`
+            && let Some(skip_item) = &items[1].meta_item()
+            && (skip_item.has_name(sym!(rustfmt_skip))
+                || skip_item
+                    .path
+                    .segments
+                    .last()
+                    .expect("empty path in attribute")
+                    .ident
+                    .name
+                    == sym::skip)
+            // Only lint outer attributes, because custom inner attributes are unstable
+            // Tracking issue: https://github.com/rust-lang/rust/issues/54726
+            && attr.style == AttrStyle::Outer
+        {
+            span_lint_and_sugg(
+                cx,
+                DEPRECATED_CFG_ATTR,
+                attr.span,
+                "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
+                "use",
+                "#[rustfmt::skip]".to_string(),
+                Applicability::MachineApplicable,
+            );
+        } else {
+            check_deprecated_cfg_recursively(cx, feature_item);
+        }
+    }
+}
+
 fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
     for item in items {
         if let NestedMetaItem::MetaItem(meta) = item {

clippy_lints/src/declared_lints.rs

@@ -51,6 +51,7 @@
     crate::attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON_INFO,
     crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO,
     crate::attrs::DEPRECATED_CFG_ATTR_INFO,
+    crate::attrs::DEPRECATED_CLIPPY_CFG_ATTR_INFO,
     crate::attrs::DEPRECATED_SEMVER_INFO,
     crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
     crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,

src/driver.rs

@@ -271,7 +271,9 @@ pub fn main() {
                 },
                 _ => Some(s.to_string()),
             })
+            // FIXME: remove this line in 1.79 to only keep `--cfg clippy`.
             .chain(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()])
+            .chain(vec!["--cfg".into(), "clippy".into()])
             .collect::<Vec<String>>();
 
         // We enable Clippy if one of the following conditions is met

tests/ui/cfg_attr_cargo_clippy.fixed

@@ -0,0 +1,13 @@
+#![warn(clippy::deprecated_clippy_cfg_attr)]
+#![allow(clippy::non_minimal_cfg)]
+#![cfg_attr(clippy, doc = "a")] //~ ERROR: `feature = "cargo-clippy"` was
+
+#[cfg_attr(clippy, derive(Debug))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg_attr(not(clippy), derive(Debug))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(clippy)] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(not(clippy))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(any(clippy))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(all(clippy))] //~ ERROR: `feature = "cargo-clippy"` was
+pub struct Bar;
+
+fn main() {}

tests/ui/cfg_attr_cargo_clippy.rs

@@ -0,0 +1,13 @@
+#![warn(clippy::deprecated_clippy_cfg_attr)]
+#![allow(clippy::non_minimal_cfg)]
+#![cfg_attr(feature = "cargo-clippy", doc = "a")] //~ ERROR: `feature = "cargo-clippy"` was
+
+#[cfg_attr(feature = "cargo-clippy", derive(Debug))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg_attr(not(feature = "cargo-clippy"), derive(Debug))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(feature = "cargo-clippy")] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(not(feature = "cargo-clippy"))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(any(feature = "cargo-clippy"))] //~ ERROR: `feature = "cargo-clippy"` was
+#[cfg(all(feature = "cargo-clippy"))] //~ ERROR: `feature = "cargo-clippy"` was
+pub struct Bar;
+
+fn main() {}

tests/ui/cfg_attr_cargo_clippy.stderr

@@ -0,0 +1,47 @@
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:5:12
+   |
+LL | #[cfg_attr(feature = "cargo-clippy", derive(Debug))]
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+   |
+   = note: `-D clippy::deprecated-clippy-cfg-attr` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::deprecated_clippy_cfg_attr)]`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:6:16
+   |
+LL | #[cfg_attr(not(feature = "cargo-clippy"), derive(Debug))]
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:7:7
+   |
+LL | #[cfg(feature = "cargo-clippy")]
+   |       ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:8:11
+   |
+LL | #[cfg(not(feature = "cargo-clippy"))]
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:9:11
+   |
+LL | #[cfg(any(feature = "cargo-clippy"))]
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:10:11
+   |
+LL | #[cfg(all(feature = "cargo-clippy"))]
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> $DIR/cfg_attr_cargo_clippy.rs:3:13
+   |
+LL | #![cfg_attr(feature = "cargo-clippy", doc = "a")]
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: aborting due to 7 previous errors
+

tests/ui/useless_attribute.fixed

@@ -6,7 +6,7 @@
 #![feature(rustc_private)]
 
 #![allow(dead_code)]
-#![cfg_attr(feature = "cargo-clippy", allow(dead_code))]
+#![cfg_attr(clippy, allow(dead_code))]
 #[rustfmt::skip]
 #[allow(unused_imports)]
 #[allow(unused_extern_crates)]

tests/ui/useless_attribute.rs

@@ -6,7 +6,7 @@
 #![feature(rustc_private)]
 
 #[allow(dead_code)]
-#[cfg_attr(feature = "cargo-clippy", allow(dead_code))]
+#[cfg_attr(clippy, allow(dead_code))]
 #[rustfmt::skip]
 #[allow(unused_imports)]
 #[allow(unused_extern_crates)]

tests/ui/useless_attribute.stderr

@@ -10,8 +10,8 @@ LL | #[allow(dead_code)]
 error: useless lint attribute
   --> $DIR/useless_attribute.rs:9:1
    |
-LL | #[cfg_attr(feature = "cargo-clippy", allow(dead_code))]
+LL | #[cfg_attr(clippy, allow(dead_code))]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(feature = "cargo-clippy", allow(dead_code)`
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(clippy, allow(dead_code)`
 
 error: useless lint attribute
   --> $DIR/useless_attribute.rs:20:5