Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lints [manual_is_infinite] and [manual_is_finite] #11049

Merged
merged 3 commits into from
Jul 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion book/src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
175 changes: 175 additions & 0 deletions clippy_lints/src/manual_float_methods.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
use clippy_utils::{
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};
use rustc_lint::{LateContext, LateLintPass, Lint, 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 manual `is_infinite` reimplementations
/// (i.e., `x == <float>::INFINITY || x == <float>::NEG_INFINITY`).
///
/// ### Why is this bad?
/// The method `is_infinite` is shorter and more readable.
///
/// ### 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 manual `is_finite` reimplementations
/// (i.e., `x != <float>::INFINITY && x != <float>::NEG_INFINITY`).
///
/// ### Why is this bad?
/// 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,
style,
"use dedicated method to check if a float 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 {
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, constants) = [lhs_lhs, lhs_rhs, rhs_lhs, rhs_rhs]
.into_iter()
.partition::<Vec<&Expr<'_>>, _>(|i| path_to_local(i).is_some())
&& let [first, second] = &*operands
&& let Some([const_1, const_2]) = constants
.into_iter()
.map(|i| constant(cx, cx.typeck_results(), i))
.collect::<Option<Vec<_>>>()
.as_deref()
&& path_to_local(first).is_some_and(|f| path_to_local(second).is_some_and(|s| f == s))
// 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,
(BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => Variant::ManualIsFinite,
_ => return,
};

span_lint_and_then(
cx,
variant.lint(),
expr.span,
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(
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
expr.span,
"use the dedicated method instead",
format!("{local_snippet}.is_finite()"),
Applicability::MaybeIncorrect,
)
.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,
)
.span_suggestion_verbose(
expr.span,
"or, for conciseness",
format!("!{local_snippet}.is_infinite()"),
Applicability::MaybeIncorrect,
);
},
}
},
);
}
}
}

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(constant: &Constant<'_>) -> bool {
match constant {
Constant::F32(float) => *float == f32::NEG_INFINITY,
Constant::F64(float) => *float == f64::NEG_INFINITY,
_ => false,
}
}
55 changes: 55 additions & 0 deletions tests/ui/manual_float_methods.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//@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;

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 {}
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
Centri3 marked this conversation as resolved.
Show resolved Hide resolved
if x.is_infinite() {}
if x.is_finite() {}
if x.abs() < f64::INFINITY {}
Centri3 marked this conversation as resolved.
Show resolved Hide resolved
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 {}
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 {}
}
}
80 changes: 80 additions & 0 deletions tests/ui/manual_float_methods.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
error: manually checking if a float is infinite
--> $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()`
|
= note: `-D clippy::manual-is-infinite` implied by `-D warnings`

error: manually checking if a float is finite
--> $DIR/manual_float_methods.rs:23:8
|
LL | if x != f32::INFINITY && x != f32::NEG_INFINITY {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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: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:25:8
|
LL | if x != INFINITE && x != NEG_INFINITE {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
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: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:28:8
|
LL | if x != f64::INFINITY && x != f64::NEG_INFINITY {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
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