Add `tests_outside_test_module` lint

6 files changed, 134 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 239631777e9..ba10cb53ec9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4936,6 +4936,7 @@ Released 2018-09-13
 [`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
+[`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module
 [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
 [`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display
 [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 7836e5ccb9a..b4510557033 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -577,6 +577,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::swap_ptr_to_ref::SWAP_PTR_TO_REF_INFO,
     crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO,
     crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO,
+    crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
     crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
     crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
     crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index ce055f16240..c63abf0979d 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -291,6 +291,7 @@ mod swap;
 mod swap_ptr_to_ref;
 mod tabs_in_doc_comments;
 mod temporary_assignment;
+mod tests_outside_test_module;
 mod to_digit_is_some;
 mod trailing_empty_array;
 mod trait_bounds;
@@ -951,6 +952,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         ))
     });
     store.register_late_pass(|_| Box::new(lines_filter_map_ok::LinesFilterMapOk));
+    store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule::new()));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/tests_outside_test_module.rs b/clippy_lints/src/tests_outside_test_module.rs
new file mode 100644
index 00000000000..d14a292a52f
--- /dev/null
+++ b/clippy_lints/src/tests_outside_test_module.rs
@@ -0,0 +1,101 @@
+use clippy_utils::{diagnostics::span_lint_and_note, is_in_cfg_test, is_in_test_function, is_test_module_or_function};
+use rustc_data_structures::sync::par_for_each_in;
+use rustc_hir::{intravisit::FnKind, Body, FnDecl, HirId, ItemKind, Mod};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{def_id::LocalDefId, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Triggers when a testing function (marked with the `#[test]` attribute) isn't inside a testing module (marked with `#[cfg(test)]`).
+    ///
+    /// ### Why is this bad?
+    ///
+    /// The idiomatic (and more performant) way of writing tests is inside a testing module (flagged with `#[cfg(test)]`), having test functions outside of this module is confusing and may lead to them being "hidden".
+    ///
+    /// ### Example
+    /// ```rust
+    /// #[test]
+    /// fn my_cool_test() {
+    ///     // [...]
+    /// }
+    ///
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     // [...]
+    /// }
+    ///
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     #[test]
+    ///     fn my_cool_test() {
+    ///         // [...]
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub TESTS_OUTSIDE_TEST_MODULE,
+    restriction,
+    "The test function `my_cool_test` is outside the testing module `tests`."
+}
+
+pub(crate) struct TestsOutsideTestModule {
+    pub test_mod_exists: bool,
+}
+
+impl TestsOutsideTestModule {
+    pub fn new() -> Self {
+        Self { test_mod_exists: false }
+    }
+}
+
+impl_lint_pass!(TestsOutsideTestModule => [TESTS_OUTSIDE_TEST_MODULE]);
+
+impl LateLintPass<'_> for TestsOutsideTestModule {
+    fn check_mod(&mut self, cx: &LateContext<'_>, _: &Mod<'_>, _: HirId) {
+        self.test_mod_exists = false;
+
+        // par_for_each_item uses Fn, while par_for_each_in uses FnMut
+        par_for_each_in(cx.tcx.hir_crate_items(()).items(), |itemid| {
+            let item = cx.tcx.hir().item(itemid);
+            if_chain! {
+                if matches!(item.kind, ItemKind::Mod(_));
+                if is_test_module_or_function(cx.tcx, item);
+                then {
+                    self.test_mod_exists = true;
+                }
+            }
+        });
+    }
+
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'_>,
+        kind: FnKind<'_>,
+        _: &FnDecl<'_>,
+        body: &Body<'_>,
+        sp: Span,
+        _: LocalDefId,
+    ) {
+        if_chain! {
+            if !matches!(kind, FnKind::Closure);
+            if self.test_mod_exists;
+            if is_in_test_function(cx.tcx, body.id().hir_id);
+            if !is_in_cfg_test(cx.tcx, body.id().hir_id);
+            then {
+                span_lint_and_note(
+                    cx,
+                    TESTS_OUTSIDE_TEST_MODULE,
+                    sp,
+                    "this function marked with #[test] is outside a #[cfg(test)] module",
+                    None,
+                    "move it to a testing module marked with #[cfg(test)]",
+                );
+            }
+        }
+    }
+}
diff --git a/tests/ui/tests_outside_test_module.rs b/tests/ui/tests_outside_test_module.rs
new file mode 100644
index 00000000000..1982b1d0107
--- /dev/null
+++ b/tests/ui/tests_outside_test_module.rs
@@ -0,0 +1,18 @@
+// compile-flags: --test
+#![allow(unused)]
+#![warn(clippy::tests_outside_test_module)]
+
+fn main() {
+    // test code goes here
+}
+
+// Should lint
+#[test]
+fn my_test() {}
+
+#[cfg(test)]
+mod tests {
+    // Should not lint
+    #[test]
+    fn my_test() {}
+}
diff --git a/tests/ui/tests_outside_test_module.stderr b/tests/ui/tests_outside_test_module.stderr
new file mode 100644
index 00000000000..125a79d6edf
--- /dev/null
+++ b/tests/ui/tests_outside_test_module.stderr
@@ -0,0 +1,11 @@
+error: this function marked with #[test] is outside a #[cfg(test)] module
+  --> $DIR/tests_outside_test_module.rs:11:1
+   |
+LL | fn my_test() {}
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: move it to a testing module marked with #[cfg(test)]
+   = note: `-D clippy::tests-outside-test-module` implied by `-D warnings`
+
+error: aborting due to previous error
+