Move IdentityOp into Operators lint pass

This commit is contained in:
Jason Newcomb 2022-06-01 01:23:57 -04:00
parent 56f50d36e7
commit 83de67cfec
6 changed files with 58 additions and 59 deletions

View File

@ -83,7 +83,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(functions::RESULT_UNIT_ERR),
LintId::of(functions::TOO_MANY_ARGUMENTS),
LintId::of(get_first::GET_FIRST),
LintId::of(identity_op::IDENTITY_OP),
LintId::of(if_let_mutex::IF_LET_MUTEX),
LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
LintId::of(infinite_iter::INFINITE_ITER),
@ -257,6 +256,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(operators::EQ_OP),
LintId::of(operators::ERASING_OP),
LintId::of(operators::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(operators::IDENTITY_OP),
LintId::of(operators::INEFFECTIVE_BIT_MASK),
LintId::of(operators::MISREFACTORED_ASSIGN_OP),
LintId::of(operators::OP_REF),

View File

@ -14,7 +14,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(explicit_write::EXPLICIT_WRITE),
LintId::of(format::USELESS_FORMAT),
LintId::of(functions::TOO_MANY_ARGUMENTS),
LintId::of(identity_op::IDENTITY_OP),
LintId::of(int_plus_one::INT_PLUS_ONE),
LintId::of(lifetimes::EXTRA_UNUSED_LIFETIMES),
LintId::of(lifetimes::NEEDLESS_LIFETIMES),
@ -71,6 +70,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(no_effect::UNNECESSARY_OPERATION),
LintId::of(operators::DOUBLE_COMPARISONS),
LintId::of(operators::DURATION_SUBSEC),
LintId::of(operators::IDENTITY_OP),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(precedence::PRECEDENCE),

View File

@ -176,7 +176,6 @@ store.register_lints(&[
functions::TOO_MANY_LINES,
future_not_send::FUTURE_NOT_SEND,
get_first::GET_FIRST,
identity_op::IDENTITY_OP,
if_let_mutex::IF_LET_MUTEX,
if_not_else::IF_NOT_ELSE,
if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
@ -432,6 +431,7 @@ store.register_lints(&[
operators::ERASING_OP,
operators::FLOAT_ARITHMETIC,
operators::FLOAT_EQUALITY_WITHOUT_ABS,
operators::IDENTITY_OP,
operators::INEFFECTIVE_BIT_MASK,
operators::INTEGER_ARITHMETIC,
operators::MISREFACTORED_ASSIGN_OP,

View File

@ -239,7 +239,6 @@ mod from_str_radix_10;
mod functions;
mod future_not_send;
mod get_first;
mod identity_op;
mod if_let_mutex;
mod if_not_else;
mod if_then_some_else_none;
@ -582,7 +581,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach));
store.register_late_pass(|| Box::new(misc::MiscLints));
store.register_late_pass(|| Box::new(eta_reduction::EtaReduction));
store.register_late_pass(|| Box::new(identity_op::IdentityOp));
store.register_late_pass(|| Box::new(mut_mut::MutMut));
store.register_late_pass(|| Box::new(mut_reference::UnnecessaryMutPassed));
store.register_late_pass(|| Box::new(len_zero::LenZero));

View File

@ -3,61 +3,40 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{clip, unsext};
use rustc_errors::Applicability;
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
declare_clippy_lint! {
/// ### What it does
/// Checks for identity operations, e.g., `x + 0`.
///
/// ### Why is this bad?
/// This code can be removed without changing the
/// meaning. So it just obscures what's going on. Delete it mercilessly.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// x / 1 + 0 * 1 - 0 | 0;
/// ```
#[clippy::version = "pre 1.29.0"]
pub IDENTITY_OP,
complexity,
"using identity operations, e.g., `x + 0` or `y / 1`"
}
use super::IDENTITY_OP;
declare_lint_pass!(IdentityOp => [IDENTITY_OP]);
impl<'tcx> LateLintPass<'tcx> for IdentityOp {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
return;
}
if let ExprKind::Binary(cmp, left, right) = &expr.kind {
if !is_allowed(cx, *cmp, left, right) {
match cmp.node {
BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
check(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right));
check(cx, right, 0, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
check(cx, right, 0, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Mul => {
check(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right));
check(cx, right, 1, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Div => check(cx, right, 1, expr.span, left.span, Parens::Unneeded),
BinOpKind::BitAnd => {
check(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right));
check(cx, right, -1, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span),
_ => (),
}
}
pub(crate) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: BinOpKind,
left: &'tcx Expr<'_>,
right: &'tcx Expr<'_>,
) {
if !is_allowed(cx, op, left, right) {
match op {
BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
check_op(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right));
check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Mul => {
check_op(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right));
check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Div => check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded),
BinOpKind::BitAnd => {
check_op(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right));
check_op(cx, right, -1, expr.span, left.span, Parens::Unneeded);
},
BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span),
_ => (),
}
}
}
@ -108,12 +87,12 @@ fn needs_parenthesis(cx: &LateContext<'_>, binary: &Expr<'_>, right: &Expr<'_>)
Parens::Needed
}
fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
fn is_allowed(cx: &LateContext<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Expr<'_>) -> bool {
// This lint applies to integers
!cx.typeck_results().expr_ty(left).peel_refs().is_integral()
|| !cx.typeck_results().expr_ty(right).peel_refs().is_integral()
// `1 << 0` is a common pattern in bit manipulation code
|| (cmp.node == BinOpKind::Shl
|| (cmp == BinOpKind::Shl
&& constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
&& constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)))
}
@ -130,7 +109,7 @@ fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span
}
}
fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens) {
fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens) {
if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) {
let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() {
ty::Int(ity) => unsext(cx.tcx, -1_i128, ity),

View File

@ -10,6 +10,7 @@ mod duration_subsec;
mod eq_op;
mod erasing_op;
mod float_equality_without_abs;
mod identity_op;
mod misrefactored_assign_op;
mod numeric_arithmetic;
mod op_ref;
@ -417,6 +418,25 @@ declare_clippy_lint! {
"float equality check without `.abs()`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for identity operations, e.g., `x + 0`.
///
/// ### Why is this bad?
/// This code can be removed without changing the
/// meaning. So it just obscures what's going on. Delete it mercilessly.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// x / 1 + 0 * 1 - 0 | 0;
/// ```
#[clippy::version = "pre 1.29.0"]
pub IDENTITY_OP,
complexity,
"using identity operations, e.g., `x + 0` or `y / 1`"
}
pub struct Operators {
arithmetic_context: numeric_arithmetic::Context,
verbose_bit_mask_threshold: u64,
@ -436,6 +456,7 @@ impl_lint_pass!(Operators => [
OP_REF,
ERASING_OP,
FLOAT_EQUALITY_WITHOUT_ABS,
IDENTITY_OP,
]);
impl Operators {
pub fn new(verbose_bit_mask_threshold: u64) -> Self {
@ -457,6 +478,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
op_ref::check(cx, e, op.node, lhs, rhs);
}
erasing_op::check(cx, e, op.node, lhs, rhs);
identity_op::check(cx, e, op.node, lhs, rhs);
}
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
bit_mask::check(cx, e, op.node, lhs, rhs);