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

Added a lint which corrects expressions like (a - b) < f32::EPSILON

Browse Source
This commit is contained in:
Bastian 2020-08-24 16:31:51 +02:00
parent 27ae4d303c
commit 680c68153b
6 changed files with 226 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1498,6 +1498,7 @@ Released 2018-09-13
[`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic
[`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp
[`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
[`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs
[`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
[`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
[`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast

112
clippy_lints/src/float_equality_without_abs.rs Normal file
View File

@ -0,0 +1,112 @@
use crate::utils::{match_qpath, snippet, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
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 unneccessary. 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
/// }
/// ```
pub FLOAT_EQUALITY_WITHOUT_ABS,
correctness,
"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, ref left, ref 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 substraction
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 match_qpath(epsilon_path, &["f32", "EPSILON"]) || match_qpath(epsilon_path, &["f64", "EPSILON"]);
// values of the substractions 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 {
// get the snippet string
let lhs_string = snippet(
cx,
lhs.span,
"(...)",
);
// format the suggestion
let suggestion = if lhs_string.starts_with('(') {
format!("{}.abs()", lhs_string)
} else {
format!("({}).abs()", lhs_string)
};
// spans the lint
span_lint_and_sugg(
cx,
FLOAT_EQUALITY_WITHOUT_ABS,
expr.span,
"float equality check without `.abs()`",
"add `.abs()`",
suggestion,
Applicability::MaybeIncorrect,
);
}
}
}
}

5
clippy_lints/src/lib.rs
View File

@ -193,6 +193,7 @@ 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;
@ -549,6 +550,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&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,
@ -1093,6 +1095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
store.register_late_pass(|| box repeat_once::RepeatOnce);
store.register_late_pass(|| box self_assignment::SelfAssignment);
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1268,6 +1271,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&eval_order_dependence::DIVERGING_SUB_EXPRESSION),
LintId::of(&eval_order_dependence::EVAL_ORDER_DEPENDENCE),
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(&formatting::POSSIBLE_MISSING_COMMA),
@ -1686,6 +1690,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
LintId::of(&eq_op::EQ_OP),
LintId::of(&erasing_op::ERASING_OP),
LintId::of(&float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
LintId::of(&if_let_mutex::IF_LET_MUTEX),

7
src/lintlist/mod.rs
View File

@ -661,6 +661,13 @@
deprecation: None,
module: "misc",
},
Lint {
name: "float_equality_without_abs",
group: "correctness",
desc: "float equality check without `.abs()`",
deprecation: None,
module: "float_equality_without_abs",
},
Lint {
name: "fn_address_comparisons",
group: "correctness",

31
tests/ui/float_equality_without_abs.rs Normal file
View File

@ -0,0 +1,31 @@
#![warn(clippy::float_equality_without_abs)]
pub fn is_roughly_equal(a: f32, b: f32) -> bool {
(a - b) < f32::EPSILON
}
pub fn main() {
// all errors
is_roughly_equal(1.0, 2.0);
let a = 0.05;
let b = 0.0500001;
let _ = (a - b) < f32::EPSILON;
let _ = a - b < f32::EPSILON;
let _ = a - b.abs() < f32::EPSILON;
let _ = (a as f64 - b as f64) < f64::EPSILON;
let _ = 1.0 - 2.0 < f32::EPSILON;
let _ = f32::EPSILON > (a - b);
let _ = f32::EPSILON > a - b;
let _ = f32::EPSILON > a - b.abs();
let _ = f64::EPSILON > (a as f64 - b as f64);
let _ = f32::EPSILON > 1.0 - 2.0;
// those are correct
let _ = (a - b).abs() < f32::EPSILON;
let _ = (a as f64 - b as f64).abs() < f64::EPSILON;
let _ = f32::EPSILON > (a - b).abs();
let _ = f64::EPSILON > (a as f64 - b as f64).abs();
}

70
tests/ui/float_equality_without_abs.stderr Normal file
View File

@ -0,0 +1,70 @@
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:4:5
|
LL | (a - b) < f32::EPSILON
| ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
|
= note: `-D clippy::float-equality-without-abs` implied by `-D warnings`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:13:13
|
LL | let _ = (a - b) < f32::EPSILON;
| ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:14:13
|
LL | let _ = a - b < f32::EPSILON;
| ^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:15:13
|
LL | let _ = a - b.abs() < f32::EPSILON;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b.abs()).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:16:13
|
LL | let _ = (a as f64 - b as f64) < f64::EPSILON;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a as f64 - b as f64).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:17:13
|
LL | let _ = 1.0 - 2.0 < f32::EPSILON;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(1.0 - 2.0).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:19:13
|
LL | let _ = f32::EPSILON > (a - b);
| ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:20:13
|
LL | let _ = f32::EPSILON > a - b;
| ^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:21:13
|
LL | let _ = f32::EPSILON > a - b.abs();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b.abs()).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:22:13
|
LL | let _ = f64::EPSILON > (a as f64 - b as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a as f64 - b as f64).abs()`
error: float equality check without `.abs()`
--> $DIR/float_equality_without_abs.rs:23:13
|
LL | let _ = f32::EPSILON > 1.0 - 2.0;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(1.0 - 2.0).abs()`
error: aborting due to 11 previous errors