fix: [std_instead_of_core] false positive for multi-imports

bushrat011899 · bushrat011899 · commit 2eda7d5f50ec · 2026-06-16T21:07:04.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,14 @@ 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_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
-use rustc_span::{Span, sym};
+use rustc_span::{Span, Symbol, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -116,44 +116,34 @@ impl StdReexports {
 }
 
 #[derive(Debug)]
-enum LintPoint {
-    Available(Span, &'static Lint, &'static str, &'static str),
-    Conflict,
+struct LintPoint {
+    span: Span,
+    used_from: Symbol,
+    available_from_core: bool,
+    available_from_alloc: bool,
 }
 
 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 is_stable = is_stable(cx, def_id, self.msrv);
+
+            let lint_point = LintPoint {
+                span: last_segment.ident.span,
+                used_from: first_segment.ident.name,
+                available_from_core: is_stable && is_available_from(cx, sym::core, path),
+                available_from_alloc: is_stable && is_available_from(cx, sym::alloc, path),
             };
 
-            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);
         }
     }
 
@@ -175,23 +165,55 @@ fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>)
         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 core_span = MultiSpan::new();
+    let mut alloc_span = MultiSpan::new();
+    let mut all_from_std = true;
+    let mut all_from_alloc = true;
+    let mut all_core = true;
+    let mut all_alloc = true;
+
+    for lint_point in lint_points {
+        all_from_std &= lint_point.used_from == sym::std;
+        all_from_alloc &= lint_point.used_from == sym::alloc;
+
+        if lint_point.available_from_core {
+            core_span.push_primary_span(lint_point.span);
+        } else {
+            all_core = false;
+        }
+
+        if !lint_point.available_from_alloc {
+            all_alloc = false;
+        }
+
+        if !lint_point.available_from_core && lint_point.available_from_alloc {
+            alloc_span.push_primary_span(lint_point.span);
         }
     }
 
-    if !has_conflict && let Some((lint, used_mod, replace_with)) = lint {
+    let mut helps = Vec::new();
+    let mut suggestions = Vec::new();
+
+    if all_from_std {
+        if all_core {
+            suggestions.push((STD_INSTEAD_OF_CORE, &sym::std, &sym::core));
+            helps.push((STD_INSTEAD_OF_ALLOC, &sym::std, &sym::alloc, alloc_span));
+        } else if all_alloc {
+            suggestions.push((STD_INSTEAD_OF_ALLOC, &sym::std, &sym::alloc));
+            helps.push((STD_INSTEAD_OF_CORE, &sym::std, &sym::core, core_span));
+        } else {
+            helps.push((STD_INSTEAD_OF_CORE, &sym::std, &sym::core, core_span));
+            helps.push((STD_INSTEAD_OF_ALLOC, &sym::std, &sym::alloc, alloc_span));
+        }
+    } else if all_from_alloc {
+        if all_core {
+            suggestions.push((ALLOC_INSTEAD_OF_CORE, &sym::alloc, &sym::core));
+        } else {
+            helps.push((ALLOC_INSTEAD_OF_CORE, &sym::alloc, &sym::core, core_span));
+        }
+    }
+
+    for (lint, used_mod, replace_with) in suggestions {
         span_lint_and_sugg(
             cx,
             lint,
@@ -201,13 +223,12 @@ fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>)
             (*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 {
+    for (lint, used_mod, replace_with, span) in helps {
+        if span.primary_spans().is_empty() {
             continue;
-        };
+        }
         span_lint_and_help(
             cx,
             lint,
@@ -219,6 +240,67 @@ fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>)
     }
 }
 
+/// Checks if the provided `path` could have its crate replaced with `krate` and still be valid.
+fn is_available_from(cx: &LateContext<'_>, krate: Symbol, path: &Path<'_>) -> bool {
+    // We're attempting to find this DefId with an alternate path.
+    // If this DefId isn't available then there's no way to do that.
+    let Some(target) = path.res.opt_def_id() else {
+        return false;
+    };
+
+    // Determine DefId for krate
+    let Some(krate) = cx
+        .tcx
+        .crates(())
+        .iter()
+        .filter(|&&krate| cx.tcx.is_user_visible_dep(krate))
+        .find(|&&krate_num| cx.tcx.crate_name(krate_num) == krate)
+        .map(|&krate| DefId {
+            index: rustc_span::def_id::CRATE_DEF_INDEX,
+            krate,
+        })
+    else {
+        return false;
+    };
+
+    path.segments
+        .iter()
+        .skip_while(|segment| {
+            // Skip `::crate` and `crate` segments
+            segment.ident.name == kw::PathRoot || segment.res.opt_def_id().is_some_and(DefId::is_crate_root)
+        })
+        .scan(krate, |cursor, segment| {
+            // Some, but not all, items from core and reexported by alloc.
+            // To find these cases, we must walk the alloc and core crates along the provided path
+            // to see if the desired item is accessible.
+            *cursor = cx
+                .tcx
+                .module_children(*cursor)
+                .iter()
+                .filter(|child| child.vis.is_public())
+                .filter(|child| {
+                    // We search by name to allow cases like `core::str` and `std::str`.
+                    // `std::str` is actually a reexport of `alloc::str`, but `alloc::str`
+                    // reexports the contents of `core::str`, so many instances of `std::str`
+                    // can be replaced with `core::str`, even if they are distinct items.
+                    child.ident.name == segment.ident.name
+                })
+                .find_map(|child| {
+                    // TyCtxt::module_children will ICE if the provided DefId isn't "module like".
+                    // The initial value will always be a crate so no need to check there.
+                    // Since what we're searching for may not be "module like", we must
+                    // exempt a non-"module like" DefId if and only if it is our target.
+                    let is_module_like = child.res.module_like_def_id().is_some();
+                    let def_id = child.res.opt_def_id()?;
+                    (def_id == target || is_module_like).then_some(def_id)
+                })?;
+
+            Some(*cursor)
+        })
+        .last()
+        .is_some_and(|cursor| cursor == target)
+}
+
 /// 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`]
diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed
@@ -96,3 +96,20 @@ fn issue15579() {
 
     let layout = alloc::Layout::new::<u8>();
 }
+
+#[warn(clippy::std_instead_of_alloc)]
+fn pr17252() {
+    use core::result::{self, Iter, Result};
+    //~^ std_instead_of_core
+
+    use alloc::str::{self as _};
+    //~^ std_instead_of_alloc
+
+    use alloc::str::{self as _, FromStr as _};
+    //~^ std_instead_of_alloc
+    //~| std_instead_of_core
+
+    use alloc::fmt::{self, Write};
+    //~^ std_instead_of_alloc
+    //~| 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,20 @@ fn issue15579() {
 
     let layout = alloc::Layout::new::<u8>();
 }
+
+#[warn(clippy::std_instead_of_alloc)]
+fn pr17252() {
+    use std::result::{self, Iter, Result};
+    //~^ std_instead_of_core
+
+    use std::str::{self as _};
+    //~^ std_instead_of_alloc
+
+    use std::str::{self as _, FromStr as _};
+    //~^ std_instead_of_alloc
+    //~| std_instead_of_core
+
+    use std::fmt::{self, Write};
+    //~^ std_instead_of_alloc
+    //~| 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,45 @@ 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 `core`
+  --> tests/ui/std_instead_of_core.rs:102:9
+   |
+LL |     use std::result::{self, Iter, Result};
+   |         ^^^ help: consider importing the item from `core`: `core`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core.rs:105:9
+   |
+LL |     use std::str::{self as _};
+   |         ^^^ help: consider importing the item from `alloc`: `alloc`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core.rs:108:9
+   |
+LL |     use std::str::{self as _, FromStr as _};
+   |         ^^^ help: consider importing the item from `alloc`: `alloc`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:108:31
+   |
+LL |     use std::str::{self as _, FromStr as _};
+   |                               ^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core.rs:112: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:112:26
+   |
+LL |     use std::fmt::{self, Write};
+   |                          ^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: aborting due to 21 previous errors
 
diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs
@@ -1,5 +1,6 @@
 #![warn(clippy::std_instead_of_core)]
 #![warn(clippy::std_instead_of_alloc)]
+#![warn(clippy::alloc_instead_of_core)]
 #![allow(unused_imports)]
 
 #[rustfmt::skip]
@@ -13,4 +14,31 @@ 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
+
+    extern crate alloc;
+
+    use alloc::str::{self as _, FromStr as _};
+    //~^ alloc_instead_of_core
 }
diff --git a/tests/ui/std_instead_of_core_unfixable.stderr b/tests/ui/std_instead_of_core_unfixable.stderr
