diff --git a/clippy_lints/src/methods/unnecessary_min_or_max.rs b/clippy_lints/src/methods/unnecessary_min_or_max.rs index 3516f0bef949..2b113b41ea80 100644 --- a/clippy_lints/src/methods/unnecessary_min_or_max.rs +++ b/clippy_lints/src/methods/unnecessary_min_or_max.rs @@ -3,17 +3,15 @@ use std::cmp::Ordering; use super::UNNECESSARY_MIN_OR_MAX; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::consts::{constant, Constant, FullInt}; -use clippy_utils::macros::HirNode; +use clippy_utils::consts::{constant, ConstEvalLateContext, Constant, FullInt}; use clippy_utils::source::snippet; -use hir::{Expr, ExprKind}; use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::Span; +use rustc_span::{sym, Span}; pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, @@ -93,12 +91,6 @@ fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option< } fn is_external_const(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - let ExprKind::Path(ref qpath) = expr.kind else { - return false; - }; - - let Some(def_id) = cx.qpath_res(qpath, expr.hir_id()).opt_def_id() else { - return false; - }; - !def_id.is_local() + let mut ctxt = ConstEvalLateContext::new(cx, cx.typeck_results()); + ctxt.expr(expr).is_some() && ctxt.expr_is_external(expr, &[sym::core]) } diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 253ae3aca689..e36bf9d8a79b 100644 --- a/clippy_utils/src/consts.rs +++ b/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}; @@ -529,6 +530,59 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } } + /// Simple constant folding to determine if an expression depends on an external crate + /// excluding crates listed in exceptions + pub fn expr_is_external(&mut self, e: &Expr<'_>, exceptions: &[Symbol]) -> bool { + match e.kind { + ExprKind::ConstBlock(ConstBlock { body, .. }) => { + self.expr_is_external(self.lcx.tcx.hir().body(body).value, exceptions) + }, + ExprKind::DropTemps(e) => self.expr_is_external(e, exceptions), + ExprKind::Path(ref qpath) => { + if let Some(def_id) = self.lcx.qpath_res(qpath, e.hir_id()).opt_def_id() { + def_id.is_local() || exceptions.iter().all(|e| self.lcx.tcx.crate_name(def_id.krate) != *e) + } else { + true + } + }, + ExprKind::Block(block, _) => { + if block.stmts.is_empty() + && let Some(expr) = block.expr + { + self.expr_is_external(expr, exceptions) + } else { + false + } + }, + ExprKind::Array(vec) => vec.iter().any(|elem| self.expr_is_external(elem, exceptions)), + ExprKind::Tup(tup) => tup.iter().any(|elem| self.expr_is_external(elem, exceptions)), + ExprKind::Repeat(value, _) => self.expr_is_external(value, exceptions), + ExprKind::Unary(_, operand) => self.expr_is_external(operand, exceptions), + ExprKind::If(cond, then, ref otherwise) => { + if let Some(Constant::Bool(b)) = self.expr(cond) { + if b { + self.expr_is_external(then, exceptions) + } else { + otherwise + .as_ref() + .is_some_and(|expr| self.expr_is_external(expr, exceptions)) + } + } else { + false + } + }, + ExprKind::Binary(_, left, right) => { + self.expr_is_external(left, exceptions) || self.expr_is_external(right, exceptions) + }, + ExprKind::Index(arr, index, _) => { + self.expr_is_external(arr, exceptions) || self.expr_is_external(index, exceptions) + }, + ExprKind::AddrOf(_, _, inner) => self.expr_is_external(inner, exceptions), + ExprKind::Field(local_expr, _) => self.expr_is_external(local_expr, exceptions), + _ => false, + } + } + #[expect(clippy::cast_possible_wrap)] fn constant_not(&self, o: &Constant<'tcx>, ty: Ty<'_>) -> Option> { use self::Constant::{Bool, Int}; diff --git a/tests/ui/unnecessary_min_or_max.fixed b/tests/ui/unnecessary_min_or_max.fixed index 1645251f2d03..d9081efa2d18 100644 --- a/tests/ui/unnecessary_min_or_max.fixed +++ b/tests/ui/unnecessary_min_or_max.fixed @@ -1,6 +1,6 @@ #![allow(unused)] #![warn(clippy::unnecessary_min_or_max)] - +#![allow(clippy::identity_op)] const X: i32 = 1; fn main() { @@ -12,6 +12,15 @@ fn main() { let _ = 6; let _ = 7_u8; + 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); + let x: u32 = 42; // unsigned with zero let _ = 0; @@ -19,18 +28,23 @@ fn main() { 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 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); } fn random_u32() -> u32 { // random number generator diff --git a/tests/ui/unnecessary_min_or_max.rs b/tests/ui/unnecessary_min_or_max.rs index c4b75a44e046..254f6c5ea036 100644 --- a/tests/ui/unnecessary_min_or_max.rs +++ b/tests/ui/unnecessary_min_or_max.rs @@ -1,6 +1,6 @@ #![allow(unused)] #![warn(clippy::unnecessary_min_or_max)] - +#![allow(clippy::identity_op)] const X: i32 = 1; fn main() { @@ -12,6 +12,15 @@ fn main() { let _ = 6.min(7_u8); let _ = 6.max(7_u8); + 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); + let x: u32 = 42; // unsigned with zero let _ = 0.min(x); @@ -19,18 +28,23 @@ fn main() { let _ = x.min(0_u32); let _ = x.max(0_u32); - // The below cases shouldn't be lint - let mut min = u32::MAX; - for _ in 0..1000 { - min = min.min(random_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()); + } } fn random_u32() -> u32 { // random number generator diff --git a/tests/ui/unnecessary_min_or_max.stderr b/tests/ui/unnecessary_min_or_max.stderr index 05b5d6d3f4ea..33c7810c7cd0 100644 --- a/tests/ui/unnecessary_min_or_max.stderr +++ b/tests/ui/unnecessary_min_or_max.stderr @@ -38,28 +38,70 @@ 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:17:13 + --> tests/ui/unnecessary_min_or_max.rs:26: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:18:13 + --> tests/ui/unnecessary_min_or_max.rs:27: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:19:13 + --> tests/ui/unnecessary_min_or_max.rs:28: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:20:13 + --> tests/ui/unnecessary_min_or_max.rs:29:13 | LL | let _ = x.max(0_u32); | ^^^^^^^^^^^^ help: try: `x` -error: aborting due to 10 previous errors +error: `i32::MIN` is never greater than `x` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:33: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:34: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:35: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:36: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:38: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:39: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:41:13 + | +LL | let _ = x.min(i32::MIN - 0); + | ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0` + +error: aborting due to 17 previous errors