refactor: use add core crate constant source

vohoanglong0107 · vohoanglong0107 · commit 6cb40f1030fd · 2024-04-29T03:32:46.000Z
diff --git a/clippy_lints/src/methods/unnecessary_min_or_max.rs b/clippy_lints/src/methods/unnecessary_min_or_max.rs
@@ -3,15 +3,14 @@ use std::cmp::Ordering;
 use super::UNNECESSARY_MIN_OR_MAX;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 
-use clippy_utils::consts::{constant, ConstEvalLateContext, Constant, FullInt};
+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::{sym, Span};
+use rustc_span::Span;
 
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
@@ -91,6 +90,11 @@ fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<
 }
 
 fn is_external_const(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    let mut ctxt = ConstEvalLateContext::new(cx, cx.typeck_results());
-    ctxt.expr(expr).is_some() && ctxt.expr_is_external(expr, &[sym::core])
+    let cv_src = constant_with_source(cx, cx.typeck_results(), expr);
+    let Some((_, const_src)) = cv_src else { return false };
+
+    !matches!(
+        const_src,
+        ConstantSource::LocalOrCoreCrateConstant | ConstantSource::Local
+    )
 }
diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs
@@ -16,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};
@@ -303,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 current crate of `core` crate.
+    LocalOrCoreCrateConstant,
 }
 impl ConstantSource {
     pub fn is_local(&self) -> bool {
@@ -416,9 +418,18 @@ 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_local_or_core_crate = if let Some(def_id) = self.lcx.qpath_res(qpath, e.hir_id()).opt_def_id() {
+                    def_id.is_local() || 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;
+                    this.source = if is_local_or_core_crate {
+                        ConstantSource::LocalOrCoreCrateConstant
+                    } else {
+                        ConstantSource::Constant
+                    };
                     Some(result)
                 })
             },
@@ -530,59 +541,6 @@ 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
@@ -12,14 +12,14 @@ 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 _ = 12;
+    let _ = X;
+    let _ = X;
+    let _ = 12;
+    let _ = 12;
+    let _ = (X + 1);
+    let _ = X - 1;
+    let _ = 12;
 
     let x: u32 = 42;
     // unsigned with zero
diff --git a/tests/ui/unnecessary_min_or_max.stderr b/tests/ui/unnecessary_min_or_max.stderr
@@ -37,6 +37,54 @@ error: `6` is never greater than `7_u8` and has therefore no effect
 LL |     let _ = 6.max(7_u8);
    |             ^^^^^^^^^^^ help: try: `7_u8`
 
+error: `X` is never greater than `12` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:15:13
+   |
+LL |     let _ = X.max(12);
+   |             ^^^^^^^^^ help: try: `12`
+
+error: `X` is never greater than `12` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:16:13
+   |
+LL |     let _ = X.min(12);
+   |             ^^^^^^^^^ help: try: `X`
+
+error: `12` is never smaller than `X` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:17:13
+   |
+LL |     let _ = 12.min(X);
+   |             ^^^^^^^^^ help: try: `X`
+
+error: `12` is never smaller than `X` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:18:13
+   |
+LL |     let _ = 12.max(X);
+   |             ^^^^^^^^^ help: try: `12`
+
+error: `(X + 1)` is never greater than `12` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:19:13
+   |
+LL |     let _ = (X + 1).max(12);
+   |             ^^^^^^^^^^^^^^^ help: try: `12`
+
+error: `(X + 1)` is never greater than `12` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:20:13
+   |
+LL |     let _ = (X + 1).min(12);
+   |             ^^^^^^^^^^^^^^^ help: try: `(X + 1)`
+
+error: `12` is never smaller than `X - 1` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:21:13
+   |
+LL |     let _ = 12.min(X - 1);
+   |             ^^^^^^^^^^^^^ help: try: `X - 1`
+
+error: `12` is never smaller than `X - 1` and has therefore no effect
+  --> tests/ui/unnecessary_min_or_max.rs:22:13
+   |
+LL |     let _ = 12.max(X - 1);
+   |             ^^^^^^^^^^^^^ help: try: `12`
+
 error: `0` is never greater than `x` and has therefore no effect
   --> tests/ui/unnecessary_min_or_max.rs:26:13
    |
@@ -103,5 +151,5 @@ error: `x` is never smaller than `i32::MIN - 0` and has therefore no effect
 LL |     let _ = x.min(i32::MIN - 0);
    |             ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0`
 
-error: aborting due to 17 previous errors
+error: aborting due to 25 previous errors
 
