mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Move FloatEqualityWithoutAbs into Operators lint pass

Browse Source
This commit is contained in:
Jason Newcomb 2022-06-01 01:06:56 -04:00
parent 3de70a4996
commit 56f50d36e7
7 changed files with 110 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
clippy_lints/src/float_equality_without_abs.rs
View File

@ -1,116 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{match_def_path, paths, sugg};
use if_chain::if_chain;
use rustc_ast::util::parser::AssocOp;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;
declare_clippy_lint! {
/// ### What it does
/// Checks for statements of the form `(a - b) < f32::EPSILON` or
/// `(a - b) < f64::EPSILON`. Notes the missing `.abs()`.
///
/// ### Why is this bad?
/// The code without `.abs()` is more likely to have a bug.
///
/// ### Known problems
/// If the user can ensure that b is larger than a, the `.abs()` is
/// technically unnecessary. However, it will make the code more robust and doesn't have any
/// large performance implications. If the abs call was deliberately left out for performance
/// reasons, it is probably better to state this explicitly in the code, which then can be done
/// with an allow.
///
/// ### Example
/// ```rust
/// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
/// (a - b) < f32::EPSILON
/// }
/// ```
/// Use instead:
/// ```rust
/// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
/// (a - b).abs() < f32::EPSILON
/// }
/// ```
#[clippy::version = "1.48.0"]
pub FLOAT_EQUALITY_WITHOUT_ABS,
suspicious,
"float equality check without `.abs()`"
}
declare_lint_pass!(FloatEqualityWithoutAbs => [FLOAT_EQUALITY_WITHOUT_ABS]);
impl<'tcx> LateLintPass<'tcx> for FloatEqualityWithoutAbs {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let lhs;
let rhs;
// check if expr is a binary expression with a lt or gt operator
if let ExprKind::Binary(op, left, right) = expr.kind {
match op.node {
BinOpKind::Lt => {
lhs = left;
rhs = right;
},
BinOpKind::Gt => {
lhs = right;
rhs = left;
},
_ => return,
};
} else {
return;
}
if_chain! {
// left hand side is a subtraction
if let ExprKind::Binary(
Spanned {
node: BinOpKind::Sub,
..
},
val_l,
val_r,
) = lhs.kind;
// right hand side matches either f32::EPSILON or f64::EPSILON
if let ExprKind::Path(ref epsilon_path) = rhs.kind;
if let Res::Def(DefKind::AssocConst, def_id) = cx.qpath_res(epsilon_path, rhs.hir_id);
if match_def_path(cx, def_id, &paths::F32_EPSILON) || match_def_path(cx, def_id, &paths::F64_EPSILON);
// values of the subtractions on the left hand side are of the type float
let t_val_l = cx.typeck_results().expr_ty(val_l);
let t_val_r = cx.typeck_results().expr_ty(val_r);
if let ty::Float(_) = t_val_l.kind();
if let ty::Float(_) = t_val_r.kind();
then {
let sug_l = sugg::Sugg::hir(cx, val_l, "..");
let sug_r = sugg::Sugg::hir(cx, val_r, "..");
// format the suggestion
let suggestion = format!("{}.abs()", sugg::make_assoc(AssocOp::Subtract, &sug_l, &sug_r).maybe_par());
// spans the lint
span_lint_and_then(
cx,
FLOAT_EQUALITY_WITHOUT_ABS,
expr.span,
"float equality check without `.abs()`",
| diag | {
diag.span_suggestion(
lhs.span,
"add `.abs()`",
suggestion,
Applicability::MaybeIncorrect,
);
}
);
}
}
}
}

2
clippy_lints/src/lib.register_all.rs
View File

@ -64,7 +64,6 @@
LintId::of(escape::BOXED_LOCAL),
LintId::of(eta_reduction::REDUNDANT_CLOSURE),
LintId::of(explicit_write::EXPLICIT_WRITE),
LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(float_literal::EXCESSIVE_PRECISION),
LintId::of(format::USELESS_FORMAT),
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),
@ -257,6 +256,7 @@
LintId::of(operators::DURATION_SUBSEC),
LintId::of(operators::EQ_OP),
LintId::of(operators::ERASING_OP),
LintId::of(operators::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(operators::INEFFECTIVE_BIT_MASK),
LintId::of(operators::MISREFACTORED_ASSIGN_OP),
LintId::of(operators::OP_REF),

2
clippy_lints/src/lib.register_lints.rs
View File

@ -151,7 +151,6 @@
exit::EXIT,
explicit_write::EXPLICIT_WRITE,
fallible_impl_from::FALLIBLE_IMPL_FROM,
float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS,
float_literal::EXCESSIVE_PRECISION,
float_literal::LOSSY_FLOAT_LITERAL,
floating_point_arithmetic::IMPRECISE_FLOPS,
@ -432,6 +431,7 @@
operators::EQ_OP,
operators::ERASING_OP,
operators::FLOAT_ARITHMETIC,
operators::FLOAT_EQUALITY_WITHOUT_ABS,
operators::INEFFECTIVE_BIT_MASK,
operators::INTEGER_ARITHMETIC,
operators::MISREFACTORED_ASSIGN_OP,

2
clippy_lints/src/lib.register_suspicious.rs
View File

@ -15,7 +15,6 @@
LintId::of(drop_forget_ref::DROP_NON_DROP),
LintId::of(drop_forget_ref::FORGET_NON_DROP),
LintId::of(duplicate_mod::DUPLICATE_MOD),
LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),
LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING),
@ -28,6 +27,7 @@
LintId::of(methods::SUSPICIOUS_MAP),
LintId::of(mut_key::MUTABLE_KEY_TYPE),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(operators::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(operators::MISREFACTORED_ASSIGN_OP),
LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),

2
clippy_lints/src/lib.rs
View File

@ -227,7 +227,6 @@ macro_rules! declare_clippy_lint {
mod exit;
mod explicit_write;
mod fallible_impl_from;
mod float_equality_without_abs;
mod float_literal;
mod floating_point_arithmetic;
mod format;
@ -842,7 +841,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(unwrap_in_result::UnwrapInResult));
store.register_late_pass(|| Box::new(self_assignment::SelfAssignment));
store.register_late_pass(|| Box::new(manual_ok_or::ManualOkOr));
store.register_late_pass(|| Box::new(float_equality_without_abs::FloatEqualityWithoutAbs));
store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned));
store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync));
let disallowed_methods = conf.disallowed_methods.clone();

71
clippy_lints/src/operators/float_equality_without_abs.rs Normal file
View File

@ -0,0 +1,71 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{match_def_path, paths, sugg};
use if_chain::if_chain;
use rustc_ast::util::parser::AssocOp;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Spanned;
use super::FLOAT_EQUALITY_WITHOUT_ABS;
pub(crate) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: BinOpKind,
lhs: &'tcx Expr<'_>,
rhs: &'tcx Expr<'_>,
) {
let (lhs, rhs) = match op {
BinOpKind::Lt => (lhs, rhs),
BinOpKind::Gt => (rhs, lhs),
_ => return,
};
if_chain! {
// left hand side is a subtraction
if let ExprKind::Binary(
Spanned {
node: BinOpKind::Sub,
..
},
val_l,
val_r,
) = lhs.kind;
// right hand side matches either f32::EPSILON or f64::EPSILON
if let ExprKind::Path(ref epsilon_path) = rhs.kind;
if let Res::Def(DefKind::AssocConst, def_id) = cx.qpath_res(epsilon_path, rhs.hir_id);
if match_def_path(cx, def_id, &paths::F32_EPSILON) || match_def_path(cx, def_id, &paths::F64_EPSILON);
// values of the subtractions on the left hand side are of the type float
let t_val_l = cx.typeck_results().expr_ty(val_l);
let t_val_r = cx.typeck_results().expr_ty(val_r);
if let ty::Float(_) = t_val_l.kind();
if let ty::Float(_) = t_val_r.kind();
then {
let sug_l = sugg::Sugg::hir(cx, val_l, "..");
let sug_r = sugg::Sugg::hir(cx, val_r, "..");
// format the suggestion
let suggestion = format!("{}.abs()", sugg::make_assoc(AssocOp::Subtract, &sug_l, &sug_r).maybe_par());
// spans the lint
span_lint_and_then(
cx,
FLOAT_EQUALITY_WITHOUT_ABS,
expr.span,
"float equality check without `.abs()`",
| diag | {
diag.span_suggestion(
lhs.span,
"add `.abs()`",
suggestion,
Applicability::MaybeIncorrect,
);
}
);
}
}
}

36
clippy_lints/src/operators/mod.rs
View File

@ -9,6 +9,7 @@
mod duration_subsec;
mod eq_op;
mod erasing_op;
mod float_equality_without_abs;
mod misrefactored_assign_op;
mod numeric_arithmetic;
mod op_ref;
@ -383,6 +384,39 @@
"using erasing operations, e.g., `x * 0` or `y & 0`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for statements of the form `(a - b) < f32::EPSILON` or
/// `(a - b) < f64::EPSILON`. Notes the missing `.abs()`.
///
/// ### Why is this bad?
/// The code without `.abs()` is more likely to have a bug.
///
/// ### Known problems
/// If the user can ensure that b is larger than a, the `.abs()` is
/// technically unnecessary. However, it will make the code more robust and doesn't have any
/// large performance implications. If the abs call was deliberately left out for performance
/// reasons, it is probably better to state this explicitly in the code, which then can be done
/// with an allow.
///
/// ### Example
/// ```rust
/// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
/// (a - b) < f32::EPSILON
/// }
/// ```
/// Use instead:
/// ```rust
/// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
/// (a - b).abs() < f32::EPSILON
/// }
/// ```
#[clippy::version = "1.48.0"]
pub FLOAT_EQUALITY_WITHOUT_ABS,
suspicious,
"float equality check without `.abs()`"
}
pub struct Operators {
arithmetic_context: numeric_arithmetic::Context,
verbose_bit_mask_threshold: u64,
@ -401,6 +435,7 @@ pub struct Operators {
EQ_OP,
OP_REF,
ERASING_OP,
FLOAT_EQUALITY_WITHOUT_ABS,
]);
impl Operators {
pub fn new(verbose_bit_mask_threshold: u64) -> Self {
@ -428,6 +463,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
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);
duration_subsec::check(cx, e, op.node, lhs, rhs);
float_equality_without_abs::check(cx, e, op.node, lhs, rhs);
},
ExprKind::AssignOp(op, lhs, rhs) => {
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);