add manual_sign_check lint

CHANGELOG.md

@@ -6482,6 +6482,7 @@ Released 2018-09-13
 [`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
 [`manual_rotate`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rotate
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
+[`manual_sign_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_sign_check
 [`manual_slice_fill`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill
 [`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
 [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once

clippy_lints/src/declared_lints.rs

@@ -594,6 +594,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::operators::INTEGER_DIVISION_INFO,
     crate::operators::MANUAL_IS_MULTIPLE_OF_INFO,
     crate::operators::MANUAL_MIDPOINT_INFO,
+    crate::operators::MANUAL_SIGN_CHECK_INFO,
     crate::operators::MISREFACTORED_ASSIGN_OP_INFO,
     crate::operators::MODULO_ARITHMETIC_INFO,
     crate::operators::MODULO_ONE_INFO,

clippy_lints/src/operators/manual_sign_check.rs

@@ -0,0 +1,67 @@
+use clippy_utils::consts::{ConstEvalCtxt, Constant};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr};
+use rustc_lint::LateContext;
+
+use super::MANUAL_SIGN_CHECK;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, left: &'tcx Expr<'_>, right: &'tcx Expr<'_>) {
+    let (float_expr, direction) = if is_zero(cx, left) && is_float(cx, right) {
+        (right, Side::Right)
+    } else if is_zero(cx, right) && is_float(cx, left) {
+        (left, Side::Left)
+    } else {
+        return;
+    };
+
+    let mut applicability = Applicability::MachineApplicable;
+    let float_snippet = snippet_with_applicability(cx, float_expr.span, "..", &mut applicability);
+    let (method, negate) = match (direction, op) {
+        (Side::Left, BinOpKind::Lt) | (Side::Right, BinOpKind::Gt) => ("is_sign_negative", false),
+        (Side::Left, BinOpKind::Le) | (Side::Right, BinOpKind::Ge) => ("is_sign_positive", true),
+        (Side::Left, BinOpKind::Gt) | (Side::Right, BinOpKind::Lt) => ("is_sign_positive", false),
+        (Side::Left, BinOpKind::Ge) | (Side::Right, BinOpKind::Le) => ("is_sign_negative", true),
+        _ => return,
+    };
+
+    let suggestion = if negate {
+        format!("!{float_snippet}.{method}()")
+    } else {
+        format!("{float_snippet}.{method}()")
+    };
+
+    span_lint_and_sugg(
+        cx,
+        MANUAL_SIGN_CHECK,
+        float_expr.span.to(right.span),
+        "checking the sign of a floating point number by comparing it to zero",
+        format!("consider using `{method}` for clarity and performance"),
+        suggestion,
+        applicability,
+    );
+}
+
+enum Side {
+    Left,
+    Right,
+}
+
+fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let ecx = ConstEvalCtxt::new(cx);
+    let Some(constant) = ecx.eval(expr) else {
+        return false;
+    };
+
+    match constant {
+        // FIXME(f16_f128): add when equality check is available on all platforms
+        Constant::F32(f) => f == 0.0,
+        Constant::F64(f) => f == 0.0,
+        _ => false,
+    }
+}
+
+fn is_float(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    cx.typeck_results().expr_ty(expr).is_floating_point()
+}

clippy_lints/src/operators/mod.rs

@@ -13,6 +13,7 @@ mod identity_op;
 mod integer_division;
 mod manual_is_multiple_of;
 mod manual_midpoint;
+mod manual_sign_check;
 mod misrefactored_assign_op;
 mod modulo_arithmetic;
 mod modulo_one;
@@ -860,6 +861,40 @@ declare_clippy_lint! {
     "manual implementation of `.is_multiple_of()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manually checking the sign of a floating point number by comparing it to zero with `<`, `>`, `<=`, or `>=`.
+    ///
+    /// ### Why is this bad?
+    /// Floating point numbers have a dedicated sign bit that can be checked with bitwise operators which uses far fewer
+    /// CPU cycles. `std` exposes the `is_sign_positive` and `is_sign_negative` methods for this purpose.
+    ///
+    /// ### Caveats
+    /// The `is_sign_positive` and `is_sign_negative` methods do not behave the same as the comparison operators in some
+    /// cases. Behavior with `NaN` is unspecified, and use with negative zero will return `true`, whereas `-0.0 < 0.0
+    /// == false`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let foo: f32 = 1.0;
+    /// if foo < 0.0 {
+    ///     println!("negative");
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// let foo: f32 = 1.0;
+    /// if foo.is_sign_negative() {
+    ///     println!("negative");
+    /// }
+    /// ```
+    #[clippy::version = "1.92.0"]
+    pub MANUAL_SIGN_CHECK,
+    pedantic,
+    "checking the sign of a floating point number by comparing it to zero, which is slow"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
@@ -906,6 +941,7 @@ impl_lint_pass!(Operators => [
     SELF_ASSIGNMENT,
     MANUAL_MIDPOINT,
     MANUAL_IS_MULTIPLE_OF,
+    MANUAL_SIGN_CHECK,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -944,6 +980,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     rhs,
                     self.modulo_arithmetic_allow_comparison_to_zero,
                 );
+                manual_sign_check::check(cx, op.node, lhs, rhs);
             },
             ExprKind::AssignOp(op, lhs, rhs) => {
                 let bin_op = op.node.into();

tests/ui/manual_sign_check.fixed

@@ -0,0 +1,17 @@
+#![allow(clippy::needless_if)]
+#![warn(clippy::manual_sign_check)]
+
+fn main() {
+    let x: f32 = 1.0;
+
+    if x.is_sign_negative() {}
+    //~^ manual_sign_check
+    if !x.is_sign_positive() {}
+    //~^ manual_sign_check
+    if x.is_sign_positive() {}
+    //~^ manual_sign_check
+    if !x.is_sign_negative() {}
+    //~^ manual_sign_check
+    if x == 0.0 {}
+    if x < 1.0 {}
+}

tests/ui/manual_sign_check.rs

@@ -0,0 +1,17 @@
+#![allow(clippy::needless_if)]
+#![warn(clippy::manual_sign_check)]
+
+fn main() {
+    let x: f32 = 1.0;
+
+    if x < 0.0 {}
+    //~^ manual_sign_check
+    if x <= 0.0 {}
+    //~^ manual_sign_check
+    if x > 0.0 {}
+    //~^ manual_sign_check
+    if x >= 0.0 {}
+    //~^ manual_sign_check
+    if x == 0.0 {}
+    if x < 1.0 {}
+}

tests/ui/manual_sign_check.stderr

@@ -0,0 +1,29 @@
+error: checking the sign of a floating point number by comparing it to zero
+  --> tests/ui/manual_sign_check.rs:7:8
+   |
+LL |     if x < 0.0 {}
+   |        ^^^^^^^ help: consider using `is_sign_negative` for clarity and performance: `x.is_sign_negative()`
+   |
+   = note: `-D clippy::manual-sign-check` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_sign_check)]`
+
+error: checking the sign of a floating point number by comparing it to zero
+  --> tests/ui/manual_sign_check.rs:9:8
+   |
+LL |     if x <= 0.0 {}
+   |        ^^^^^^^^ help: consider using `is_sign_positive` for clarity and performance: `!x.is_sign_positive()`
+
+error: checking the sign of a floating point number by comparing it to zero
+  --> tests/ui/manual_sign_check.rs:11:8
+   |
+LL |     if x > 0.0 {}
+   |        ^^^^^^^ help: consider using `is_sign_positive` for clarity and performance: `x.is_sign_positive()`
+
+error: checking the sign of a floating point number by comparing it to zero
+  --> tests/ui/manual_sign_check.rs:13:8
+   |
+LL |     if x >= 0.0 {}
+   |        ^^^^^^^^ help: consider using `is_sign_negative` for clarity and performance: `!x.is_sign_negative()`
+
+error: aborting due to 4 previous errors
+