Skip to content

Commit

Permalink
avoid eq_op in test code
Browse files Browse the repository at this point in the history
  • Loading branch information
llogiq committed Oct 19, 2021
1 parent e1871ba commit e88c956
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 13 deletions.
11 changes: 8 additions & 3 deletions clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then};
use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of};
use clippy_utils::{
ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
Expand Down Expand Up @@ -81,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
if macro_args.len() == 2;
let (lhs, rhs) = (macro_args[0], macro_args[1]);
if eq_expr_value(cx, lhs, rhs);

if !is_in_test_function(cx.tcx, e.hir_id);
then {
span_lint(
cx,
Expand All @@ -108,7 +110,10 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
return;
}
if is_useless_with_eq_exprs(op.node.into()) && eq_expr_value(cx, left, right) {
if is_useless_with_eq_exprs(op.node.into())
&& eq_expr_value(cx, left, right)
&& !is_in_test_function(cx.tcx, e.hir_id)
{
span_lint(
cx,
EQ_OP,
Expand Down
78 changes: 68 additions & 10 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,13 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::hir_id::{HirIdMap, HirIdSet};
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk};
use rustc_hir::{
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat,
PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp,
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs,
HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node,
Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
UnOp,
};
use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export;
Expand Down Expand Up @@ -2064,16 +2066,72 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
false
}

/// Checks whether item either has `test` attribute applied, or
/// is a module with `test` in its name.
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
if let Some(def_id) = tcx.hir().opt_local_def_id(item.hir_id()) {
if tcx.has_attr(def_id.to_def_id(), sym::test) {
return true;
struct VisitConstTestStruct<'tcx> {
tcx: TyCtxt<'tcx>,
names: Vec<Symbol>,
found: bool,
}
impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> {
fn visit_item(&mut self, item: &Item<'_>) {
if let ItemKind::Const(ty, _body) = item.kind {
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
// We could also check for the type name `test::TestDescAndFn`
// and the `#[rustc_test_marker]` attribute?
if let Res::Def(DefKind::Struct, _) = path.res {
let has_test_marker = self
.tcx
.hir()
.attrs(item.hir_id())
.iter()
.any(|a| a.has_name(sym::rustc_test_marker));
if has_test_marker && self.names.contains(&item.ident.name) {
self.found = true;
}
}
}
}
}
fn visit_trait_item(&mut self, _: &TraitItem<'_>) {}
fn visit_impl_item(&mut self, _: &ImplItem<'_>) {}
fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {}
}

matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
/// Checks if the function containing the given `HirId` is a `#[test]` function
///
/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`.
pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
let names: Vec<_> = tcx
.hir()
.parent_iter(id)
// Since you can nest functions we need to collect all until we leave
// function scope
.filter_map(|(_id, node)| {
if let Node::Item(item) = node {
if let ItemKind::Fn(_, _, _) = item.kind {
return Some(item.ident.name);
}
}
None
})
.collect();
let parent_mod = tcx.parent_module(id);
let mut vis = VisitConstTestStruct {
tcx,
names,
found: false,
};
tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis);
vis.found
}

/// Checks whether item either has `test` attribute appelied, or
/// is a module with `test` in its name.
///
/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`.
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
is_in_test_function(tcx, item.hir_id())
|| matches!(item.kind, ItemKind::Mod(..))
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
}

macro_rules! op_utils {
Expand Down
14 changes: 14 additions & 0 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ fn run_ui(cfg: &mut compiletest::Config) {
compiletest::run_tests(cfg);
}

fn run_ui_test(cfg: &mut compiletest::Config) {
cfg.mode = TestMode::Ui;
cfg.src_base = Path::new("tests").join("ui_test");
let _g = VarGuard::set("CARGO_MANIFEST_DIR", std::fs::canonicalize("tests").unwrap());
let rustcflags = cfg.target_rustcflags.get_or_insert_with(Default::default);
let len = rustcflags.len();
rustcflags.push_str(" --test");
compiletest::run_tests(cfg);
if let Some(ref mut flags) = &mut cfg.target_rustcflags {
flags.truncate(len);
}
}

fn run_internal_tests(cfg: &mut compiletest::Config) {
// only run internal tests with the internal-tests feature
if !RUN_INTERNAL_TESTS {
Expand Down Expand Up @@ -312,6 +325,7 @@ fn compile_test() {
prepare_env();
let mut config = default_config();
run_ui(&mut config);
run_ui_test(&mut config);
run_ui_toml(&mut config);
run_ui_cargo(&mut config);
run_internal_tests(&mut config);
Expand Down
15 changes: 15 additions & 0 deletions tests/ui_test/eq_op.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#[warn(clippy::eq_op)]
#[test]
fn eq_op_shouldnt_trigger_in_tests() {
let a = 1;
let result = a + 1 == 1 + a;
assert!(result);
}

#[test]
fn eq_op_macros_shouldnt_trigger_in_tests() {
let a = 1;
let b = 2;
assert_eq!(a, a);
assert_eq!(a + b, b + a);
}

0 comments on commit e88c956

Please sign in to comment.