|
| 1 | +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; |
| 2 | +use clippy_utils::is_test_function; |
| 3 | +use clippy_utils::visitors::for_each_expr; |
| 4 | +use rustc_errors::Applicability; |
| 5 | +use rustc_hir::intravisit::FnKind; |
| 6 | +use rustc_hir::{self as hir, Body, ExprKind, FnDecl}; |
| 7 | +use rustc_lexer::is_ident; |
| 8 | +use rustc_lint::{LateContext, LateLintPass}; |
| 9 | +use rustc_session::{declare_lint_pass}; |
| 10 | +use rustc_span::def_id::LocalDefId; |
| 11 | +use rustc_span::{Span, Symbol, edition}; |
| 12 | +use std::ops::ControlFlow; |
| 13 | + |
| 14 | +declare_clippy_lint! { |
| 15 | + /// ### What it does |
| 16 | + /// Checks for test functions (functions annotated with `#[test]`) that are prefixed |
| 17 | + /// with `test_` which is redundant. |
| 18 | + /// |
| 19 | + /// ### Why is this bad? |
| 20 | + /// This is redundant because test functions are already annotated with `#[test]`. |
| 21 | + /// Moreover, it clutters the output of `cargo test` since test functions are expanded as |
| 22 | + /// `module::tests::test_use_case` in the output. Without the redundant prefix, the output |
| 23 | + /// becomes `module::tests::use_case`, which is more readable. |
| 24 | + /// |
| 25 | + /// ### Example |
| 26 | + /// ```no_run |
| 27 | + /// #[cfg(test)] |
| 28 | + /// mod tests { |
| 29 | + /// use super::*; |
| 30 | + /// |
| 31 | + /// #[test] |
| 32 | + /// fn test_use_case() { |
| 33 | + /// // test code |
| 34 | + /// } |
| 35 | + /// } |
| 36 | + /// ``` |
| 37 | + /// Use instead: |
| 38 | + /// ```no_run |
| 39 | + /// #[cfg(test)] |
| 40 | + /// mod tests { |
| 41 | + /// use super::*; |
| 42 | + /// |
| 43 | + /// #[test] |
| 44 | + /// fn use_case() { |
| 45 | + /// // test code |
| 46 | + /// } |
| 47 | + /// } |
| 48 | + /// ``` |
| 49 | + #[clippy::version = "1.88.0"] |
| 50 | + pub REDUNDANT_TEST_PREFIX, |
| 51 | + restriction, |
| 52 | + "redundant `test_` prefix in test function name" |
| 53 | +} |
| 54 | + |
| 55 | +declare_lint_pass!(RedundantTestPrefix => [REDUNDANT_TEST_PREFIX]); |
| 56 | + |
| 57 | +impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix { |
| 58 | + fn check_fn( |
| 59 | + &mut self, |
| 60 | + cx: &LateContext<'tcx>, |
| 61 | + kind: FnKind<'_>, |
| 62 | + _decl: &FnDecl<'_>, |
| 63 | + body: &'tcx Body<'_>, |
| 64 | + _span: Span, |
| 65 | + fn_def_id: LocalDefId, |
| 66 | + ) { |
| 67 | + // Ignore methods and closures. |
| 68 | + let FnKind::ItemFn(ref ident, ..) = kind else { |
| 69 | + return; |
| 70 | + }; |
| 71 | + |
| 72 | + // Skip the lint if the function is within a macro expansion. |
| 73 | + if ident.span.from_expansion() { |
| 74 | + return; |
| 75 | + } |
| 76 | + |
| 77 | + // Skip if the function name does not start with `test_`. |
| 78 | + if !ident.as_str().starts_with("test_") { |
| 79 | + return; |
| 80 | + } |
| 81 | + |
| 82 | + if is_test_function(cx.tcx, cx.tcx.local_def_id_to_hir_id(fn_def_id)) { |
| 83 | + let msg = "redundant `test_` prefix in test function name"; |
| 84 | + let mut non_prefixed = ident.as_str().trim_start_matches("test_").to_string(); |
| 85 | + |
| 86 | + if is_invalid_ident(&non_prefixed) { |
| 87 | + // If the prefix-trimmed name is not a valid function name, do not provide an |
| 88 | + // automatic fix, just suggest renaming the function. |
| 89 | + span_lint_and_help( |
| 90 | + cx, |
| 91 | + REDUNDANT_TEST_PREFIX, |
| 92 | + ident.span, |
| 93 | + msg, |
| 94 | + None, |
| 95 | + "consider function renaming (just removing `test_` prefix will produce invalid function name)", |
| 96 | + ); |
| 97 | + } else if name_conflicts(cx, body, &non_prefixed) { |
| 98 | + // If `non_prefixed` conflicts with another function in the same module/scope, |
| 99 | + // do not provide an automatic fix, but still emit a fix suggestion. |
| 100 | + non_prefixed.push_str("_works"); |
| 101 | + span_lint_and_sugg( |
| 102 | + cx, |
| 103 | + REDUNDANT_TEST_PREFIX, |
| 104 | + ident.span, |
| 105 | + msg, |
| 106 | + "consider function renaming (just removing `test_` prefix will cause a name conflict)", |
| 107 | + non_prefixed, |
| 108 | + Applicability::MaybeIncorrect, |
| 109 | + ); |
| 110 | + } else { |
| 111 | + // If `non_prefixed` is a valid identifier and does not conflict with another function, |
| 112 | + // so we can suggest an auto-fix. |
| 113 | + span_lint_and_sugg( |
| 114 | + cx, |
| 115 | + REDUNDANT_TEST_PREFIX, |
| 116 | + ident.span, |
| 117 | + msg, |
| 118 | + "consider removing the `test_` prefix", |
| 119 | + non_prefixed, |
| 120 | + Applicability::MaybeIncorrect, |
| 121 | + ); |
| 122 | + } |
| 123 | + } |
| 124 | + } |
| 125 | +} |
| 126 | + |
| 127 | +/// Checks whether removal of the `_test` prefix from the function name will cause a name conflict. |
| 128 | +/// |
| 129 | +/// There should be no other function with the same name in the same module/scope. Also, there |
| 130 | +/// should not be any function call with the same name within the body of the function, to avoid |
| 131 | +/// recursion. |
| 132 | +fn name_conflicts<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'_>, fn_name: &str) -> bool { |
| 133 | + let tcx = cx.tcx; |
| 134 | + let id = body.id().hir_id; |
| 135 | + |
| 136 | + // Iterate over items in the same module/scope |
| 137 | + let (module, _module_span, _module_hir) = tcx.hir_get_module(tcx.parent_module(id)); |
| 138 | + for item in module.item_ids { |
| 139 | + let item = tcx.hir_item(*item); |
| 140 | + if let hir::ItemKind::Fn { ident, .. } = item.kind |
| 141 | + && ident.name.as_str() == fn_name |
| 142 | + { |
| 143 | + // Name conflict found |
| 144 | + return true; |
| 145 | + } |
| 146 | + } |
| 147 | + |
| 148 | + // Also check that within the body of the function there is also no function call |
| 149 | + // with the same name (since it will result in recursion) |
| 150 | + for_each_expr(cx, body, |expr| { |
| 151 | + if let ExprKind::Path(qpath) = &expr.kind |
| 152 | + && let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id() |
| 153 | + && let Some(name) = tcx.opt_item_name(def_id) |
| 154 | + && name.as_str() == fn_name |
| 155 | + { |
| 156 | + // Function call with the same name found |
| 157 | + return ControlFlow::Break(()); |
| 158 | + } |
| 159 | + |
| 160 | + ControlFlow::Continue(()) |
| 161 | + }) |
| 162 | + .is_some() |
| 163 | +} |
| 164 | + |
| 165 | +fn is_invalid_ident(ident: &str) -> bool { |
| 166 | + // The identifier is either a reserved keyword, or starts with an invalid sequence. |
| 167 | + Symbol::intern(ident).is_reserved(|| edition::LATEST_STABLE_EDITION) || !is_ident(ident) |
| 168 | +} |
0 commit comments