Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identical arguments on assert macro family #6167

Merged
merged 6 commits into from
Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
use crate::utils::ast_utils::eq_expr;
use crate::utils::{
eq_expr_value, implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then,
};
use if_chain::if_chain;
use rustc_ast::{ast, token};
use rustc_errors::Applicability;
use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_parse::parser;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
Expand All @@ -23,6 +28,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`)"
Expand Down Expand Up @@ -52,6 +63,41 @@ declare_clippy_lint! {

declare_lint_pass!(EqOp => [EQ_OP, OP_REF]);

impl EarlyLintPass for EqOp {
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &ast::MacCall) {
let macro_list = [
sym!(assert_eq),
sym!(assert_ne),
sym!(debug_assert_eq),
sym!(debug_assert_ne),
];
if_chain! {
if !in_external_macro(cx.sess, mac.span());
if mac.path.segments.len() == 1;
let macro_name = mac.path.segments[0].ident.name;
if macro_list.contains(&macro_name);
let tokens = mac.args.inner_tokens();
let mut parser = parser::Parser::new(
&cx.sess.parse_sess, tokens, false, None);
if let Ok(left) = parser.parse_expr();
if parser.eat(&token::Comma);
if let Ok(right) = parser.parse_expr();
let left_expr = left.into_inner();
let right_expr = right.into_inner();
if eq_expr(&left_expr, &right_expr);

then {
span_lint(
cx,
EQ_OP,
left_expr.span.to(right_expr.span),
&format!("identical args used in this `{}!` macro call", macro_name),
);
}
}
}
}

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<'_>) {
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) {
store.register_pre_expansion_pass(|| box write::Write::default());
store.register_pre_expansion_pass(|| box attrs::EarlyAttributes);
store.register_pre_expansion_pass(|| box dbg_macro::DbgMacro);
store.register_pre_expansion_pass(|| box eq_op::EqOp);
ThibsG marked this conversation as resolved.
Show resolved Hide resolved
}

#[doc(hidden)]
Expand Down Expand Up @@ -910,6 +911,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let vec_box_size_threshold = conf.vec_box_size_threshold;
store.register_late_pass(move || box types::Types::new(vec_box_size_threshold));
store.register_late_pass(|| box booleans::NonminimalBool);
store.register_early_pass(|| box eq_op::EqOp);
store.register_late_pass(|| box eq_op::EqOp);
store.register_late_pass(|| box enum_clike::UnportableVariant);
store.register_late_pass(|| box float_literal::FloatLiteral);
Expand Down
1 change: 1 addition & 0 deletions tests/ui/auxiliary/proc_macro_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#![crate_type = "proc-macro"]
#![feature(repr128, proc_macro_quote)]
#![allow(clippy::eq_op)]

extern crate proc_macro;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::double_parens)]
#![allow(dead_code)]
#![allow(dead_code, clippy::eq_op)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip]

Expand Down
38 changes: 38 additions & 0 deletions tests/ui/eq_op_early.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![warn(clippy::eq_op)]

fn main() {
let a = 1;
let b = 2;

// lint identical args in `assert_eq!` (see #3574)
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);
}
52 changes: 52 additions & 0 deletions tests/ui/eq_op_early.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
error: identical args used in this `assert_eq!` macro call
--> $DIR/eq_op_early.rs:8:16
|
LL | assert_eq!(a, a);
| ^^^^
|
= note: `-D clippy::eq-op` implied by `-D warnings`

error: identical args used in this `assert_eq!` macro call
--> $DIR/eq_op_early.rs:9:16
|
LL | assert_eq!(a + 1, a + 1);
| ^^^^^^^^^^^^

error: identical args used in this `assert_ne!` macro call
--> $DIR/eq_op_early.rs:16:16
|
LL | assert_ne!(a, a);
| ^^^^

error: identical args used in this `assert_ne!` macro call
--> $DIR/eq_op_early.rs:17:16
|
LL | assert_ne!(a + 1, a + 1);
| ^^^^^^^^^^^^

error: identical args used in this `debug_assert_eq!` macro call
--> $DIR/eq_op_early.rs:24:22
|
LL | debug_assert_eq!(a, a);
| ^^^^

error: identical args used in this `debug_assert_eq!` macro call
--> $DIR/eq_op_early.rs:25:22
|
LL | debug_assert_eq!(a + 1, a + 1);
| ^^^^^^^^^^^^

error: identical args used in this `debug_assert_ne!` macro call
--> $DIR/eq_op_early.rs:32:22
|
LL | debug_assert_ne!(a, a);
| ^^^^

error: identical args used in this `debug_assert_ne!` macro call
--> $DIR/eq_op_early.rs:33:22
|
LL | debug_assert_ne!(a + 1, a + 1);
| ^^^^^^^^^^^^

error: aborting due to 8 previous errors

2 changes: 1 addition & 1 deletion tests/ui/used_underscore_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down