Skip to content

Commit 0d738be

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

File tree

5 files changed

+104
-13
lines changed

5 files changed

+104
-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

+68-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,72 @@ 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<'tcx> {
2070+
tcx: TyCtxt<'tcx>,
2071+
names: Vec<Symbol>,
2072+
found: bool,
2073+
}
2074+
impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> {
2075+
fn visit_item(&mut self, item: &Item<'_>) {
2076+
if let ItemKind::Const(ty, _body) = item.kind {
2077+
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
2078+
// We could also check for the type name `test::TestDescAndFn`
2079+
// and the `#[rustc_test_marker]` attribute?
2080+
if let Res::Def(DefKind::Struct, _) = path.res {
2081+
let has_test_marker = self
2082+
.tcx
2083+
.hir()
2084+
.attrs(item.hir_id())
2085+
.iter()
2086+
.any(|a| a.has_name(sym::rustc_test_marker));
2087+
if has_test_marker && self.names.contains(&item.ident.name) {
2088+
self.found = true;
2089+
}
2090+
}
2091+
}
20732092
}
20742093
}
2094+
fn visit_trait_item(&mut self, _: &TraitItem<'_>) {}
2095+
fn visit_impl_item(&mut self, _: &ImplItem<'_>) {}
2096+
fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {}
2097+
}
20752098

2076-
matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
2099+
/// Checks if the function containing the given `HirId` is a `#[test]` function
2100+
///
2101+
/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`.
2102+
pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
2103+
let names: Vec<_> = tcx
2104+
.hir()
2105+
.parent_iter(id)
2106+
// Since you can nest functions we need to collect all until we leave
2107+
// function scope
2108+
.filter_map(|(_id, node)| {
2109+
if let Node::Item(item) = node {
2110+
if let ItemKind::Fn(_, _, _) = item.kind {
2111+
return Some(item.ident.name);
2112+
}
2113+
}
2114+
None
2115+
})
2116+
.collect();
2117+
let parent_mod = tcx.parent_module(id);
2118+
let mut vis = VisitConstTestStruct {
2119+
tcx,
2120+
names,
2121+
found: false,
2122+
};
2123+
tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis);
2124+
vis.found
2125+
}
2126+
2127+
/// Checks whether item either has `test` attribute appelied, or
2128+
/// is a module with `test` in its name.
2129+
///
2130+
/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`.
2131+
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
2132+
is_in_test_function(tcx, item.hir_id())
2133+
|| matches!(item.kind, ItemKind::Mod(..))
2134+
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
20772135
}
20782136

20792137
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)