Make suggestion give multiple alternatives

This commit is contained in:
Catherine 2023-07-02 11:43:05 -05:00
parent f12edfdb53
commit 5949f762bf
4 changed files with 117 additions and 65 deletions

View File

@ -1,9 +1,9 @@
use clippy_utils::{ 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_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind}; 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_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint}; 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]); 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 { impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !in_external_macro(cx.sess(), expr.span) 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(kind, lhs, rhs) = expr.kind
&& let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind && let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind
&& let ExprKind::Binary(rhs_kind, rhs_lhs, rhs_rhs) = rhs.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] && let (operands, consts) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs]
.into_iter() .into_iter()
.partition::<Vec<&Expr<'_>>, _>(|i| path_to_local(i).is_some()) .partition::<Vec<&Expr<'_>>, _>(|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) && let Some(local_snippet) = snippet_opt(cx, first.span)
&& !is_from_proc_macro(cx, expr) && !is_from_proc_macro(cx, expr)
{ {
let (msg, lint, sugg_fn) = match (kind.node, lhs_kind.node, rhs_kind.node) { let variant = match (kind.node, lhs_kind.node, rhs_kind.node) {
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => { (BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => Variant::ManualIsInfinite,
("manually checking if a float is infinite", MANUAL_IS_INFINITE, "is_infinite") (BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => Variant::ManualIsFinite,
},
(BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => {
("manually checking if a float is finite", MANUAL_IS_FINITE, "is_finite")
},
_ => return, _ => return,
}; };
span_lint_and_sugg( span_lint_and_then(
cx, cx,
lint, variant.lint(),
expr.span, expr.span,
msg, variant.msg(),
"try", |diag| {
format!("{local_snippet}.{sugg_fn}()"), match variant {
Applicability::MachineApplicable, 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,
);
}
}
}
); );
} }
} }

View File

@ -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 {}
}
}

View File

@ -1,4 +1,3 @@
//@run-rustfix
//@aux-build:proc_macros.rs:proc-macro //@aux-build:proc_macros.rs:proc-macro
#![allow(clippy::needless_if, unused)] #![allow(clippy::needless_if, unused)]
#![warn(clippy::manual_is_infinite, clippy::manual_is_finite)] #![warn(clippy::manual_is_infinite, clippy::manual_is_finite)]

View File

@ -1,42 +1,80 @@
error: manually checking if a float is infinite 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 {} 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` = note: `-D clippy::manual-is-infinite` implied by `-D warnings`
error: manually checking if a float is finite 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 {} LL | if x != f32::INFINITY && x != f32::NEG_INFINITY {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_finite()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
= note: `-D clippy::manual-is-finite` implied by `-D warnings` = 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 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 {} 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 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 {} 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 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 {} 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 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 {} 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 error: aborting due to 6 previous errors