Add global_variables lint

CHANGELOG.md

@@ -6167,6 +6167,7 @@ Released 2018-09-13
 [`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first
 [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
 [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
+[`global_variables`]: https://rust-lang.github.io/rust-clippy/master/index.html#global_variables
 [`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#host_endian_bytes
 [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
 [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

clippy_lints/src/declared_lints.rs

@@ -563,6 +563,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::non_canonical_impls::NON_CANONICAL_PARTIAL_ORD_IMPL_INFO,
     crate::non_copy_const::BORROW_INTERIOR_MUTABLE_CONST_INFO,
     crate::non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST_INFO,
+    crate::non_copy_const::GLOBAL_VARIABLES_INFO,
     crate::non_expressive_names::JUST_UNDERSCORES_AND_DIGITS_INFO,
     crate::non_expressive_names::MANY_SINGLE_CHAR_NAMES_INFO,
     crate::non_expressive_names::SIMILAR_NAMES_INFO,

clippy_lints/src/non_copy_const.rs

@@ -19,7 +19,7 @@
 
 use clippy_config::Conf;
 use clippy_utils::consts::{ConstEvalCtxt, Constant};
-use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then};
 use clippy_utils::is_in_const_context;
 use clippy_utils::macros::macro_backtrace;
 use clippy_utils::paths::{PathNS, lookup_path_str};
@@ -159,6 +159,51 @@ declare_clippy_lint! {
     "referencing `const` with interior mutability"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks whether a global variable defined.
+    ///
+    /// ### Why restrict this?
+    ///
+    /// - Global variables can be modified from any part of the program, making it difficult to
+    ///   track and control their state.
+    /// - Global variables introduce implicit dependencies that are not visible in function
+    ///   signatures, making the code harder to understand and maintain.
+    /// - Global variables introduce persistent state, complicating unit tests and making them
+    ///   prone to side effects.
+    /// - Global variables create tight coupling between different parts of the program, making it
+    ///   harder to modify one part without affecting others.
+    ///
+    /// ### Example
+    ///
+    /// ```no_run
+    /// use std::sync::Mutex;
+    ///
+    /// struct State {}
+    ///
+    /// static STATE: Mutex<State> = Mutex::new(State {});
+    ///
+    /// fn foo() {
+    ///    // Access global variable `STATE`.
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    ///
+    /// ```no_run
+    /// struct State {}
+    ///
+    /// fn foo(state: &mut State) {
+    ///    // Access `state` argument instead of a global variable.
+    /// }
+    /// ```
+    #[clippy::version = "1.88.0"]
+    pub GLOBAL_VARIABLES,
+    nursery,
+    "declaring global variables"
+}
+
 #[derive(Clone, Copy)]
 enum IsFreeze {
     /// The type and all possible values are `Freeze`
@@ -256,7 +301,7 @@ pub struct NonCopyConst<'tcx> {
     freeze_tys: FxHashMap<Ty<'tcx>, IsFreeze>,
 }
 
-impl_lint_pass!(NonCopyConst<'_> => [DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST]);
+impl_lint_pass!(NonCopyConst<'_> => [DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST, GLOBAL_VARIABLES]);
 
 impl<'tcx> NonCopyConst<'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf) -> Self {
@@ -695,43 +740,81 @@ impl<'tcx> NonCopyConst<'tcx> {
 
 impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
-        if let ItemKind::Const(ident, .., body_id) = item.kind
-            && !ident.is_special()
-            && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
-            && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) {
-                IsFreeze::No => true,
-                IsFreeze::Yes => false,
-                IsFreeze::Maybe => match cx.tcx.const_eval_poly(item.owner_id.to_def_id()) {
-                    Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx.tcx, cx.typing_env(), ty, val) => !is_freeze,
-                    _ => !self.is_init_expr_freeze(
-                        cx.tcx,
-                        cx.typing_env(),
-                        cx.tcx.typeck(item.owner_id),
-                        GenericArgs::identity_for_item(cx.tcx, item.owner_id),
-                        cx.tcx.hir_body(body_id).value,
-                    ),
-                },
-            }
-            && !item.span.in_external_macro(cx.sess().source_map())
+        let ty = || cx.tcx.type_of(item.owner_id).instantiate_identity();
+
+        let is_init_expr_freeze = |this: &mut Self, body_id| {
+            this.is_init_expr_freeze(
+                cx.tcx,
+                cx.typing_env(),
+                cx.tcx.typeck(item.owner_id),
+                GenericArgs::identity_for_item(cx.tcx, item.owner_id),
+                cx.tcx.hir_body(body_id).value,
+            )
+        };
+
+        let is_thread_local_or_external_macro = || {
+            item.span.in_external_macro(cx.sess().source_map())
             // Only needed when compiling `std`
-            && !is_thread_local(cx, item)
-        {
-            span_lint_and_then(
-                cx,
-                DECLARE_INTERIOR_MUTABLE_CONST,
-                ident.span,
-                "named constant with interior mutability",
-                |diag| {
-                    let Some(sync_trait) = cx.tcx.lang_items().sync_trait() else {
-                        return;
-                    };
-                    if implements_trait(cx, ty, sync_trait, &[]) {
-                        diag.help("did you mean to make this a `static` item");
-                    } else {
-                        diag.help("did you mean to make this a `thread_local!` item");
+            || is_thread_local(cx, item)
+        };
+
+        match item.kind {
+            ItemKind::Const(ident, .., body_id) => {
+                if !clippy_utils::is_lint_allowed(cx, DECLARE_INTERIOR_MUTABLE_CONST, item.hir_id())
+                    && !ident.is_special()
+                    && let ty = ty()
+                    && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) {
+                        IsFreeze::No => true,
+                        IsFreeze::Yes => false,
+                        IsFreeze::Maybe => match cx.tcx.const_eval_poly(item.owner_id.to_def_id()) {
+                            Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx.tcx, cx.typing_env(), ty, val) => {
+                                !is_freeze
+                            },
+                            _ => !is_init_expr_freeze(self, body_id),
+                        },
                     }
-                },
-            );
+                    && !is_thread_local_or_external_macro()
+                {
+                    span_lint_and_then(
+                        cx,
+                        DECLARE_INTERIOR_MUTABLE_CONST,
+                        ident.span,
+                        "named constant with interior mutability",
+                        |diag| {
+                            let Some(sync_trait) = cx.tcx.lang_items().sync_trait() else {
+                                return;
+                            };
+
+                            diag.help(if implements_trait(cx, ty, sync_trait, &[]) {
+                                "did you mean to make this a `static` item"
+                            } else {
+                                "did you mean to make this a `thread_local!` item"
+                            });
+                        },
+                    );
+                }
+            },
+            ItemKind::Static(_, ident, _, body_id) => {
+                if !clippy_utils::is_lint_allowed(cx, GLOBAL_VARIABLES, item.hir_id())
+                    && !ident.is_special()
+                    && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty()) {
+                        IsFreeze::No => true,
+                        IsFreeze::Yes => false,
+                        IsFreeze::Maybe => !is_init_expr_freeze(self, body_id),
+                    }
+                    && !is_thread_local_or_external_macro()
+                {
+                    span_lint_and_help(
+                        cx,
+                        GLOBAL_VARIABLES,
+                        ident.span,
+                        "global variable should not be used",
+                        None,
+                        "consider passing this as a function argument or using a `thread_local`",
+                    );
+                }
+            },
+            _ => {},
         }
     }
 

tests/ui/global_variables.rs

@@ -0,0 +1,50 @@
+#![warn(clippy::global_variables)]
+
+use std::sync::Mutex;
+use std::sync::atomic::AtomicU32;
+
+macro_rules! define_global_variables_with_macro {
+    () => {
+        static GLOBAL_VARIABLE_0: AtomicU32 = AtomicU32::new(2);
+        //~^ global_variables
+
+        static GLOBAL_VARIABLE_1: Mutex<u32> = Mutex::new(3);
+        //~^ global_variables
+    };
+}
+
+#[allow(clippy::missing_const_for_thread_local)]
+fn main() {
+    define_global_variables_with_macro!();
+
+    static GLOBAL_VARIABLE_2: AtomicU32 = AtomicU32::new(2);
+    //~^ global_variables
+
+    static GLOBAL_VARIABLE_3: Mutex<u32> = Mutex::new(3);
+    //~^ global_variables
+
+    static GLOBAL_VARIABLE_4: Option<AtomicU32> = Some(AtomicU32::new(0));
+    //~^ global_variables
+
+    static NOT_GLOBAL_VARIABLE_0: u32 = 1;
+
+    // ZST fields (like `PhantomData`) should be ignored.
+    static NOT_GLOBAL_VARIABLE_1: Vec<AtomicU32> = Vec::new();
+
+    // Having a initializer with a freeze value should be ignored.
+    static NOT_GLOBAL_VARIABLE_2: Option<AtomicU32> = None;
+
+    // Thread-local variables are ignored.
+    thread_local! {
+        static THREAD_LOCAL_VARIABLE_0: u32 = 0;
+        static THREAD_LOCAL_VARIABLE_1: AtomicU32 = AtomicU32::new(0);
+        static THREAD_LOCAL_VARIABLE_2: u32 = const { 0 };
+        static THREAD_LOCAL_VARIABLE_3: AtomicU32 = const { AtomicU32::new(0) };
+
+        static THREAD_LOCAL_VARIABLE_4: () = {
+            // Global variables inside a thread-local initializer are also considered.
+            static GLOBAL_VARIABLE_IN_THREAD_LOCAL: AtomicU32 = AtomicU32::new(0);
+            //~^ global_variables
+        };
+    }
+}

tests/ui/global_variables.stderr

@@ -0,0 +1,60 @@
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:8:16
+   |
+LL |         static GLOBAL_VARIABLE_0: AtomicU32 = AtomicU32::new(2);
+   |                ^^^^^^^^^^^^^^^^^
+...
+LL |     define_global_variables_with_macro!();
+   |     ------------------------------------- in this macro invocation
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+   = note: `-D clippy::global-variables` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::global_variables)]`
+   = note: this error originates in the macro `define_global_variables_with_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:11:16
+   |
+LL |         static GLOBAL_VARIABLE_1: Mutex<u32> = Mutex::new(3);
+   |                ^^^^^^^^^^^^^^^^^
+...
+LL |     define_global_variables_with_macro!();
+   |     ------------------------------------- in this macro invocation
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+   = note: this error originates in the macro `define_global_variables_with_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:20:12
+   |
+LL |     static GLOBAL_VARIABLE_2: AtomicU32 = AtomicU32::new(2);
+   |            ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:23:12
+   |
+LL |     static GLOBAL_VARIABLE_3: Mutex<u32> = Mutex::new(3);
+   |            ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:26:12
+   |
+LL |     static GLOBAL_VARIABLE_4: Option<AtomicU32> = Some(AtomicU32::new(0));
+   |            ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+
+error: global variable should not be used
+  --> tests/ui/global_variables.rs:46:20
+   |
+LL |             static GLOBAL_VARIABLE_IN_THREAD_LOCAL: AtomicU32 = AtomicU32::new(0);
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider passing this as a function argument or using a `thread_local`
+
+error: aborting due to 6 previous errors
+