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

…lip1995 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`
rust-lang · Feb 16, 2024 · 32c006c · 32c006c
2 parents 61daf67 + f4a3db8
commit 32c006c
Show file tree

Hide file tree

Showing 11 changed files with 181 additions and 38 deletions.
diff --git a/CHANGELOG.md b/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

diff --git a/book/src/configuration.md b/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.
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
@@ -433,6 +433,32 @@ declare_clippy_lint! {
     "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 @@ impl_lint_pass!(EarlyAttributes => [
     EMPTY_LINE_AFTER_DOC_COMMENTS,
     NON_MINIMAL_CFG,
     MAYBE_MISUSED_CFG,
+    DEPRECATED_CLIPPY_CFG_ATTR,
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
@@ -803,6 +830,7 @@ impl EarlyLintPass for EarlyAttributes {
 
     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,39 +885,80 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
     }
 }
 
+fn check_cargo_clippy_attr(cx: &EarlyContext<'_>, item: &rustc_ast::MetaItem) {
+    if item.has_name(sym::feature) && item.value_str().is_some_and(|v| v.as_str() == "cargo-clippy") {
+        span_lint_and_sugg(
+            cx,
+            DEPRECATED_CLIPPY_CFG_ATTR,
+            item.span,
+            "`feature = \"cargo-clippy\"` was replaced by `clippy`",
+            "replace with",
+            "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) {
-    if msrv.meets(msrvs::TOOL_ATTRIBUTES)
-        // check cfg_attr
-        && attr.has_name(sym::cfg_attr)
+    // check cfg_attr
+    if 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,
-            attr.span,
-            "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
-            "use",
-            "#[rustfmt::skip]".to_string(),
-            Applicability::MachineApplicable,
-        );
+        // 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);
+        }
     }
 }
 

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -51,6 +51,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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,

diff --git a/src/driver.rs b/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

diff --git a/tests/ui/cfg_attr_cargo_clippy.fixed b/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() {}
diff --git a/tests/ui/cfg_attr_cargo_clippy.rs b/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() {}
diff --git a/tests/ui/cfg_attr_cargo_clippy.stderr b/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
+
diff --git a/tests/ui/useless_attribute.fixed b/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)]

diff --git a/tests/ui/useless_attribute.rs b/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)]

diff --git a/tests/ui/useless_attribute.stderr b/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))]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(feature = "cargo-clippy", allow(dead_code)`
+LL | #[cfg_attr(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