Skip to content

Commit e48de8a

Browse files
committed
fix: redundant_test_prefix allow nested prefixed functions
1 parent c9cecf2 commit e48de8a

File tree

4 files changed

+82
-42
lines changed

4 files changed

+82
-42
lines changed

clippy_lints/src/redundant_test_prefix.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::visitors::for_each_expr;
4-
use clippy_utils::{is_in_cfg_test, is_in_test_function};
4+
use clippy_utils::{is_in_cfg_test, is_test_function};
55
use rustc_errors::Applicability;
66
use rustc_hir::intravisit::FnKind;
77
use rustc_hir::{self as hir, Body, ExprKind, FnDecl};
@@ -65,21 +65,21 @@ impl<'tcx> LateLintPass<'tcx> for RedundantTestPrefix {
6565
_decl: &FnDecl<'_>,
6666
body: &'tcx Body<'_>,
6767
_span: Span,
68-
_fn_def_id: LocalDefId,
68+
fn_def_id: LocalDefId,
6969
) {
70+
// Ignore methods and closures.
71+
let FnKind::ItemFn(ref ident, ..) = kind else {
72+
return;
73+
};
74+
7075
// Skip the lint if the function is not within a node with `#[cfg(test)]` attribute,
7176
// which is true for integration tests. If the lint is enabled for integration tests,
7277
// via configuration value, ignore this check.
7378
if !(self.in_integration_tests || is_in_cfg_test(cx.tcx, body.id().hir_id)) {
7479
return;
7580
}
7681

77-
// Ignore methods and closures.
78-
let FnKind::ItemFn(ref ident, ..) = kind else {
79-
return;
80-
};
81-
82-
if is_in_test_function(cx.tcx, body.id().hir_id) && ident.as_str().starts_with("test_") {
82+
if is_test_function(cx.tcx, cx.tcx.local_def_id_to_hir_id(fn_def_id)) && ident.as_str().starts_with("test_") {
8383
let mut non_prefixed = ident.as_str().trim_start_matches("test_").to_string();
8484
let mut help_msg = "consider removing the `test_` prefix";
8585
// If `non_prefixed` conflicts with another function in the same module/scope,

clippy_utils/src/lib.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2606,6 +2606,28 @@ pub fn is_in_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool {
26062606
})
26072607
}
26082608

2609+
/// Checks if `id` has a `#[test]` attribute applied
2610+
///
2611+
/// This only checks directly applied attributes, to see if a node has a parent function marked with
2612+
/// `#[test]` use [`is_in_test_function`].
2613+
///
2614+
/// Note: Add `//@compile-flags: --test` to UI tests with a `#[test]` function
2615+
pub fn is_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool {
2616+
with_test_item_names(tcx, tcx.parent_module(id), |names| {
2617+
let node = tcx.hir_node(id);
2618+
once((id, node)).any(|(_id, node)| {
2619+
if let Node::Item(item) = node {
2620+
if let ItemKind::Fn(_, _, _) = item.kind {
2621+
// Note that we have sorted the item names in the visitor,
2622+
// so the binary_search gets the same as `contains`, but faster.
2623+
return names.binary_search(&item.ident.name).is_ok();
2624+
}
2625+
}
2626+
false
2627+
})
2628+
})
2629+
}
2630+
26092631
/// Checks if `id` has a `#[cfg(test)]` attribute applied
26102632
///
26112633
/// This only checks directly applied attributes, to see if a node is inside a `#[cfg(test)]` parent

tests/ui/redundant_test_prefix.fixed

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -160,43 +160,52 @@ mod tests_no_annotations {
160160
}
161161

162162
#[test]
163-
fn test_foo() {
164-
}
163+
fn test_foo() {}
165164

166165
#[test]
167166
fn test_foo_with_call() {
168167
main();
169168
}
170169

171170
#[test]
172-
fn test_f1() {
173-
}
171+
fn test_f1() {}
174172

175173
#[test]
176-
fn test_f2() {
177-
}
174+
fn test_f2() {}
178175

179176
#[test]
180-
fn test_f3() {
181-
}
177+
fn test_f3() {}
182178

183179
#[test]
184-
fn test_f4() {
185-
}
180+
fn test_f4() {}
186181

187182
#[test]
188-
fn test_f5() {
189-
}
183+
fn test_f5() {}
190184

191185
#[test]
192-
fn test_f6() {
193-
}
186+
fn test_f6() {}
194187

195188
#[test]
196-
fn test_f7() {
197-
}
189+
fn test_f7() {}
198190

199191
#[test]
200-
fn test_f8() {
192+
fn test_f8() {}
193+
}
194+
195+
// This test is inspired by real test in `clippy_utils/src/sugg.rs`.
196+
// The `is_in_test_function()` checks whether any identifier within a given node's parents is
197+
// marked with `#[test]` attribute. Thus flagging false positives when nested functions are
198+
// prefixed with `test_`. Therefore `is_test_function()` has been defined in `clippy_utils`,
199+
// allowing to select only functions that are immediately marked with `#[test]` annotation.
200+
//
201+
// This test case ensures that for such nested functions no error is emitted.
202+
#[test]
203+
fn not_op() {
204+
fn test_not(foo: bool) {
205+
assert!(foo);
201206
}
207+
208+
// Use helper function
209+
test_not(true);
210+
test_not(false);
202211
}

tests/ui/redundant_test_prefix.rs

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -160,43 +160,52 @@ mod tests_no_annotations {
160160
}
161161

162162
#[test]
163-
fn test_foo() {
164-
}
163+
fn test_foo() {}
165164

166165
#[test]
167166
fn test_foo_with_call() {
168167
main();
169168
}
170169

171170
#[test]
172-
fn test_f1() {
173-
}
171+
fn test_f1() {}
174172

175173
#[test]
176-
fn test_f2() {
177-
}
174+
fn test_f2() {}
178175

179176
#[test]
180-
fn test_f3() {
181-
}
177+
fn test_f3() {}
182178

183179
#[test]
184-
fn test_f4() {
185-
}
180+
fn test_f4() {}
186181

187182
#[test]
188-
fn test_f5() {
189-
}
183+
fn test_f5() {}
190184

191185
#[test]
192-
fn test_f6() {
193-
}
186+
fn test_f6() {}
194187

195188
#[test]
196-
fn test_f7() {
197-
}
189+
fn test_f7() {}
198190

199191
#[test]
200-
fn test_f8() {
192+
fn test_f8() {}
193+
}
194+
195+
// This test is inspired by real test in `clippy_utils/src/sugg.rs`.
196+
// The `is_in_test_function()` checks whether any identifier within a given node's parents is
197+
// marked with `#[test]` attribute. Thus flagging false positives when nested functions are
198+
// prefixed with `test_`. Therefore `is_test_function()` has been defined in `clippy_utils`,
199+
// allowing to select only functions that are immediately marked with `#[test]` annotation.
200+
//
201+
// This test case ensures that for such nested functions no error is emitted.
202+
#[test]
203+
fn not_op() {
204+
fn test_not(foo: bool) {
205+
assert!(foo);
201206
}
207+
208+
// Use helper function
209+
test_not(true);
210+
test_not(false);
202211
}

0 commit comments

Comments
 (0)