New lint: manual_midpoint

Author: samueltardieu

CHANGELOG.md

@@ -5718,6 +5718,7 @@ Released 2018-09-13
 [`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
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
+[`manual_midpoint`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint
 [`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or

book/src/lint_configuration.md

@@ -709,6 +709,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_hash_one`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one)
 * [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check)
 * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else)
+* [`manual_midpoint`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint)
 * [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive)
 * [`manual_pattern_char_comparison`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison)
 * [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains)

clippy_config/src/conf.rs

@@ -592,6 +592,7 @@ define_Conf! {
         manual_hash_one,
         manual_is_ascii_check,
         manual_let_else,
+        manual_midpoint,
         manual_non_exhaustive,
         manual_pattern_char_comparison,
         manual_range_contains,

clippy_lints/src/declared_lints.rs

@@ -593,6 +593,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::operators::IMPOSSIBLE_COMPARISONS_INFO,
     crate::operators::INEFFECTIVE_BIT_MASK_INFO,
     crate::operators::INTEGER_DIVISION_INFO,
+    crate::operators::MANUAL_MIDPOINT_INFO,
     crate::operators::MISREFACTORED_ASSIGN_OP_INFO,
     crate::operators::MODULO_ARITHMETIC_INFO,
     crate::operators::MODULO_ONE_INFO,

clippy_lints/src/operators/manual_midpoint.rs

@@ -0,0 +1,69 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{is_floating_point_integer_literal, is_integer_literal};
+use rustc_ast::{BinOpKind, LitKind};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+
+use super::MANUAL_MIDPOINT;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    op: BinOpKind,
+    left: &'tcx Expr<'_>,
+    right: &'tcx Expr<'_>,
+    msrv: &Msrv,
+) {
+    if msrv.meets(msrvs::UINT_FLOAT_MIDPOINT)
+        && !left.span.from_expansion()
+        && !right.span.from_expansion()
+        && op == BinOpKind::Div
+        && (is_integer_literal(right, 2) || is_floating_point_integer_literal(right, 2))
+        && let Some((ll_expr, lr_expr)) = add_operands(left)
+        && add_operands(ll_expr).is_none() && add_operands(lr_expr).is_none()
+        && let left_ty = cx.typeck_results().expr_ty_adjusted(ll_expr)
+        && let right_ty = cx.typeck_results().expr_ty_adjusted(lr_expr)
+        && left_ty == right_ty
+        // Do not lint on `(_+1)/2` and `(1+_)/2`, it is likely a `div_ceil()` operation
+        && !is_1_integer_literal(ll_expr) && !is_1_integer_literal(lr_expr)
+        // FIXME: Also lint on signed integers when rust-lang/rust#134340 is merged
+        && matches!(left_ty.kind(), ty::Uint(_) | ty::Float(_))
+    {
+        let mut app = Applicability::MachineApplicable;
+        let left_sugg = Sugg::hir_with_context(cx, ll_expr, expr.span.ctxt(), "..", &mut app);
+        let right_sugg = Sugg::hir_with_context(cx, lr_expr, expr.span.ctxt(), "..", &mut app);
+        let sugg = format!("{left_ty}::midpoint({left_sugg}, {right_sugg})");
+        span_lint_and_sugg(
+            cx,
+            MANUAL_MIDPOINT,
+            expr.span,
+            "manual implementation of `midpoint` which can overflow",
+            format!("use `{left_ty}::midpoint` instead"),
+            sugg,
+            app,
+        );
+    }
+}
+
+/// Return the left and right operands if `expr` represents an addition
+fn add_operands<'e, 'tcx>(expr: &'e Expr<'tcx>) -> Option<(&'e Expr<'tcx>, &'e Expr<'tcx>)> {
+    match expr.kind {
+        ExprKind::Binary(op, left, right) if op.node == BinOpKind::Add => Some((left, right)),
+        _ => None,
+    }
+}
+
+/// Check if the expression represents the integer 1 literal
+fn is_1_integer_literal(expr: &Expr<'_>) -> bool {
+    if let ExprKind::Lit(lit) = expr.kind
+        && let LitKind::Int(value, _) = lit.node
+    {
+        value.get() == 1
+    } else {
+        false
+    }
+}

clippy_lints/src/operators/mod.rs

@@ -11,6 +11,7 @@ mod float_cmp;
 mod float_equality_without_abs;
 mod identity_op;
 mod integer_division;
+mod manual_midpoint;
 mod misrefactored_assign_op;
 mod modulo_arithmetic;
 mod modulo_one;
@@ -24,6 +25,7 @@ mod verbose_bit_mask;
 pub(crate) mod arithmetic_side_effects;
 
 use clippy_config::Conf;
+use clippy_utils::msrvs::Msrv;
 use rustc_hir::{Body, Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
@@ -837,17 +839,43 @@ declare_clippy_lint! {
     "explicit self-assignment"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementation of `midpoint`.
+    ///
+    /// ### Why is this bad?
+    /// Using `(x + y) / 2` might cause an overflow on the intermediate
+    /// addition result.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let a: u32 = 0;
+    /// let c = (a + 10) / 2;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let a: u32 = 0;
+    /// let c = u32::midpoint(a, 10);
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub MANUAL_MIDPOINT,
+    correctness,
+    "manual implementation of `midpoint` which can overflow"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
     modulo_arithmetic_allow_comparison_to_zero: bool,
+    msrv: Msrv,
 }
 impl Operators {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
             arithmetic_context: numeric_arithmetic::Context::default(),
             verbose_bit_mask_threshold: conf.verbose_bit_mask_threshold,
             modulo_arithmetic_allow_comparison_to_zero: conf.allow_comparison_to_zero,
+            msrv: conf.msrv.clone(),
         }
     }
 }
@@ -879,6 +907,7 @@ impl_lint_pass!(Operators => [
     NEEDLESS_BITWISE_BOOL,
     PTR_EQ,
     SELF_ASSIGNMENT,
+    MANUAL_MIDPOINT,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -896,6 +925,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     identity_op::check(cx, e, op.node, lhs, rhs);
                     needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
                     ptr_eq::check(cx, e, op.node, lhs, rhs);
+                    manual_midpoint::check(cx, e, op.node, lhs, rhs, &self.msrv);
                 }
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
                 bit_mask::check(cx, e, op.node, lhs, rhs);
@@ -946,6 +976,8 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
     fn check_body_post(&mut self, cx: &LateContext<'tcx>, b: &Body<'_>) {
         self.arithmetic_context.body_post(cx, b);
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 fn macro_with_not_op(e: &Expr<'_>) -> bool {

clippy_utils/src/msrvs.rs

@@ -18,6 +18,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
+    1,85,0 { UINT_FLOAT_MIDPOINT }
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY }
     1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
     1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE }

tests/ui/manual_midpoint.fixed

@@ -0,0 +1,56 @@
+#![warn(clippy::manual_midpoint)]
+
+macro_rules! mac {
+    ($a: expr, $b: expr) => {
+        ($a + $b) / 2
+    };
+}
+
+macro_rules! add {
+    ($a: expr, $b: expr) => {
+        ($a + $b)
+    };
+}
+
+macro_rules! two {
+    () => {
+        2
+    };
+}
+
+fn main() {
+    let a: u32 = 10;
+    let _ = u32::midpoint(a, 5); //~ ERROR: manual implementation of `midpoint`
+
+    let f: f32 = 10.0;
+    let _ = f32::midpoint(f, 5.0); //~ ERROR: manual implementation of `midpoint`
+
+    let _: u32 = 5 + u32::midpoint(8, 8) + 2; //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = const { u32::midpoint(8, 8) }; //~ ERROR: manual implementation of `midpoint`
+    let _: f64 = const { f64::midpoint(8.0f64, 8.) }; //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = u32::midpoint(u32::default(), u32::default()); //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = u32::midpoint(two!(), two!()); //~ ERROR: manual implementation of `midpoint`
+
+    // Do not lint in presence of an addition with more than 2 operands
+    let _: u32 = (10 + 20 + 30) / 2;
+
+    // Do not lint if whole or part is coming from a macro
+    let _ = mac!(10, 20);
+    let _: u32 = add!(10u32, 20u32) / 2;
+    let _: u32 = (10 + 20) / two!();
+
+    // Do not lint if a literal is not present
+    let _ = (f + 5.0) / (1.0 + 1.0);
+
+    // Do not lint on signed integer types
+    let i: i32 = 10;
+    let _ = (i + 5) / 2;
+
+    // Do not lint on (x+1)/2 or (1+x)/2 as this looks more like a `div_ceil()` operation
+    let _ = (i + 1) / 2;
+    let _ = (1 + i) / 2;
+
+    // But if we see (x+1.0)/2.0 or (x+1.0)/2.0, it is probably a midpoint operation
+    let _ = f32::midpoint(f, 1.0); //~ ERROR: manual implementation of `midpoint`
+    let _ = f32::midpoint(1.0, f); //~ ERROR: manual implementation of `midpoint`
+}

tests/ui/manual_midpoint.rs

@@ -0,0 +1,56 @@
+#![warn(clippy::manual_midpoint)]
+
+macro_rules! mac {
+    ($a: expr, $b: expr) => {
+        ($a + $b) / 2
+    };
+}
+
+macro_rules! add {
+    ($a: expr, $b: expr) => {
+        ($a + $b)
+    };
+}
+
+macro_rules! two {
+    () => {
+        2
+    };
+}
+
+fn main() {
+    let a: u32 = 10;
+    let _ = (a + 5) / 2; //~ ERROR: manual implementation of `midpoint`
+
+    let f: f32 = 10.0;
+    let _ = (f + 5.0) / 2.0; //~ ERROR: manual implementation of `midpoint`
+
+    let _: u32 = 5 + (8 + 8) / 2 + 2; //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = const { (8 + 8) / 2 }; //~ ERROR: manual implementation of `midpoint`
+    let _: f64 = const { (8.0f64 + 8.) / 2. }; //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = (u32::default() + u32::default()) / 2; //~ ERROR: manual implementation of `midpoint`
+    let _: u32 = (two!() + two!()) / 2; //~ ERROR: manual implementation of `midpoint`
+
+    // Do not lint in presence of an addition with more than 2 operands
+    let _: u32 = (10 + 20 + 30) / 2;
+
+    // Do not lint if whole or part is coming from a macro
+    let _ = mac!(10, 20);
+    let _: u32 = add!(10u32, 20u32) / 2;
+    let _: u32 = (10 + 20) / two!();
+
+    // Do not lint if a literal is not present
+    let _ = (f + 5.0) / (1.0 + 1.0);
+
+    // Do not lint on signed integer types
+    let i: i32 = 10;
+    let _ = (i + 5) / 2;
+
+    // Do not lint on (x+1)/2 or (1+x)/2 as this looks more like a `div_ceil()` operation
+    let _ = (i + 1) / 2;
+    let _ = (1 + i) / 2;
+
+    // But if we see (x+1.0)/2.0 or (x+1.0)/2.0, it is probably a midpoint operation
+    let _ = (f + 1.0) / 2.0; //~ ERROR: manual implementation of `midpoint`
+    let _ = (1.0 + f) / 2.0; //~ ERROR: manual implementation of `midpoint`
+}

tests/ui/manual_midpoint.stderr

@@ -0,0 +1,59 @@
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:23:13
+   |
+LL |     let _ = (a + 5) / 2;
+   |             ^^^^^^^^^^^ help: use `u32::midpoint` instead: `u32::midpoint(a, 5)`
+   |
+   = note: `-D clippy::manual-midpoint` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_midpoint)]`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:26:13
+   |
+LL |     let _ = (f + 5.0) / 2.0;
+   |             ^^^^^^^^^^^^^^^ help: use `f32::midpoint` instead: `f32::midpoint(f, 5.0)`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:28:22
+   |
+LL |     let _: u32 = 5 + (8 + 8) / 2 + 2;
+   |                      ^^^^^^^^^^^ help: use `u32::midpoint` instead: `u32::midpoint(8, 8)`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:29:26
+   |
+LL |     let _: u32 = const { (8 + 8) / 2 };
+   |                          ^^^^^^^^^^^ help: use `u32::midpoint` instead: `u32::midpoint(8, 8)`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:30:26
+   |
+LL |     let _: f64 = const { (8.0f64 + 8.) / 2. };
+   |                          ^^^^^^^^^^^^^^^^^^ help: use `f64::midpoint` instead: `f64::midpoint(8.0f64, 8.)`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:31:18
+   |
+LL |     let _: u32 = (u32::default() + u32::default()) / 2;
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `u32::midpoint` instead: `u32::midpoint(u32::default(), u32::default())`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:32:18
+   |
+LL |     let _: u32 = (two!() + two!()) / 2;
+   |                  ^^^^^^^^^^^^^^^^^^^^^ help: use `u32::midpoint` instead: `u32::midpoint(two!(), two!())`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:54:13
+   |
+LL |     let _ = (f + 1.0) / 2.0;
+   |             ^^^^^^^^^^^^^^^ help: use `f32::midpoint` instead: `f32::midpoint(f, 1.0)`
+
+error: manual implementation of `midpoint` which can overflow
+  --> tests/ui/manual_midpoint.rs:55:13
+   |
+LL |     let _ = (1.0 + f) / 2.0;
+   |             ^^^^^^^^^^^^^^^ help: use `f32::midpoint` instead: `f32::midpoint(1.0, f)`
+
+error: aborting due to 9 previous errors
+

tests/ui/option_if_let_else.fixed

@@ -4,6 +4,7 @@
     clippy::equatable_if_let,
     clippy::let_unit_value,
     clippy::redundant_locals,
+    clippy::manual_midpoint,
     clippy::manual_unwrap_or_default,
     clippy::manual_unwrap_or
 )]

tests/ui/option_if_let_else.rs

@@ -4,6 +4,7 @@
     clippy::equatable_if_let,
     clippy::let_unit_value,
     clippy::redundant_locals,
+    clippy::manual_midpoint,
     clippy::manual_unwrap_or_default,
     clippy::manual_unwrap_or
 )]