Auto merge of rust-lang#11293 - mrnossiom:11234, r=Jarcho

feat: add cfg_not_test lint

<!--
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
-->

Fixes rust-lang#11234

changelog: new lint: [`cfg_not_test`]

I don't know whether to lint only the `attr` or also the item associated to it. I guess this would mean putting the check in another place than `check_attribute` but I can't find a way to get the associated item to the attribute.

Also, I'm not sure how to document this lint, I feel like my explications are bad.

Author: bors

CHANGELOG.md

@@ -5254,6 +5254,7 @@ Released 2018-09-13
 [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss
 [`cast_slice_different_sizes`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_slice_different_sizes
 [`cast_slice_from_raw_parts`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_slice_from_raw_parts
+[`cfg_not_test`]: https://rust-lang.github.io/rust-clippy/master/index.html#cfg_not_test
 [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8
 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp
 [`chars_next_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp

clippy_lints/src/cfg_not_test.rs

@@ -0,0 +1,60 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use rustc_ast::NestedMetaItem;
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `cfg` that excludes code from `test` builds. (i.e., `#{cfg(not(test))]`)
+    ///
+    /// ### Why is this bad?
+    /// This may give the false impression that a codebase has 100% coverage, yet actually has untested code.
+    /// Enabling this also guards against excessive mockery as well, which is an anti-pattern.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # fn important_check() {}
+    /// #[cfg(not(test))]
+    /// important_check(); // I'm not actually tested, but not including me will falsely increase coverage!
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # fn important_check() {}
+    /// important_check();
+    /// ```
+    #[clippy::version = "1.73.0"]
+    pub CFG_NOT_TEST,
+    restriction,
+    "enforce against excluding code from test builds"
+}
+
+declare_lint_pass!(CfgNotTest => [CFG_NOT_TEST]);
+
+impl EarlyLintPass for CfgNotTest {
+    fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &rustc_ast::Attribute) {
+        if attr.has_name(rustc_span::sym::cfg) && contains_not_test(attr.meta_item_list().as_deref(), false) {
+            span_lint_and_then(
+                cx,
+                CFG_NOT_TEST,
+                attr.span,
+                "code is excluded from test builds",
+                |diag| {
+                    diag.help("consider not excluding any code from test builds");
+                    diag.note_once("this could increase code coverage despite not actually being tested");
+                },
+            );
+        }
+    }
+}
+
+fn contains_not_test(list: Option<&[NestedMetaItem]>, not: bool) -> bool {
+    list.is_some_and(|list| {
+        list.iter().any(|item| {
+            item.ident().is_some_and(|ident| match ident.name {
+                rustc_span::sym::not => contains_not_test(item.meta_item_list(), !not),
+                rustc_span::sym::test => not,
+                _ => contains_not_test(item.meta_item_list(), not),
+            })
+        })
+    })
+}

clippy_lints/src/declared_lints.rs

@@ -104,6 +104,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::casts::REF_AS_PTR_INFO,
     crate::casts::UNNECESSARY_CAST_INFO,
     crate::casts::ZERO_PTR_INFO,
+    crate::cfg_not_test::CFG_NOT_TEST_INFO,
     crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
     crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
     crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO,

clippy_lints/src/lib.rs

@@ -92,6 +92,7 @@ mod box_default;
 mod byte_char_slices;
 mod cargo;
 mod casts;
+mod cfg_not_test;
 mod checked_conversions;
 mod cognitive_complexity;
 mod collapsible_if;
@@ -1176,6 +1177,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
     store.register_late_pass(|_| Box::new(set_contains_or_insert::HashsetInsertAfterContains));
     store.register_early_pass(|| Box::new(byte_char_slices::ByteCharSlice));
+    store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

tests/ui/cfg_not_test.rs

@@ -0,0 +1,32 @@
+#![allow(unused)]
+#![warn(clippy::cfg_not_test)]
+
+fn important_check() {}
+
+fn main() {
+    // Statement
+    #[cfg(not(test))]
+    let answer = 42;
+
+    // Expression
+    #[cfg(not(test))]
+    important_check();
+
+    // Make sure only not(test) are checked, not other attributes
+    #[cfg(not(foo))]
+    important_check();
+}
+
+#[cfg(not(not(test)))]
+struct CfgNotTest;
+
+// Deeply nested `not(test)`
+#[cfg(not(test))]
+fn foo() {}
+#[cfg(all(debug_assertions, not(test)))]
+fn bar() {}
+#[cfg(not(any(not(debug_assertions), test)))]
+fn baz() {}
+
+#[cfg(test)]
+mod tests {}

tests/ui/cfg_not_test.stderr

@@ -0,0 +1,45 @@
+error: code is excluded from test builds
+  --> tests/ui/cfg_not_test.rs:8:5
+   |
+LL |     #[cfg(not(test))]
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider not excluding any code from test builds
+   = note: this could increase code coverage despite not actually being tested
+   = note: `-D clippy::cfg-not-test` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::cfg_not_test)]`
+
+error: code is excluded from test builds
+  --> tests/ui/cfg_not_test.rs:12:5
+   |
+LL |     #[cfg(not(test))]
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider not excluding any code from test builds
+
+error: code is excluded from test builds
+  --> tests/ui/cfg_not_test.rs:24:1
+   |
+LL | #[cfg(not(test))]
+   | ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider not excluding any code from test builds
+
+error: code is excluded from test builds
+  --> tests/ui/cfg_not_test.rs:26:1
+   |
+LL | #[cfg(all(debug_assertions, not(test)))]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider not excluding any code from test builds
+
+error: code is excluded from test builds
+  --> tests/ui/cfg_not_test.rs:28:1
+   |
+LL | #[cfg(not(any(not(debug_assertions), test)))]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider not excluding any code from test builds
+
+error: aborting due to 5 previous errors
+