squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Auto merge of #6167 - ThibsG:IdenticalArgumentsAssertEq3574, r=ebroto

Browse source 
Identical arguments on assert macro family

Lint when identical args are used on `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` macros.

Added to the lint `eq_op`.

Common functions added to `utils/higher.rs`

Fixes: #3574
Fixes: #4694

changelog: Lint on identical args when calling `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` macros
This commit is contained in:
bors 2020-10-19 19:50:12 +00:00
parent 74530adcba 16b5f37b5a
commit eaffd0e418
8 changed files with 254 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
clippy_lints/src/eq_op.rs
View file

@ -1,8 +1,10 @@
use crate::utils::{
eq_expr_value, implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then,
eq_expr_value, higher, implements_trait, in_macro, is_copy, is_expn_of, multispan_sugg, snippet, span_lint,
span_lint_and_then,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind};
use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -23,6 +25,12 @@ declare_clippy_lint! {
/// # let x = 1;
/// if x + 1 == x + 1 {}
/// ```
/// or
/// ```rust
/// # let a = 3;
/// # let b = 4;
/// assert_eq!(a, a);
/// ```
pub EQ_OP,
correctness,
"equal operands on both sides of a comparison or bitwise combination (e.g., `x == x`)"
@ -52,9 +60,34 @@ declare_clippy_lint! {
declare_lint_pass!(EqOp => [EQ_OP, OP_REF]);
const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"];
impl<'tcx> LateLintPass<'tcx> for EqOp {
#[allow(clippy::similar_names, clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Block(ref block, _) = e.kind {
for stmt in block.stmts {
for amn in &ASSERT_MACRO_NAMES {
if_chain! {
if is_expn_of(stmt.span, amn).is_some();
if let StmtKind::Semi(ref matchexpr) = stmt.kind;
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
if macro_args.len() == 2;
let (lhs, rhs) = (macro_args[0], macro_args[1]);
if eq_expr_value(cx, lhs, rhs);
then {
span_lint(
cx,
EQ_OP,
lhs.span.to(rhs.span),
&format!("identical args used in this `{}!` macro call", amn),
);
}
}
}
}
}
if let ExprKind::Binary(op, ref left, ref right) = e.kind {
if e.span.from_expansion() {
return;

66
clippy_lints/src/mutable_debug_assertion.rs
View file

@ -1,7 +1,6 @@
use crate::utils::{is_direct_expn_of, span_lint};
use if_chain::if_chain;
use crate::utils::{higher, is_direct_expn_of, span_lint};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability, StmtKind, UnOp};
use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
@ -39,66 +38,23 @@ impl<'tcx> LateLintPass<'tcx> for DebugAssertWithMutCall {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
for dmn in &DEBUG_MACRO_NAMES {
if is_direct_expn_of(e.span, dmn).is_some() {
if let Some(span) = extract_call(cx, e) {
span_lint(
cx,
DEBUG_ASSERT_WITH_MUT_CALL,
span,
&format!("do not call a function with mutable arguments inside of `{}!`", dmn),
);
}
}
}
}
}
//HACK(hellow554): remove this when #4694 is implemented
fn extract_call<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option<Span> {
if_chain! {
if let ExprKind::Block(ref block, _) = e.kind;
if block.stmts.len() == 1;
if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind;
then {
// debug_assert
if_chain! {
if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind;
if let ExprKind::DropTemps(ref droptmp) = ifclause.kind;
if let ExprKind::Unary(UnOp::UnNot, ref condition) = droptmp.kind;
then {
let mut visitor = MutArgVisitor::new(cx);
visitor.visit_expr(condition);
return visitor.expr_span();
}
}
// debug_assert_{eq,ne}
if_chain! {
if let ExprKind::Block(ref matchblock, _) = matchexpr.kind;
if let Some(ref matchheader) = matchblock.expr;
if let ExprKind::Match(ref headerexpr, _, _) = matchheader.kind;
if let ExprKind::Tup(ref conditions) = headerexpr.kind;
if conditions.len() == 2;
then {
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref lhs) = conditions[0].kind {
if let Some(macro_args) = higher::extract_assert_macro_args(e) {
for arg in macro_args {
let mut visitor = MutArgVisitor::new(cx);
visitor.visit_expr(lhs);
visitor.visit_expr(arg);
if let Some(span) = visitor.expr_span() {
return Some(span);
}
}
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref rhs) = conditions[1].kind {
let mut visitor = MutArgVisitor::new(cx);
visitor.visit_expr(rhs);
if let Some(span) = visitor.expr_span() {
return Some(span);
span_lint(
cx,
DEBUG_ASSERT_WITH_MUT_CALL,
span,
&format!("do not call a function with mutable arguments inside of `{}!`", dmn),
);
}
}
}
}
}
}
None
}
struct MutArgVisitor<'a, 'tcx> {

54
clippy_lints/src/utils/higher.rs
View file

@ -7,6 +7,7 @@ use crate::utils::{is_expn_of, match_def_path, paths};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_hir as hir;
use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp};
use rustc_lint::LateContext;
/// Converts a hir binary operator to the corresponding `ast` type.
@ -241,3 +242,56 @@ pub fn vec_macro<'e>(cx: &LateContext<'_>, expr: &'e hir::Expr<'_>) -> Option<Ve
None
}
/// Extract args from an assert-like macro.
/// Currently working with:
/// - `assert!`, `assert_eq!` and `assert_ne!`
/// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!`
/// For example:
/// `assert!(expr)` will return Some([expr])
/// `debug_assert_eq!(a, b)` will return Some([a, b])
pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option<Vec<&'tcx Expr<'tcx>>> {
/// Try to match the AST for a pattern that contains a match, for example when two args are
/// compared
fn ast_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option<Vec<&Expr<'_>>> {
if_chain! {
if let ExprKind::Match(ref headerexpr, _, _) = &matchblock_expr.kind;
if let ExprKind::Tup([lhs, rhs]) = &headerexpr.kind;
if let ExprKind::AddrOf(BorrowKind::Ref, _, lhs) = lhs.kind;
if let ExprKind::AddrOf(BorrowKind::Ref, _, rhs) = rhs.kind;
then {
return Some(vec![lhs, rhs]);
}
}
None
}
if let ExprKind::Block(ref block, _) = e.kind {
if block.stmts.len() == 1 {
if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind {
// macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`)
if_chain! {
if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind;
if let ExprKind::DropTemps(ref droptmp) = ifclause.kind;
if let ExprKind::Unary(UnOp::UnNot, condition) = droptmp.kind;
then {
return Some(vec![condition]);
}
}
// debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
if_chain! {
if let ExprKind::Block(ref matchblock,_) = matchexpr.kind;
if let Some(ref matchblock_expr) = matchblock.expr;
then {
return ast_matchblock(matchblock_expr);
}
}
}
} else if let Some(matchblock_expr) = block.expr {
// macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
return ast_matchblock(&matchblock_expr);
}
}
None
}

1
tests/ui/auxiliary/proc_macro_derive.rs
View file

@ -3,6 +3,7 @@
#![crate_type = "proc-macro"]
#![feature(repr128, proc_macro_quote)]
#![allow(clippy::eq_op)]
extern crate proc_macro;

2
tests/ui/double_parens.rs
View file

@ -1,5 +1,5 @@
#![warn(clippy::double_parens)]
#![allow(dead_code)]
#![allow(dead_code, clippy::eq_op)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip]

56
tests/ui/eq_op_macros.rs Normal file
View file

@ -0,0 +1,56 @@
#![warn(clippy::eq_op)]
// lint also in macro definition
macro_rules! assert_in_macro_def {
() => {
let a = 42;
assert_eq!(a, a);
assert_ne!(a, a);
debug_assert_eq!(a, a);
debug_assert_ne!(a, a);
};
}
// lint identical args in assert-like macro invocations (see #3574)
fn main() {
assert_in_macro_def!();
let a = 1;
let b = 2;
// lint identical args in `assert_eq!`
assert_eq!(a, a);
assert_eq!(a + 1, a + 1);
// ok
assert_eq!(a, b);
assert_eq!(a, a + 1);
assert_eq!(a + 1, b + 1);
// lint identical args in `assert_ne!`
assert_ne!(a, a);
assert_ne!(a + 1, a + 1);
// ok
assert_ne!(a, b);
assert_ne!(a, a + 1);
assert_ne!(a + 1, b + 1);
// lint identical args in `debug_assert_eq!`
debug_assert_eq!(a, a);
debug_assert_eq!(a + 1, a + 1);
// ok
debug_assert_eq!(a, b);
debug_assert_eq!(a, a + 1);
debug_assert_eq!(a + 1, b + 1);
// lint identical args in `debug_assert_ne!`
debug_assert_ne!(a, a);
debug_assert_ne!(a + 1, a + 1);
// ok
debug_assert_ne!(a, b);
debug_assert_ne!(a, a + 1);
debug_assert_ne!(a + 1, b + 1);
let my_vec = vec![1; 5];
let mut my_iter = my_vec.iter();
assert_ne!(my_iter.next(), my_iter.next());
}

95
tests/ui/eq_op_macros.stderr Normal file
View file

@ -0,0 +1,95 @@
error: identical args used in this `assert_eq!` macro call
--> $DIR/eq_op_macros.rs:7:20
|
LL | assert_eq!(a, a);
| ^^^^
...
LL | assert_in_macro_def!();
| ----------------------- in this macro invocation
|
= note: `-D clippy::eq-op` implied by `-D warnings`
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: identical args used in this `assert_ne!` macro call
--> $DIR/eq_op_macros.rs:8:20
|
LL | assert_ne!(a, a);
| ^^^^
...
LL | assert_in_macro_def!();
| ----------------------- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: identical args used in this `assert_eq!` macro call
--> $DIR/eq_op_macros.rs:22:16
|
LL | assert_eq!(a, a);
| ^^^^
error: identical args used in this `assert_eq!` macro call
--> $DIR/eq_op_macros.rs:23:16
|
LL | assert_eq!(a + 1, a + 1);
| ^^^^^^^^^^^^
error: identical args used in this `assert_ne!` macro call
--> $DIR/eq_op_macros.rs:30:16
|
LL | assert_ne!(a, a);
| ^^^^
error: identical args used in this `assert_ne!` macro call
--> $DIR/eq_op_macros.rs:31:16
|
LL | assert_ne!(a + 1, a + 1);
| ^^^^^^^^^^^^
error: identical args used in this `debug_assert_eq!` macro call
--> $DIR/eq_op_macros.rs:9:26
|
LL | debug_assert_eq!(a, a);
| ^^^^
...
LL | assert_in_macro_def!();
| ----------------------- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: identical args used in this `debug_assert_ne!` macro call
--> $DIR/eq_op_macros.rs:10:26
|
LL | debug_assert_ne!(a, a);
| ^^^^
...
LL | assert_in_macro_def!();
| ----------------------- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: identical args used in this `debug_assert_eq!` macro call
--> $DIR/eq_op_macros.rs:38:22
|
LL | debug_assert_eq!(a, a);
| ^^^^
error: identical args used in this `debug_assert_eq!` macro call
--> $DIR/eq_op_macros.rs:39:22
|
LL | debug_assert_eq!(a + 1, a + 1);
| ^^^^^^^^^^^^
error: identical args used in this `debug_assert_ne!` macro call
--> $DIR/eq_op_macros.rs:46:22
|
LL | debug_assert_ne!(a, a);
| ^^^^
error: identical args used in this `debug_assert_ne!` macro call
--> $DIR/eq_op_macros.rs:47:22
|
LL | debug_assert_ne!(a + 1, a + 1);
| ^^^^^^^^^^^^
error: aborting due to 12 previous errors

2
tests/ui/used_underscore_binding.rs
View file

@ -3,7 +3,7 @@
#![feature(rustc_private)]
#![warn(clippy::all)]
#![allow(clippy::blacklisted_name)]
#![allow(clippy::blacklisted_name, clippy::eq_op)]
#![warn(clippy::used_underscore_binding)]
#[macro_use]