From 9342210f921ffb1022b6f4258caae1aba3070186 Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Fri, 24 Jul 2026 19:12:51 +1000 Subject: [PATCH] fix [std_instead_of_core]: rewrite around `check_item` --- clippy_lints/src/std_instead_of_core.rs | 238 ++++++++++++------ tests/ui/std_instead_of_core.fixed | 30 ++- tests/ui/std_instead_of_core.rs | 30 ++- tests/ui/std_instead_of_core.stderr | 36 ++- tests/ui/std_instead_of_core_unfixable.rs | 93 ++++++- tests/ui/std_instead_of_core_unfixable.stderr | 112 ++++++++- 6 files changed, 432 insertions(+), 107 deletions(-) diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 3f4ba4724e94..705b1fd3dac2 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/clippy_lints/src/std_instead_of_core.rs @@ -1,15 +1,15 @@ +use LintKind::{AllocInsteadOfCore, StdInsteadOfAlloc, StdInsteadOfCore}; use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::Msrv; -use rustc_errors::Applicability; -use rustc_hir::def::{DefKind, Res}; +use rustc_errors::{Applicability, MultiSpan}; use rustc_hir::def_id::DefId; -use rustc_hir::{Block, Body, HirId, Path, PathSegment, StabilityLevel, StableSince}; -use rustc_lint::{LateContext, LateLintPass, Lint, LintContext as _}; +use rustc_hir::{Block, Body, HirId, Item, ItemKind, Path, PathSegment, StabilityLevel, StableSince}; +use rustc_lint::{LateContext, LateLintPass, LintContext as _}; use rustc_session::impl_lint_pass; use rustc_span::symbol::kw; -use rustc_span::{Span, sym}; +use rustc_span::{Ident, Span, sym}; declare_clippy_lint! { /// ### What it does @@ -93,8 +93,9 @@ impl_lint_pass!(StdReexports => [ ]); pub struct StdReexports { - lint_points: Option<(Span, Vec)>, + lint_points: Option<(LintPoint, Vec, Vec, usize)>, msrv: Msrv, + paths_to_skip: usize, } impl StdReexports { @@ -102,58 +103,83 @@ impl StdReexports { Self { lint_points: Option::default(), msrv: conf.msrv.into(), - } - } - - fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) { - match &mut self.lint_points { - Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => { - prev_lints.push(lint_point); - }, - _ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))), + paths_to_skip: 0, } } } #[derive(Debug)] -enum LintPoint { - Available(Span, &'static Lint, &'static str, &'static str), - Conflict, +struct LintPoint { + ident: Ident, + is_crate: bool, + from: UsedFrom, +} + +impl LintPoint { + fn try_new(cx: &LateContext<'_>, &PathSegment { ident, res, .. }: &PathSegment<'_>) -> Option { + let def_id = res.opt_def_id()?; + let is_crate = def_id.is_crate_root(); + let from = match cx.tcx.crate_name(def_id.krate) { + sym::std => UsedFrom::Std, + sym::alloc => UsedFrom::Alloc, + _ => return None, + }; + Some(LintPoint { ident, is_crate, from }) + } } 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 - && !matches!(def_kind, DefKind::Macro(_)) - && let Some(first_segment) = get_first_segment(path) - && let Res::Def(DefKind::Mod, crate_def_id) = first_segment.res - && crate_def_id.is_crate_root() - && is_stable(cx, def_id, self.msrv) - && !path.span.in_external_macro(cx.sess().source_map()) - && !is_from_proc_macro(cx, &first_segment.ident) - && let Some(last_segment) = path.segments.last() + let Some((a, b)) = get_end_segments(path) else { return }; + + if let Some(n) = self.paths_to_skip.checked_sub(1) { + self.paths_to_skip = n; + } else if let Some(lint_point) = LintPoint::try_new(cx, a) + && let Some(b_def_id) = path.res.opt_def_id() + && let Some(defined_in) = DefinedIn::try_new(cx, b_def_id) + && let Some(kind) = LintKind::try_new(lint_point.from, defined_in) + && is_stable(cx, b_def_id, self.msrv) { - 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; - }, - }; + emit_lints(cx, self.lint_points.take()); + emit_lint(cx, &lint_point, &kind, false, b.ident.span); + } + } + + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + let ItemKind::Use(path, ..) = item.kind else { return }; + let Some((a, b)) = get_end_segments(path) else { return }; - self.lint_if_finish( - cx, - first_segment.ident.span, - LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with), - ); + self.paths_to_skip += path.res.present_items().count(); + + if self + .lint_points + .as_ref() + .is_none_or(|(x, ..)| x.ident.span != a.ident.span) + { + emit_lints(cx, self.lint_points.take()); + self.lint_points = LintPoint::try_new(cx, a).map(|a| (a, Vec::new(), Vec::new(), 0)); + } + + if let Some((lint_point, in_core, in_alloc, conflicts)) = self.lint_points.as_mut() { + let lint_kind = path + .res + .iter() + .flatten() + .try_fold(DefinedIn::Core, |acc, res| { + let def_id = res.opt_def_id()?; + match (acc, DefinedIn::try_new(cx, def_id)?, is_stable(cx, def_id, self.msrv)) { + (_, _, false) => None, + (DefinedIn::Core, DefinedIn::Core, _) => Some(DefinedIn::Core), + _ => Some(DefinedIn::Alloc), + } + }) + .and_then(|defined_in| LintKind::try_new(lint_point.from, defined_in)); + + match lint_kind { + Some(AllocInsteadOfCore | StdInsteadOfCore) => in_core.push(b.ident.span), + Some(StdInsteadOfAlloc) => in_alloc.push(b.ident.span), + None => *conflicts += 1, + } } } @@ -170,64 +196,71 @@ 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 { +fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(LintPoint, Vec, Vec, usize)>) { + let Some((lint_point, in_core, in_alloc, conflicts)) = 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; - }, + let total = in_core.len() + in_alloc.len() + conflicts; + for (spans, defined_in) in [(in_core, DefinedIn::Core), (in_alloc, DefinedIn::Alloc)] { + let Some(lint_kind) = LintKind::try_new(lint_point.from, defined_in) else { + continue; + }; + + if !spans.is_empty() { + emit_lint(cx, &lint_point, &lint_kind, spans.len() < total, spans); } } +} - if !has_conflict && let Some((lint, used_mod, replace_with)) = lint { +fn emit_lint(cx: &LateContext<'_>, point: &LintPoint, kind: &LintKind, has_conflict: bool, span: impl Into) { + if point.ident.span.in_external_macro(cx.sess().source_map()) || is_from_proc_macro(cx, &point.ident) { + return; + } + + let (lint, lint_message) = match kind { + StdInsteadOfCore => (STD_INSTEAD_OF_CORE, "used import from `std` instead of `core`"), + StdInsteadOfAlloc => (STD_INSTEAD_OF_ALLOC, "used import from `std` instead of `alloc`"), + AllocInsteadOfCore => (ALLOC_INSTEAD_OF_CORE, "used import from `alloc` instead of `core`"), + }; + + let (help_message, replace_with) = match kind { + StdInsteadOfCore | AllocInsteadOfCore => ("consider importing the item from `core`", &sym::core), + StdInsteadOfAlloc => ("consider importing the item from `alloc`", &sym::alloc), + }; + + if !has_conflict && point.is_crate { span_lint_and_sugg( cx, lint, - krate_span, - format!("used import from `{used_mod}` instead of `{replace_with}`"), - format!("consider importing the item from `{replace_with}`"), + point.ident.span, + lint_message, + help_message, (*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}`"), - ); + let leaf_spans = span.into(); + if leaf_spans.primary_spans().len() == 1 { + span_lint_and_help(cx, lint, leaf_spans, lint_message, None, help_message); + } else { + span_lint_and_then(cx, lint, point.ident.span, lint_message, |diag| { + diag.span_help(leaf_spans, help_message); + }); } } -/// Returns the first named segment of a [`Path`]. +/// Returns the first and last named segments of a [`Path`]. /// /// If this is a global path (such as `::std::fmt::Debug`), then the segment after [`kw::PathRoot`] /// is returned. -fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>> { +fn get_end_segments<'tcx, T>(path: &Path<'tcx, T>) -> Option<(&'tcx PathSegment<'tcx>, &'tcx PathSegment<'tcx>)> { match path.segments { // A global path will have PathRoot as the first segment. In this case, return the segment after. - [x, y, ..] if x.ident.name == kw::PathRoot => Some(y), - [x, ..] => Some(x), + [x, y, .., z] if x.ident.name == kw::PathRoot => Some((y, z)), + [x, .., y] => Some((x, y)), _ => None, } } @@ -262,3 +295,42 @@ fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool { } } } + +enum LintKind { + StdInsteadOfCore, + StdInsteadOfAlloc, + AllocInsteadOfCore, +} + +impl LintKind { + fn try_new(used_from: UsedFrom, defined_in: DefinedIn) -> Option { + match (used_from, defined_in) { + (UsedFrom::Alloc, DefinedIn::Core) => Some(AllocInsteadOfCore), + (UsedFrom::Std, DefinedIn::Core) => Some(StdInsteadOfCore), + (UsedFrom::Std, DefinedIn::Alloc) => Some(StdInsteadOfAlloc), + _ => None, + } + } +} + +#[derive(Debug, Clone, Copy)] +enum DefinedIn { + Core, + Alloc, +} + +impl DefinedIn { + fn try_new(cx: &LateContext<'_>, def_id: DefId) -> Option { + match cx.tcx.crate_name(def_id.krate) { + sym::alloc => Some(DefinedIn::Alloc), + sym::core => Some(DefinedIn::Core), + _ => None, + } + } +} + +#[derive(Debug, Clone, Copy)] +enum UsedFrom { + Alloc, + Std, +} diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed index 63d0e204d72f..5be46c89d21b 100644 --- a/tests/ui/std_instead_of_core.fixed +++ b/tests/ui/std_instead_of_core.fixed @@ -89,13 +89,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. @@ -115,3 +108,26 @@ 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_core)] +fn issue17260() { + use core::concat; + //~^ std_instead_of_core +} + +#[warn(clippy::std_instead_of_alloc)] +fn non_use_paths() { + type Foo = alloc::sync::Arc>; + //~^ std_instead_of_alloc + + let _x = core::iter::repeat(u8::default()); + //~^ std_instead_of_core + + fn impl_trait(_: impl core::fmt::Display) {} + //~^ std_instead_of_core + + struct Bar { + layout: core::alloc::Layout, + //~^ std_instead_of_core + } +} diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs index e5cde188bedd..54f727c9d9bc 100644 --- a/tests/ui/std_instead_of_core.rs +++ b/tests/ui/std_instead_of_core.rs @@ -89,13 +89,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. @@ -115,3 +108,26 @@ 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_core)] +fn issue17260() { + use std::concat; + //~^ std_instead_of_core +} + +#[warn(clippy::std_instead_of_alloc)] +fn non_use_paths() { + type Foo = std::sync::Arc>; + //~^ std_instead_of_alloc + + let _x = std::iter::repeat(u8::default()); + //~^ std_instead_of_core + + fn impl_trait(_: impl std::fmt::Display) {} + //~^ std_instead_of_core + + struct Bar { + layout: std::alloc::Layout, + //~^ std_instead_of_core + } +} diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr index 7045d4448211..5082e68c1642 100644 --- a/tests/ui/std_instead_of_core.stderr +++ b/tests/ui/std_instead_of_core.stderr @@ -98,16 +98,46 @@ 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:109:33 + --> tests/ui/std_instead_of_core.rs:102: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:116:33 + --> tests/ui/std_instead_of_core.rs:109: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:114:9 + | +LL | use std::concat; + | ^^^ help: consider importing the item from `core`: `core` + +error: used import from `std` instead of `alloc` + --> tests/ui/std_instead_of_core.rs:120:16 + | +LL | type Foo = std::sync::Arc>; + | ^^^ help: consider importing the item from `alloc`: `alloc` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core.rs:123:14 + | +LL | let _x = std::iter::repeat(u8::default()); + | ^^^ help: consider importing the item from `core`: `core` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core.rs:126:27 + | +LL | fn impl_trait(_: impl std::fmt::Display) {} + | ^^^ help: consider importing the item from `core`: `core` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core.rs:130:17 + | +LL | layout: std::alloc::Layout, + | ^^^ help: consider importing the item from `core`: `core` + +error: aborting due to 22 previous errors diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs index 459db5e8944a..ff63766d14f7 100644 --- a/tests/ui/std_instead_of_core_unfixable.rs +++ b/tests/ui/std_instead_of_core_unfixable.rs @@ -15,11 +15,100 @@ fn issue15143() { #[rustfmt::skip] fn pr16964() { + //~v std_instead_of_alloc use std::{ borrow::Cow, - //~^ std_instead_of_alloc collections::BTreeSet, - //~^ std_instead_of_alloc ffi::OsString, }; } + +#[warn(clippy::alloc_instead_of_core)] +fn issue15579() { + use std::alloc; + + let layout = alloc::Layout::new::(); + //~^ std_instead_of_core +} + +#[rustfmt::skip] +fn issue12468() { + use std::{ + fmt::Result, //~ std_instead_of_core + io::Write, + }; + + use std::sync::{ + Arc, //~ std_instead_of_alloc + Mutex, + }; +} + +#[allow(clippy::legacy_numeric_constants)] +#[warn(clippy::alloc_instead_of_core)] +#[rustfmt::skip] +fn pr17252_large() { + extern crate alloc; + + use { + std::sync::Mutex, + ::{ + //~v std_instead_of_core + std::{ + fmt::{*, Formatter}, //~ std_instead_of_alloc + fs, + sync::atomic, + sync::atomic::{ + AtomicUsize, + AtomicU8, + Ordering, + }, + }, + core::u32, + std::collections::HashMap, + }, + alloc::{ + fmt::Result as FmtResult, //~ alloc_instead_of_core + format, + } + }; +} + +#[warn(clippy::alloc_instead_of_core)] +#[rustfmt::skip] +fn issue11159() { + extern crate alloc; + + use alloc::fmt; + + 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!() + } + } +} + +#[warn(clippy::alloc_instead_of_core)] +#[rustfmt::skip] +fn mixed_name_spaces() { + extern crate alloc; + + use std::{ + io, + iter::repeat_with, //~ std_instead_of_core + }; + + use alloc::alloc::{ + alloc, + dealloc, + Layout, //~ alloc_instead_of_core + }; +} diff --git a/tests/ui/std_instead_of_core_unfixable.stderr b/tests/ui/std_instead_of_core_unfixable.stderr index c79b3e48cad3..6183c261ca71 100644 --- a/tests/ui/std_instead_of_core_unfixable.stderr +++ b/tests/ui/std_instead_of_core_unfixable.stderr @@ -27,20 +27,122 @@ 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:19:17 + --> tests/ui/std_instead_of_core_unfixable.rs:19:9 + | +LL | use std::{ + | ^^^ + | +help: consider importing the item from `alloc` + --> tests/ui/std_instead_of_core_unfixable.rs:20:17 | LL | borrow::Cow, | ^^^ +LL | collections::BTreeSet, + | ^^^^^^^^ + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:30:25 + | +LL | let layout = alloc::Layout::new::(); + | ^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:37:14 + | +LL | fmt::Result, + | ^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `std` instead of `alloc` + --> tests/ui/std_instead_of_core_unfixable.rs:42:9 + | +LL | Arc, + | ^^^ | = help: consider importing the item from `alloc` +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:57:13 + | +LL | std::{ + | ^^^ + | +help: consider importing the item from `core` + --> tests/ui/std_instead_of_core_unfixable.rs:58:26 + | +LL | fmt::{*, Formatter}, + | ^^^^^^^^^ +LL | fs, +LL | sync::atomic, + | ^^^^^^ +LL | sync::atomic::{ +LL | AtomicUsize, + | ^^^^^^^^^^^ +LL | AtomicU8, + | ^^^^^^^^ +LL | Ordering, + | ^^^^^^^^ + error: used import from `std` instead of `alloc` - --> tests/ui/std_instead_of_core_unfixable.rs:21:22 + --> tests/ui/std_instead_of_core_unfixable.rs:58:17 | -LL | collections::BTreeSet, - | ^^^^^^^^ +LL | fmt::{*, Formatter}, + | ^^^ | = 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:71:18 + | +LL | fmt::Result as FmtResult, + | ^^^^^^ + | + = 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 `alloc` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:86: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:90: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:92: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:106:15 + | +LL | iter::repeat_with, + | ^^^^^^^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `alloc` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:112:9 + | +LL | Layout, + | ^^^^^^ + | + = help: consider importing the item from `core` + +error: aborting due to 15 previous errors