diff --git a/CHANGELOG.md b/CHANGELOG.md index 47a503510c10..ab3033ae4e33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4964,6 +4964,7 @@ Released 2018-09-13 [`unchecked_duration_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction [`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks [`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops +[`unflagged_test_modules`]: https://rust-lang.github.io/rust-clippy/master/index.html#unflagged_test_modules [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2331e857b1f7..f048b3551fae 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -606,6 +606,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::types::VEC_BOX_INFO, crate::undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS_INFO, crate::undocumented_unsafe_blocks::UNNECESSARY_SAFETY_COMMENT_INFO, + crate::unflagged_test_modules::UNFLAGGED_TEST_MODULES_INFO, crate::unicode::INVISIBLE_CHARACTERS_INFO, crate::unicode::NON_ASCII_LITERAL_INFO, crate::unicode::UNICODE_NOT_NFC_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 100cba456793..2dbc40e67bc9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -295,6 +295,7 @@ mod trait_bounds; mod transmute; mod types; mod undocumented_unsafe_blocks; +mod unflagged_test_modules; mod unicode; mod uninit_vec; mod unit_return_expecting_ord; @@ -938,6 +939,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped)); store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute)); store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv()))); + store.register_late_pass(|_| Box::new(unflagged_test_modules::UnflaggedTestModules)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unflagged_test_modules.rs b/clippy_lints/src/unflagged_test_modules.rs new file mode 100644 index 000000000000..658d92b04b4a --- /dev/null +++ b/clippy_lints/src/unflagged_test_modules.rs @@ -0,0 +1,73 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::{symbol::sym, BytePos}; + +declare_clippy_lint! { + /// ### What it does + /// + /// Triggers when a testing module (that contains "test" or "tests" in its name) isn't flagged with the `#[cfg(test)]` attribute. + /// + /// ### Why is this bad? + /// + /// The attribute `#[cfg(test)]` is used to tell Rust to compile and run the test code only when you run `cargo test` and not when you run `cargo build`. This saves compile time and space in the resulting compiled artifact because tests are not included. So not using `#[cfg(test)]` for tests is both a waste of time and space. + /// + /// ### Example + /// ```rust + /// mod my_cool_tests { + /// // [...] + /// } + /// ``` + /// Use instead: + /// ```rust + /// #[cfg(test)] + /// mod my_cool_tests { + /// // [...] + /// } + /// ``` + #[clippy::version = "1.70.0"] + pub UNFLAGGED_TEST_MODULES, + pedantic, + "the testing module `my_cool_tests` wasn't marked with `#[cfg(test)]`" +} +declare_lint_pass!(UnflaggedTestModules => [UNFLAGGED_TEST_MODULES]); + +impl LateLintPass<'_> for UnflaggedTestModules { + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) { + if let ItemKind::Mod(_) = item.kind { + // If module name contains *test* or *tests*. + if item + .ident + .name + .to_ident_string() + .split('_') + .any(|seg| seg == "test" || seg == "tests") + { + 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(items) = attr.meta_item_list(); + if let [item] = &*items; + if item.has_name(sym::test); + then { + return; + } + } + } + // If no #[cfg(test)] is found + span_lint_and_sugg( + cx, + UNFLAGGED_TEST_MODULES, + item.ident.span.with_lo( + item.ident.span.lo() - BytePos(4), // Add `mod` keyword + ), + "this testing module isn't flagged with #[cfg(test)]", + "add the attribute", + format!("#[cfg(test)]\nmod {}", item.ident.as_str()), + rustc_errors::Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/tests/ui/unflagged_test_modules.fixed b/tests/ui/unflagged_test_modules.fixed new file mode 100644 index 000000000000..427202338876 --- /dev/null +++ b/tests/ui/unflagged_test_modules.fixed @@ -0,0 +1,18 @@ +// run-rustfix +// compile-flags: --test +#![allow(unused)] +#![warn(clippy::unflagged_test_modules)] + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod tests { + fn my_test() {} +} + +#[cfg(test)] +mod test { + fn my_other_test() {} +} diff --git a/tests/ui/unflagged_test_modules.rs b/tests/ui/unflagged_test_modules.rs new file mode 100644 index 000000000000..602ec95057b8 --- /dev/null +++ b/tests/ui/unflagged_test_modules.rs @@ -0,0 +1,17 @@ +// run-rustfix +// compile-flags: --test +#![allow(unused)] +#![warn(clippy::unflagged_test_modules)] + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod tests { + fn my_test() {} +} + +mod test { + fn my_other_test() {} +} diff --git a/tests/ui/unflagged_test_modules.stderr b/tests/ui/unflagged_test_modules.stderr new file mode 100644 index 000000000000..3dfc408f167c --- /dev/null +++ b/tests/ui/unflagged_test_modules.stderr @@ -0,0 +1,15 @@ +error: this testing module isn't flagged with #[cfg(test)] + --> $DIR/unflagged_test_modules.rs:15:1 + | +LL | mod test { + | ^^^^^^^^ + | + = note: `-D clippy::unflagged-test-modules` implied by `-D warnings` +help: add the attribute + | +LL + #[cfg(test)] +LL ~ mod test { + | + +error: aborting due to previous error +