Add a new lint to check for use of std::env::set_var in tests

CHANGELOG.md

@@ -6766,6 +6766,7 @@ Released 2018-09-13
 [`separated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#separated_literal_suffix
 [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
 [`set_contains_or_insert`]: https://rust-lang.github.io/rust-clippy/master/index.html#set_contains_or_insert
+[`set_env_in_tests`]: https://rust-lang.github.io/rust-clippy/master/index.html#set_env_in_tests
 [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
 [`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same
 [`shadow_unrelated`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated

clippy_lints/src/declared_lints.rs

@@ -669,6 +669,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED_INFO,
     crate::serde_api::SERDE_API_MISUSE_INFO,
     crate::set_contains_or_insert::SET_CONTAINS_OR_INSERT_INFO,
+    crate::set_env_in_tests::SET_ENV_IN_TESTS_INFO,
     crate::shadow::SHADOW_REUSE_INFO,
     crate::shadow::SHADOW_SAME_INFO,
     crate::shadow::SHADOW_UNRELATED_INFO,

clippy_lints/src/lib.rs

@@ -324,6 +324,7 @@ mod semicolon_block;
 mod semicolon_if_nothing_returned;
 mod serde_api;
 mod set_contains_or_insert;
+mod set_env_in_tests;
 mod shadow;
 mod significant_drop_tightening;
 mod single_call_fn;
@@ -821,5 +822,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
     store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
     store.register_late_pass(|_| Box::new(replace_box::ReplaceBox));
+    store.register_late_pass(|_| Box::new(set_env_in_tests::SetEnvInTests));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/set_env_in_tests.rs

@@ -0,0 +1,55 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::paths::{PathNS, lookup_path_str};
+use clippy_utils::{fn_def_id, is_in_test_function};
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for use of `std::env::set_env` in tests.
+    ///
+    /// ### Why restrict this?
+    /// Setting environment varibales in tests often means the subject code
+    /// is reading and acting on the environment. By default, rust tests
+    /// are run concurrently, and setting environment variables cannot be
+    /// done in a way that is scoped only to the test. Even if care is taken
+    /// to clean up any mutations, concurrent test runs will affect each
+    /// other's environment.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     fn my_test() {
+    ///         unsafe { std::env::set_var("MY_VAR", "1"); }
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.92.0"]
+    pub SET_ENV_IN_TESTS,
+    restriction,
+    "use of set_env in tests"
+}
+declare_lint_pass!(SetEnvInTests => [SET_ENV_IN_TESTS]);
+
+impl<'tcx> LateLintPass<'tcx> for SetEnvInTests {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        let set_var_ids = lookup_path_str(cx.tcx, PathNS::Value, "std::env::set_var");
+
+        if matches!(expr.kind, ExprKind::Call(..))
+            && let Some(call_def_id) = fn_def_id(cx, expr)
+            && is_in_test_function(cx.tcx, expr.hir_id)
+            && set_var_ids.contains(&call_def_id)
+        {
+            span_lint_and_help(
+                cx,
+                SET_ENV_IN_TESTS,
+                expr.span,
+                "env::set_var called from a test",
+                None,
+                "this might indicate state leakage and cause flaky tests",
+            );
+        }
+    }
+}

tests/ui/set_env_in_tests.rs

@@ -0,0 +1,25 @@
+#![warn(clippy::set_env_in_tests)]
+
+use std::env;
+
+fn main() {
+    unsafe { env::set_var("CLIPPY_TESTS_THIS_IS_OK", "1") }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::env;
+    use std::env::set_var;
+
+    #[test]
+    fn my_test() {
+        unsafe { set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+        //~^ set_env_in_tests
+
+        unsafe { env::set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+        //~^ set_env_in_tests
+
+        unsafe { std::env::set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+        //~^ set_env_in_tests
+    }
+}

tests/ui/set_env_in_tests.stderr

@@ -0,0 +1,28 @@
+error: env::set_var called from a test
+  --> tests/ui/set_env_in_tests.rs:16:18
+   |
+LL |         unsafe { set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: this might indicate state leakage and cause flaky tests
+   = note: `-D clippy::set-env-in-tests` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::set_env_in_tests)]`
+
+error: env::set_var called from a test
+  --> tests/ui/set_env_in_tests.rs:19:18
+   |
+LL |         unsafe { env::set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: this might indicate state leakage and cause flaky tests
+
+error: env::set_var called from a test
+  --> tests/ui/set_env_in_tests.rs:22:18
+   |
+LL |         unsafe { std::env::set_var("CLIPPY_TESTS_THIS_IS_NOT_OK", "1") }
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: this might indicate state leakage and cause flaky tests
+
+error: aborting due to 3 previous errors
+