From f12edfdb53b13008dcff9eba2a3fd9a19370d2ee Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Fri, 30 Jun 2023 11:31:08 -0500 Subject: [PATCH 1/3] `manual_float_methods` --- CHANGELOG.md | 2 + clippy_lints/src/declared_lints.rs | 2 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_float_methods.rs | 106 +++++++++++++++++++++++ clippy_utils/src/consts.rs | 11 +++ tests/ui/manual_float_methods.fixed | 37 ++++++++ tests/ui/manual_float_methods.rs | 37 ++++++++ tests/ui/manual_float_methods.stderr | 42 +++++++++ 8 files changed, 239 insertions(+) create mode 100644 clippy_lints/src/manual_float_methods.rs create mode 100644 tests/ui/manual_float_methods.fixed create mode 100644 tests/ui/manual_float_methods.rs create mode 100644 tests/ui/manual_float_methods.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index e2004c2931d5..59719080e440 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4941,6 +4941,8 @@ Released 2018-09-13 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check +[`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite +[`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index ca97db040792..f8b8b94dd29a 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -273,6 +273,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::manual_async_fn::MANUAL_ASYNC_FN_INFO, crate::manual_bits::MANUAL_BITS_INFO, crate::manual_clamp::MANUAL_CLAMP_INFO, + crate::manual_float_methods::MANUAL_IS_FINITE_INFO, + crate::manual_float_methods::MANUAL_IS_INFINITE_INFO, crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO, crate::manual_let_else::MANUAL_LET_ELSE_INFO, crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 00d46025caa8..9abcbe01176d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -184,6 +184,7 @@ mod manual_assert; mod manual_async_fn; mod manual_bits; mod manual_clamp; +mod manual_float_methods; mod manual_is_ascii_check; mod manual_let_else; mod manual_main_separator_str; @@ -1073,6 +1074,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns)); store.register_early_pass(|| Box::new(visibility::Visibility)); store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() })); + store.register_late_pass(|_| Box::new(manual_float_methods::ManualFloatMethods)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_float_methods.rs b/clippy_lints/src/manual_float_methods.rs new file mode 100644 index 000000000000..abf1eb788dd0 --- /dev/null +++ b/clippy_lints/src/manual_float_methods.rs @@ -0,0 +1,106 @@ +use clippy_utils::{ + consts::constant, diagnostics::span_lint_and_sugg, is_from_proc_macro, path_to_local, source::snippet_opt, +}; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `x == ::INFINITY || x == ::NEG_INFINITY`. + /// + /// ### Why is this bad? + /// This should use the dedicated method instead, `is_infinite`. + /// + /// ### Example + /// ```rust + /// # let x = 1.0f32; + /// if x == f32::INFINITY || x == f32::NEG_INFINITY {} + /// ``` + /// Use instead: + /// ```rust + /// # let x = 1.0f32; + /// if x.is_infinite() {} + /// ``` + #[clippy::version = "1.72.0"] + pub MANUAL_IS_INFINITE, + style, + "use dedicated method to check if a float is infinite" +} +declare_clippy_lint! { + /// ### What it does + /// Checks for `x != ::INFINITY && x != ::NEG_INFINITY`. + /// + /// ### Why is this bad? + /// This should use the dedicated method instead, `is_finite`. + /// + /// ### Example + /// ```rust + /// # let x = 1.0f32; + /// if x != f32::INFINITY && x != f32::NEG_INFINITY {} + /// ``` + /// Use instead: + /// ```rust + /// # let x = 1.0f32; + /// if x.is_finite() {} + /// ``` + #[clippy::version = "1.72.0"] + pub MANUAL_IS_FINITE, + style, + "use dedicated method to check if a float is finite" +} +declare_lint_pass!(ManualFloatMethods => [MANUAL_IS_INFINITE, MANUAL_IS_FINITE]); + +impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if !in_external_macro(cx.sess(), expr.span) + && let ExprKind::Binary(kind, lhs, rhs) = expr.kind + && let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind + && let ExprKind::Binary(rhs_kind, rhs_lhs, rhs_rhs) = rhs.kind + && let (operands, consts) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs] + .into_iter() + .partition::>, _>(|i| path_to_local(i).is_some()) + && let [first, second] = &*operands + && let Some([const_1, const_2]) = consts + .into_iter() + .map(|i| constant(cx, cx.typeck_results(), i).and_then(|c| c.to_bits())) + .collect::>>() + .as_deref() + && path_to_local(first).is_some_and(|f| path_to_local(second).is_some_and(|s| f == s)) + && (is_infinity(*const_1) && is_neg_infinity(*const_2) + || is_neg_infinity(*const_1) && is_infinity(*const_2)) + && let Some(local_snippet) = snippet_opt(cx, first.span) + && !is_from_proc_macro(cx, expr) + { + let (msg, lint, sugg_fn) = match (kind.node, lhs_kind.node, rhs_kind.node) { + (BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => { + ("manually checking if a float is infinite", MANUAL_IS_INFINITE, "is_infinite") + }, + (BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => { + ("manually checking if a float is finite", MANUAL_IS_FINITE, "is_finite") + }, + _ => return, + }; + + span_lint_and_sugg( + cx, + lint, + expr.span, + msg, + "try", + format!("{local_snippet}.{sugg_fn}()"), + Applicability::MachineApplicable, + ); + } + } +} + +fn is_infinity(bits: u128) -> bool { + bits == 0x7f80_0000 || bits == 0x7ff0_0000_0000_0000 +} + +fn is_neg_infinity(bits: u128) -> bool { + bits == 0xff80_0000 || bits == 0xfff0_0000_0000_0000 +} diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 87d85d742ce4..b9f8eefeb8e7 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -190,6 +190,17 @@ impl<'tcx> Constant<'tcx> { } } + /// Returns the bit representation if `self` is a bool, integer, or float. + pub fn to_bits(&self) -> Option { + match self { + Constant::Int(int) => Some(*int), + Constant::F32(float) => Some(u128::from(float.to_bits())), + Constant::F64(float) => Some(u128::from(float.to_bits())), + Constant::Bool(bool) => Some(u128::from(*bool)), + _ => None, + } + } + /// Returns the integer value or `None` if `self` or `val_type` is not integer type. pub fn int_value(&self, cx: &LateContext<'_>, val_type: Ty<'_>) -> Option { if let Constant::Int(const_int) = *self { diff --git a/tests/ui/manual_float_methods.fixed b/tests/ui/manual_float_methods.fixed new file mode 100644 index 000000000000..87a23b5a75b7 --- /dev/null +++ b/tests/ui/manual_float_methods.fixed @@ -0,0 +1,37 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow(clippy::needless_if, unused)] +#![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] + +#[macro_use] +extern crate proc_macros; + +const INFINITE: f32 = f32::INFINITY; +const NEG_INFINITE: f32 = f32::NEG_INFINITY; + +fn main() { + let x = 1.0f32; + if x.is_infinite() {} + if x.is_finite() {} + if x.is_infinite() {} + if x.is_finite() {} + let x = 1.0f64; + if x.is_infinite() {} + if x.is_finite() {} + // Don't lint + if x.is_infinite() {} + if x.is_finite() {} + // If they're doing it this way, they probably know what they're doing + if x.abs() < f64::INFINITY {} + external! { + let x = 1.0; + if x == f32::INFINITY || x == f32::NEG_INFINITY {} + if x != f32::INFINITY && x != f32::NEG_INFINITY {} + } + with_span! { + span + let x = 1.0; + if x == f32::INFINITY || x == f32::NEG_INFINITY {} + if x != f32::INFINITY && x != f32::NEG_INFINITY {} + } +} diff --git a/tests/ui/manual_float_methods.rs b/tests/ui/manual_float_methods.rs new file mode 100644 index 000000000000..9255bf934187 --- /dev/null +++ b/tests/ui/manual_float_methods.rs @@ -0,0 +1,37 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow(clippy::needless_if, unused)] +#![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] + +#[macro_use] +extern crate proc_macros; + +const INFINITE: f32 = f32::INFINITY; +const NEG_INFINITE: f32 = f32::NEG_INFINITY; + +fn main() { + let x = 1.0f32; + if x == f32::INFINITY || x == f32::NEG_INFINITY {} + if x != f32::INFINITY && x != f32::NEG_INFINITY {} + if x == INFINITE || x == NEG_INFINITE {} + if x != INFINITE && x != NEG_INFINITE {} + let x = 1.0f64; + if x == f64::INFINITY || x == f64::NEG_INFINITY {} + if x != f64::INFINITY && x != f64::NEG_INFINITY {} + // Don't lint + if x.is_infinite() {} + if x.is_finite() {} + // If they're doing it this way, they probably know what they're doing + if x.abs() < f64::INFINITY {} + external! { + let x = 1.0; + if x == f32::INFINITY || x == f32::NEG_INFINITY {} + if x != f32::INFINITY && x != f32::NEG_INFINITY {} + } + with_span! { + span + let x = 1.0; + if x == f32::INFINITY || x == f32::NEG_INFINITY {} + if x != f32::INFINITY && x != f32::NEG_INFINITY {} + } +} diff --git a/tests/ui/manual_float_methods.stderr b/tests/ui/manual_float_methods.stderr new file mode 100644 index 000000000000..00227593a9ea --- /dev/null +++ b/tests/ui/manual_float_methods.stderr @@ -0,0 +1,42 @@ +error: manually checking if a float is infinite + --> $DIR/manual_float_methods.rs:14:8 + | +LL | if x == f32::INFINITY || x == f32::NEG_INFINITY {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + | + = note: `-D clippy::manual-is-infinite` implied by `-D warnings` + +error: manually checking if a float is finite + --> $DIR/manual_float_methods.rs:15:8 + | +LL | if x != f32::INFINITY && x != f32::NEG_INFINITY {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + | + = note: `-D clippy::manual-is-finite` implied by `-D warnings` + +error: manually checking if a float is infinite + --> $DIR/manual_float_methods.rs:16:8 + | +LL | if x == INFINITE || x == NEG_INFINITE {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + +error: manually checking if a float is finite + --> $DIR/manual_float_methods.rs:17:8 + | +LL | if x != INFINITE && x != NEG_INFINITE {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + +error: manually checking if a float is infinite + --> $DIR/manual_float_methods.rs:19:8 + | +LL | if x == f64::INFINITY || x == f64::NEG_INFINITY {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + +error: manually checking if a float is finite + --> $DIR/manual_float_methods.rs:20:8 + | +LL | if x != f64::INFINITY && x != f64::NEG_INFINITY {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + +error: aborting due to 6 previous errors + From 5949f762bfa15e62657771e556c160abe4caee8b Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Sun, 2 Jul 2023 11:43:05 -0500 Subject: [PATCH 2/3] Make suggestion give multiple alternatives --- clippy_lints/src/manual_float_methods.rs | 82 +++++++++++++++++++----- tests/ui/manual_float_methods.fixed | 37 ----------- tests/ui/manual_float_methods.rs | 1 - tests/ui/manual_float_methods.stderr | 62 ++++++++++++++---- 4 files changed, 117 insertions(+), 65 deletions(-) delete mode 100644 tests/ui/manual_float_methods.fixed diff --git a/clippy_lints/src/manual_float_methods.rs b/clippy_lints/src/manual_float_methods.rs index abf1eb788dd0..0f488627b4e9 100644 --- a/clippy_lints/src/manual_float_methods.rs +++ b/clippy_lints/src/manual_float_methods.rs @@ -1,9 +1,9 @@ use clippy_utils::{ - consts::constant, diagnostics::span_lint_and_sugg, is_from_proc_macro, path_to_local, source::snippet_opt, + consts::constant, diagnostics::span_lint_and_then, is_from_proc_macro, path_to_local, source::snippet_opt, }; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -53,12 +53,37 @@ declare_clippy_lint! { } declare_lint_pass!(ManualFloatMethods => [MANUAL_IS_INFINITE, MANUAL_IS_FINITE]); +#[derive(Clone, Copy)] +enum Variant { + ManualIsInfinite, + ManualIsFinite, +} + +impl Variant { + pub fn lint(self) -> &'static Lint { + match self { + Self::ManualIsInfinite => MANUAL_IS_INFINITE, + Self::ManualIsFinite => MANUAL_IS_FINITE, + } + } + + pub fn msg(self) -> &'static str { + match self { + Self::ManualIsInfinite => "manually checking if a float is infinite", + Self::ManualIsFinite => "manually checking if a float is finite", + } + } +} + impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if !in_external_macro(cx.sess(), expr.span) + && (!cx.param_env.is_const() || cx.tcx.features().active(sym!(const_float_classify))) && let ExprKind::Binary(kind, lhs, rhs) = expr.kind && let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind && let ExprKind::Binary(rhs_kind, rhs_lhs, rhs_rhs) = rhs.kind + // Checking all possible scenarios using a function would be a hopeless task, as we have + // 16 possible alignments of constants/operands. For now, let's use `partition`. && let (operands, consts) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs] .into_iter() .partition::>, _>(|i| path_to_local(i).is_some()) @@ -74,24 +99,51 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { && let Some(local_snippet) = snippet_opt(cx, first.span) && !is_from_proc_macro(cx, expr) { - let (msg, lint, sugg_fn) = match (kind.node, lhs_kind.node, rhs_kind.node) { - (BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => { - ("manually checking if a float is infinite", MANUAL_IS_INFINITE, "is_infinite") - }, - (BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => { - ("manually checking if a float is finite", MANUAL_IS_FINITE, "is_finite") - }, + let variant = match (kind.node, lhs_kind.node, rhs_kind.node) { + (BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => Variant::ManualIsInfinite, + (BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => Variant::ManualIsFinite, _ => return, }; - span_lint_and_sugg( + span_lint_and_then( cx, - lint, + variant.lint(), expr.span, - msg, - "try", - format!("{local_snippet}.{sugg_fn}()"), - Applicability::MachineApplicable, + variant.msg(), + |diag| { + match variant { + Variant::ManualIsInfinite => { + diag.span_suggestion( + expr.span, + "use the dedicated method instead", + format!("{local_snippet}.is_infinite()"), + Applicability::MachineApplicable, + ); + }, + Variant::ManualIsFinite => { + // TODO: There's probably some better way to do this, i.e., create + // multiple suggestions with notes between each of them + diag.span_suggestion_verbose( + expr.span, + "use the dedicated method instead", + format!("{local_snippet}.is_finite()"), + Applicability::MaybeIncorrect, + ); + diag.span_suggestion_verbose( + expr.span, + "this will alter how it handles NaN; if that is a problem, use instead", + format!("{local_snippet}.is_finite() || {local_snippet}.is_nan()"), + Applicability::MaybeIncorrect, + ); + diag.span_suggestion_verbose( + expr.span, + "or, for conciseness", + format!("!{local_snippet}.is_infinite()"), + Applicability::MaybeIncorrect, + ); + } + } + } ); } } diff --git a/tests/ui/manual_float_methods.fixed b/tests/ui/manual_float_methods.fixed deleted file mode 100644 index 87a23b5a75b7..000000000000 --- a/tests/ui/manual_float_methods.fixed +++ /dev/null @@ -1,37 +0,0 @@ -//@run-rustfix -//@aux-build:proc_macros.rs:proc-macro -#![allow(clippy::needless_if, unused)] -#![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] - -#[macro_use] -extern crate proc_macros; - -const INFINITE: f32 = f32::INFINITY; -const NEG_INFINITE: f32 = f32::NEG_INFINITY; - -fn main() { - let x = 1.0f32; - if x.is_infinite() {} - if x.is_finite() {} - if x.is_infinite() {} - if x.is_finite() {} - let x = 1.0f64; - if x.is_infinite() {} - if x.is_finite() {} - // Don't lint - if x.is_infinite() {} - if x.is_finite() {} - // If they're doing it this way, they probably know what they're doing - if x.abs() < f64::INFINITY {} - external! { - let x = 1.0; - if x == f32::INFINITY || x == f32::NEG_INFINITY {} - if x != f32::INFINITY && x != f32::NEG_INFINITY {} - } - with_span! { - span - let x = 1.0; - if x == f32::INFINITY || x == f32::NEG_INFINITY {} - if x != f32::INFINITY && x != f32::NEG_INFINITY {} - } -} diff --git a/tests/ui/manual_float_methods.rs b/tests/ui/manual_float_methods.rs index 9255bf934187..b772202c003f 100644 --- a/tests/ui/manual_float_methods.rs +++ b/tests/ui/manual_float_methods.rs @@ -1,4 +1,3 @@ -//@run-rustfix //@aux-build:proc_macros.rs:proc-macro #![allow(clippy::needless_if, unused)] #![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] diff --git a/tests/ui/manual_float_methods.stderr b/tests/ui/manual_float_methods.stderr index 00227593a9ea..7f4a4d2e95b3 100644 --- a/tests/ui/manual_float_methods.stderr +++ b/tests/ui/manual_float_methods.stderr @@ -1,42 +1,80 @@ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:14:8 + --> $DIR/manual_float_methods.rs:13:8 | LL | if x == f32::INFINITY || x == f32::NEG_INFINITY {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` | = note: `-D clippy::manual-is-infinite` implied by `-D warnings` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:15:8 + --> $DIR/manual_float_methods.rs:14:8 | LL | if x != f32::INFINITY && x != f32::NEG_INFINITY {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-is-finite` implied by `-D warnings` +help: use the dedicated method instead + | +LL | if x.is_finite() {} + | ~~~~~~~~~~~~~ +help: this will alter how it handles NaN; if that is a problem, use instead + | +LL | if x.is_finite() || x.is_nan() {} + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: or, for conciseness + | +LL | if !x.is_infinite() {} + | ~~~~~~~~~~~~~~~~ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:16:8 + --> $DIR/manual_float_methods.rs:15:8 | LL | if x == INFINITE || x == NEG_INFINITE {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:17:8 + --> $DIR/manual_float_methods.rs:16:8 | LL | if x != INFINITE && x != NEG_INFINITE {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use the dedicated method instead + | +LL | if x.is_finite() {} + | ~~~~~~~~~~~~~ +help: this will alter how it handles NaN; if that is a problem, use instead + | +LL | if x.is_finite() || x.is_nan() {} + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: or, for conciseness + | +LL | if !x.is_infinite() {} + | ~~~~~~~~~~~~~~~~ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:19:8 + --> $DIR/manual_float_methods.rs:18:8 | LL | if x == f64::INFINITY || x == f64::NEG_INFINITY {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_infinite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:20:8 + --> $DIR/manual_float_methods.rs:19:8 | LL | if x != f64::INFINITY && x != f64::NEG_INFINITY {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use the dedicated method instead + | +LL | if x.is_finite() {} + | ~~~~~~~~~~~~~ +help: this will alter how it handles NaN; if that is a problem, use instead + | +LL | if x.is_finite() || x.is_nan() {} + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: or, for conciseness + | +LL | if !x.is_infinite() {} + | ~~~~~~~~~~~~~~~~ error: aborting due to 6 previous errors From 41438c2b90f0324494c6ca22d205bf0acec4c03c Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Tue, 4 Jul 2023 12:49:15 -0500 Subject: [PATCH 3/3] Refactor, remove `Constant::to_bits` --- README.md | 2 +- book/src/README.md | 2 +- clippy_lints/src/manual_float_methods.rs | 59 +++++++++++++++--------- clippy_utils/src/consts.rs | 11 ----- tests/ui/manual_float_methods.rs | 21 ++++++++- tests/ui/manual_float_methods.stderr | 12 ++--- 6 files changed, 66 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index d712d3e67507..5d490645d897 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 600 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. diff --git a/book/src/README.md b/book/src/README.md index 3b6270962680..486ea3df7042 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 600 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how diff --git a/clippy_lints/src/manual_float_methods.rs b/clippy_lints/src/manual_float_methods.rs index 0f488627b4e9..09b39634333f 100644 --- a/clippy_lints/src/manual_float_methods.rs +++ b/clippy_lints/src/manual_float_methods.rs @@ -1,5 +1,8 @@ use clippy_utils::{ - consts::constant, diagnostics::span_lint_and_then, is_from_proc_macro, path_to_local, source::snippet_opt, + consts::{constant, Constant}, + diagnostics::span_lint_and_then, + is_from_proc_macro, path_to_local, + source::snippet_opt, }; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; @@ -9,10 +12,11 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// Checks for `x == ::INFINITY || x == ::NEG_INFINITY`. + /// Checks for manual `is_infinite` reimplementations + /// (i.e., `x == ::INFINITY || x == ::NEG_INFINITY`). /// /// ### Why is this bad? - /// This should use the dedicated method instead, `is_infinite`. + /// The method `is_infinite` is shorter and more readable. /// /// ### Example /// ```rust @@ -31,20 +35,23 @@ declare_clippy_lint! { } declare_clippy_lint! { /// ### What it does - /// Checks for `x != ::INFINITY && x != ::NEG_INFINITY`. + /// Checks for manual `is_finite` reimplementations + /// (i.e., `x != ::INFINITY && x != ::NEG_INFINITY`). /// /// ### Why is this bad? - /// This should use the dedicated method instead, `is_finite`. + /// The method `is_finite` is shorter and more readable. /// /// ### Example /// ```rust /// # let x = 1.0f32; /// if x != f32::INFINITY && x != f32::NEG_INFINITY {} + /// if x.abs() < f32::INFINITY {} /// ``` /// Use instead: /// ```rust /// # let x = 1.0f32; /// if x.is_finite() {} + /// if x.is_finite() {} /// ``` #[clippy::version = "1.72.0"] pub MANUAL_IS_FINITE, @@ -84,20 +91,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { && let ExprKind::Binary(rhs_kind, rhs_lhs, rhs_rhs) = rhs.kind // Checking all possible scenarios using a function would be a hopeless task, as we have // 16 possible alignments of constants/operands. For now, let's use `partition`. - && let (operands, consts) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs] + && let (operands, constants) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs] .into_iter() .partition::>, _>(|i| path_to_local(i).is_some()) && let [first, second] = &*operands - && let Some([const_1, const_2]) = consts + && let Some([const_1, const_2]) = constants .into_iter() - .map(|i| constant(cx, cx.typeck_results(), i).and_then(|c| c.to_bits())) + .map(|i| constant(cx, cx.typeck_results(), i)) .collect::>>() .as_deref() && path_to_local(first).is_some_and(|f| path_to_local(second).is_some_and(|s| f == s)) - && (is_infinity(*const_1) && is_neg_infinity(*const_2) - || is_neg_infinity(*const_1) && is_infinity(*const_2)) - && let Some(local_snippet) = snippet_opt(cx, first.span) + // The actual infinity check, we also allow `NEG_INFINITY` before` INFINITY` just in + // case somebody does that for some reason + && (is_infinity(const_1) && is_neg_infinity(const_2) + || is_neg_infinity(const_1) && is_infinity(const_2)) && !is_from_proc_macro(cx, expr) + && let Some(local_snippet) = snippet_opt(cx, first.span) { let variant = match (kind.node, lhs_kind.node, rhs_kind.node) { (BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => Variant::ManualIsInfinite, @@ -128,31 +137,39 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { "use the dedicated method instead", format!("{local_snippet}.is_finite()"), Applicability::MaybeIncorrect, - ); - diag.span_suggestion_verbose( + ) + .span_suggestion_verbose( expr.span, "this will alter how it handles NaN; if that is a problem, use instead", format!("{local_snippet}.is_finite() || {local_snippet}.is_nan()"), Applicability::MaybeIncorrect, - ); - diag.span_suggestion_verbose( + ) + .span_suggestion_verbose( expr.span, "or, for conciseness", format!("!{local_snippet}.is_infinite()"), Applicability::MaybeIncorrect, ); - } + }, } - } + }, ); } } } -fn is_infinity(bits: u128) -> bool { - bits == 0x7f80_0000 || bits == 0x7ff0_0000_0000_0000 +fn is_infinity(constant: &Constant<'_>) -> bool { + match constant { + Constant::F32(float) => *float == f32::INFINITY, + Constant::F64(float) => *float == f64::INFINITY, + _ => false, + } } -fn is_neg_infinity(bits: u128) -> bool { - bits == 0xff80_0000 || bits == 0xfff0_0000_0000_0000 +fn is_neg_infinity(constant: &Constant<'_>) -> bool { + match constant { + Constant::F32(float) => *float == f32::NEG_INFINITY, + Constant::F64(float) => *float == f64::NEG_INFINITY, + _ => false, + } } diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index b9f8eefeb8e7..87d85d742ce4 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -190,17 +190,6 @@ impl<'tcx> Constant<'tcx> { } } - /// Returns the bit representation if `self` is a bool, integer, or float. - pub fn to_bits(&self) -> Option { - match self { - Constant::Int(int) => Some(*int), - Constant::F32(float) => Some(u128::from(float.to_bits())), - Constant::F64(float) => Some(u128::from(float.to_bits())), - Constant::Bool(bool) => Some(u128::from(*bool)), - _ => None, - } - } - /// Returns the integer value or `None` if `self` or `val_type` is not integer type. pub fn int_value(&self, cx: &LateContext<'_>, val_type: Ty<'_>) -> Option { if let Constant::Int(const_int) = *self { diff --git a/tests/ui/manual_float_methods.rs b/tests/ui/manual_float_methods.rs index b772202c003f..af9076cfb71f 100644 --- a/tests/ui/manual_float_methods.rs +++ b/tests/ui/manual_float_methods.rs @@ -1,6 +1,7 @@ //@aux-build:proc_macros.rs:proc-macro #![allow(clippy::needless_if, unused)] #![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] +#![feature(inline_const)] #[macro_use] extern crate proc_macros; @@ -8,6 +9,14 @@ extern crate proc_macros; const INFINITE: f32 = f32::INFINITY; const NEG_INFINITE: f32 = f32::NEG_INFINITY; +fn fn_test() -> f64 { + f64::NEG_INFINITY +} + +fn fn_test_not_inf() -> f64 { + 112.0 +} + fn main() { let x = 1.0f32; if x == f32::INFINITY || x == f32::NEG_INFINITY {} @@ -20,8 +29,18 @@ fn main() { // Don't lint if x.is_infinite() {} if x.is_finite() {} - // If they're doing it this way, they probably know what they're doing if x.abs() < f64::INFINITY {} + if f64::INFINITY > x.abs() {} + if f64::abs(x) < f64::INFINITY {} + if f64::INFINITY > f64::abs(x) {} + // Is not evaluated by `clippy_utils::constant` + if x != f64::INFINITY && x != fn_test() {} + // Not -inf + if x != f64::INFINITY && x != fn_test_not_inf() {} + const X: f64 = 1.0f64; + // Will be linted if `const_float_classify` is enabled + if const { X == f64::INFINITY || X == f64::NEG_INFINITY } {} + if const { X != f64::INFINITY && X != f64::NEG_INFINITY } {} external! { let x = 1.0; if x == f32::INFINITY || x == f32::NEG_INFINITY {} diff --git a/tests/ui/manual_float_methods.stderr b/tests/ui/manual_float_methods.stderr index 7f4a4d2e95b3..a56118b316ae 100644 --- a/tests/ui/manual_float_methods.stderr +++ b/tests/ui/manual_float_methods.stderr @@ -1,5 +1,5 @@ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:13:8 + --> $DIR/manual_float_methods.rs:22:8 | LL | if x == f32::INFINITY || x == f32::NEG_INFINITY {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` @@ -7,7 +7,7 @@ LL | if x == f32::INFINITY || x == f32::NEG_INFINITY {} = note: `-D clippy::manual-is-infinite` implied by `-D warnings` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:14:8 + --> $DIR/manual_float_methods.rs:23:8 | LL | if x != f32::INFINITY && x != f32::NEG_INFINITY {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -27,13 +27,13 @@ LL | if !x.is_infinite() {} | ~~~~~~~~~~~~~~~~ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:15:8 + --> $DIR/manual_float_methods.rs:24:8 | LL | if x == INFINITE || x == NEG_INFINITE {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:16:8 + --> $DIR/manual_float_methods.rs:25:8 | LL | if x != INFINITE && x != NEG_INFINITE {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -52,13 +52,13 @@ LL | if !x.is_infinite() {} | ~~~~~~~~~~~~~~~~ error: manually checking if a float is infinite - --> $DIR/manual_float_methods.rs:18:8 + --> $DIR/manual_float_methods.rs:27:8 | LL | if x == f64::INFINITY || x == f64::NEG_INFINITY {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the dedicated method instead: `x.is_infinite()` error: manually checking if a float is finite - --> $DIR/manual_float_methods.rs:19:8 + --> $DIR/manual_float_methods.rs:28:8 | LL | if x != f64::INFINITY && x != f64::NEG_INFINITY {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^