fix: [std_instead_of_core] false positive for multi-imports

bushrat011899 · bushrat011899 · commit 5e134b4f330f · 2026-06-23T11:13:45.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_hir::def::{DefKind, Res};
+use clippy_utils::paths::{PathNS, lookup_path};
+use rustc_errors::{Applicability, MultiSpan};
+use rustc_hir::def::{DefKind, Namespace, 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,43 +117,55 @@ impl StdReexports {
 }
 
 #[derive(Debug)]
-enum LintPoint {
-    Available(Span, &'static Lint, &'static str, &'static str),
-    Conflict,
+struct LintPoint {
+    span: Span,
+    used_from: Symbol,
+    is_stable: bool,
+    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 namespace = match def_kind.ns() {
+                Some(Namespace::TypeNS) => PathNS::Type,
+                Some(Namespace::ValueNS) => PathNS::Value,
+                Some(Namespace::MacroNS) => PathNS::Macro,
+                None => PathNS::Arbitrary,
             };
 
+            let mut path_new = path
+                .segments
+                .iter()
+                .map(|segment| segment.ident.name)
+                .skip_while(|&segment| segment == kw::PathRoot)
+                .collect::<Vec<_>>();
+
+            path_new[0] = sym::core;
+            let available_from_core = lookup_path(cx.tcx, namespace, &path_new).contains(&def_id);
+
+            path_new[0] = sym::alloc;
+            let available_from_alloc = lookup_path(cx.tcx, namespace, &path_new).contains(&def_id);
+
             self.lint_if_finish(
                 cx,
                 first_segment.ident.span,
-                LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with),
+                LintPoint {
+                    span: last_segment.ident.span,
+                    used_from: first_segment.ident.name,
+                    is_stable: is_stable(cx, def_id, self.msrv),
+                    available_from_core,
+                    available_from_alloc,
+                },
             );
         }
     }
@@ -171,27 +184,78 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
 }
 
 fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) {
-    let Some((krate_span, lint_points)) = lint_points else {
+    let Some((krate_span, mut 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;
-            },
+    // It's possible for multiple items to come from the same path.
+    // For example, `std::vec` refers to a macro and a module.
+    // In these cases, it's possible for them to have different availabilities.
+    // `std::vec` as a macro and a module are both defined in `alloc` and unavailable in `core`.
+    // Whereas, `std::env` is not available in `alloc`, and only the macro is available in `core`.
+    // Since we aren't checking which of the shadowed items the user needs, we take the intersection
+    // of these availabilities to ensure we don't provide the user a false positive.
+    lint_points.sort_by_key(|lint_point| lint_point.span);
+    lint_points.dedup_by(|a, b| {
+        if a.span == b.span {
+            b.is_stable &= a.is_stable;
+            b.available_from_alloc &= a.available_from_alloc;
+            b.available_from_core &= a.available_from_core;
+            true
+        } else {
+            false
+        }
+    });
+
+    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.is_stable && lint_point.available_from_core {
+            core_span.push_primary_span(lint_point.span);
+        } else {
+            all_core = false;
+        }
+
+        if lint_point.is_stable && lint_point.available_from_alloc {
+            if !lint_point.available_from_core {
+                alloc_span.push_primary_span(lint_point.span);
+            }
+        } else {
+            all_alloc = false;
         }
     }
 
-    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,21 +265,19 @@ 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 {
-            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}`"),
-        );
+    for (lint, used_mod, replace_with, span) in helps {
+        for &span in span.primary_spans() {
+            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}`"),
+            );
+        }
     }
 }
 
diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed
@@ -116,3 +116,23 @@ 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_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
+
+    // Macros aren't currently linted.
+    use std::writeln;
+}
diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs
@@ -116,3 +116,23 @@ 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_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
+
+    // Macros aren't currently linted.
+    use std::writeln;
+}
diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr
@@ -109,5 +109,45 @@ error: used import from `std` instead of `core`
 LL | fn issue13158_msrv_1_81(_: &dyn std::error::Error) {}
    |                                 ^^^ help: consider importing the item from `core`: `core`
 
-error: aborting due to 17 previous errors
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:122: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:125: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:128: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:128: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:132: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:132:26
+   |
+LL |     use std::fmt::{self, Write};
+   |                          ^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: aborting due to 23 previous errors
 
diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs
@@ -25,3 +25,26 @@ fn pr16964() {
         ffi::OsString,
     };
 }
+
+#[warn(clippy::alloc_instead_of_core)]
+#[rustfmt::skip]
+fn pr17252() {
+    extern crate alloc;
+
+    use alloc::str::{self as _, FromStr as _};
+    //~^ alloc_instead_of_core
+
+    use std::sync::{
+        Arc,
+        Mutex,
+        Weak,
+        atomic::{
+            AtomicPtr,
+            Ordering,
+        },
+    };
+    //~^^^^^^^^ std_instead_of_alloc
+    //~^^^^^^^ std_instead_of_alloc
+    //~^^^^^^ std_instead_of_core
+    //~^^^^^^ std_instead_of_core
+}
diff --git a/tests/ui/std_instead_of_core_unfixable.stderr b/tests/ui/std_instead_of_core_unfixable.stderr
@@ -42,5 +42,47 @@ LL |         collections::BTreeSet,
    |
    = help: consider importing the item from `alloc`
 
-error: aborting due to 5 previous errors
+error: used import from `alloc` instead of `core`
+  --> tests/ui/std_instead_of_core_unfixable.rs:34:33
+   |
+LL |     use alloc::str::{self as _, FromStr as _};
+   |                                 ^^^^^^^
+   |
+   = help: consider importing the item from `core`
+   = note: `-D clippy::alloc-instead-of-core` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::alloc_instead_of_core)]`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core_unfixable.rs:42:13
+   |
+LL |             AtomicPtr,
+   |             ^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core_unfixable.rs:43:13
+   |
+LL |             Ordering,
+   |             ^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core_unfixable.rs:38:9
+   |
+LL |         Arc,
+   |         ^^^
+   |
+   = help: consider importing the item from `alloc`
+
+error: used import from `std` instead of `alloc`
+  --> tests/ui/std_instead_of_core_unfixable.rs:40:9
+   |
+LL |         Weak,
+   |         ^^^^
+   |
+   = help: consider importing the item from `alloc`
+
+error: aborting due to 10 previous errors
 
