diff --git a/CHANGELOG.md b/CHANGELOG.md index dd3124ee9a3b..5788e44c9534 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 @@ -6214,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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 670b5cbef823..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. diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index c3b1fc83af0a..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, 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/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/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/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/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/redundant_test_prefix.rs b/clippy_lints/src/redundant_test_prefix.rs new file mode 100644 index 000000000000..0f84e1a91de2 --- /dev/null +++ b/clippy_lints/src/redundant_test_prefix.rs @@ -0,0 +1,143 @@ +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_test_function}; +use rustc_errors::Applicability; +use rustc_hir::intravisit::FnKind; +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 are prefixed + /// with `test_` which is redundant. + /// + /// ### Why is this bad? + /// 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_use_case() { + /// // test code + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn use_case() { + /// // test code + /// } + /// ``` + #[clippy::version = "1.84.0"] + pub REDUNDANT_TEST_PREFIX, + pedantic, + "redundant `test_` prefix in test function name" +} + +pub struct RedundantTestPrefix { + check_outside_test_cfg: bool, + custom_sufix: String, +} + +impl RedundantTestPrefix { + pub fn new(conf: &'static Conf) -> Self { + Self { + check_outside_test_cfg: conf.redundant_test_prefix_check_outside_cfg_test, + custom_sufix: conf.redundant_test_prefix_custom_suffix.clone(), + } + } +} + +impl_lint_pass!(RedundantTestPrefix => [REDUNDANT_TEST_PREFIX]); + +impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'_>, + _decl: &FnDecl<'_>, + body: &'tcx Body<'_>, + _span: Span, + 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 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; + } + + 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, + // add extra suffix to avoid name conflict. + if name_conflicts(cx, body, &non_prefixed) { + non_prefixed.push_str(&self.custom_sufix); + help_msg = "consider removing the `test_` prefix (suffix avoids name conflict)"; + } + span_lint_and_sugg( + cx, + REDUNDANT_TEST_PREFIX, + ident.span, + "redundant `test_` prefix in test function name", + help_msg, + non_prefixed, + Applicability::MachineApplicable, + ); + } + } +} + +/// 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/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/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/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/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/default/clippy.toml b/tests/ui-toml/redundant_test_prefix/default/clippy.toml new file mode 100644 index 000000000000..ba28999fdaa6 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/default/clippy.toml @@ -0,0 +1 @@ +redundant-test-prefix-check-outside-cfg-test = false 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.custom_suffix.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.fixed new file mode 100644 index 000000000000..7fb3e9c373b1 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.fixed @@ -0,0 +1,55 @@ +//@revisions: default outside_cfg_test custom_suffix +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[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)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn has_annotation() { + //~[outside_cfg_test]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn main_module_has_annotation() { + //~[outside_cfg_test]^ 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() { + //~[custom_suffix]^ 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.custom_suffix.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.stderr new file mode 100644 index 000000000000..e9614161c485 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.custom_suffix.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 + 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..c0e65de285fa --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.default.fixed @@ -0,0 +1,55 @@ +//@revisions: default outside_cfg_test custom_suffix +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[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)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn test_has_annotation() { + //~[outside_cfg_test]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn test_main_module_has_annotation() { + //~[outside_cfg_test]^ 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() { + //~[custom_suffix]^ 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.outside_cfg_test.fixed b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.fixed new file mode 100644 index 000000000000..8cf2c2bccfd8 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.fixed @@ -0,0 +1,55 @@ +//@revisions: default outside_cfg_test custom_suffix +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[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)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn has_annotation() { + //~[outside_cfg_test]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn main_module_has_annotation() { + //~[outside_cfg_test]^ 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 bar_works() { + //~[custom_suffix]^ 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.outside_cfg_test.stderr b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.stderr new file mode 100644 index 000000000000..c8a63c0ab078 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.outside_cfg_test.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_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 new file mode 100644 index 000000000000..15a3ffb7ba91 --- /dev/null +++ b/tests/ui-toml/redundant_test_prefix/redundant_test_prefix.rs @@ -0,0 +1,55 @@ +//@revisions: default outside_cfg_test custom_suffix +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/redundant_test_prefix/default +//@[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)] + +fn main() {} + +mod tests_no_annotations { + use super::*; + + #[test] + fn test_has_annotation() { + //~[outside_cfg_test]^ redundant_test_prefix + } + + fn no_annotation() {} +} + +#[test] +fn test_main_module_has_annotation() { + //~[outside_cfg_test]^ redundant_test_prefix +} + +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() { + //~[custom_suffix]^ 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/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 diff --git a/tests/ui/redundant_test_prefix.fixed b/tests/ui/redundant_test_prefix.fixed new file mode 100644 index 000000000000..26c5b82de567 --- /dev/null +++ b/tests/ui/redundant_test_prefix.fixed @@ -0,0 +1,211 @@ +#![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() {} +} + +// 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 new file mode 100644 index 000000000000..a14e62133bef --- /dev/null +++ b/tests/ui/redundant_test_prefix.rs @@ -0,0 +1,211 @@ +#![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 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)] +mod tests { + use super::*; + + #[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() {} +} + +// 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.stderr b/tests/ui/redundant_test_prefix.stderr new file mode 100644 index 000000000000..35adaa26095f --- /dev/null +++ b/tests/ui/redundant_test_prefix.stderr @@ -0,0 +1,95 @@ +error: redundant `test_` prefix in test function name + --> tests/ui/redundant_test_prefix.rs:24:4 + | +LL | fn test_f4() { + | ^^^^^^^ help: consider removing the `test_` prefix: `f4` + | + = 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/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 +