diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 0f7cbd503bcd..bf0ce801c838 100644 --- 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::{DUMMY_SP, Ident, Span, Symbol, sym}; declare_clippy_lint! { /// ### What it does @@ -116,43 +117,64 @@ 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::>(); + + let used_from = if crate_def_id.is_crate_root() { + first_segment.ident + } else { + let mut base_path = cx.get_def_path(crate_def_id); + base_path.pop(); + base_path.extend_from_slice(&path_new); + path_new = base_path; + Ident::new(path_new[0], DUMMY_SP) }; + 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), + used_from.span, + LintPoint { + span: last_segment.ident.span, + used_from: used_from.name, + is_stable: is_stable(cx, def_id, self.msrv), + available_from_core, + available_from_alloc, + }, ); } } @@ -171,27 +193,78 @@ 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 = !krate_span.is_dummy(); + let mut all_alloc = !krate_span.is_dummy(); + + 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; + } + } + + 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)); } } - if !has_conflict && let Some((lint, used_mod, replace_with)) = lint { + for (lint, used_mod, replace_with) in suggestions { span_lint_and_sugg( cx, lint, @@ -201,21 +274,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}`"), + ); + } } } diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed index 3020ed5c6f39..226fe418be1f 100644 --- a/tests/ui/std_instead_of_core.fixed +++ b/tests/ui/std_instead_of_core.fixed @@ -90,13 +90,6 @@ fn msrv_1_76(_: std::net::IpAddr) {} fn msrv_1_77(_: core::net::IpAddr) {} //~^ std_instead_of_core -#[warn(clippy::alloc_instead_of_core)] -fn issue15579() { - use std::alloc; - - let layout = alloc::Layout::new::(); -} - #[warn(clippy::std_instead_of_core)] fn issue13158_core_io() { // items moved from std::io into core::io are stable in an unstable module. @@ -116,3 +109,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..2ca4248233b3 100644 --- a/tests/ui/std_instead_of_core.rs +++ b/tests/ui/std_instead_of_core.rs @@ -90,13 +90,6 @@ fn msrv_1_76(_: std::net::IpAddr) {} fn msrv_1_77(_: std::net::IpAddr) {} //~^ std_instead_of_core -#[warn(clippy::alloc_instead_of_core)] -fn issue15579() { - use std::alloc; - - let layout = alloc::Layout::new::(); -} - #[warn(clippy::std_instead_of_core)] fn issue13158_core_io() { // items moved from std::io into core::io are stable in an unstable module. @@ -116,3 +109,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..cac027fda67e 100644 --- a/tests/ui/std_instead_of_core.stderr +++ b/tests/ui/std_instead_of_core.stderr @@ -98,16 +98,56 @@ LL | fn msrv_1_77(_: std::net::IpAddr) {} | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:110:33 + --> tests/ui/std_instead_of_core.rs:103:33 | LL | fn issue13158_msrv_1_41(_: &dyn std::panic::UnwindSafe) {} | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:117:33 + --> tests/ui/std_instead_of_core.rs:110:33 | 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:115: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:118: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:121: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:121: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:125: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:125: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 index 66d834b5e427..4ae80caa7078 100644 --- a/tests/ui/std_instead_of_core_unfixable.rs +++ b/tests/ui/std_instead_of_core_unfixable.rs @@ -1,3 +1,4 @@ +//@no-rustfix #![warn(clippy::std_instead_of_core)] #![warn(clippy::std_instead_of_alloc)] #![allow(unused_imports)] @@ -25,3 +26,54 @@ 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 +} + +#[warn(clippy::alloc_instead_of_core)] +#[rustfmt::skip] +fn issue11159() { + use std::fmt; + //~^ std_instead_of_alloc + + struct S; + + impl fmt::Display for S { + //~^ alloc_instead_of_core + fn fmt( + &self, + _: &mut fmt::Formatter<'_> + //~^ alloc_instead_of_core + ) -> fmt::Result { + //~^ alloc_instead_of_core + todo!() + } + } +} + +fn issue15579() { + use std::alloc; + + let layout = alloc::Layout::new::(); + //~^ 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..fead6293a3c3 100644 --- a/tests/ui/std_instead_of_core_unfixable.stderr +++ b/tests/ui/std_instead_of_core_unfixable.stderr @@ -1,5 +1,5 @@ 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:8:43 | LL | use std::{collections::HashMap, hash::Hash}; | ^^^^ @@ -9,7 +9,7 @@ LL | use std::{collections::HashMap, hash::Hash}; = 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:14:22 | LL | use std::{error::Error, vec::Vec, fs::File}; | ^^^^^ @@ -17,7 +17,7 @@ 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:14:34 | LL | use std::{error::Error, vec::Vec, fs::File}; | ^^^ @@ -27,7 +27,7 @@ LL | use std::{error::Error, vec::Vec, fs::File}; = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]` error: used import from `std` instead of `alloc` - --> tests/ui/std_instead_of_core_unfixable.rs:21:17 + --> tests/ui/std_instead_of_core_unfixable.rs:22:17 | LL | borrow::Cow, | ^^^ @@ -35,12 +35,92 @@ LL | borrow::Cow, = help: consider importing the item from `alloc` error: used import from `std` instead of `alloc` - --> tests/ui/std_instead_of_core_unfixable.rs:23:22 + --> tests/ui/std_instead_of_core_unfixable.rs:24:22 | 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:35: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:43: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:44: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:39: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:41:9 + | +LL | Weak, + | ^^^^ + | + = help: consider importing the item from `alloc` + +error: used import from `std` instead of `alloc` + --> tests/ui/std_instead_of_core_unfixable.rs:56:9 + | +LL | use std::fmt; + | ^^^ help: consider importing the item from `alloc`: `alloc` + +error: used import from `alloc` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:61:15 + | +LL | impl fmt::Display for S { + | ^^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `alloc` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:65:26 + | +LL | _: &mut fmt::Formatter<'_> + | ^^^^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `alloc` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:67:19 + | +LL | ) -> fmt::Result { + | ^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:77:25 + | +LL | let layout = alloc::Layout::new::(); + | ^^^^^^ + | + = help: consider importing the item from `core` + +error: aborting due to 15 previous errors