Add lint items_after_test_module

Author: blyxyas

CHANGELOG.md

@@ -4615,6 +4615,7 @@ Released 2018-09-13
 [`invisible_characters`]: https://rust-lang.github.io/rust-clippy/master/index.html#invisible_characters
 [`is_digit_ascii_radix`]: https://rust-lang.github.io/rust-clippy/master/index.html#is_digit_ascii_radix
 [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
+[`items_after_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module
 [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count
 [`iter_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map

clippy_lints/src/declared_lints.rs

@@ -215,6 +215,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS_INFO,
     crate::invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED_INFO,
     crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO,
+    crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO,
     crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO,
     crate::large_const_arrays::LARGE_CONST_ARRAYS_INFO,
     crate::large_enum_variant::LARGE_ENUM_VARIANT_INFO,

clippy_lints/src/items_after_test_module.rs

@@ -0,0 +1,94 @@
+use clippy_utils::{diagnostics::span_lint_and_then, is_in_cfg_test, source::snippet};
+use rustc_errors::Applicability;
+use rustc_hir::{HirId, ItemId, ItemKind, Mod};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{sym, BytePos, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Triggers if an item is declared after the testing module marked with `#[cfg(test)]`.
+    /// ### Why is this bad?
+    /// Having items declared after the testing module is confusing and may lead to bad test coverage.
+    /// ### Example
+    /// ```rust
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     // [...]
+    /// }
+    ///
+    /// fn my_function() {
+    ///     // [...]
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn my_function() {
+    ///     // [...]
+    /// }
+    ///
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     // [...]
+    /// }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub ITEMS_AFTER_TEST_MODULE,
+    style,
+    "An item was found after the testing module `tests`"
+}
+
+declare_lint_pass!(ItemsAfterTestModule => [ITEMS_AFTER_TEST_MODULE]);
+
+impl LateLintPass<'_> for ItemsAfterTestModule {
+    fn check_mod(&mut self, cx: &LateContext<'_>, _: &Mod<'_>, _: HirId) {
+        let mut was_test_mod_visited = false;
+        let mut when_was_visited = 0;
+
+        let hir = cx.tcx.hir();
+        let items = hir.items().collect::<Vec<ItemId>>();
+
+        for (i, itid) in items.iter().enumerate() {
+            let item = hir.item(*itid);
+
+            if_chain! {
+                if was_test_mod_visited;
+                if !matches!(item.kind, ItemKind::Mod(_) | ItemKind::Use(_, _));
+                if !is_in_cfg_test(cx.tcx, itid.hir_id()); // The item isn't in the testing module itself
+                if !in_external_macro(cx.sess(), item.span);
+                then {
+                    was_test_mod_visited = false;
+                    span_lint_and_then(cx, ITEMS_AFTER_TEST_MODULE, item.span, "an item was found after the testing module", |diag| {
+                        diag.multipart_suggestion("move the item to before the testing module was defined", vec![
+                            (item.span, String::new()), // Remove the item
+                            (
+                                Span::new(
+                                hir.item(items[when_was_visited - 1]).span.hi() + BytePos(1),
+                                hir.item(items[when_was_visited]).span.lo() - BytePos(1),
+                                item.span.ctxt(), item.span.parent()),
+
+                                format!("\n{}\n", snippet(cx, item.span, ".."))
+                            ) // ^ Copy the item to the new location
+                        ], Applicability::MachineApplicable);
+                    });
+                }
+            }
+
+            if matches!(item.kind, ItemKind::Mod(_)) {
+                for attr in cx.tcx.get_attrs(item.owner_id.to_def_id(), sym::cfg) {
+                    if_chain! {
+                        if attr.has_name(sym::cfg);
+                        if let Some(mitems) = attr.meta_item_list();
+                        if let [mitem] = &*mitems;
+                        if mitem.has_name(sym::test);
+                        then {
+                            was_test_mod_visited = true;
+                            when_was_visited = i;
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -160,6 +160,7 @@ mod int_plus_one;
 mod invalid_upcast_comparisons;
 mod invalid_utf8_in_unchecked;
 mod items_after_statements;
+mod items_after_test_module;
 mod iter_not_returning_iterator;
 mod large_const_arrays;
 mod large_enum_variant;
@@ -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(items_after_test_module::ItemsAfterTestModule));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

tests/ui/items_after_test_module.fixed

@@ -0,0 +1,17 @@
+// compile-flags: --test
+// run-rustfix
+#![allow(unused)]
+#![warn(clippy::items_after_test_module)]
+
+fn main() {}
+
+fn should_not_lint() {}
+
+fn should_lint() {}
+
+mod tests {
+    #[test]
+    fn hi() {}
+}
+
+

tests/ui/items_after_test_module.rs

@@ -0,0 +1,16 @@
+// compile-flags: --test
+// run-rustfix
+#![allow(unused)]
+#![warn(clippy::items_after_test_module)]
+
+fn main() {}
+
+fn should_not_lint() {}
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn hi() {}
+}
+
+fn should_lint() {}

tests/ui/items_after_test_module.stderr

@@ -0,0 +1,16 @@
+error: an item was found after the testing module
+  --> $DIR/items_after_test_module.rs:16:1
+   |
+LL | fn should_lint() {}
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::items-after-test-module` implied by `-D warnings`
+help: move the item to before the testing module was defined
+   |
+LL + 
+LL + fn should_lint() {}
+LL + 
+   |
+
+error: aborting due to previous error
+