fix: exclude core from external crates

l0ngvh · l0ngvh · commit f446e4eb313f · 2024-04-28T08:31:08.000Z
diff --git 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
@@ -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<Constant<'tcx>> {
         use self::Constant::{Bool, Int};
diff --git 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,25 +12,39 @@ 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;
     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 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
@@ -1,6 +1,6 @@
 #![allow(unused)]
 #![warn(clippy::unnecessary_min_or_max)]
-
+#![allow(clippy::identity_op)]
 const X: i32 = 1;
 
 fn main() {
@@ -12,25 +12,39 @@ 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);
     let _ = 0.max(x);
     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
@@ -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
 
