fix [std_instead_of_core]: don't assume alloc/core are in the extern prelude

clippy_lints/src/std_instead_of_core.rs

@@ -2,14 +2,17 @@ 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_data_structures::fx::FxHashSet;
+use rustc_errors::{Applicability, MultiSpan};
+use rustc_hir::attrs::AttributeKind;
+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_hir::{Attribute, Block, Body, HirId, Item, ItemKind, Node, Path, PathSegment, StabilityLevel, StableSince};
+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 +119,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 +186,89 @@ 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));
+        }
+    }
+
+    let extern_prelude = extern_prelude(cx);
+
+    for (lint, used_mod, replace_with) in suggestions {
+        // If the target crate isn't in the extern crate prelude,
+        // the suggestion is likely to fail.
+        // It's possible it could still work (e.g., extern crate in a local scope),
+        // but we'll be cautious and downgrade from a suggestion to a help.
+        if !extern_prelude.contains(replace_with) {
+            helps.push((lint, used_mod, replace_with, krate_span.into()));
+            continue;
+        }
+
         span_lint_and_sugg(
             cx,
             lint,
@@ -201,21 +278,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}`"),
+            );
+        }
     }
 }
 
@@ -262,3 +337,43 @@ fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool {
         }
     }
 }
+
+fn extern_prelude(cx: &LateContext<'_>) -> FxHashSet<Symbol> {
+    let mut extern_prelude = cx
+        .tcx
+        .hir_root_module()
+        .item_ids
+        .iter()
+        .filter_map(|item| match cx.tcx.hir_node(item.hir_id()) {
+            Node::Item(Item {
+                kind: ItemKind::ExternCrate(original_name, ident),
+                ..
+            }) => Some(original_name.unwrap_or(ident.name)),
+            _ => None,
+        })
+        .collect::<FxHashSet<_>>();
+
+    let implicit_core = !cx.tcx.hir_krate_attrs().iter().any(|attr| {
+        matches!(
+            attr,
+            Attribute::Parsed(AttributeKind::NoCore | AttributeKind::NoImplicitPrelude)
+        )
+    });
+
+    if implicit_core {
+        extern_prelude.insert(sym::core);
+    }
+
+    let implicit_std = !cx.tcx.hir_krate_attrs().iter().any(|attr| {
+        matches!(
+            attr,
+            Attribute::Parsed(AttributeKind::NoStd | AttributeKind::NoImplicitPrelude)
+        )
+    });
+
+    if implicit_std {
+        extern_prelude.insert(sym::std);
+    }
+
+    extern_prelude
+}

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;
+}

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;
+}

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
 

tests/ui/std_instead_of_core_no_implicit_prelude.fixed

@@ -0,0 +1,16 @@
+#![warn(clippy::std_instead_of_core)]
+#![warn(clippy::std_instead_of_alloc)]
+#![allow(unused_imports)]
+#![no_implicit_prelude]
+
+extern crate alloc;
+extern crate core;
+extern crate std;
+
+fn pr17252() {
+    use core::result::Result;
+    //~^ std_instead_of_core
+
+    use alloc::vec::Vec;
+    //~^ std_instead_of_alloc
+}