Auto merge of #10543 - blyxyas:tests_outside_test_module, r=flip1995

Add `tests_outside_test_module` lint

Adds `tests_outside_test_module` from #10506. This PR **doesn't** close the issue, just resolves task 1.

changelog: [`tests_outside_test_module`]: The lint has been added

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

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,

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;
@@ -955,6 +956,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));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/tests_outside_test_module.rs

@@ -0,0 +1,71 @@
+use clippy_utils::{diagnostics::span_lint_and_note, is_in_cfg_test, is_in_test_function};
+use rustc_hir::{intravisit::FnKind, Body, FnDecl};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+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,
+    "A test function is outside the testing module."
+}
+
+declare_lint_pass!(TestsOutsideTestModule => [TESTS_OUTSIDE_TEST_MODULE]);
+
+impl LateLintPass<'_> for TestsOutsideTestModule {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'_>,
+        kind: FnKind<'_>,
+        _: &FnDecl<'_>,
+        body: &Body<'_>,
+        sp: Span,
+        _: LocalDefId,
+    ) {
+        if_chain! {
+            if !matches!(kind, FnKind::Closure);
+            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)]",
+                );
+            }
+        }
+    }
+}

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() {}
+}

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
+