Skip to content

Commit ebfa8a1

Browse files
committed
avoid eq_op in test code
1 parent 5c97b27 commit ebfa8a1

File tree

5 files changed

+63
-25
lines changed

5 files changed

+63
-25
lines changed

clippy_lints/src/eq_op.rs

+23-18
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then};
22
use clippy_utils::source::snippet;
33
use clippy_utils::ty::{implements_trait, is_copy};
4-
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of};
4+
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function};
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
@@ -72,23 +72,25 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
7272
#[allow(clippy::similar_names, clippy::too_many_lines)]
7373
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
7474
if let ExprKind::Block(block, _) = e.kind {
75-
for stmt in block.stmts {
76-
for amn in &ASSERT_MACRO_NAMES {
77-
if_chain! {
78-
if is_expn_of(stmt.span, amn).is_some();
79-
if let StmtKind::Semi(matchexpr) = stmt.kind;
80-
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
81-
if macro_args.len() == 2;
82-
let (lhs, rhs) = (macro_args[0], macro_args[1]);
83-
if eq_expr_value(cx, lhs, rhs);
75+
if !is_in_test_function(cx.tcx, e.hir_id) {
76+
for stmt in block.stmts {
77+
for amn in &ASSERT_MACRO_NAMES {
78+
if_chain! {
79+
if is_expn_of(stmt.span, amn).is_some();
80+
if let StmtKind::Semi(matchexpr) = stmt.kind;
81+
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
82+
if macro_args.len() == 2;
83+
let (lhs, rhs) = (macro_args[0], macro_args[1]);
84+
if eq_expr_value(cx, lhs, rhs);
8485

85-
then {
86-
span_lint(
87-
cx,
88-
EQ_OP,
89-
lhs.span.to(rhs.span),
90-
&format!("identical args used in this `{}!` macro call", amn),
91-
);
86+
then {
87+
span_lint(
88+
cx,
89+
EQ_OP,
90+
lhs.span.to(rhs.span),
91+
&format!("identical args used in this `{}!` macro call", amn),
92+
);
93+
}
9294
}
9395
}
9496
}
@@ -108,7 +110,10 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
108110
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
109111
return;
110112
}
111-
if is_useless_with_eq_exprs(op.node.into()) && eq_expr_value(cx, left, right) {
113+
if is_useless_with_eq_exprs(op.node.into())
114+
&& !is_in_test_function(cx.tcx, e.hir_id)
115+
&& eq_expr_value(cx, left, right)
116+
{
112117
span_lint(
113118
cx,
114119
EQ_OP,

clippy_utils/src/lib.rs

+16-7
Original file line numberDiff line numberDiff line change
@@ -2064,16 +2064,25 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
20642064
false
20652065
}
20662066

2067+
/// Checks if the HirId belongs to a function annotated with the `#[test]` attribute
2068+
pub fn is_test_function(tcx: TyCtxt<'_>, fn_id: HirId) -> bool {
2069+
if let Some(def_id) = tcx.hir().opt_local_def_id(fn_id) {
2070+
return tcx.has_attr(def_id.to_def_id(), sym::test);
2071+
}
2072+
false
2073+
}
2074+
2075+
/// Checks if the node this HirId identifies is in a function annotated with `#[test]`
2076+
pub fn is_in_test_function(tcx: TyCtxt<'_>, elem_id: HirId) -> bool {
2077+
is_test_function(tcx, tcx.hir().get_parent_item(elem_id))
2078+
}
2079+
20672080
/// Checks whether item either has `test` attribute applied, or
20682081
/// is a module with `test` in its name.
20692082
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
2070-
if let Some(def_id) = tcx.hir().opt_local_def_id(item.hir_id()) {
2071-
if tcx.has_attr(def_id.to_def_id(), sym::test) {
2072-
return true;
2073-
}
2074-
}
2075-
2076-
matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
2083+
is_test_function(tcx, item.hir_id())
2084+
|| matches!(item.kind, ItemKind::Mod(..))
2085+
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
20772086
}
20782087

20792088
macro_rules! op_utils {

tests/compile-test.rs

+9
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,14 @@ fn run_ui(cfg: &mut compiletest::Config) {
149149
compiletest::run_tests(cfg);
150150
}
151151

152+
fn run_ui_test(cfg: &mut compiletest::Config) {
153+
cfg.mode = TestMode::Ui;
154+
cfg.src_base = Path::new("tests").join("ui_test");
155+
let _g = VarGuard::set("CARGO_MANIFEST_DIR", std::fs::canonicalize("tests").unwrap());
156+
cfg.target_rustcflags.get_or_insert_with(Default::default).push_str(" --test");
157+
compiletest::run_tests(cfg);
158+
}
159+
152160
fn run_internal_tests(cfg: &mut compiletest::Config) {
153161
// only run internal tests with the internal-tests feature
154162
if !RUN_INTERNAL_TESTS {
@@ -312,6 +320,7 @@ fn compile_test() {
312320
prepare_env();
313321
let mut config = default_config();
314322
run_ui(&mut config);
323+
run_ui_test(&mut config);
315324
run_ui_toml(&mut config);
316325
run_ui_cargo(&mut config);
317326
run_internal_tests(&mut config);

tests/ui/eq_op_macros.rs

+8
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,11 @@ fn main() {
5454
let mut my_iter = my_vec.iter();
5555
assert_ne!(my_iter.next(), my_iter.next());
5656
}
57+
58+
#[test]
59+
fn eq_op_shouldnt_trigger_in_tests() {
60+
let a = 1;
61+
let b = 2;
62+
assert_eq!(a, a);
63+
assert_eq!(a + b, b + a);
64+
}

tests/ui_test/eq_op.rs

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
#[test]
3+
fn eq_op_shouldnt_trigger_in_tests() {
4+
let a = 1;
5+
let result = a + 1 == 1 + a;
6+
assert!(result);
7+
}

0 commit comments

Comments
 (0)