Skip to content

Commit f34abb4

Browse files
committed
avoid eq_op in test code
1 parent e1871ba commit f34abb4

File tree

5 files changed

+89
-13
lines changed

5 files changed

+89
-13
lines changed

clippy_lints/src/eq_op.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
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::{
5+
ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function,
6+
};
57
use if_chain::if_chain;
68
use rustc_errors::Applicability;
79
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
@@ -81,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
8183
if macro_args.len() == 2;
8284
let (lhs, rhs) = (macro_args[0], macro_args[1]);
8385
if eq_expr_value(cx, lhs, rhs);
84-
86+
if !is_in_test_function(cx.tcx, e.hir_id);
8587
then {
8688
span_lint(
8789
cx,
@@ -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+
&& eq_expr_value(cx, left, right)
115+
&& !is_in_test_function(cx.tcx, e.hir_id)
116+
{
112117
span_lint(
113118
cx,
114119
EQ_OP,

clippy_utils/src/lib.rs

+53-10
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,13 @@ use rustc_hir::def::{DefKind, Res};
6969
use rustc_hir::def_id::DefId;
7070
use rustc_hir::hir_id::{HirIdMap, HirIdSet};
7171
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
72+
use rustc_hir::itemlikevisit::ItemLikeVisitor;
7273
use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk};
7374
use rustc_hir::{
74-
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
75-
ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat,
76-
PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp,
75+
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs,
76+
HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node,
77+
Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
78+
UnOp,
7779
};
7880
use rustc_lint::{LateContext, Level, Lint, LintContext};
7981
use rustc_middle::hir::exports::Export;
@@ -2064,16 +2066,57 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
20642066
false
20652067
}
20662068

2067-
/// Checks whether item either has `test` attribute applied, or
2068-
/// is a module with `test` in its name.
2069-
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;
2069+
struct VisitConstTestStruct {
2070+
names: Vec<Symbol>,
2071+
found: bool,
2072+
}
2073+
impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct {
2074+
fn visit_item(&mut self, item: &Item<'_>) {
2075+
if let ItemKind::Const(ty, _body) = item.kind {
2076+
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
2077+
// We could also check for the type name `test::TestDescAndFn`
2078+
// and the `#[rustc_test_marker]` attribute?
2079+
if let Res::Def(DefKind::Struct, _) = path.res {
2080+
if self.names.contains(&item.ident.name) {
2081+
self.found = true;
2082+
}
2083+
}
2084+
}
20732085
}
20742086
}
2087+
fn visit_trait_item(&mut self, _: &TraitItem<'_>) {}
2088+
fn visit_impl_item(&mut self, _: &ImplItem<'_>) {}
2089+
fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {}
2090+
}
2091+
2092+
/// Checks if the function containing the given `HirId` is a `#[test]` function
2093+
pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
2094+
let names: Vec<_> = tcx
2095+
.hir()
2096+
.parent_iter(id)
2097+
// Since you can nest functions we need to collect all until we leave
2098+
// function scope
2099+
.filter_map(|(_id, node)| {
2100+
if let Node::Item(item) = node {
2101+
if let ItemKind::Fn(_, _, _) = item.kind {
2102+
return Some(item.ident.name);
2103+
}
2104+
}
2105+
None
2106+
})
2107+
.collect();
2108+
let parent_mod = tcx.parent_module(id);
2109+
let mut vis = VisitConstTestStruct { names, found: false };
2110+
tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis);
2111+
vis.found
2112+
}
20752113

2076-
matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
2114+
/// Checks whether item either has `test` attribute appelied, or
2115+
/// is a module with `test` in its name.
2116+
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
2117+
is_in_test_function(tcx, item.hir_id())
2118+
|| matches!(item.kind, ItemKind::Mod(..))
2119+
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
20772120
}
20782121

20792122
macro_rules! op_utils {

tests/compile-test.rs

+14
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,19 @@ 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+
let rustcflags = cfg.target_rustcflags.get_or_insert_with(Default::default);
157+
let len = rustcflags.len();
158+
rustcflags.push_str(" --test");
159+
compiletest::run_tests(cfg);
160+
if let Some(ref mut flags) = &mut cfg.target_rustcflags {
161+
flags.truncate(len);
162+
}
163+
}
164+
152165
fn run_internal_tests(cfg: &mut compiletest::Config) {
153166
// only run internal tests with the internal-tests feature
154167
if !RUN_INTERNAL_TESTS {
@@ -312,6 +325,7 @@ fn compile_test() {
312325
prepare_env();
313326
let mut config = default_config();
314327
run_ui(&mut config);
328+
run_ui_test(&mut config);
315329
run_ui_toml(&mut config);
316330
run_ui_cargo(&mut config);
317331
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

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

0 commit comments

Comments
 (0)