From a37bdca5d8e67463e30ee09acf9d36b02882c0fc Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Wed, 13 Nov 2024 00:34:34 +0300 Subject: [PATCH 1/7] new lint: `redundant_test_prefix` basic help, no auto-fix suggestion --- CHANGELOG.md | 2 + book/src/lint_configuration.md | 10 +++ clippy_config/src/conf.rs | 3 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/redundant_test_prefix.rs | 87 +++++++++++++++++++++++ tests/ui/redundant_test_prefix.rs | 25 +++++++ tests/ui/redundant_test_prefix.stderr | 14 ++++ 8 files changed, 144 insertions(+) create mode 100644 clippy_lints/src/redundant_test_prefix.rs create mode 100644 tests/ui/redundant_test_prefix.rs create mode 100644 tests/ui/redundant_test_prefix.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index dd3124ee9a3b..440e77386e23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5907,6 +5907,7 @@ Released 2018-09-13 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes +[`redundant_test_prefix`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix [`redundant_type_annotations`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_type_annotations [`ref_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_as_ptr [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference @@ -6235,4 +6236,5 @@ Released 2018-09-13 [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold [`warn-on-all-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-on-all-wildcard-imports [`warn-unsafe-macro-metavars-in-private-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-unsafe-macro-metavars-in-private-macros +[`redundant-test-prefix-in-integration-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-in-integration-tests diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 670b5cbef823..6aed03720bf2 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -982,3 +982,13 @@ Whether to also emit warnings for unsafe blocks with metavariable expansions in --- **Affected lints:** * [`macro_metavars_in_unsafe`](https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe) + + +## `redundant-test-prefix-in-integration-tests` +Whether to include integration tests in the linting process or not. + +**Default Value:** `false` + +--- +**Affected lints:** +* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index c3b1fc83af0a..563521df6471 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -706,6 +706,9 @@ define_Conf! { /// Whether to also emit warnings for unsafe blocks with metavariable expansions in **private** macros. #[lints(macro_metavars_in_unsafe)] warn_unsafe_macro_metavars_in_private_macros: bool = false, + /// Whether to include integration tests in the linting process or not. + #[lints(redundant_test_prefix)] + redundant_test_prefix_in_integration_tests: bool = false, } /// Search for the configuration file. diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index dff60f76b746..2ddcb82e5303 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -642,6 +642,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::redundant_slicing::DEREF_BY_SLICING_INFO, crate::redundant_slicing::REDUNDANT_SLICING_INFO, crate::redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES_INFO, + crate::redundant_test_prefix::REDUNDANT_TEST_PREFIX_INFO, crate::redundant_type_annotations::REDUNDANT_TYPE_ANNOTATIONS_INFO, crate::ref_option_ref::REF_OPTION_REF_INFO, crate::ref_patterns::REF_PATTERNS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c9064df25ac8..c664ab647573 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -313,6 +313,7 @@ mod redundant_locals; mod redundant_pub_crate; mod redundant_slicing; mod redundant_static_lifetimes; +mod redundant_test_prefix; mod redundant_type_annotations; mod ref_option_ref; mod ref_patterns; @@ -963,5 +964,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); + store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix::new(conf))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs new file mode 100644 index 000000000000..e5ffee8f361e --- /dev/null +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -0,0 +1,87 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::{is_in_cfg_test, is_in_test_function}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, FnDecl}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{ impl_lint_pass}; +use rustc_span::Span; +use rustc_span::def_id::LocalDefId; + +declare_clippy_lint! { + /// ### What it does + /// Checks for test functions (functions annotated with `#[test]`) that prefixed with `test_` + /// which is redundant. + /// + /// ### Why is this bad? + /// This is redundant because the test functions are already annotated with `#[test]`. + /// Moreover, it clutters the output of `cargo test` as test functions are expanded as + /// `module::tests::test_name` in the output. + /// + /// ### Example + /// ```no_run + /// #[test] + /// fn test_my_use_case() { + /// // test code + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn my_use_case() { + /// // test code + /// } + /// ``` + #[clippy::version = "1.84.0"] + pub REDUNDANT_TEST_PREFIX, + pedantic, + "redundant `test_` prefix in test function name" +} + +pub struct RedundantTestPrefix { + in_integration_tests: bool, +} + +impl RedundantTestPrefix { + pub fn new(conf: &'static Conf) -> Self { + Self { + in_integration_tests: conf.redundant_test_prefix_in_integration_tests, + } + } +} + +impl_lint_pass!(RedundantTestPrefix => [REDUNDANT_TEST_PREFIX]); + +impl LateLintPass<'_> for RedundantTestPrefix { + fn check_fn( + &mut self, + cx: &LateContext<'_>, + fn_kind: FnKind<'_>, + _: &FnDecl<'_>, + body: &Body<'_>, + span: Span, + _: LocalDefId, + ) { + // Skip the lint if the function is not within a node with `#[cfg(test)]` attribute, + // which is true for integration tests. If the lint is enabled for integration tests, + // via configuration value, ignore this check. + if !(self.in_integration_tests || is_in_cfg_test(cx.tcx, body.id().hir_id)) { + return; + } + + if is_in_test_function(cx.tcx, body.id().hir_id) && has_redundant_test_prefix(fn_kind) { + span_lint_and_help( + cx, + REDUNDANT_TEST_PREFIX, + span, + "redundant `test_` prefix in test function name", + None, + "consider removing the `test_` prefix", + ); + } + } +} + +/// Checks if the function has redundant `test_` prefix in its name. +fn has_redundant_test_prefix(fn_kind: FnKind<'_>) -> bool { + matches!(fn_kind, FnKind::ItemFn(ident, ..) if ident.name.as_str().starts_with("test_")) +} diff --git a/tests/ui/redundant_test_prefix.rs b/tests/ui/redundant_test_prefix.rs new file mode 100644 index 000000000000..a56b7915ac2d --- /dev/null +++ b/tests/ui/redundant_test_prefix.rs @@ -0,0 +1,25 @@ +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_main() { + main(); + } +} + +mod tests_no_annotations { + use super::*; + + #[test] + fn test_main() { + main(); + } +} diff --git a/tests/ui/redundant_test_prefix.stderr b/tests/ui/redundant_test_prefix.stderr new file mode 100644 index 000000000000..c42647f4e7a0 --- /dev/null +++ b/tests/ui/redundant_test_prefix.stderr @@ -0,0 +1,14 @@ +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:13:5 + | +LL | / fn test_main() { +LL | | main(); +LL | | } + | |_____^ + | + = help: consider removing the `test_` prefix + = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` + +error: aborting due to 1 previous error + From de984125da79693a2df4c9599d644a0a3f9e6ee2 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Wed, 13 Nov 2024 13:16:43 +0300 Subject: [PATCH 2/7] refactor: `redundant_test_prefix` configuration tests --- clippy_lints/src/redundant_test_prefix.rs | 2 +- .../redundant_test_prefix/default/clippy.toml | 1 + .../in_integration_tests/clippy.toml | 1 + .../redundant_test_prefix.integ_tests.stderr | 24 +++++++++++++++ .../redundant_test_prefix.rs | 29 +++++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/ui-toml/redundant_test_prefix/default/clippy.toml create mode 100644 tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs index e5ffee8f361e..16b3f5a9febf 100644 --- a/clippy_lints/src/redundant_test_prefix.rs +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -4,7 +4,7 @@ use clippy_utils::{is_in_cfg_test, is_in_test_function}; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, FnDecl}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{ impl_lint_pass}; +use rustc_session::impl_lint_pass; use rustc_span::Span; use rustc_span::def_id::LocalDefId; diff --git a/tests/ui-toml/redundant_test_prefix/default/clippy.toml b/tests/ui-toml/redundant_test_prefix/default/clippy.toml new file mode 100644 index 000000000000..a6a3ec9ba305 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/default/clippy.toml @@ -0,0 +1 @@ +redundant-test-prefix-in-integration-tests = false diff --git a/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml b/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml new file mode 100644 index 000000000000..1f6f792607ff --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml @@ -0,0 +1 @@ +redundant-test-prefix-in-integration-tests = true diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr new file mode 100644 index 000000000000..9a22205bd940 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr @@ -0,0 +1,24 @@ +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:14:5 + | +LL | / fn test_has_annotation() { +LL | | +LL | | } + | |_____^ + | + = help: consider removing the `test_` prefix + = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` + +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:22:1 + | +LL | / fn test_main_module_has_annotation() { +LL | | +LL | | } + | |_^ + | + = help: consider removing the `test_` prefix + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs new file mode 100644 index 000000000000..21dadda13aaf --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs @@ -0,0 +1,29 @@ +//@revisions: default integ_tests +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@compile-flags: --test +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn test_has_annotation() { + //~[integ_tests]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn test_main_module_has_annotation() { + //~[integ_tests]^ redundant_test_prefix +} + +fn test_main_module_no_annotation() {} + +#[cfg(test)] +mod tests {} From c45551c2a36ed63cdd127db3d88009505861abe9 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Tue, 19 Nov 2024 01:15:58 +0300 Subject: [PATCH 3/7] refactor: `redundant_test_prefix` rustfix --- clippy_lints/src/redundant_test_prefix.rs | 102 ++++++--- .../redundant_test_prefix.integ_tests.fixed | 29 +++ .../redundant_test_prefix.integ_tests.stderr | 19 +- tests/ui/redundant_test_prefix.fixed | 202 ++++++++++++++++++ tests/ui/redundant_test_prefix.rs | 179 +++++++++++++++- tests/ui/redundant_test_prefix.stderr | 95 +++++++- 6 files changed, 581 insertions(+), 45 deletions(-) create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed create mode 100644 tests/ui/redundant_test_prefix.fixed diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs index 16b3f5a9febf..6a7b857a6685 100644 --- a/clippy_lints/src/redundant_test_prefix.rs +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -1,33 +1,37 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{is_in_cfg_test, is_in_test_function}; +use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, FnDecl}; +use rustc_hir::{self as hir, Body, ExprKind, FnDecl}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::Span; use rustc_span::def_id::LocalDefId; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does - /// Checks for test functions (functions annotated with `#[test]`) that prefixed with `test_` - /// which is redundant. + /// Checks for test functions (functions annotated with `#[test]`) that are prefixed + /// with `test_` which is redundant. /// /// ### Why is this bad? - /// This is redundant because the test functions are already annotated with `#[test]`. - /// Moreover, it clutters the output of `cargo test` as test functions are expanded as - /// `module::tests::test_name` in the output. + /// This is redundant because test functions are already annotated with `#[test]`. + /// Moreover, it clutters the output of `cargo test` since test functions are expanded as + /// `module::tests::test_use_case` in the output. Without the redundant prefix, the output + /// becomes `module::tests::use_case`, which is more readable. /// /// ### Example /// ```no_run /// #[test] - /// fn test_my_use_case() { + /// fn test_use_case() { /// // test code /// } /// ``` /// Use instead: /// ```no_run - /// fn my_use_case() { + /// fn use_case() { /// // test code /// } /// ``` @@ -51,15 +55,15 @@ impl RedundantTestPrefix { impl_lint_pass!(RedundantTestPrefix => [REDUNDANT_TEST_PREFIX]); -impl LateLintPass<'_> for RedundantTestPrefix { +impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { fn check_fn( &mut self, - cx: &LateContext<'_>, - fn_kind: FnKind<'_>, - _: &FnDecl<'_>, - body: &Body<'_>, - span: Span, - _: LocalDefId, + cx: &LateContext<'tcx>, + kind: FnKind<'_>, + _decl: &FnDecl<'_>, + body: &'tcx Body<'_>, + _span: Span, + _fn_def_id: LocalDefId, ) { // Skip the lint if the function is not within a node with `#[cfg(test)]` attribute, // which is true for integration tests. If the lint is enabled for integration tests, @@ -68,20 +72,70 @@ impl LateLintPass<'_> for RedundantTestPrefix { return; } - if is_in_test_function(cx.tcx, body.id().hir_id) && has_redundant_test_prefix(fn_kind) { - span_lint_and_help( + // Ignore methods and closures. + let FnKind::ItemFn(ref ident, ..) = kind else { + return; + }; + + if is_in_test_function(cx.tcx, body.id().hir_id) && ident.as_str().starts_with("test_") { + let mut non_prefixed = ident.as_str().trim_start_matches("test_").to_string(); + let mut help_msg = "consider removing the `test_` prefix"; + // If `non_prefixed` conflicts with another function in the same module/scope, + // add extra suffix to avoid name conflict. + if name_conflicts(cx, body, &non_prefixed) { + non_prefixed.push_str("_works"); + help_msg = "consider removing the `test_` prefix (suffix avoids name conflict)"; + } + span_lint_and_sugg( cx, REDUNDANT_TEST_PREFIX, - span, + ident.span, "redundant `test_` prefix in test function name", - None, - "consider removing the `test_` prefix", + help_msg, + non_prefixed, + Applicability::MachineApplicable, ); } } } -/// Checks if the function has redundant `test_` prefix in its name. -fn has_redundant_test_prefix(fn_kind: FnKind<'_>) -> bool { - matches!(fn_kind, FnKind::ItemFn(ident, ..) if ident.name.as_str().starts_with("test_")) +/// Checks whether removal of the `_test` prefix from the function name will cause a name conflict. +/// +/// There should be no other function with the same name in the same module/scope. Also, there +/// should not be any function call with the same name within the body of the function, to avoid +/// recursion. +fn name_conflicts<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'_>, fn_name: &str) -> bool { + let tcx = cx.tcx; + let id = body.id().hir_id; + + // Iterate over items in the same module/scope + let (module, _module_span, _module_hir) = tcx.hir().get_module(tcx.parent_module(id)); + for item in module.item_ids { + let item = tcx.hir().item(*item); + if let hir::ItemKind::Fn(..) = item.kind { + if item.ident.name.as_str() == fn_name { + // Name conflict found + return true; + } + } + } + + // Also check that within the body of the function there is also no function call + // with the same name (since it will result in recursion) + for_each_expr(cx, body, |expr| { + if let ExprKind::Call(path_expr, _) = expr.kind { + if let ExprKind::Path(qpath) = &path_expr.kind { + if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() { + if let Some(name) = tcx.opt_item_name(def_id) { + if name.as_str() == fn_name { + // Function call with the same name found + return ControlFlow::Break(()); + } + } + } + } + } + ControlFlow::Continue(()) + }) + .is_some() } diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed new file mode 100644 index 000000000000..2bbfebf8985e --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed @@ -0,0 +1,29 @@ +//@revisions: default integ_tests +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@compile-flags: --test +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn has_annotation() { + //~[integ_tests]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn main_module_has_annotation() { + //~[integ_tests]^ redundant_test_prefix +} + +fn test_main_module_no_annotation() {} + +#[cfg(test)] +mod tests {} diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr index 9a22205bd940..2af1681949aa 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr @@ -1,24 +1,17 @@ error: redundant `test_` prefix in test function name - --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:14:5 + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:14:8 | -LL | / fn test_has_annotation() { -LL | | -LL | | } - | |_____^ +LL | fn test_has_annotation() { + | ^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `has_annotation` | - = help: consider removing the `test_` prefix = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` error: redundant `test_` prefix in test function name - --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:22:1 + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:22:4 | -LL | / fn test_main_module_has_annotation() { -LL | | -LL | | } - | |_^ - | - = help: consider removing the `test_` prefix +LL | fn test_main_module_has_annotation() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `main_module_has_annotation` error: aborting due to 2 previous errors diff --git a/tests/ui/redundant_test_prefix.fixed b/tests/ui/redundant_test_prefix.fixed new file mode 100644 index 000000000000..7263a3ad0ba1 --- /dev/null +++ b/tests/ui/redundant_test_prefix.fixed @@ -0,0 +1,202 @@ +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() { + // Normal function, no redundant prefix. +} + +fn f1() { + // Normal function, no redundant prefix. +} + +fn test_f2() { + // Has prefix, but no `#[test]` attribute, ignore. +} + +#[test] +fn test_f3() { + // Has prefix, has `#[test]` attribute. + // Not within a `#[cfg(test)]`, ignore (by default, configurable -- see TOML tests). +} + +#[cfg(test)] +#[test] +fn f4() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + //~| NOTE: `-D clippy::redundant-test-prefix` implied by `-D warnings` + + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision with other functions, should `test_` prefix be removed. +} + +fn f5() {} + +#[cfg(test)] +#[test] +fn f5_works() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix `_works` is added. +} + +mod m1 { + pub fn f6() {} + pub fn f7() {} +} + +#[cfg(test)] +#[test] +fn f6_works() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + use m1::f6; + + f6(); + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision, but has a function call that will result in recursion. +} + +#[cfg(test)] +#[test] +fn f8() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + + use m1::f7; + + f7(); + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision, has function call, but it will not result in recursion. +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn main_works() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + main(); + } + + #[test] + fn foo() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn foo_with_call() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + + main(); + } + + #[test] + fn f1() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f2() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f3() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f4() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f5() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f6() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f7() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn f8() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } +} + +mod tests_no_annotations { + use super::*; + + use super::*; + + #[test] + fn test_main() { + main(); + } + + #[test] + fn test_foo() { + } + + #[test] + fn test_foo_with_call() { + main(); + } + + #[test] + fn test_f1() { + } + + #[test] + fn test_f2() { + } + + #[test] + fn test_f3() { + } + + #[test] + fn test_f4() { + } + + #[test] + fn test_f5() { + } + + #[test] + fn test_f6() { + } + + #[test] + fn test_f7() { + } + + #[test] + fn test_f8() { + } +} diff --git a/tests/ui/redundant_test_prefix.rs b/tests/ui/redundant_test_prefix.rs index a56b7915ac2d..872536958844 100644 --- a/tests/ui/redundant_test_prefix.rs +++ b/tests/ui/redundant_test_prefix.rs @@ -2,7 +2,76 @@ #![warn(clippy::redundant_test_prefix)] fn main() { - // test code goes here + // Normal function, no redundant prefix. +} + +fn f1() { + // Normal function, no redundant prefix. +} + +fn test_f2() { + // Has prefix, but no `#[test]` attribute, ignore. +} + +#[test] +fn test_f3() { + // Has prefix, has `#[test]` attribute. + // Not within a `#[cfg(test)]`, ignore (by default, configurable -- see TOML tests). +} + +#[cfg(test)] +#[test] +fn test_f4() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + //~| NOTE: `-D clippy::redundant-test-prefix` implied by `-D warnings` + + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision with other functions, should `test_` prefix be removed. +} + +fn f5() {} + +#[cfg(test)] +#[test] +fn test_f5() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix `_works` is added. +} + +mod m1 { + pub fn f6() {} + pub fn f7() {} +} + +#[cfg(test)] +#[test] +fn test_f6() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + use m1::f6; + + f6(); + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision, but has a function call that will result in recursion. +} + +#[cfg(test)] +#[test] +fn test_f8() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + + use m1::f7; + + f7(); + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // No collision, has function call, but it will not result in recursion. } #[cfg(test)] @@ -11,15 +80,123 @@ mod tests { #[test] fn test_main() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + main(); + } + + #[test] + fn test_foo() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_foo_with_call() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + main(); } + + #[test] + fn test_f1() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f2() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f3() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f4() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f5() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f6() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f7() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } + + #[test] + fn test_f8() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix + } } mod tests_no_annotations { use super::*; + use super::*; + #[test] fn test_main() { main(); } + + #[test] + fn test_foo() { + } + + #[test] + fn test_foo_with_call() { + main(); + } + + #[test] + fn test_f1() { + } + + #[test] + fn test_f2() { + } + + #[test] + fn test_f3() { + } + + #[test] + fn test_f4() { + } + + #[test] + fn test_f5() { + } + + #[test] + fn test_f6() { + } + + #[test] + fn test_f7() { + } + + #[test] + fn test_f8() { + } } diff --git a/tests/ui/redundant_test_prefix.stderr b/tests/ui/redundant_test_prefix.stderr index c42647f4e7a0..35adaa26095f 100644 --- a/tests/ui/redundant_test_prefix.stderr +++ b/tests/ui/redundant_test_prefix.stderr @@ -1,14 +1,95 @@ error: redundant `test_` prefix in test function name - --> tests/ui/redundant_test_prefix.rs:13:5 + --> tests/ui/redundant_test_prefix.rs:24:4 | -LL | / fn test_main() { -LL | | main(); -LL | | } - | |_____^ +LL | fn test_f4() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f4` | - = help: consider removing the `test_` prefix = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` -error: aborting due to 1 previous error +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:37:4 + | +LL | fn test_f5() { + | ^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `f5_works` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:53:4 + | +LL | fn test_f6() { + | ^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `f6_works` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:66:4 + | +LL | fn test_f8() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f8` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:82:8 + | +LL | fn test_main() { + | ^^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `main_works` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:90:8 + | +LL | fn test_foo() { + | ^^^^^^^^ help: consider removing the `test_` prefix: `foo` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:96:8 + | +LL | fn test_foo_with_call() { + | ^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `foo_with_call` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:104:8 + | +LL | fn test_f1() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f1` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:110:8 + | +LL | fn test_f2() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f2` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:116:8 + | +LL | fn test_f3() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f3` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:122:8 + | +LL | fn test_f4() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f4` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:128:8 + | +LL | fn test_f5() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f5` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:134:8 + | +LL | fn test_f6() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f6` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:140:8 + | +LL | fn test_f7() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f7` + +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:146:8 + | +LL | fn test_f8() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f8` + +error: aborting due to 15 previous errors From c9cecf2490b99bed6a3c1d193b5cf86d9f800d22 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Tue, 19 Nov 2024 02:05:12 +0300 Subject: [PATCH 4/7] refactor: `redundant_test_prefix` make suffix configurable --- CHANGELOG.md | 1 + book/src/lint_configuration.md | 14 +++++ clippy_config/src/conf.rs | 7 +++ clippy_lints/src/redundant_test_prefix.rs | 4 +- .../custom_suffix_tests/clippy.toml | 2 + .../redundant_test_prefix.default.fixed | 55 +++++++++++++++++++ .../redundant_test_prefix.default.stderr | 11 ++++ .../redundant_test_prefix.integ_tests.fixed | 28 +++++++++- .../redundant_test_prefix.integ_tests.stderr | 18 +++++- .../redundant_test_prefix.rs | 28 +++++++++- .../redundant_test_prefix.suffix_tests.fixed | 55 +++++++++++++++++++ .../redundant_test_prefix.suffix_tests.stderr | 29 ++++++++++ 12 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.stderr create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed create mode 100644 tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 440e77386e23..18dd18627952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6237,4 +6237,5 @@ Released 2018-09-13 [`warn-on-all-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-on-all-wildcard-imports [`warn-unsafe-macro-metavars-in-private-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-unsafe-macro-metavars-in-private-macros [`redundant-test-prefix-in-integration-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-in-integration-tests +[`redundant-test-prefix-custom-suffix`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-custom-suffix diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 6aed03720bf2..f7601e77bc08 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -992,3 +992,17 @@ Whether to include integration tests in the linting process or not. --- **Affected lints:** * [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) + + +## `redundant-test-prefix-custom-suffix` +What suffix to use to avoid function name collisions when `test_` prefix is removed. + +If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. +Suffix is added only when there is a collision with an existing function name, +otherwise just `test_` prefix is removed (and no suffix added). + +**Default Value:** `"_works"` + +--- +**Affected lints:** +* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 563521df6471..181235d32408 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -709,6 +709,13 @@ define_Conf! { /// Whether to include integration tests in the linting process or not. #[lints(redundant_test_prefix)] redundant_test_prefix_in_integration_tests: bool = false, + /// What suffix to use to avoid function name collisions when `test_` prefix is removed. + /// + /// If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. + /// Suffix is added only when there is a collision with an existing function name, + /// otherwise just `test_` prefix is removed (and no suffix added). + #[lints(redundant_test_prefix)] + redundant_test_prefix_custom_suffix: String = String::from("_works"), } /// Search for the configuration file. diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs index 6a7b857a6685..c5f3f553fbd7 100644 --- a/clippy_lints/src/redundant_test_prefix.rs +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -43,12 +43,14 @@ declare_clippy_lint! { pub struct RedundantTestPrefix { in_integration_tests: bool, + custom_sufix: String, } impl RedundantTestPrefix { pub fn new(conf: &'static Conf) -> Self { Self { in_integration_tests: conf.redundant_test_prefix_in_integration_tests, + custom_sufix: conf.redundant_test_prefix_custom_suffix.clone(), } } } @@ -83,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { // If `non_prefixed` conflicts with another function in the same module/scope, // add extra suffix to avoid name conflict. if name_conflicts(cx, body, &non_prefixed) { - non_prefixed.push_str("_works"); + non_prefixed.push_str(&self.custom_sufix); help_msg = "consider removing the `test_` prefix (suffix avoids name conflict)"; } span_lint_and_sugg( diff --git a/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml b/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml new file mode 100644 index 000000000000..85b11330d071 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml @@ -0,0 +1,2 @@ +redundant-test-prefix-custom-suffix = "_ok" +redundant-test-prefix-in-integration-tests = true diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed new file mode 100644 index 000000000000..3f30d4360adc --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed @@ -0,0 +1,55 @@ +//@revisions: default integ_tests suffix_tests +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@compile-flags: --test +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn test_has_annotation() { + //~[integ_tests]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn test_main_module_has_annotation() { + //~[integ_tests]^ redundant_test_prefix +} + +fn test_main_module_no_annotation() {} + +fn foo() {} + +#[cfg(test)] +#[test] +fn foo_works() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix is added. +} + +fn bar() {} + +#[test] +fn test_bar() { + //~[suffix_tests]^ redundant_test_prefix + + todo!() + // Has prefix, has `#[test]` attribute. + // NOT within a `#[cfg(test)]`, but the lint is enabled for integration tests. + // Collision with existing function, so suffix is added. +} + +#[cfg(test)] +mod tests {} diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.stderr new file mode 100644 index 000000000000..368e8b87aea8 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.stderr @@ -0,0 +1,11 @@ +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:33:4 + | +LL | fn test_foo() { + | ^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `foo_works` + | + = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed index 2bbfebf8985e..f690afe84ffd 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed @@ -1,6 +1,7 @@ -//@revisions: default integ_tests +//@revisions: default integ_tests suffix_tests //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default //@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -25,5 +26,30 @@ fn main_module_has_annotation() { fn test_main_module_no_annotation() {} +fn foo() {} + +#[cfg(test)] +#[test] +fn foo_works() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix is added. +} + +fn bar() {} + +#[test] +fn bar_works() { + //~[suffix_tests]^ redundant_test_prefix + + todo!() + // Has prefix, has `#[test]` attribute. + // NOT within a `#[cfg(test)]`, but the lint is enabled for integration tests. + // Collision with existing function, so suffix is added. +} + #[cfg(test)] mod tests {} diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr index 2af1681949aa..c8a63c0ab078 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr @@ -1,5 +1,5 @@ error: redundant `test_` prefix in test function name - --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:14:8 + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:15:8 | LL | fn test_has_annotation() { | ^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `has_annotation` @@ -8,10 +8,22 @@ LL | fn test_has_annotation() { = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` error: redundant `test_` prefix in test function name - --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:22:4 + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:23:4 | LL | fn test_main_module_has_annotation() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `main_module_has_annotation` -error: aborting due to 2 previous errors +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:33:4 + | +LL | fn test_foo() { + | ^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `foo_works` + +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:45:4 + | +LL | fn test_bar() { + | ^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `bar_works` + +error: aborting due to 4 previous errors diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs index 21dadda13aaf..5053b69da07f 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs @@ -1,6 +1,7 @@ -//@revisions: default integ_tests +//@revisions: default integ_tests suffix_tests //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default //@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -25,5 +26,30 @@ fn test_main_module_has_annotation() { fn test_main_module_no_annotation() {} +fn foo() {} + +#[cfg(test)] +#[test] +fn test_foo() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix is added. +} + +fn bar() {} + +#[test] +fn test_bar() { + //~[suffix_tests]^ redundant_test_prefix + + todo!() + // Has prefix, has `#[test]` attribute. + // NOT within a `#[cfg(test)]`, but the lint is enabled for integration tests. + // Collision with existing function, so suffix is added. +} + #[cfg(test)] mod tests {} diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed new file mode 100644 index 000000000000..18970949ce04 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed @@ -0,0 +1,55 @@ +//@revisions: default integ_tests suffix_tests +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests +//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@compile-flags: --test +#![allow(dead_code)] +#![warn(clippy::redundant_test_prefix)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn has_annotation() { + //~[integ_tests]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn main_module_has_annotation() { + //~[integ_tests]^ redundant_test_prefix +} + +fn test_main_module_no_annotation() {} + +fn foo() {} + +#[cfg(test)] +#[test] +fn foo_ok() { + //~^ ERROR: redundant `test_` prefix in test function name + //~| HELP: consider removing the `test_` prefix (suffix avoids name conflict) + + todo!() + // Has prefix, has `#[test]` attribute, within a `#[cfg(test)]`. + // Collision with existing function, so suffix is added. +} + +fn bar() {} + +#[test] +fn bar_ok() { + //~[suffix_tests]^ redundant_test_prefix + + todo!() + // Has prefix, has `#[test]` attribute. + // NOT within a `#[cfg(test)]`, but the lint is enabled for integration tests. + // Collision with existing function, so suffix is added. +} + +#[cfg(test)] +mod tests {} diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr new file mode 100644 index 000000000000..e9614161c485 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr @@ -0,0 +1,29 @@ +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:15:8 + | +LL | fn test_has_annotation() { + | ^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `has_annotation` + | + = note: `-D clippy::redundant-test-prefix` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_test_prefix)]` + +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:23:4 + | +LL | fn test_main_module_has_annotation() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `test_` prefix: `main_module_has_annotation` + +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:33:4 + | +LL | fn test_foo() { + | ^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `foo_ok` + +error: redundant `test_` prefix in test function name + --> tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs:45:4 + | +LL | fn test_bar() { + | ^^^^^^^^ help: consider removing the `test_` prefix (suffix avoids name conflict): `bar_ok` + +error: aborting due to 4 previous errors + From e48de8afaa50e360b0af1af8ff843dd2ddc3f987 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Tue, 19 Nov 2024 22:49:27 +0300 Subject: [PATCH 5/7] fix: `redundant_test_prefix` allow nested prefixed functions --- clippy_lints/src/redundant_test_prefix.rs | 16 ++++----- clippy_utils/src/lib.rs | 22 ++++++++++++ tests/ui/redundant_test_prefix.fixed | 43 ++++++++++++++--------- tests/ui/redundant_test_prefix.rs | 43 ++++++++++++++--------- 4 files changed, 82 insertions(+), 42 deletions(-) diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs index c5f3f553fbd7..993d4ee5be68 100644 --- a/clippy_lints/src/redundant_test_prefix.rs +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -1,7 +1,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::visitors::for_each_expr; -use clippy_utils::{is_in_cfg_test, is_in_test_function}; +use clippy_utils::{is_in_cfg_test, is_test_function}; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{self as hir, Body, ExprKind, FnDecl}; @@ -65,8 +65,13 @@ impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { _decl: &FnDecl<'_>, body: &'tcx Body<'_>, _span: Span, - _fn_def_id: LocalDefId, + fn_def_id: LocalDefId, ) { + // Ignore methods and closures. + let FnKind::ItemFn(ref ident, ..) = kind else { + return; + }; + // Skip the lint if the function is not within a node with `#[cfg(test)]` attribute, // which is true for integration tests. If the lint is enabled for integration tests, // via configuration value, ignore this check. @@ -74,12 +79,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { return; } - // Ignore methods and closures. - let FnKind::ItemFn(ref ident, ..) = kind else { - return; - }; - - if is_in_test_function(cx.tcx, body.id().hir_id) && ident.as_str().starts_with("test_") { + if is_test_function(cx.tcx, cx.tcx.local_def_id_to_hir_id(fn_def_id)) && ident.as_str().starts_with("test_") { let mut non_prefixed = ident.as_str().trim_start_matches("test_").to_string(); let mut help_msg = "consider removing the `test_` prefix"; // If `non_prefixed` conflicts with another function in the same module/scope, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 6ad796fdd61f..31b4e5a1964f 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2606,6 +2606,28 @@ pub fn is_in_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool { }) } +/// Checks if `id` has a `#[test]` attribute applied +/// +/// This only checks directly applied attributes, to see if a node has a parent function marked with +/// `#[test]` use [`is_in_test_function`]. +/// +/// Note: Add `//@compile-flags: --test` to UI tests with a `#[test]` function +pub fn is_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool { + with_test_item_names(tcx, tcx.parent_module(id), |names| { + let node = tcx.hir_node(id); + once((id, node)).any(|(_id, node)| { + if let Node::Item(item) = node { + if let ItemKind::Fn(_, _, _) = item.kind { + // Note that we have sorted the item names in the visitor, + // so the binary_search gets the same as `contains`, but faster. + return names.binary_search(&item.ident.name).is_ok(); + } + } + false + }) + }) +} + /// Checks if `id` has a `#[cfg(test)]` attribute applied /// /// This only checks directly applied attributes, to see if a node is inside a `#[cfg(test)]` parent diff --git a/tests/ui/redundant_test_prefix.fixed b/tests/ui/redundant_test_prefix.fixed index 7263a3ad0ba1..26c5b82de567 100644 --- a/tests/ui/redundant_test_prefix.fixed +++ b/tests/ui/redundant_test_prefix.fixed @@ -160,8 +160,7 @@ mod tests_no_annotations { } #[test] - fn test_foo() { - } + fn test_foo() {} #[test] fn test_foo_with_call() { @@ -169,34 +168,44 @@ mod tests_no_annotations { } #[test] - fn test_f1() { - } + fn test_f1() {} #[test] - fn test_f2() { - } + fn test_f2() {} #[test] - fn test_f3() { - } + fn test_f3() {} #[test] - fn test_f4() { - } + fn test_f4() {} #[test] - fn test_f5() { - } + fn test_f5() {} #[test] - fn test_f6() { - } + fn test_f6() {} #[test] - fn test_f7() { - } + fn test_f7() {} #[test] - fn test_f8() { + fn test_f8() {} +} + +// This test is inspired by real test in `clippy_utils/src/sugg.rs`. +// The `is_in_test_function()` checks whether any identifier within a given node's parents is +// marked with `#[test]` attribute. Thus flagging false positives when nested functions are +// prefixed with `test_`. Therefore `is_test_function()` has been defined in `clippy_utils`, +// allowing to select only functions that are immediately marked with `#[test]` annotation. +// +// This test case ensures that for such nested functions no error is emitted. +#[test] +fn not_op() { + fn test_not(foo: bool) { + assert!(foo); } + + // Use helper function + test_not(true); + test_not(false); } diff --git a/tests/ui/redundant_test_prefix.rs b/tests/ui/redundant_test_prefix.rs index 872536958844..a14e62133bef 100644 --- a/tests/ui/redundant_test_prefix.rs +++ b/tests/ui/redundant_test_prefix.rs @@ -160,8 +160,7 @@ mod tests_no_annotations { } #[test] - fn test_foo() { - } + fn test_foo() {} #[test] fn test_foo_with_call() { @@ -169,34 +168,44 @@ mod tests_no_annotations { } #[test] - fn test_f1() { - } + fn test_f1() {} #[test] - fn test_f2() { - } + fn test_f2() {} #[test] - fn test_f3() { - } + fn test_f3() {} #[test] - fn test_f4() { - } + fn test_f4() {} #[test] - fn test_f5() { - } + fn test_f5() {} #[test] - fn test_f6() { - } + fn test_f6() {} #[test] - fn test_f7() { - } + fn test_f7() {} #[test] - fn test_f8() { + fn test_f8() {} +} + +// This test is inspired by real test in `clippy_utils/src/sugg.rs`. +// The `is_in_test_function()` checks whether any identifier within a given node's parents is +// marked with `#[test]` attribute. Thus flagging false positives when nested functions are +// prefixed with `test_`. Therefore `is_test_function()` has been defined in `clippy_utils`, +// allowing to select only functions that are immediately marked with `#[test]` annotation. +// +// This test case ensures that for such nested functions no error is emitted. +#[test] +fn not_op() { + fn test_not(foo: bool) { + assert!(foo); } + + // Use helper function + test_not(true); + test_not(false); } From 24aa7838f663e70eb2ed9c9bce8902a764640307 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Tue, 19 Nov 2024 23:54:58 +0300 Subject: [PATCH 6/7] refactor: `redundant_test_prefix` rename option --- CHANGELOG.md | 4 +- book/src/lint_configuration.md | 52 ++++++++++--------- clippy_lints/src/redundant_test_prefix.rs | 12 ++--- .../custom_suffix/clippy.toml | 2 + .../custom_suffix_tests/clippy.toml | 2 - .../redundant_test_prefix/default/clippy.toml | 2 +- .../in_integration_tests/clippy.toml | 1 - .../outside_cfg_test/clippy.toml | 1 + ...redundant_test_prefix.custom_suffix.fixed} | 12 ++--- ...edundant_test_prefix.custom_suffix.stderr} | 0 .../redundant_test_prefix.default.fixed | 12 ++--- ...undant_test_prefix.outside_cfg_test.fixed} | 12 ++--- ...ndant_test_prefix.outside_cfg_test.stderr} | 0 .../redundant_test_prefix.rs | 12 ++--- 14 files changed, 64 insertions(+), 60 deletions(-) create mode 100644 tests/ui-toml/redundant_test_prefix/custom_suffix/clippy.toml delete mode 100644 tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml delete mode 100644 tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml create mode 100644 tests/ui-toml/redundant_test_prefix/outside_cfg_test/clippy.toml rename tests/ui-toml/redundant_test_prefix/{redundant_test_prefix.suffix_tests.fixed => redundant_test_prefix.custom_suffix.fixed} (71%) rename tests/ui-toml/redundant_test_prefix/{redundant_test_prefix.suffix_tests.stderr => redundant_test_prefix.custom_suffix.stderr} (100%) rename tests/ui-toml/redundant_test_prefix/{redundant_test_prefix.integ_tests.fixed => redundant_test_prefix.outside_cfg_test.fixed} (71%) rename tests/ui-toml/redundant_test_prefix/{redundant_test_prefix.integ_tests.stderr => redundant_test_prefix.outside_cfg_test.stderr} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18dd18627952..5788e44c9534 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6215,6 +6215,8 @@ Released 2018-09-13 [`msrv`]: https://doc.rust-lang.org/clippy/lint_configuration.html#msrv [`pass-by-value-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pass-by-value-size-limit [`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior +[`redundant-test-prefix-check-outside-cfg-test`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-check-outside-cfg-test +[`redundant-test-prefix-custom-suffix`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-custom-suffix [`semicolon-inside-block-ignore-singleline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-inside-block-ignore-singleline [`semicolon-outside-block-ignore-multiline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-outside-block-ignore-multiline [`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold @@ -6236,6 +6238,4 @@ Released 2018-09-13 [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold [`warn-on-all-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-on-all-wildcard-imports [`warn-unsafe-macro-metavars-in-private-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-unsafe-macro-metavars-in-private-macros -[`redundant-test-prefix-in-integration-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-in-integration-tests -[`redundant-test-prefix-custom-suffix`]: https://doc.rust-lang.org/clippy/lint_configuration.html#redundant-test-prefix-custom-suffix diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index f7601e77bc08..88831c03c92c 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -764,6 +764,34 @@ exported visibility, or whether they are marked as "pub". * [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields) +## `redundant-test-prefix-check-outside-cfg-test` +Whether to include functions outside of `#[cfg(test)]` in the linting process or not. + +This option allows running the lint against the integration tests: test functions located +there are not inside a node marked with `#[cfg(test)]` annotation (although they are +still marked using `#[test]` annotation and thus can have redundant "test_" prefix). + +**Default Value:** `false` + +--- +**Affected lints:** +* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) + + +## `redundant-test-prefix-custom-suffix` +What suffix to use to avoid function name collisions when `test_` prefix is removed. + +If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. +Suffix is added only when there is a collision with an existing function name, +otherwise just `test_` prefix is removed (and no suffix added). + +**Default Value:** `"_works"` + +--- +**Affected lints:** +* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) + + ## `semicolon-inside-block-ignore-singleline` Whether to lint only if it's multiline. @@ -982,27 +1010,3 @@ Whether to also emit warnings for unsafe blocks with metavariable expansions in --- **Affected lints:** * [`macro_metavars_in_unsafe`](https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe) - - -## `redundant-test-prefix-in-integration-tests` -Whether to include integration tests in the linting process or not. - -**Default Value:** `false` - ---- -**Affected lints:** -* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) - - -## `redundant-test-prefix-custom-suffix` -What suffix to use to avoid function name collisions when `test_` prefix is removed. - -If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. -Suffix is added only when there is a collision with an existing function name, -otherwise just `test_` prefix is removed (and no suffix added). - -**Default Value:** `"_works"` - ---- -**Affected lints:** -* [`redundant_test_prefix`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_test_prefix) diff --git a/clippy_lints/src/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs index 993d4ee5be68..0f84e1a91de2 100644 --- a/clippy_lints/src/redundant_test_prefix.rs +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -42,14 +42,14 @@ declare_clippy_lint! { } pub struct RedundantTestPrefix { - in_integration_tests: bool, + check_outside_test_cfg: bool, custom_sufix: String, } impl RedundantTestPrefix { pub fn new(conf: &'static Conf) -> Self { Self { - in_integration_tests: conf.redundant_test_prefix_in_integration_tests, + check_outside_test_cfg: conf.redundant_test_prefix_check_outside_cfg_test, custom_sufix: conf.redundant_test_prefix_custom_suffix.clone(), } } @@ -72,10 +72,10 @@ impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { return; }; - // Skip the lint if the function is not within a node with `#[cfg(test)]` attribute, - // which is true for integration tests. If the lint is enabled for integration tests, - // via configuration value, ignore this check. - if !(self.in_integration_tests || is_in_cfg_test(cx.tcx, body.id().hir_id)) { + // Skip the lint if the function is not within a node marked with `#[cfg(test)]` attribute. + // Continue if the function is inside the node marked with `#[cfg(test)]` or lint is enforced + // via configuration (most likely to include integration tests in lint's scope). + if !(self.check_outside_test_cfg || is_in_cfg_test(cx.tcx, body.id().hir_id)) { return; } diff --git a/tests/ui-toml/redundant_test_prefix/custom_suffix/clippy.toml b/tests/ui-toml/redundant_test_prefix/custom_suffix/clippy.toml new file mode 100644 index 000000000000..4e37a5ac69bd --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/custom_suffix/clippy.toml @@ -0,0 +1,2 @@ +redundant-test-prefix-custom-suffix = "_ok" +redundant-test-prefix-check-outside-cfg-test = true diff --git a/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml b/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml deleted file mode 100644 index 85b11330d071..000000000000 --- a/tests/ui-toml/redundant_test_prefix/custom_suffix_tests/clippy.toml +++ /dev/null @@ -1,2 +0,0 @@ -redundant-test-prefix-custom-suffix = "_ok" -redundant-test-prefix-in-integration-tests = true diff --git a/tests/ui-toml/redundant_test_prefix/default/clippy.toml b/tests/ui-toml/redundant_test_prefix/default/clippy.toml index a6a3ec9ba305..ba28999fdaa6 100644 --- a/tests/ui-toml/redundant_test_prefix/default/clippy.toml +++ b/tests/ui-toml/redundant_test_prefix/default/clippy.toml @@ -1 +1 @@ -redundant-test-prefix-in-integration-tests = false +redundant-test-prefix-check-outside-cfg-test = false diff --git a/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml b/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml deleted file mode 100644 index 1f6f792607ff..000000000000 --- a/tests/ui-toml/redundant_test_prefix/in_integration_tests/clippy.toml +++ /dev/null @@ -1 +0,0 @@ -redundant-test-prefix-in-integration-tests = true diff --git a/tests/ui-toml/redundant_test_prefix/outside_cfg_test/clippy.toml b/tests/ui-toml/redundant_test_prefix/outside_cfg_test/clippy.toml new file mode 100644 index 000000000000..84707b1b0849 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/outside_cfg_test/clippy.toml @@ -0,0 +1 @@ +redundant-test-prefix-check-outside-cfg-test = true diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.fixed similarity index 71% rename from tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed rename to tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.fixed index 18970949ce04..7fb3e9c373b1 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.fixed +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.fixed @@ -1,7 +1,7 @@ -//@revisions: default integ_tests suffix_tests +//@revisions: default outside_cfg_test custom_suffix //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default -//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests -//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@[outside_cfg_test] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/outside_cfg_test +//@[custom_suffix] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -13,7 +13,7 @@ mod tests_no_annotations { #[test] fn has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn no_annotation() {} @@ -21,7 +21,7 @@ mod tests_no_annotations { #[test] fn main_module_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn test_main_module_no_annotation() {} @@ -43,7 +43,7 @@ fn bar() {} #[test] fn bar_ok() { - //~[suffix_tests]^ redundant_test_prefix + //~[custom_suffix]^ redundant_test_prefix todo!() // Has prefix, has `#[test]` attribute. diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.stderr similarity index 100% rename from tests/ui-toml/redundant_test_prefix/redundant_test_prefix.suffix_tests.stderr rename to tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.stderr diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed index 3f30d4360adc..c0e65de285fa 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed @@ -1,7 +1,7 @@ -//@revisions: default integ_tests suffix_tests +//@revisions: default outside_cfg_test custom_suffix //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default -//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests -//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@[outside_cfg_test] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/outside_cfg_test +//@[custom_suffix] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -13,7 +13,7 @@ mod tests_no_annotations { #[test] fn test_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn no_annotation() {} @@ -21,7 +21,7 @@ mod tests_no_annotations { #[test] fn test_main_module_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn test_main_module_no_annotation() {} @@ -43,7 +43,7 @@ fn bar() {} #[test] fn test_bar() { - //~[suffix_tests]^ redundant_test_prefix + //~[custom_suffix]^ redundant_test_prefix todo!() // Has prefix, has `#[test]` attribute. diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.fixed similarity index 71% rename from tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed rename to tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.fixed index f690afe84ffd..8cf2c2bccfd8 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.fixed +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.fixed @@ -1,7 +1,7 @@ -//@revisions: default integ_tests suffix_tests +//@revisions: default outside_cfg_test custom_suffix //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default -//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests -//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@[outside_cfg_test] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/outside_cfg_test +//@[custom_suffix] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -13,7 +13,7 @@ mod tests_no_annotations { #[test] fn has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn no_annotation() {} @@ -21,7 +21,7 @@ mod tests_no_annotations { #[test] fn main_module_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn test_main_module_no_annotation() {} @@ -43,7 +43,7 @@ fn bar() {} #[test] fn bar_works() { - //~[suffix_tests]^ redundant_test_prefix + //~[custom_suffix]^ redundant_test_prefix todo!() // Has prefix, has `#[test]` attribute. diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.stderr similarity index 100% rename from tests/ui-toml/redundant_test_prefix/redundant_test_prefix.integ_tests.stderr rename to tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.stderr diff --git a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs index 5053b69da07f..15a3ffb7ba91 100644 --- a/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs @@ -1,7 +1,7 @@ -//@revisions: default integ_tests suffix_tests +//@revisions: default outside_cfg_test custom_suffix //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default -//@[integ_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/in_integration_tests -//@[suffix_tests] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix_tests +//@[outside_cfg_test] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/outside_cfg_test +//@[custom_suffix] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/custom_suffix //@compile-flags: --test #![allow(dead_code)] #![warn(clippy::redundant_test_prefix)] @@ -13,7 +13,7 @@ mod tests_no_annotations { #[test] fn test_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn no_annotation() {} @@ -21,7 +21,7 @@ mod tests_no_annotations { #[test] fn test_main_module_has_annotation() { - //~[integ_tests]^ redundant_test_prefix + //~[outside_cfg_test]^ redundant_test_prefix } fn test_main_module_no_annotation() {} @@ -43,7 +43,7 @@ fn bar() {} #[test] fn test_bar() { - //~[suffix_tests]^ redundant_test_prefix + //~[custom_suffix]^ redundant_test_prefix todo!() // Has prefix, has `#[test]` attribute. From eb06e2683c4a42b4921a9667906662c32342b854 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Wed, 20 Nov 2024 00:00:19 +0300 Subject: [PATCH 7/7] fix: `redundant_test_prefix` dogfooding --- clippy_config/src/conf.rs | 24 +++++++++++-------- clippy_dev/src/update_lints.rs | 6 ++--- clippy_lints/src/empty_with_brackets.rs | 2 +- clippy_lints/src/needless_continue.rs | 4 ++-- clippy_lints/src/tabs_in_doc_comments.rs | 22 ++++++++--------- clippy_utils/src/source.rs | 8 +++---- rustc_tools_util/src/lib.rs | 6 ++--- .../toml_unknown_key/conf_unknown_key.stderr | 6 +++++ 8 files changed, 44 insertions(+), 34 deletions(-) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 181235d32408..479019eae0fe 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -633,6 +633,20 @@ define_Conf! { /// exported visibility, or whether they are marked as "pub". #[lints(pub_underscore_fields)] pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PubliclyExported, + /// Whether to include functions outside of `#[cfg(test)]` in the linting process or not. + /// + /// This option allows running the lint against the integration tests: test functions located + /// there are not inside a node marked with `#[cfg(test)]` annotation (although they are + /// still marked using `#[test]` annotation and thus can have redundant "test_" prefix). + #[lints(redundant_test_prefix)] + redundant_test_prefix_check_outside_cfg_test: bool = false, + /// What suffix to use to avoid function name collisions when `test_` prefix is removed. + /// + /// If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. + /// Suffix is added only when there is a collision with an existing function name, + /// otherwise just `test_` prefix is removed (and no suffix added). + #[lints(redundant_test_prefix)] + redundant_test_prefix_custom_suffix: String = String::from("_works"), /// Whether to lint only if it's multiline. #[lints(semicolon_inside_block)] semicolon_inside_block_ignore_singleline: bool = false, @@ -706,16 +720,6 @@ define_Conf! { /// Whether to also emit warnings for unsafe blocks with metavariable expansions in **private** macros. #[lints(macro_metavars_in_unsafe)] warn_unsafe_macro_metavars_in_private_macros: bool = false, - /// Whether to include integration tests in the linting process or not. - #[lints(redundant_test_prefix)] - redundant_test_prefix_in_integration_tests: bool = false, - /// What suffix to use to avoid function name collisions when `test_` prefix is removed. - /// - /// If set to `"_works"`, the lint will suggest renaming `test_foo` to `foo_works`. - /// Suffix is added only when there is a collision with an existing function name, - /// otherwise just `test_` prefix is removed (and no suffix added). - #[lints(redundant_test_prefix)] - redundant_test_prefix_custom_suffix: String = String::from("_works"), } /// Search for the configuration file. diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 795456ad3c56..8333531a60f6 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -963,7 +963,7 @@ mod tests { use super::*; #[test] - fn test_parse_contents() { + fn parse_contents_works() { static CONTENTS: &str = r#" declare_clippy_lint! { #[clippy::version = "Hello Clippy!"] @@ -1006,7 +1006,7 @@ mod tests { } #[test] - fn test_usable_lints() { + fn usable_lints_works() { let lints = vec![ Lint::new( "should_assert_eq2", @@ -1041,7 +1041,7 @@ mod tests { } #[test] - fn test_by_lint_group() { + fn by_lint_group_works() { let lints = vec![ Lint::new("should_assert_eq", "group1", "\"abc\"", "module_name", Range::default()), Lint::new( diff --git a/clippy_lints/src/empty_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs index 743ec5b9ea7f..008667604c48 100644 --- a/clippy_lints/src/empty_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -147,7 +147,7 @@ mod unit_test { use super::*; #[test] - fn test_has_no_ident_token() { + fn has_no_ident_token_works() { let input = "{ field: u8 }"; assert!(!has_no_ident_token(input)); diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index c48232f99058..4b52041b41b1 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -410,7 +410,7 @@ mod test { #[test] #[rustfmt::skip] - fn test_erode_from_back() { + fn erode_from_back_works() { let input = "\ { let x = 5; @@ -428,7 +428,7 @@ mod test { #[test] #[rustfmt::skip] - fn test_erode_from_back_no_brace() { + fn erode_from_back_no_brace() { let input = "\ let x = 5; let y = something(); diff --git a/clippy_lints/src/tabs_in_doc_comments.rs b/clippy_lints/src/tabs_in_doc_comments.rs index af9e13dba36e..2b1fc43c1e72 100644 --- a/clippy_lints/src/tabs_in_doc_comments.rs +++ b/clippy_lints/src/tabs_in_doc_comments.rs @@ -152,77 +152,77 @@ mod tests_for_get_chunks_of_tabs { use super::get_chunks_of_tabs; #[test] - fn test_unicode_han_string() { + fn unicode_han_string() { let res = get_chunks_of_tabs(" \u{4f4d}\t"); assert_eq!(res, vec![(4, 5)]); } #[test] - fn test_empty_string() { + fn empty_string() { let res = get_chunks_of_tabs(""); assert_eq!(res, vec![]); } #[test] - fn test_simple() { + fn simple() { let res = get_chunks_of_tabs("sd\t\t\taa"); assert_eq!(res, vec![(2, 5)]); } #[test] - fn test_only_t() { + fn only_t() { let res = get_chunks_of_tabs("\t\t"); assert_eq!(res, vec![(0, 2)]); } #[test] - fn test_only_one_t() { + fn only_one_t() { let res = get_chunks_of_tabs("\t"); assert_eq!(res, vec![(0, 1)]); } #[test] - fn test_double() { + fn double() { let res = get_chunks_of_tabs("sd\tasd\t\taa"); assert_eq!(res, vec![(2, 3), (6, 8)]); } #[test] - fn test_start() { + fn start() { let res = get_chunks_of_tabs("\t\taa"); assert_eq!(res, vec![(0, 2)]); } #[test] - fn test_end() { + fn end() { let res = get_chunks_of_tabs("aa\t\t"); assert_eq!(res, vec![(2, 4)]); } #[test] - fn test_start_single() { + fn start_single() { let res = get_chunks_of_tabs("\taa"); assert_eq!(res, vec![(0, 1)]); } #[test] - fn test_end_single() { + fn end_single() { let res = get_chunks_of_tabs("aa\t"); assert_eq!(res, vec![(2, 3)]); } #[test] - fn test_no_tabs() { + fn no_tabs() { let res = get_chunks_of_tabs("dsfs"); assert_eq!(res, vec![]); diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index eecbfb3936ac..b1383187de50 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -744,7 +744,7 @@ mod test { use super::reindent_multiline; #[test] - fn test_reindent_multiline_single_line() { + fn reindent_multiline_single_line() { assert_eq!("", reindent_multiline("".into(), false, None)); assert_eq!("...", reindent_multiline("...".into(), false, None)); assert_eq!("...", reindent_multiline(" ...".into(), false, None)); @@ -754,7 +754,7 @@ mod test { #[test] #[rustfmt::skip] - fn test_reindent_multiline_block() { + fn reindent_multiline_block() { assert_eq!("\ if x { y @@ -779,7 +779,7 @@ mod test { #[test] #[rustfmt::skip] - fn test_reindent_multiline_empty_line() { + fn reindent_multiline_empty_line() { assert_eq!("\ if x { y @@ -796,7 +796,7 @@ mod test { #[test] #[rustfmt::skip] - fn test_reindent_multiline_lines_deeper() { + fn reindent_multiline_lines_deeper() { assert_eq!("\ if x { y diff --git a/rustc_tools_util/src/lib.rs b/rustc_tools_util/src/lib.rs index 16be02f4a40f..33ca983b2674 100644 --- a/rustc_tools_util/src/lib.rs +++ b/rustc_tools_util/src/lib.rs @@ -175,7 +175,7 @@ mod test { use super::*; #[test] - fn test_struct_local() { + fn struct_local() { let vi = get_version_info!(); assert_eq!(vi.major, 0); assert_eq!(vi.minor, 4); @@ -187,13 +187,13 @@ mod test { } #[test] - fn test_display_local() { + fn display_local() { let vi = get_version_info!(); assert_eq!(vi.to_string(), "rustc_tools_util 0.4.0"); } #[test] - fn test_debug_local() { + fn debug_local() { let vi = get_version_info!(); let s = format!("{vi:?}"); assert_eq!( diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 6fa583fc0417..8b8859d65131 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -58,6 +58,8 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect msrv pass-by-value-size-limit pub-underscore-fields-behavior + redundant-test-prefix-check-outside-cfg-test + redundant-test-prefix-custom-suffix semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold @@ -145,6 +147,8 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect msrv pass-by-value-size-limit pub-underscore-fields-behavior + redundant-test-prefix-check-outside-cfg-test + redundant-test-prefix-custom-suffix semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold @@ -232,6 +236,8 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni msrv pass-by-value-size-limit pub-underscore-fields-behavior + redundant-test-prefix-check-outside-cfg-test + redundant-test-prefix-custom-suffix semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold