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] 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 abf1eb788dd..0f488627b4e 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 87a23b5a75b..00000000000 --- 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 9255bf93418..b772202c003 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 00227593a9e..7f4a4d2e95b 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