Move DoubleComparison into Operators lint pass

This commit is contained in:
Jason Newcomb 2022-05-31 22:32:13 -04:00
parent 68a7fd22ad
commit 4ac3626f2e
7 changed files with 88 additions and 101 deletions

View File

@ -1,96 +0,0 @@
//! Lint on unnecessary double comparisons. Some examples:
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eq_expr_value;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
declare_clippy_lint! {
/// ### What it does
/// Checks for double comparisons that could be simplified to a single expression.
///
///
/// ### Why is this bad?
/// Readability.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x == y || x < y {}
/// ```
///
/// Use instead:
///
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x <= y {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub DOUBLE_COMPARISONS,
complexity,
"unnecessary double comparisons that can be simplified"
}
declare_lint_pass!(DoubleComparisons => [DOUBLE_COMPARISONS]);
impl<'tcx> DoubleComparisons {
#[expect(clippy::similar_names)]
fn check_binop(cx: &LateContext<'tcx>, op: BinOpKind, lhs: &'tcx Expr<'_>, rhs: &'tcx Expr<'_>, span: Span) {
let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (&lhs.kind, &rhs.kind) {
(ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => {
(lb.node, llhs, lrhs, rb.node, rlhs, rrhs)
},
_ => return,
};
if !(eq_expr_value(cx, llhs, rlhs) && eq_expr_value(cx, lrhs, rrhs)) {
return;
}
macro_rules! lint_double_comparison {
($op:tt) => {{
let mut applicability = Applicability::MachineApplicable;
let lhs_str = snippet_with_applicability(cx, llhs.span, "", &mut applicability);
let rhs_str = snippet_with_applicability(cx, lrhs.span, "", &mut applicability);
let sugg = format!("{} {} {}", lhs_str, stringify!($op), rhs_str);
span_lint_and_sugg(
cx,
DOUBLE_COMPARISONS,
span,
"this binary expression can be simplified",
"try",
sugg,
applicability,
);
}};
}
#[rustfmt::skip]
match (op, lkind, rkind) {
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Lt) | (BinOpKind::Or, BinOpKind::Lt, BinOpKind::Eq) => {
lint_double_comparison!(<=);
},
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Gt) | (BinOpKind::Or, BinOpKind::Gt, BinOpKind::Eq) => {
lint_double_comparison!(>=);
},
(BinOpKind::Or, BinOpKind::Lt, BinOpKind::Gt) | (BinOpKind::Or, BinOpKind::Gt, BinOpKind::Lt) => {
lint_double_comparison!(!=);
},
(BinOpKind::And, BinOpKind::Le, BinOpKind::Ge) | (BinOpKind::And, BinOpKind::Ge, BinOpKind::Le) => {
lint_double_comparison!(==);
},
_ => (),
};
}
}
impl<'tcx> LateLintPass<'tcx> for DoubleComparisons {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Binary(ref kind, lhs, rhs) = expr.kind {
Self::check_binop(cx, kind.node, lhs, rhs, expr.span);
}
}
}

View File

@ -48,7 +48,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(disallowed_types::DISALLOWED_TYPES),
LintId::of(doc::MISSING_SAFETY_DOC),
LintId::of(doc::NEEDLESS_DOCTEST_MAIN),
LintId::of(double_comparison::DOUBLE_COMPARISONS),
LintId::of(double_parens::DOUBLE_PARENS),
LintId::of(drop_forget_ref::DROP_COPY),
LintId::of(drop_forget_ref::DROP_NON_DROP),
@ -258,6 +257,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(operators::ABSURD_EXTREME_COMPARISONS),
LintId::of(operators::ASSIGN_OP_PATTERN),
LintId::of(operators::BAD_BIT_MASK),
LintId::of(operators::DOUBLE_COMPARISONS),
LintId::of(operators::INEFFECTIVE_BIT_MASK),
LintId::of(operators::MISREFACTORED_ASSIGN_OP),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),

View File

@ -10,7 +10,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(casts::CHAR_LIT_AS_U8),
LintId::of(casts::UNNECESSARY_CAST),
LintId::of(derivable_impls::DERIVABLE_IMPLS),
LintId::of(double_comparison::DOUBLE_COMPARISONS),
LintId::of(double_parens::DOUBLE_PARENS),
LintId::of(duration_subsec::DURATION_SUBSEC),
LintId::of(explicit_write::EXPLICIT_WRITE),
@ -71,6 +70,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
LintId::of(no_effect::NO_EFFECT),
LintId::of(no_effect::UNNECESSARY_OPERATION),
LintId::of(operators::DOUBLE_COMPARISONS),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(precedence::PRECEDENCE),

View File

@ -122,7 +122,6 @@ store.register_lints(&[
doc::MISSING_SAFETY_DOC,
doc::NEEDLESS_DOCTEST_MAIN,
doc_link_with_quotes::DOC_LINK_WITH_QUOTES,
double_comparison::DOUBLE_COMPARISONS,
double_parens::DOUBLE_PARENS,
drop_forget_ref::DROP_COPY,
drop_forget_ref::DROP_NON_DROP,
@ -432,6 +431,7 @@ store.register_lints(&[
operators::ABSURD_EXTREME_COMPARISONS,
operators::ASSIGN_OP_PATTERN,
operators::BAD_BIT_MASK,
operators::DOUBLE_COMPARISONS,
operators::FLOAT_ARITHMETIC,
operators::INEFFECTIVE_BIT_MASK,
operators::INTEGER_ARITHMETIC,

View File

@ -209,7 +209,6 @@ mod disallowed_script_idents;
mod disallowed_types;
mod doc;
mod doc_link_with_quotes;
mod double_comparison;
mod double_parens;
mod drop_forget_ref;
mod duplicate_mod;
@ -726,7 +725,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(useless_conversion::UselessConversion::default()));
store.register_late_pass(|| Box::new(implicit_hasher::ImplicitHasher));
store.register_late_pass(|| Box::new(fallible_impl_from::FallibleImplFrom));
store.register_late_pass(|| Box::new(double_comparison::DoubleComparisons));
store.register_late_pass(|| Box::new(question_mark::QuestionMark));
store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings));
store.register_late_pass(|| Box::new(suspicious_trait_impl::SuspiciousImpl));

View File

@ -0,0 +1,54 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eq_expr_value;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::source_map::Span;
use super::DOUBLE_COMPARISONS;
#[expect(clippy::similar_names)]
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, lhs: &'tcx Expr<'_>, rhs: &'tcx Expr<'_>, span: Span) {
let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (&lhs.kind, &rhs.kind) {
(ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => {
(lb.node, llhs, lrhs, rb.node, rlhs, rrhs)
},
_ => return,
};
if !(eq_expr_value(cx, llhs, rlhs) && eq_expr_value(cx, lrhs, rrhs)) {
return;
}
macro_rules! lint_double_comparison {
($op:tt) => {{
let mut applicability = Applicability::MachineApplicable;
let lhs_str = snippet_with_applicability(cx, llhs.span, "", &mut applicability);
let rhs_str = snippet_with_applicability(cx, lrhs.span, "", &mut applicability);
let sugg = format!("{} {} {}", lhs_str, stringify!($op), rhs_str);
span_lint_and_sugg(
cx,
DOUBLE_COMPARISONS,
span,
"this binary expression can be simplified",
"try",
sugg,
applicability,
);
}};
}
match (op, lkind, rkind) {
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Lt) | (BinOpKind::Or, BinOpKind::Lt, BinOpKind::Eq) => {
lint_double_comparison!(<=);
},
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Gt) | (BinOpKind::Or, BinOpKind::Gt, BinOpKind::Eq) => {
lint_double_comparison!(>=);
},
(BinOpKind::Or, BinOpKind::Lt, BinOpKind::Gt) | (BinOpKind::Or, BinOpKind::Gt, BinOpKind::Lt) => {
lint_double_comparison!(!=);
},
(BinOpKind::And, BinOpKind::Le, BinOpKind::Ge) | (BinOpKind::And, BinOpKind::Ge, BinOpKind::Le) => {
lint_double_comparison!(==);
},
_ => (),
};
}

View File

@ -5,6 +5,7 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
mod absurd_extreme_comparisons;
mod assign_op_pattern;
mod bit_mask;
mod double_comparison;
mod misrefactored_assign_op;
mod numeric_arithmetic;
mod verbose_bit_mask;
@ -242,6 +243,34 @@ declare_clippy_lint! {
"expressions where a bit mask is less readable than the corresponding method call"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for double comparisons that could be simplified to a single expression.
///
///
/// ### Why is this bad?
/// Readability.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x == y || x < y {}
/// ```
///
/// Use instead:
///
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x <= y {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub DOUBLE_COMPARISONS,
complexity,
"unnecessary double comparisons that can be simplified"
}
pub struct Operators {
arithmetic_context: numeric_arithmetic::Context,
verbose_bit_mask_threshold: u64,
@ -255,6 +284,7 @@ impl_lint_pass!(Operators => [
BAD_BIT_MASK,
INEFFECTIVE_BIT_MASK,
VERBOSE_BIT_MASK,
DOUBLE_COMPARISONS,
]);
impl Operators {
pub fn new(verbose_bit_mask_threshold: u64) -> Self {
@ -274,6 +304,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
bit_mask::check(cx, e, op.node, lhs, rhs);
verbose_bit_mask::check(cx, e, op.node, lhs, rhs, self.verbose_bit_mask_threshold);
double_comparison::check(cx, op.node, lhs, rhs, e.span);
},
ExprKind::AssignOp(op, lhs, rhs) => {
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);