Skip to content

Fix std-instead-of-core FP when not all items come from the new crate #15165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 76 additions & 33 deletions clippy_lints/src/std_instead_of_core.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
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_attr_data_structures::{StabilityLevel, StableSince};
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{Block, Body, HirId, Path, PathSegment};
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
Expand Down Expand Up @@ -88,94 +88,137 @@ declare_clippy_lint! {
}

pub struct StdReexports {
lint_point: (Span, Option<LintPoint>),
lint_points: Option<(Span, Vec<LintPoint>)>,
msrv: Msrv,
}

impl StdReexports {
pub fn new(conf: &'static Conf) -> Self {
Self {
lint_point: Default::default(),
lint_points: Option::default(),
msrv: conf.msrv,
}
}

fn lint_if_finish(&mut self, cx: &LateContext<'_>, (span, item): (Span, Option<LintPoint>)) {
if span.source_equal(self.lint_point.0) {
return;
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]))),
}

if !self.lint_point.0.is_dummy() {
emit_lints(cx, &self.lint_point);
}

self.lint_point = (span, item);
}
}

impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);

type LintPoint = (&'static Lint, &'static str, &'static str);
#[derive(Debug)]
enum LintPoint {
Available(Span, &'static Lint, &'static str, &'static str),
Conflict,
}

impl<'tcx> LateLintPass<'tcx> for StdReexports {
fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
if let Res::Def(_, def_id) = path.res
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 (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, None));
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")
} else {
self.lint_if_finish(cx, (first_segment.ident.span, None));
self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
return;
}
},
_ => return,
_ => {
self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
return;
},
};

self.lint_if_finish(cx, (first_segment.ident.span, Some((lint, used_mod, replace_with))));
self.lint_if_finish(
cx,
first_segment.ident.span,
LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with),
);
}
}

fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &Block<'tcx>) {
self.lint_if_finish(cx, Default::default());
emit_lints(cx, self.lint_points.take());
}

fn check_body_post(&mut self, cx: &LateContext<'tcx>, _: &Body<'tcx>) {
self.lint_if_finish(cx, Default::default());
emit_lints(cx, self.lint_points.take());
}

fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
self.lint_if_finish(cx, Default::default());
emit_lints(cx, self.lint_points.take());
}
}

fn emit_lints(cx: &LateContext<'_>, (span, item): &(Span, Option<LintPoint>)) {
let Some((lint, used_mod, replace_with)) = item else {
fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) {
let Some((krate_span, lint_points)) = lint_points else {
return;
};

span_lint_and_sugg(
cx,
lint,
*span,
format!("used import from `{used_mod}` instead of `{replace_with}`"),
format!("consider importing the item from `{replace_with}`"),
(*replace_with).to_string(),
Applicability::MachineApplicable,
);
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;
},
}
}

if !has_conflict && let Some((lint, used_mod, replace_with)) = lint {
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}`"),
(*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}`"),
);
}
}

/// Returns the first named segment of a [`Path`].
Expand Down
7 changes: 0 additions & 7 deletions tests/ui/std_instead_of_core.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ extern crate alloc;
#[macro_use]
extern crate proc_macro_derive;

#[warn(clippy::std_instead_of_core)]
fn std_instead_of_core() {
// Regular import
use core::hash::Hasher;
Expand Down Expand Up @@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {}
#[clippy::msrv = "1.77"]
fn msrv_1_77(_: core::net::IpAddr) {}
//~^ std_instead_of_core

#[warn(clippy::std_instead_of_core)]
#[rustfmt::skip]
fn issue14982() {
use std::{collections::HashMap, hash::Hash};
}
7 changes: 0 additions & 7 deletions tests/ui/std_instead_of_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ extern crate alloc;
#[macro_use]
extern crate proc_macro_derive;

#[warn(clippy::std_instead_of_core)]
fn std_instead_of_core() {
// Regular import
use std::hash::Hasher;
Expand Down Expand Up @@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {}
#[clippy::msrv = "1.77"]
fn msrv_1_77(_: std::net::IpAddr) {}
//~^ std_instead_of_core

#[warn(clippy::std_instead_of_core)]
#[rustfmt::skip]
fn issue14982() {
use std::{collections::HashMap, hash::Hash};
}
30 changes: 15 additions & 15 deletions tests/ui/std_instead_of_core.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:14:9
--> tests/ui/std_instead_of_core.rs:13:9
|
LL | use std::hash::Hasher;
| ^^^ help: consider importing the item from `core`: `core`
Expand All @@ -8,61 +8,61 @@ LL | use std::hash::Hasher;
= 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.rs:17:11
--> tests/ui/std_instead_of_core.rs:16:11
|
LL | use ::std::hash::Hash;
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:23:9
--> tests/ui/std_instead_of_core.rs:22:9
|
LL | use std::fmt::{Debug, Result};
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:28:9
--> tests/ui/std_instead_of_core.rs:27:9
|
LL | use std::{
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:35:15
--> tests/ui/std_instead_of_core.rs:34:15
|
LL | let ptr = std::ptr::null::<u32>();
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:37:21
--> tests/ui/std_instead_of_core.rs:36:21
|
LL | let ptr_mut = ::std::ptr::null_mut::<usize>();
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:41:16
--> tests/ui/std_instead_of_core.rs:40:16
|
LL | let cell = std::cell::Cell::new(8u32);
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:43:27
--> tests/ui/std_instead_of_core.rs:42:27
|
LL | let cell_absolute = ::std::cell::Cell::new(8u32);
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:48:9
--> tests/ui/std_instead_of_core.rs:47:9
|
LL | use std::error::Error;
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:52:9
--> tests/ui/std_instead_of_core.rs:51:9
|
LL | use std::iter::Iterator;
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `alloc`
--> tests/ui/std_instead_of_core.rs:59:9
--> tests/ui/std_instead_of_core.rs:58:9
|
LL | use std::vec;
| ^^^ help: consider importing the item from `alloc`: `alloc`
Expand All @@ -71,13 +71,13 @@ LL | use std::vec;
= 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.rs:61:9
--> tests/ui/std_instead_of_core.rs:60:9
|
LL | use std::vec::Vec;
| ^^^ help: consider importing the item from `alloc`: `alloc`

error: used import from `alloc` instead of `core`
--> tests/ui/std_instead_of_core.rs:67:9
--> tests/ui/std_instead_of_core.rs:66:9
|
LL | use alloc::slice::from_ref;
| ^^^^^ help: consider importing the item from `core`: `core`
Expand All @@ -86,13 +86,13 @@ LL | use alloc::slice::from_ref;
= 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.rs:82:9
--> tests/ui/std_instead_of_core.rs:81:9
|
LL | std::intrinsics::copy(a, b, 1);
| ^^^ help: consider importing the item from `core`: `core`

error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core.rs:91:17
--> tests/ui/std_instead_of_core.rs:90:17
|
LL | fn msrv_1_77(_: std::net::IpAddr) {}
| ^^^ help: consider importing the item from `core`: `core`
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/std_instead_of_core_unfixable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@no-rustfix

#![warn(clippy::std_instead_of_core)]
#![warn(clippy::std_instead_of_alloc)]
#![allow(unused_imports)]

#[rustfmt::skip]
fn issue14982() {
use std::{collections::HashMap, hash::Hash};
//~^ std_instead_of_core
}

#[rustfmt::skip]
fn issue15143() {
use std::{error::Error, vec::Vec, fs::File};
//~^ std_instead_of_core
//~| std_instead_of_alloc
}
30 changes: 30 additions & 0 deletions tests/ui/std_instead_of_core_unfixable.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error: used import from `std` instead of `core`
--> tests/ui/std_instead_of_core_unfixable.rs:9:43
|
LL | use std::{collections::HashMap, hash::Hash};
| ^^^^
|
= 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 `core`
--> tests/ui/std_instead_of_core_unfixable.rs:15:22
|
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:15:34
|
LL | use std::{error::Error, vec::Vec, fs::File};
| ^^^
|
= 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: aborting due to 3 previous errors