fix: [std_instead_of_core] false positive for multi-imports

bushrat011899 · bushrat011899 · commit db1918ae8850 · 2026-06-15T23:59:22.000+10:00
diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs
@@ -2,14 +2,15 @@ use clippy_config::Conf;
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::is_from_proc_macro;
 use clippy_utils::msrvs::Msrv;
-use rustc_errors::Applicability;
+use rustc_errors::{Applicability, MultiSpan};
 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};
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
-use rustc_span::{Span, sym};
+use rustc_span::{Span, Symbol, sym};
+use std::collections::BTreeMap;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -93,7 +94,7 @@ impl_lint_pass!(StdReexports => [
 ]);
 
 pub struct StdReexports {
-    lint_points: Option<(Span, Vec<LintPoint>)>,
+    lint_points: Option<(Span, LintPoints)>,
     msrv: Msrv,
 }
 
@@ -105,55 +106,105 @@ impl StdReexports {
         }
     }
 
-    fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) {
-        match &mut self.lint_points {
+    fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint, path: &Path<'_>) {
+        let mut lint_points = match &mut self.lint_points {
             Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => {
-                prev_lints.push(lint_point);
+                prev_lints.lint_point.update(lint_point);
+                prev_lints
             },
-            _ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))),
+            _ => {
+                emit_lints(cx, self.lint_points.replace((krate, LintPoints::new(lint_point))));
+                &mut self.lint_points.as_mut().unwrap().1
+            },
+        };
+
+        for segment in path
+            .segments
+            .iter()
+            .skip_while(|segment| segment.ident.name == kw::PathRoot)
+        {
+            lint_points = lint_points
+                .children
+                .entry(segment.ident.span)
+                .or_insert(LintPoints::new(lint_point));
+            lint_points.lint_point.update(lint_point);
         }
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
 enum LintPoint {
-    Available(Span, &'static Lint, &'static str, &'static str),
+    Available(Symbol, Symbol),
     Conflict,
 }
 
+impl LintPoint {
+    fn into_lint(self) -> Option<(&'static Lint, &'static Symbol, &'static Symbol)> {
+        match self {
+            Self::Available(sym::alloc, sym::core) => Some((ALLOC_INSTEAD_OF_CORE, &sym::alloc, &sym::core)),
+            Self::Available(sym::std, sym::alloc) => Some((STD_INSTEAD_OF_ALLOC, &sym::std, &sym::alloc)),
+            Self::Available(sym::std, sym::core) => Some((STD_INSTEAD_OF_CORE, &sym::std, &sym::core)),
+            _ => None,
+        }
+    }
+
+    fn from_symbols(used: Symbol, defined_in: Symbol) -> Self {
+        match (used, defined_in) {
+            (sym::std, sym::core) => Self::Available(sym::std, sym::core),
+            (sym::std, sym::alloc) => Self::Available(sym::std, sym::alloc),
+            (sym::alloc, sym::core) => Self::Available(sym::alloc, sym::core),
+            _ => Self::Conflict,
+        }
+    }
+
+    fn update(&mut self, other: Self) {
+        *self = match (*self, other) {
+            (Self::Conflict, _) | (_, Self::Conflict) => Self::Conflict,
+            (Self::Available(a_used, a_replace), Self::Available(b_used, b_replace)) => {
+                if a_used != b_used {
+                    Self::Conflict
+                } else if a_replace == b_replace {
+                    other
+                } else {
+                    Self::Available(a_used, sym::alloc)
+                }
+            },
+        };
+    }
+}
+
+#[derive(Debug)]
+struct LintPoints {
+    lint_point: LintPoint,
+    children: BTreeMap<Span, LintPoints>,
+}
+
+impl LintPoints {
+    fn new(lint_point: LintPoint) -> Self {
+        Self {
+            lint_point,
+            children: BTreeMap::new(),
+        }
+    }
+}
+
 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()
         {
-            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;
-                },
+            let lint_point = if is_stable(cx, def_id, self.msrv) {
+                LintPoint::from_symbols(first_segment.ident.name, cx.tcx.crate_name(def_id.krate))
+            } else {
+                LintPoint::Conflict
             };
 
-            self.lint_if_finish(
-                cx,
-                first_segment.ident.span,
-                LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with),
-            );
+            self.lint_if_finish(cx, first_segment.ident.span, lint_point, path);
         }
     }
 
@@ -170,28 +221,31 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
     }
 }
 
-fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) {
+fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, LintPoints)>) {
     let Some((krate_span, lint_points)) = lint_points else {
         return;
     };
 
-    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;
-            },
+    let mut combined = BTreeMap::new();
+    let mut stack = vec![(krate_span, lint_points)];
+    while let Some((span, lint_points)) = stack.pop() {
+        if let LintPoint::Conflict = lint_points.lint_point {
+            stack.extend(lint_points.children);
         }
+
+        combined
+            .entry(lint_points.lint_point)
+            .or_insert_with(MultiSpan::new)
+            .push_primary_span(span);
     }
+    let mut combined = combined.into_iter().collect::<Vec<_>>();
+    combined.sort_by_key(|(_, span)| span.primary_span());
 
-    if !has_conflict && let Some((lint, used_mod, replace_with)) = lint {
+    if let Some((lint, used_mod, replace_with)) = combined
+        .first()
+        .filter(|_| combined.len() == 1)
+        .and_then(|first| first.0.into_lint())
+    {
         span_lint_and_sugg(
             cx,
             lint,
@@ -204,8 +258,8 @@ fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>)
         return;
     }
 
-    for lint_point in lint_points {
-        let LintPoint::Available(span, lint, used_mod, replace_with) = lint_point else {
+    for (lint_point, span) in combined {
+        let Some((lint, used_mod, replace_with)) = lint_point.into_lint() else {
             continue;
         };
         span_lint_and_help(
diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed
@@ -96,3 +96,12 @@ fn issue15579() {
 
     let layout = alloc::Layout::new::<u8>();
 }
+
+#[warn(clippy::std_instead_of_alloc)]
+fn pr17252() {
+    use alloc::fmt::{self, Write};
+    //~^ std_instead_of_alloc
+
+    use core::result::{self, Iter, Result};
+    //~^ std_instead_of_core
+}
diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs
@@ -96,3 +96,12 @@ fn issue15579() {
 
     let layout = alloc::Layout::new::<u8>();
 }
+
+#[warn(clippy::std_instead_of_alloc)]
+fn pr17252() {
+    use std::fmt::{self, Write};
+    //~^ std_instead_of_alloc
+
+    use std::result::{self, Iter, Result};
+    //~^ std_instead_of_core
+}
diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr
@@ -97,5 +97,17 @@ error: used import from `std` instead of `core`
 LL | fn msrv_1_77(_: std::net::IpAddr) {}
    |                 ^^^ help: consider importing the item from `core`: `core`
 
-error: aborting due to 15 previous errors
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core.rs:102:9
+   |
+LL |     use std::fmt::{self, Write};
+   |         ^^^ help: consider importing the item from `alloc`: `alloc`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:105:9
+   |
+LL |     use std::result::{self, Iter, Result};
+   |         ^^^ help: consider importing the item from `core`: `core`
+
+error: aborting due to 17 previous errors
 
diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs
@@ -13,4 +13,26 @@ fn issue15143() {
     use std::{error::Error, vec::Vec, fs::File};
     //~^ std_instead_of_core
     //~| std_instead_of_alloc
+
+    use std::sync::{
+        Arc,
+        Mutex,
+        Weak,
+        atomic::{
+            AtomicPtr,
+            Ordering,
+        },
+    };
+    //~^^^^^^ std_instead_of_alloc
+    //~^^^^^^ std_instead_of_core
+}
+
+#[rustfmt::skip]
+fn pr16964() {
+    use std::{
+        borrow::Cow,
+        collections::BTreeSet,
+        ffi::OsString,
+    };
+    //~^^^ std_instead_of_alloc
 }
diff --git a/tests/ui/std_instead_of_core_unfixable.stderr b/tests/ui/std_instead_of_core_unfixable.stderr
@@ -1,30 +1,59 @@
 error: used import from `std` instead of `core`
-  --> tests/ui/std_instead_of_core_unfixable.rs:7:43
+  --> tests/ui/std_instead_of_core_unfixable.rs:7:37
    |
 LL |     use std::{collections::HashMap, hash::Hash};
-   |                                           ^^^^
+   |                                     ^^^^
    |
    = help: consider importing the item from `core`
    = note: `-D clippy::std-instead-of-core` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]`
 
 error: used import from `std` instead of `core`
-  --> tests/ui/std_instead_of_core_unfixable.rs:13:22
+  --> tests/ui/std_instead_of_core_unfixable.rs:13:15
    |
 LL |     use std::{error::Error, vec::Vec, fs::File};
-   |                      ^^^^^
+   |               ^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `alloc`
-  --> tests/ui/std_instead_of_core_unfixable.rs:13:34
+  --> tests/ui/std_instead_of_core_unfixable.rs:13:29
    |
 LL |     use std::{error::Error, vec::Vec, fs::File};
-   |                                  ^^^
+   |                             ^^^
    |
    = help: consider importing the item from `alloc`
    = note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]`
 
-error: aborting due to 3 previous errors
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core_unfixable.rs:18:9
+   |
+LL |         Arc,
+   |         ^^^
+LL |         Mutex,
+LL |         Weak,
+   |         ^^^^
+   |
+   = help: consider importing the item from `alloc`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core_unfixable.rs:21:9
+   |
+LL |         atomic::{
+   |         ^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core_unfixable.rs:33:9
+   |
+LL |         borrow::Cow,
+   |         ^^^^^^
+LL |         collections::BTreeSet,
+   |         ^^^^^^^^^^^
+   |
+   = help: consider importing the item from `alloc`
+
+error: aborting due to 6 previous errors
 
