Unnecessary call to min/max method

CHANGELOG.md

@@ -5917,6 +5917,7 @@ Released 2018-09-13
 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
 [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
 [`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
+[`unnecessary_min_or_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_or_max
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

clippy_lints/src/declared_lints.rs

@@ -471,6 +471,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::UNNECESSARY_JOIN_INFO,
     crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
     crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
+    crate::methods::UNNECESSARY_MIN_OR_MAX_INFO,
     crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO,
     crate::methods::UNNECESSARY_SORT_BY_INFO,
     crate::methods::UNNECESSARY_TO_OWNED_INFO,

clippy_lints/src/methods/mod.rs

@@ -117,6 +117,7 @@ mod unnecessary_iter_cloned;
 mod unnecessary_join;
 mod unnecessary_lazy_eval;
 mod unnecessary_literal_unwrap;
+mod unnecessary_min_or_max;
 mod unnecessary_result_map_or_else;
 mod unnecessary_sort_by;
 mod unnecessary_to_owned;
@@ -3945,6 +3946,31 @@ declare_clippy_lint! {
     "cloning an `Option` via `as_ref().cloned()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for unnecessary calls to `min()` or `max()` in the following cases
+    /// - Either both side is constant
+    /// - One side is clearly larger than the other, like i32::MIN and an i32 variable
+    ///
+    /// ### Why is this bad?
+    ///
+    /// In the aformentioned cases it is not necessary to call `min()` or `max()`
+    /// to compare values, it may even cause confusion.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let _ = 0.min(7_u32);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let _ = 0;
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub UNNECESSARY_MIN_OR_MAX,
+    complexity,
+    "using 'min()/max()' when there is no need for it"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `.map_or_else()` "map closure" for `Result` type.
@@ -4267,6 +4293,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_GET_THEN_CHECK,
     NEEDLESS_CHARACTER_ITERATION,
     MANUAL_INSPECT,
+    UNNECESSARY_MIN_OR_MAX,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4566,6 +4593,9 @@ impl Methods {
                     Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
                     _ => {},
                 },
+                ("min" | "max", [arg]) => {
+                    unnecessary_min_or_max::check(cx, expr, name, recv, arg);
+                },
                 ("drain", ..) => {
                     if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.parent_hir_node(expr.hir_id)
                         && matches!(kind, StmtKind::Semi(_))

clippy_lints/src/methods/unnecessary_min_or_max.rs

@@ -0,0 +1,90 @@
+use std::cmp::Ordering;
+
+use super::UNNECESSARY_MIN_OR_MAX;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+
+use clippy_utils::consts::{constant, constant_with_source, Constant, ConstantSource, FullInt};
+use clippy_utils::source::snippet;
+
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_span::Span;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    name: &str,
+    recv: &'tcx Expr<'_>,
+    arg: &'tcx Expr<'_>,
+) {
+    let typeck_results = cx.typeck_results();
+    if let Some((left, ConstantSource::Local | ConstantSource::CoreConstant)) =
+        constant_with_source(cx, typeck_results, recv)
+        && let Some((right, ConstantSource::Local | ConstantSource::CoreConstant)) =
+            constant_with_source(cx, typeck_results, arg)
+    {
+        let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else {
+            return;
+        };
+
+        lint(cx, expr, name, recv.span, arg.span, ord);
+    } else if let Some(extrema) = detect_extrema(cx, recv) {
+        let ord = match extrema {
+            Extrema::Minimum => Ordering::Less,
+            Extrema::Maximum => Ordering::Greater,
+        };
+        lint(cx, expr, name, recv.span, arg.span, ord);
+    } else if let Some(extrema) = detect_extrema(cx, arg) {
+        let ord = match extrema {
+            Extrema::Minimum => Ordering::Greater,
+            Extrema::Maximum => Ordering::Less,
+        };
+        lint(cx, expr, name, recv.span, arg.span, ord);
+    }
+}
+
+fn lint(cx: &LateContext<'_>, expr: &Expr<'_>, name: &str, lhs: Span, rhs: Span, order: Ordering) {
+    let cmp_str = if order.is_ge() { "smaller" } else { "greater" };
+
+    let suggested_value = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) {
+        snippet(cx, rhs, "..")
+    } else {
+        snippet(cx, lhs, "..")
+    };
+
+    span_lint_and_sugg(
+        cx,
+        UNNECESSARY_MIN_OR_MAX,
+        expr.span,
+        format!(
+            "`{}` is never {} than `{}` and has therefore no effect",
+            snippet(cx, lhs, ".."),
+            cmp_str,
+            snippet(cx, rhs, "..")
+        ),
+        "try",
+        suggested_value.to_string(),
+        Applicability::MachineApplicable,
+    );
+}
+
+#[derive(Debug)]
+enum Extrema {
+    Minimum,
+    Maximum,
+}
+fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Extrema> {
+    let ty = cx.typeck_results().expr_ty(expr);
+
+    let cv = constant(cx, cx.typeck_results(), expr)?;
+
+    match (cv.int_value(cx, ty)?, ty.kind()) {
+        (FullInt::S(i), &ty::Int(ity)) if i == i128::MIN >> (128 - ity.bit_width()?) => Some(Extrema::Minimum),
+        (FullInt::S(i), &ty::Int(ity)) if i == i128::MAX >> (128 - ity.bit_width()?) => Some(Extrema::Maximum),
+        (FullInt::U(i), &ty::Uint(uty)) if i == u128::MAX >> (128 - uty.bit_width()?) => Some(Extrema::Maximum),
+        (FullInt::U(0), &ty::Uint(_)) => Some(Extrema::Minimum),
+        _ => None,
+    }
+}

clippy_utils/src/consts.rs

@@ -1,5 +1,6 @@
 #![allow(clippy::float_cmp)]
 
+use crate::macros::HirNode;
 use crate::source::{get_source_text, walk_span_to_context};
 use crate::{clip, is_direct_expn_of, sext, unsext};
 
@@ -15,7 +16,7 @@ use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List,
 use rustc_middle::{bug, mir, span_bug};
 use rustc_span::def_id::DefId;
 use rustc_span::symbol::{Ident, Symbol};
-use rustc_span::SyntaxContext;
+use rustc_span::{sym, SyntaxContext};
 use rustc_target::abi::Size;
 use std::cmp::Ordering;
 use std::hash::{Hash, Hasher};
@@ -302,6 +303,8 @@ pub enum ConstantSource {
     Local,
     /// The value is dependent on a defined constant.
     Constant,
+    /// The value is dependent on a constant defined in `core` crate.
+    CoreConstant,
 }
 impl ConstantSource {
     pub fn is_local(&self) -> bool {
@@ -415,9 +418,19 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
             ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr(self.lcx.tcx.hir().body(body).value),
             ExprKind::DropTemps(e) => self.expr(e),
             ExprKind::Path(ref qpath) => {
+                let is_core_crate = if let Some(def_id) = self.lcx.qpath_res(qpath, e.hir_id()).opt_def_id() {
+                    self.lcx.tcx.crate_name(def_id.krate) == sym::core
+                } else {
+                    false
+                };
                 self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| {
                     let result = mir_to_const(this.lcx, result)?;
-                    this.source = ConstantSource::Constant;
+                    // If source is already Constant we wouldn't want to override it with CoreConstant
+                    this.source = if is_core_crate && !matches!(this.source, ConstantSource::Constant) {
+                        ConstantSource::CoreConstant
+                    } else {
+                        ConstantSource::Constant
+                    };
                     Some(result)
                 })
             },

tests/ui/auxiliary/external_consts.rs

@@ -0,0 +1 @@
+pub const MAGIC_NUMBER: i32 = 1;

tests/ui/cast.rs

@@ -12,6 +12,7 @@
 #![allow(
     clippy::cast_abs_to_unsigned,
     clippy::no_effect,
+    clippy::unnecessary_min_or_max,
     clippy::unnecessary_operation,
     clippy::unnecessary_literal_unwrap,
     clippy::identity_op

tests/ui/unnecessary_min_or_max.fixed

@@ -0,0 +1,67 @@
+//@aux-build:external_consts.rs
+
+#![allow(unused)]
+#![warn(clippy::unnecessary_min_or_max)]
+#![allow(clippy::identity_op)]
+
+extern crate external_consts;
+
+const X: i32 = 1;
+
+fn main() {
+    // Both are Literals
+    let _ = (-6_i32);
+    let _ = 9;
+    let _ = 6;
+    let _ = 9_u32;
+    let _ = 6;
+    let _ = 7_u8;
+
+    let x: u32 = 42;
+    // unsigned with zero
+    let _ = 0;
+    let _ = x;
+    let _ = 0_u32;
+    let _ = x;
+
+    let x: i32 = 42;
+    // signed MIN
+    let _ = i32::MIN;
+    let _ = x;
+    let _ = i32::MIN;
+    let _ = x;
+
+    let _ = i32::MIN - 0;
+    let _ = x;
+
+    let _ = i32::MIN - 0;
+
+    // The below cases shouldn't be lint
+    let mut min = u32::MAX;
+    for _ in 0..1000 {
+        min = min.min(random_u32());
+    }
+
+    let _ = 2.min(external_consts::MAGIC_NUMBER);
+    let _ = 2.max(external_consts::MAGIC_NUMBER);
+    let _ = external_consts::MAGIC_NUMBER.min(2);
+    let _ = external_consts::MAGIC_NUMBER.max(2);
+
+    let _ = X.min(external_consts::MAGIC_NUMBER);
+    let _ = X.max(external_consts::MAGIC_NUMBER);
+    let _ = external_consts::MAGIC_NUMBER.min(X);
+    let _ = external_consts::MAGIC_NUMBER.max(X);
+
+    let _ = X.max(12);
+    let _ = X.min(12);
+    let _ = 12.min(X);
+    let _ = 12.max(X);
+    let _ = (X + 1).max(12);
+    let _ = (X + 1).min(12);
+    let _ = 12.min(X - 1);
+    let _ = 12.max(X - 1);
+}
+fn random_u32() -> u32 {
+    // random number generator
+    0
+}

tests/ui/unnecessary_min_or_max.rs

@@ -0,0 +1,67 @@
+//@aux-build:external_consts.rs
+
+#![allow(unused)]
+#![warn(clippy::unnecessary_min_or_max)]
+#![allow(clippy::identity_op)]
+
+extern crate external_consts;
+
+const X: i32 = 1;
+
+fn main() {
+    // Both are Literals
+    let _ = (-6_i32).min(9);
+    let _ = (-6_i32).max(9);
+    let _ = 9_u32.min(6);
+    let _ = 9_u32.max(6);
+    let _ = 6.min(7_u8);
+    let _ = 6.max(7_u8);
+
+    let x: u32 = 42;
+    // unsigned with zero
+    let _ = 0.min(x);
+    let _ = 0.max(x);
+    let _ = x.min(0_u32);
+    let _ = x.max(0_u32);
+
+    let x: i32 = 42;
+    // signed MIN
+    let _ = i32::MIN.min(x);
+    let _ = i32::MIN.max(x);
+    let _ = x.min(i32::MIN);
+    let _ = x.max(i32::MIN);
+
+    let _ = x.min(i32::MIN - 0);
+    let _ = x.max(i32::MIN);
+
+    let _ = x.min(i32::MIN - 0);
+
+    // The below cases shouldn't be lint
+    let mut min = u32::MAX;
+    for _ in 0..1000 {
+        min = min.min(random_u32());
+    }
+
+    let _ = 2.min(external_consts::MAGIC_NUMBER);
+    let _ = 2.max(external_consts::MAGIC_NUMBER);
+    let _ = external_consts::MAGIC_NUMBER.min(2);
+    let _ = external_consts::MAGIC_NUMBER.max(2);
+
+    let _ = X.min(external_consts::MAGIC_NUMBER);
+    let _ = X.max(external_consts::MAGIC_NUMBER);
+    let _ = external_consts::MAGIC_NUMBER.min(X);
+    let _ = external_consts::MAGIC_NUMBER.max(X);
+
+    let _ = X.max(12);
+    let _ = X.min(12);
+    let _ = 12.min(X);
+    let _ = 12.max(X);
+    let _ = (X + 1).max(12);
+    let _ = (X + 1).min(12);
+    let _ = 12.min(X - 1);
+    let _ = 12.max(X - 1);
+}
+fn random_u32() -> u32 {
+    // random number generator
+    0
+}

tests/ui/unnecessary_min_or_max.stderr

@@ -0,0 +1,107 @@
+error: `(-6_i32)` is never greater than `9` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:13:13
+   |
+LL |     let _ = (-6_i32).min(9);
+   |             ^^^^^^^^^^^^^^^ help: try: `(-6_i32)`
+   |
+   = note: `-D clippy::unnecessary-min-or-max` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_min_or_max)]`
+
+error: `(-6_i32)` is never greater than `9` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:14:13
+   |
+LL |     let _ = (-6_i32).max(9);
+   |             ^^^^^^^^^^^^^^^ help: try: `9`
+
+error: `9_u32` is never smaller than `6` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:15:13
+   |
+LL |     let _ = 9_u32.min(6);
+   |             ^^^^^^^^^^^^ help: try: `6`
+
+error: `9_u32` is never smaller than `6` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:16:13
+   |
+LL |     let _ = 9_u32.max(6);
+   |             ^^^^^^^^^^^^ help: try: `9_u32`
+
+error: `6` is never greater than `7_u8` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:17:13
+   |
+LL |     let _ = 6.min(7_u8);
+   |             ^^^^^^^^^^^ help: try: `6`
+
+error: `6` is never greater than `7_u8` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:18:13
+   |
+LL |     let _ = 6.max(7_u8);
+   |             ^^^^^^^^^^^ help: try: `7_u8`
+
+error: `0` is never greater than `x` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:22:13
+   |
+LL |     let _ = 0.min(x);
+   |             ^^^^^^^^ help: try: `0`
+
+error: `0` is never greater than `x` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:23:13
+   |
+LL |     let _ = 0.max(x);
+   |             ^^^^^^^^ help: try: `x`
+
+error: `x` is never smaller than `0_u32` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:24:13
+   |
+LL |     let _ = x.min(0_u32);
+   |             ^^^^^^^^^^^^ help: try: `0_u32`
+
+error: `x` is never smaller than `0_u32` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:25:13
+   |
+LL |     let _ = x.max(0_u32);
+   |             ^^^^^^^^^^^^ help: try: `x`
+
+error: `i32::MIN` is never greater than `x` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:29:13
+   |
+LL |     let _ = i32::MIN.min(x);
+   |             ^^^^^^^^^^^^^^^ help: try: `i32::MIN`
+
+error: `i32::MIN` is never greater than `x` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:30:13
+   |
+LL |     let _ = i32::MIN.max(x);
+   |             ^^^^^^^^^^^^^^^ help: try: `x`
+
+error: `x` is never smaller than `i32::MIN` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:31:13
+   |
+LL |     let _ = x.min(i32::MIN);
+   |             ^^^^^^^^^^^^^^^ help: try: `i32::MIN`
+
+error: `x` is never smaller than `i32::MIN` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:32:13
+   |
+LL |     let _ = x.max(i32::MIN);
+   |             ^^^^^^^^^^^^^^^ help: try: `x`
+
+error: `x` is never smaller than `i32::MIN - 0` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:34:13
+   |
+LL |     let _ = x.min(i32::MIN - 0);
+   |             ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0`
+
+error: `x` is never smaller than `i32::MIN` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:35:13
+   |
+LL |     let _ = x.max(i32::MIN);
+   |             ^^^^^^^^^^^^^^^ help: try: `x`
+
+error: `x` is never smaller than `i32::MIN - 0` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:37:13
+   |
+LL |     let _ = x.min(i32::MIN - 0);
+   |             ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0`
+
+error: aborting due to 17 previous errors
+