Merge commit '1e8fdf4928' into clippyup
This commit is contained in:
Philipp Krones
parent
a1ab2d765f
commit
f730a2655a
72 changed files with 1543 additions and 378 deletions
207
clippy_lints/src/operators/const_comparisons.rs
Normal file
207
clippy_lints/src/operators/const_comparisons.rs
Normal file
|
|
@ -0,0 +1,207 @@
|
|||
#![allow(clippy::match_same_arms)]
|
||||
|
||||
use std::cmp::Ordering;
|
||||
|
||||
use clippy_utils::consts::{constant, Constant};
|
||||
use if_chain::if_chain;
|
||||
use rustc_hir::{BinOpKind, Expr, ExprKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty::layout::HasTyCtxt;
|
||||
use rustc_middle::ty::{Ty, TypeckResults};
|
||||
use rustc_span::source_map::{Span, Spanned};
|
||||
|
||||
use clippy_utils::diagnostics::span_lint_and_note;
|
||||
use clippy_utils::source::snippet;
|
||||
use clippy_utils::SpanlessEq;
|
||||
|
||||
use super::{IMPOSSIBLE_COMPARISONS, REDUNDANT_COMPARISONS};
|
||||
|
||||
// Extract a comparison between a const and non-const
|
||||
// Flip yoda conditionals, turnings expressions like `42 < x` into `x > 42`
|
||||
fn comparison_to_const<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
typeck: &TypeckResults<'tcx>,
|
||||
expr: &'tcx Expr<'tcx>,
|
||||
) -> Option<(CmpOp, &'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Constant<'tcx>, Ty<'tcx>)> {
|
||||
if_chain! {
|
||||
if let ExprKind::Binary(operator, left, right) = expr.kind;
|
||||
if let Ok(cmp_op) = CmpOp::try_from(operator.node);
|
||||
then {
|
||||
match (constant(cx, typeck, left), constant(cx, typeck, right)) {
|
||||
(Some(_), Some(_)) => None,
|
||||
(_, Some(con)) => Some((cmp_op, left, right, con, typeck.expr_ty(right))),
|
||||
(Some(con), _) => Some((cmp_op.reverse(), right, left, con, typeck.expr_ty(left))),
|
||||
_ => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
and_op: Spanned<BinOpKind>,
|
||||
left_cond: &'tcx Expr<'tcx>,
|
||||
right_cond: &'tcx Expr<'tcx>,
|
||||
span: Span,
|
||||
) {
|
||||
if_chain! {
|
||||
// Ensure that the binary operator is &&
|
||||
if and_op.node == BinOpKind::And;
|
||||
|
||||
// Check that both operands to '&&' are themselves a binary operation
|
||||
// The `comparison_to_const` step also checks this, so this step is just an optimization
|
||||
if let ExprKind::Binary(_, _, _) = left_cond.kind;
|
||||
if let ExprKind::Binary(_, _, _) = right_cond.kind;
|
||||
|
||||
let typeck = cx.typeck_results();
|
||||
|
||||
// Check that both operands to '&&' compare a non-literal to a literal
|
||||
if let Some((left_cmp_op, left_expr, left_const_expr, left_const, left_type)) =
|
||||
comparison_to_const(cx, typeck, left_cond);
|
||||
if let Some((right_cmp_op, right_expr, right_const_expr, right_const, right_type)) =
|
||||
comparison_to_const(cx, typeck, right_cond);
|
||||
|
||||
if left_type == right_type;
|
||||
|
||||
// Check that the same expression is compared in both comparisons
|
||||
if SpanlessEq::new(cx).eq_expr(left_expr, right_expr);
|
||||
|
||||
if !left_expr.can_have_side_effects();
|
||||
|
||||
// Compare the two constant expressions
|
||||
if let Some(ordering) = Constant::partial_cmp(cx.tcx(), left_type, &left_const, &right_const);
|
||||
|
||||
// Rule out the `x >= 42 && x <= 42` corner case immediately
|
||||
// Mostly to simplify the implementation, but it is also covered by `clippy::double_comparisons`
|
||||
if !matches!(
|
||||
(&left_cmp_op, &right_cmp_op, ordering),
|
||||
(CmpOp::Le | CmpOp::Ge, CmpOp::Le | CmpOp::Ge, Ordering::Equal)
|
||||
);
|
||||
|
||||
then {
|
||||
if left_cmp_op.direction() == right_cmp_op.direction() {
|
||||
let lhs_str = snippet(cx, left_cond.span, "<lhs>");
|
||||
let rhs_str = snippet(cx, right_cond.span, "<rhs>");
|
||||
// We already know that either side of `&&` has no effect,
|
||||
// but emit a different error message depending on which side it is
|
||||
if left_side_is_useless(left_cmp_op, ordering) {
|
||||
span_lint_and_note(
|
||||
cx,
|
||||
REDUNDANT_COMPARISONS,
|
||||
span,
|
||||
"left-hand side of `&&` operator has no effect",
|
||||
Some(left_cond.span.until(right_cond.span)),
|
||||
&format!("`if `{rhs_str}` evaluates to true, {lhs_str}` will always evaluate to true as well"),
|
||||
);
|
||||
} else {
|
||||
span_lint_and_note(
|
||||
cx,
|
||||
REDUNDANT_COMPARISONS,
|
||||
span,
|
||||
"right-hand side of `&&` operator has no effect",
|
||||
Some(and_op.span.to(right_cond.span)),
|
||||
&format!("`if `{lhs_str}` evaluates to true, {rhs_str}` will always evaluate to true as well"),
|
||||
);
|
||||
}
|
||||
// We could autofix this error but choose not to,
|
||||
// because code triggering this lint probably not behaving correctly in the first place
|
||||
}
|
||||
else if !comparison_is_possible(left_cmp_op.direction(), ordering) {
|
||||
let expr_str = snippet(cx, left_expr.span, "..");
|
||||
let lhs_str = snippet(cx, left_const_expr.span, "<lhs>");
|
||||
let rhs_str = snippet(cx, right_const_expr.span, "<rhs>");
|
||||
let note = match ordering {
|
||||
Ordering::Less => format!("since `{lhs_str}` < `{rhs_str}`, the expression evaluates to false for any value of `{expr_str}`"),
|
||||
Ordering::Equal => format!("`{expr_str}` cannot simultaneously be greater than and less than `{lhs_str}`"),
|
||||
Ordering::Greater => format!("since `{lhs_str}` > `{rhs_str}`, the expression evaluates to false for any value of `{expr_str}`"),
|
||||
};
|
||||
span_lint_and_note(
|
||||
cx,
|
||||
IMPOSSIBLE_COMPARISONS,
|
||||
span,
|
||||
"boolean expression will never evaluate to 'true'",
|
||||
None,
|
||||
¬e,
|
||||
);
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn left_side_is_useless(left_cmp_op: CmpOp, ordering: Ordering) -> bool {
|
||||
// Special-case for equal constants with an inclusive comparison
|
||||
if ordering == Ordering::Equal {
|
||||
match left_cmp_op {
|
||||
CmpOp::Lt | CmpOp::Gt => false,
|
||||
CmpOp::Le | CmpOp::Ge => true,
|
||||
}
|
||||
} else {
|
||||
match (left_cmp_op.direction(), ordering) {
|
||||
(CmpOpDirection::Lesser, Ordering::Less) => false,
|
||||
(CmpOpDirection::Lesser, Ordering::Equal) => false,
|
||||
(CmpOpDirection::Lesser, Ordering::Greater) => true,
|
||||
(CmpOpDirection::Greater, Ordering::Less) => true,
|
||||
(CmpOpDirection::Greater, Ordering::Equal) => false,
|
||||
(CmpOpDirection::Greater, Ordering::Greater) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn comparison_is_possible(left_cmp_direction: CmpOpDirection, ordering: Ordering) -> bool {
|
||||
match (left_cmp_direction, ordering) {
|
||||
(CmpOpDirection::Lesser, Ordering::Less | Ordering::Equal) => false,
|
||||
(CmpOpDirection::Lesser, Ordering::Greater) => true,
|
||||
(CmpOpDirection::Greater, Ordering::Greater | Ordering::Equal) => false,
|
||||
(CmpOpDirection::Greater, Ordering::Less) => true,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(PartialEq, Eq, Clone, Copy)]
|
||||
enum CmpOpDirection {
|
||||
Lesser,
|
||||
Greater,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum CmpOp {
|
||||
Lt,
|
||||
Le,
|
||||
Ge,
|
||||
Gt,
|
||||
}
|
||||
|
||||
impl CmpOp {
|
||||
fn reverse(self) -> Self {
|
||||
match self {
|
||||
CmpOp::Lt => CmpOp::Gt,
|
||||
CmpOp::Le => CmpOp::Ge,
|
||||
CmpOp::Ge => CmpOp::Le,
|
||||
CmpOp::Gt => CmpOp::Lt,
|
||||
}
|
||||
}
|
||||
|
||||
fn direction(self) -> CmpOpDirection {
|
||||
match self {
|
||||
CmpOp::Lt => CmpOpDirection::Lesser,
|
||||
CmpOp::Le => CmpOpDirection::Lesser,
|
||||
CmpOp::Ge => CmpOpDirection::Greater,
|
||||
CmpOp::Gt => CmpOpDirection::Greater,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl TryFrom<BinOpKind> for CmpOp {
|
||||
type Error = ();
|
||||
|
||||
fn try_from(bin_op: BinOpKind) -> Result<Self, Self::Error> {
|
||||
match bin_op {
|
||||
BinOpKind::Lt => Ok(CmpOp::Lt),
|
||||
BinOpKind::Le => Ok(CmpOp::Le),
|
||||
BinOpKind::Ge => Ok(CmpOp::Ge),
|
||||
BinOpKind::Gt => Ok(CmpOp::Gt),
|
||||
_ => Err(()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -2,6 +2,7 @@ mod absurd_extreme_comparisons;
|
|||
mod assign_op_pattern;
|
||||
mod bit_mask;
|
||||
mod cmp_owned;
|
||||
mod const_comparisons;
|
||||
mod double_comparison;
|
||||
mod duration_subsec;
|
||||
mod eq_op;
|
||||
|
|
@ -298,6 +299,45 @@ declare_clippy_lint! {
|
|||
"unnecessary double comparisons that can be simplified"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for double comparisons that can never succeed
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// The whole expression can be replaced by `false`,
|
||||
/// which is probably not the programmer's intention
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// # let status_code = 200;
|
||||
/// if status_code <= 400 && status_code > 500 {}
|
||||
/// ```
|
||||
#[clippy::version = "1.71.0"]
|
||||
pub IMPOSSIBLE_COMPARISONS,
|
||||
correctness,
|
||||
"double comparisons that will never evaluate to `true`"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for ineffective double comparisons against constants.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// Only one of the comparisons has any effect on the result, the programmer
|
||||
/// probably intended to flip one of the comparison operators, or compare a
|
||||
/// different value entirely.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// # let status_code = 200;
|
||||
/// if status_code <= 400 && status_code < 500 {}
|
||||
/// ```
|
||||
#[clippy::version = "1.71.0"]
|
||||
pub REDUNDANT_COMPARISONS,
|
||||
correctness,
|
||||
"double comparisons where one of them can be removed"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for calculation of subsecond microseconds or milliseconds
|
||||
|
|
@ -742,6 +782,8 @@ impl_lint_pass!(Operators => [
|
|||
INEFFECTIVE_BIT_MASK,
|
||||
VERBOSE_BIT_MASK,
|
||||
DOUBLE_COMPARISONS,
|
||||
IMPOSSIBLE_COMPARISONS,
|
||||
REDUNDANT_COMPARISONS,
|
||||
DURATION_SUBSEC,
|
||||
EQ_OP,
|
||||
OP_REF,
|
||||
|
|
@ -786,6 +828,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
|
|||
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);
|
||||
const_comparisons::check(cx, op, lhs, rhs, e.span);
|
||||
duration_subsec::check(cx, e, op.node, lhs, rhs);
|
||||
float_equality_without_abs::check(cx, e, op.node, lhs, rhs);
|
||||
integer_division::check(cx, e, op.node, lhs, rhs);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue