fix [std_instead_of_core]: rewrite around check_item

Author: bushrat011899

clippy_lints/src/lib.rs

@@ -757,7 +757,7 @@ rustc_lint::late_lint_methods!(
         ManualRetain: manual_retain::ManualRetain = manual_retain::ManualRetain::new(conf),
         ManualRotate: manual_rotate::ManualRotate = manual_rotate::ManualRotate,
         Operators: operators::Operators = operators::Operators::new(conf),
-        StdReexports: std_instead_of_core::StdReexports = std_instead_of_core::StdReexports::new(conf),
+        StdReexports: std_instead_of_core::StdReexports<'tcx> = std_instead_of_core::StdReexports::new(conf),
         UncheckedTimeSubtraction: time_subtraction::UncheckedTimeSubtraction = time_subtraction::UncheckedTimeSubtraction::new(conf),
         PartialeqToNone: partialeq_to_none::PartialeqToNone = partialeq_to_none::PartialeqToNone,
         ManualAbsDiff: manual_abs_diff::ManualAbsDiff = manual_abs_diff::ManualAbsDiff::new(conf),

clippy_lints/src/std_instead_of_core.rs

@@ -1,15 +1,14 @@
 use clippy_config::Conf;
-use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::is_from_proc_macro;
 use clippy_utils::msrvs::Msrv;
 use rustc_errors::Applicability;
-use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
-use rustc_hir::{Block, Body, HirId, Path, PathSegment, StabilityLevel, StableSince};
-use rustc_lint::{LateContext, LateLintPass, Lint, LintContext as _};
+use rustc_hir::{Block, Body, HirId, Item, ItemKind, Path, PathSegment, StabilityLevel, StableSince};
+use rustc_lint::{LateContext, LateLintPass, LintContext as _};
 use rustc_session::impl_lint_pass;
+use rustc_span::sym;
 use rustc_span::symbol::kw;
-use rustc_span::{Span, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -86,144 +85,106 @@ declare_clippy_lint! {
     "type is imported from std when available in core"
 }
 
-impl_lint_pass!(StdReexports => [
+impl_lint_pass!(StdReexports<'_> => [
     ALLOC_INSTEAD_OF_CORE,
     STD_INSTEAD_OF_ALLOC,
     STD_INSTEAD_OF_CORE,
 ]);
 
-pub struct StdReexports {
-    lint_points: Option<(Span, Vec<LintPoint>)>,
+pub struct StdReexports<'a> {
+    lint_points: Vec<LintPoint<'a>>,
     msrv: Msrv,
+    use_item_depth: usize,
 }
 
-impl StdReexports {
+impl<'a> StdReexports<'a> {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
-            lint_points: Option::default(),
+            lint_points: Vec::new(),
             msrv: conf.msrv,
-        }
-    }
-
-    fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) {
-        match &mut self.lint_points {
-            Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => {
-                prev_lints.push(lint_point);
-            },
-            _ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))),
+            use_item_depth: 0,
         }
     }
 }
 
-#[derive(Debug)]
-enum LintPoint {
-    Available(Span, &'static Lint, &'static str, &'static str),
-    Conflict,
+#[derive(Debug, Clone, Copy)]
+struct LintPoint<'a> {
+    root: PathSegment<'a>,
+    tail: PathSegment<'a>,
 }
 
-impl<'tcx> LateLintPass<'tcx> for StdReexports {
-    fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
-        if let Res::Def(def_kind, def_id) = path.res
-            && let Some(first_segment) = get_first_segment(path)
-            && is_stable(cx, def_id, self.msrv)
-            && !path.span.in_external_macro(cx.sess().source_map())
-            && !is_from_proc_macro(cx, &first_segment.ident)
-            && !matches!(def_kind, DefKind::Macro(_))
-            && let Some(last_segment) = path.segments.last()
-            && let Res::Def(DefKind::Mod, crate_def_id) = first_segment.res
-            && crate_def_id.is_crate_root()
+impl<'tcx> LateLintPass<'tcx> for StdReexports<'tcx> {
+    fn check_path(&mut self, _: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
+        if self.use_item_depth == 0
+            && path.segments.len() > 1
+            && let Some(&root) = get_first_segment(path)
+            && let Some(&tail) = path.segments.last()
         {
-            let (lint, used_mod, replace_with) = match first_segment.ident.name {
-                sym::std => match cx.tcx.crate_name(def_id.krate) {
-                    sym::core => (STD_INSTEAD_OF_CORE, "std", "core"),
-                    sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"),
-                    _ => {
-                        self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
-                        return;
-                    },
-                },
-                sym::alloc if cx.tcx.crate_name(def_id.krate) == sym::core => (ALLOC_INSTEAD_OF_CORE, "alloc", "core"),
-                _ => {
-                    self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
-                    return;
-                },
-            };
+            self.lint_points.push(LintPoint {
+                root,
+                tail: PathSegment { res: path.res, ..tail },
+            });
+        }
+    }
 
-            self.lint_if_finish(
-                cx,
-                first_segment.ident.span,
-                LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with),
-            );
+    fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let ItemKind::Use(path, _use_kind) = item.kind {
+            if path.segments.len() > 1
+                && let Some(&root) = get_first_segment(path)
+                && let Some(&tail) = path.segments.last()
+            {
+                for res in path.res.into_iter().flatten() {
+                    self.lint_points.push(LintPoint {
+                        root,
+                        tail: PathSegment { res, ..tail },
+                    });
+                }
+            }
+            self.use_item_depth += 1;
+        }
+    }
+
+    fn check_item_post(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if matches!(item.kind, ItemKind::Use(_path, _use_kind)) {
+            self.use_item_depth -= 1;
         }
     }
 
     fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &Block<'tcx>) {
-        emit_lints(cx, self.lint_points.take());
+        emit_lints(cx, &mut self.lint_points, self.msrv);
     }
 
     fn check_body_post(&mut self, cx: &LateContext<'tcx>, _: &Body<'tcx>) {
-        emit_lints(cx, self.lint_points.take());
+        emit_lints(cx, &mut self.lint_points, self.msrv);
     }
 
     fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
-        emit_lints(cx, self.lint_points.take());
+        emit_lints(cx, &mut self.lint_points, self.msrv);
     }
 }
 
-fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) {
-    let Some((krate_span, lint_points)) = lint_points else {
-        return;
-    };
+fn emit_lints(cx: &LateContext<'_>, lint_points: &mut Vec<LintPoint<'_>>, msrv: Msrv) {
+    // This should already be the case, but we'll assert that it is
+    lint_points.sort_by_key(|x| (x.root.ident.span, x.tail.ident.span));
 
-    let mut lint: Option<(&'static Lint, &'static str, &'static str)> = None;
-    let mut has_conflict = false;
-    for lint_point in &lint_points {
-        match lint_point {
-            LintPoint::Available(_, l, used_mod, replace_with)
-                if lint.is_none_or(|(prev_l, ..)| l.name == prev_l.name) =>
-            {
-                lint = Some((l, used_mod, replace_with));
-            },
-            _ => {
-                has_conflict = true;
-                break;
-            },
+    for shared_root in lint_points.chunk_by(|a, b| a.root.ident.span == b.root.ident.span) {
+        if let Some(root) = shared_root.iter().find_map(|a| a.root.res.opt_def_id())
+            && !(root.is_crate_root() && LintPoint::try_emit(shared_root, cx, msrv, true, root))
+        {
+            for shared_tail in shared_root.chunk_by(|a, b| a.tail.ident.span == b.tail.ident.span) {
+                LintPoint::try_emit(shared_tail, cx, msrv, false, root);
+            }
         }
     }
 
-    if !has_conflict && let Some((lint, used_mod, replace_with)) = lint {
-        span_lint_and_sugg(
-            cx,
-            lint,
-            krate_span,
-            format!("used import from `{used_mod}` instead of `{replace_with}`"),
-            format!("consider importing the item from `{replace_with}`"),
-            (*replace_with).to_string(),
-            Applicability::MachineApplicable,
-        );
-        return;
-    }
-
-    for lint_point in lint_points {
-        let LintPoint::Available(span, lint, used_mod, replace_with) = lint_point else {
-            continue;
-        };
-        span_lint_and_help(
-            cx,
-            lint,
-            span,
-            format!("used import from `{used_mod}` instead of `{replace_with}`"),
-            None,
-            format!("consider importing the item from `{replace_with}`"),
-        );
-    }
+    lint_points.clear();
 }
 
 /// Returns the first named segment of a [`Path`].
 ///
 /// If this is a global path (such as `::std::fmt::Debug`), then the segment after [`kw::PathRoot`]
 /// is returned.
-fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
+fn get_first_segment<'tcx, T>(path: &Path<'tcx, T>) -> Option<&'tcx PathSegment<'tcx>> {
     match path.segments {
         // A global path will have PathRoot as the first segment. In this case, return the segment after.
         [x, y, ..] if x.ident.name == kw::PathRoot => Some(y),
@@ -232,11 +193,12 @@ fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>>
     }
 }
 
-/// Checks if all ancestors of `def_id` meet `msrv` to avoid linting [unstable moves](https://github.com/rust-lang/rust/pull/95956)
-/// or now stable moves that were once unstable.
+/// Checks if all ancestors of `def_id` meet `msrv` at `node` to avoid linting
+/// [unstable moves](https://github.com/rust-lang/rust/pull/95956) or now stable
+/// moves that were once unstable.
 ///
 /// Does not catch individually moved items
-fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool {
+fn is_stable_at(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv, node: HirId) -> bool {
     loop {
         if let Some(stability) = cx.tcx.lookup_stability(def_id) {
             match stability.level {
@@ -247,8 +209,8 @@ fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool {
                     ..
                 } => return true,
                 StabilityLevel::Stable { since, .. } => match since {
-                    StableSince::Version(v) if !msrv.meets(cx, v) => return false,
-                    StableSince::Current if msrv.current(cx).is_none() => return false,
+                    StableSince::Version(v) if !msrv.meets_at(cx.tcx, node, v) => return false,
+                    StableSince::Current if msrv.at(cx.tcx, node).is_none() => return false,
                     StableSince::Err(_) => return false,
                     StableSince::Version(_) | StableSince::Current => {},
                 },
@@ -262,3 +224,53 @@ fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool {
         }
     }
 }
+
+impl<'a> LintPoint<'a> {
+    fn try_emit(lint_points: &[Self], cx: &LateContext<'_>, msrv: Msrv, is_suggestion: bool, root: DefId) -> bool {
+        if let Some(first) = lint_points.first()
+            && let Some(tail) = first.tail.res.opt_def_id()
+            && !first.root.ident.span.in_external_macro(cx.sess().source_map())
+            && !is_from_proc_macro(cx, &first.root.ident)
+            && lint_points
+                .iter()
+                .map(|lint_point| lint_point.tail.res.opt_def_id())
+                .all(|id| id.is_some_and(|id| id.krate == tail.krate && is_stable_at(cx, id, msrv, first.tail.hir_id)))
+        {
+            let root = cx.tcx.crate_name(root.krate);
+            let tail = cx.tcx.crate_name(tail.krate);
+
+            let (lint, message, help) = match (root, tail) {
+                (sym::alloc, sym::core) => (
+                    ALLOC_INSTEAD_OF_CORE,
+                    "used import from `alloc` instead of `core`",
+                    "consider importing the item from `core`",
+                ),
+                (sym::std, sym::alloc) => (
+                    STD_INSTEAD_OF_ALLOC,
+                    "used import from `std` instead of `alloc`",
+                    "consider importing the item from `alloc`",
+                ),
+                (sym::std, sym::core) => (
+                    STD_INSTEAD_OF_CORE,
+                    "used import from `std` instead of `core`",
+                    "consider importing the item from `core`",
+                ),
+                _ => return false,
+            };
+
+            if is_suggestion {
+                span_lint_hir_and_then(cx, lint, first.root.hir_id, first.root.ident.span, message, |diag| {
+                    diag.span_suggestion(first.root.ident.span, help, tail, Applicability::MachineApplicable);
+                });
+            } else {
+                span_lint_hir_and_then(cx, lint, first.tail.hir_id, first.tail.ident.span, message, |diag| {
+                    diag.help(help);
+                });
+            }
+
+            true
+        } else {
+            false
+        }
+    }
+}

tests/ui/std_instead_of_core.fixed

@@ -89,13 +89,6 @@ fn msrv_1_76(_: std::net::IpAddr) {}
 fn msrv_1_77(_: core::net::IpAddr) {}
 //~^ std_instead_of_core
 
-#[warn(clippy::alloc_instead_of_core)]
-fn issue15579() {
-    use std::alloc;
-
-    let layout = alloc::Layout::new::<u8>();
-}
-
 #[warn(clippy::std_instead_of_core)]
 fn issue13158_core_io() {
     // items moved from std::io into core::io are stable in an unstable module.
@@ -115,3 +108,26 @@ fn issue13158_msrv_1_80(_: &dyn std::error::Error) {}
 #[clippy::msrv = "1.81"]
 fn issue13158_msrv_1_81(_: &dyn core::error::Error) {}
 //~^ std_instead_of_core
+
+#[warn(clippy::std_instead_of_core)]
+fn issue17260() {
+    use core::concat;
+    //~^ std_instead_of_core
+}
+
+#[warn(clippy::std_instead_of_alloc)]
+fn non_use_paths() {
+    type Foo = alloc::sync::Arc<std::sync::Mutex<bool>>;
+    //~^ std_instead_of_alloc
+
+    let _x = core::iter::repeat(u8::default());
+    //~^ std_instead_of_core
+
+    fn impl_trait(_: impl core::fmt::Display) {}
+    //~^ std_instead_of_core
+
+    struct Bar {
+        layout: core::alloc::Layout,
+        //~^ std_instead_of_core
+    }
+}

tests/ui/std_instead_of_core.rs

@@ -89,13 +89,6 @@ fn msrv_1_76(_: std::net::IpAddr) {}
 fn msrv_1_77(_: std::net::IpAddr) {}
 //~^ std_instead_of_core
 
-#[warn(clippy::alloc_instead_of_core)]
-fn issue15579() {
-    use std::alloc;
-
-    let layout = alloc::Layout::new::<u8>();
-}
-
 #[warn(clippy::std_instead_of_core)]
 fn issue13158_core_io() {
     // items moved from std::io into core::io are stable in an unstable module.
@@ -115,3 +108,26 @@ fn issue13158_msrv_1_80(_: &dyn std::error::Error) {}
 #[clippy::msrv = "1.81"]
 fn issue13158_msrv_1_81(_: &dyn std::error::Error) {}
 //~^ std_instead_of_core
+
+#[warn(clippy::std_instead_of_core)]
+fn issue17260() {
+    use std::concat;
+    //~^ std_instead_of_core
+}
+
+#[warn(clippy::std_instead_of_alloc)]
+fn non_use_paths() {
+    type Foo = std::sync::Arc<std::sync::Mutex<bool>>;
+    //~^ std_instead_of_alloc
+
+    let _x = std::iter::repeat(u8::default());
+    //~^ std_instead_of_core
+
+    fn impl_trait(_: impl std::fmt::Display) {}
+    //~^ std_instead_of_core
+
+    struct Bar {
+        layout: std::alloc::Layout,
+        //~^ std_instead_of_core
+    }
+}