mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #7811 - rust-lang:eq-op-testless, r=xFrednet

Browse Source 
avoid `eq_op` in test code

Add a check to `eq_op` that will avoid linting in functions annotated with `#[test]`

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: avoid `eq_op` in test functions
This commit is contained in:
bors 2021-10-19 19:54:40 +00:00
commit c1e7a07c9c
4 changed files with 105 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
clippy_lints/src/eq_op.rs
View File

@ -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};
@ -81,7 +83,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
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,
@ -108,7 +110,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
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,

78
clippy_utils/src/lib.rs
View File

@ -69,11 +69,13 @@
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;
@ -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 {

14
tests/compile-test.rs
View File

@ -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 {
@ -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);

15
tests/ui_test/eq_op.rs Normal file
View File

@ -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);
}