diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 0f7cbd503bcd..9807b6dbea4c 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/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,16 +119,18 @@ 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(_)) @@ -133,26 +138,36 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { && 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::>(); + + 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)>) { - 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)>) (*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 { + 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::>(); + + 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 +} diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed index 3020ed5c6f39..f1a2e4e301ba 100644 --- 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 index 49b4218aa898..7879b9788399 100644 --- 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 index 0363852cf8d4..ce8da60cbfe2 100644 --- 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_no_implicit_prelude.fixed b/tests/ui/std_instead_of_core_no_implicit_prelude.fixed new file mode 100644 index 000000000000..f2b2a3c5a928 --- /dev/null +++ b/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 +} diff --git a/tests/ui/std_instead_of_core_no_implicit_prelude.rs b/tests/ui/std_instead_of_core_no_implicit_prelude.rs new file mode 100644 index 000000000000..f2f5f6eb6398 --- /dev/null +++ b/tests/ui/std_instead_of_core_no_implicit_prelude.rs @@ -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 std::result::Result; + //~^ std_instead_of_core + + use std::vec::Vec; + //~^ std_instead_of_alloc +} diff --git a/tests/ui/std_instead_of_core_no_implicit_prelude.stderr b/tests/ui/std_instead_of_core_no_implicit_prelude.stderr new file mode 100644 index 000000000000..0ea9ab40fd40 --- /dev/null +++ b/tests/ui/std_instead_of_core_no_implicit_prelude.stderr @@ -0,0 +1,20 @@ +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_no_implicit_prelude.rs:11:9 + | +LL | use std::result::Result; + | ^^^ help: consider importing the item from `core`: `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 `alloc` + --> tests/ui/std_instead_of_core_no_implicit_prelude.rs:14:9 + | +LL | use std::vec::Vec; + | ^^^ help: consider importing the item from `alloc`: `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 2 previous errors + diff --git a/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs b/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs new file mode 100644 index 000000000000..a28a058b2ff7 --- /dev/null +++ b/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs @@ -0,0 +1,27 @@ +#![warn(clippy::std_instead_of_core)] +#![warn(clippy::std_instead_of_alloc)] +#![allow(unused_imports)] +#![no_implicit_prelude] + +extern crate std; + +fn pr17252_no_core() { + // This wont have a suggestion since `core` isn't in scope + use std::result::Result; + //~^ std_instead_of_core +} + +fn pr17252_no_alloc() { + // This wont have a suggestion since `alloc` isn't in scope + use std::vec::Vec; + //~^ std_instead_of_alloc +} + +fn pr17252_local_core() { + extern crate core; + + // FIXME: This *should* have a suggestion, but we're not currently + // checking local scope, only the extern crate prelude. + use std::result::Result; + //~^ std_instead_of_core +} diff --git a/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.stderr b/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.stderr new file mode 100644 index 000000000000..ee19e5511c99 --- /dev/null +++ b/tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.stderr @@ -0,0 +1,30 @@ +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs:10:9 + | +LL | use std::result::Result; + | ^^^ + | + = 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 `alloc` + --> tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs:16:9 + | +LL | use std::vec::Vec; + | ^^^ + | + = 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: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_no_implicit_prelude_unfixable.rs:25:9 + | +LL | use std::result::Result; + | ^^^ + | + = help: consider importing the item from `core` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs index 66d834b5e427..b625a9f0f7d1 100644 --- 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 index 6fa8f47a4d6f..cbbd43f10fe6 100644 --- 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