Skip to content

Commit 9eb8512

Browse files
profetiaprofetia
committed
fix: std-instead-of-core FP when not all items come from the new crate
1 parent 0732085 commit 9eb8512

File tree

6 files changed

+139
-62
lines changed

6 files changed

+139
-62
lines changed

‎clippy_lints/src/std_instead_of_core.rs

Lines changed: 76 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_config::Conf;
2-
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
33
use clippy_utils::is_from_proc_macro;
44
use clippy_utils::msrvs::Msrv;
55
use rustc_attr_data_structures::{StabilityLevel, StableSince};
66
use rustc_errors::Applicability;
7-
use rustc_hir::def::Res;
7+
use rustc_hir::def::{DefKind, Res};
88
use rustc_hir::def_id::DefId;
99
use rustc_hir::{Block, Body, HirId, Path, PathSegment};
1010
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
@@ -88,94 +88,137 @@ declare_clippy_lint! {
8888
}
8989

9090
pub struct StdReexports {
91-
lint_point: (Span, Option<LintPoint>),
91+
lint_points: Option<(Span, Vec<LintPoint>)>,
9292
msrv: Msrv,
9393
}
9494

9595
impl StdReexports {
9696
pub fn new(conf: &'static Conf) -> Self {
9797
Self {
98-
lint_point: Default::default(),
98+
lint_points: Option::default(),
9999
msrv: conf.msrv,
100100
}
101101
}
102102

103-
fn lint_if_finish(&mut self, cx: &LateContext<'_>, (span, item): (Span, Option<LintPoint>)) {
104-
if span.source_equal(self.lint_point.0) {
105-
return;
103+
fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) {
104+
match &mut self.lint_points {
105+
Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => {
106+
prev_lints.push(lint_point);
107+
},
108+
_ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))),
106109
}
107-
108-
if !self.lint_point.0.is_dummy() {
109-
emit_lints(cx, &self.lint_point);
110-
}
111-
112-
self.lint_point = (span, item);
113110
}
114111
}
115112

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

118-
type LintPoint = (&'static Lint, &'static str, &'static str);
115+
#[derive(Debug)]
116+
enum LintPoint {
117+
Available(Span, &'static Lint, &'static str, &'static str),
118+
Conflict,
119+
}
119120

120121
impl<'tcx> LateLintPass<'tcx> for StdReexports {
121122
fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
122-
if let Res::Def(_, def_id) = path.res
123+
if let Res::Def(def_kind, def_id) = path.res
123124
&& let Some(first_segment) = get_first_segment(path)
124125
&& is_stable(cx, def_id, self.msrv)
125126
&& !path.span.in_external_macro(cx.sess().source_map())
126127
&& !is_from_proc_macro(cx, &first_segment.ident)
128+
&& !matches!(def_kind, DefKind::Macro(_))
129+
&& let Some(last_segment) = path.segments.last()
127130
{
128131
let (lint, used_mod, replace_with) = match first_segment.ident.name {
129132
sym::std => match cx.tcx.crate_name(def_id.krate) {
130133
sym::core => (STD_INSTEAD_OF_CORE, "std", "core"),
131134
sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"),
132135
_ => {
133-
self.lint_if_finish(cx, (first_segment.ident.span, None));
136+
self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
134137
return;
135138
},
136139
},
137140
sym::alloc => {
138141
if cx.tcx.crate_name(def_id.krate) == sym::core {
139142
(ALLOC_INSTEAD_OF_CORE, "alloc", "core")
140143
} else {
141-
self.lint_if_finish(cx, (first_segment.ident.span, None));
144+
self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
142145
return;
143146
}
144147
},
145-
_ => return,
148+
_ => {
149+
self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict);
150+
return;
151+
},
146152
};
147153

148-
self.lint_if_finish(cx, (first_segment.ident.span, Some((lint, used_mod, replace_with))));
154+
self.lint_if_finish(
155+
cx,
156+
first_segment.ident.span,
157+
LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with),
158+
);
149159
}
150160
}
151161

152162
fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &Block<'tcx>) {
153-
self.lint_if_finish(cx, Default::default());
163+
emit_lints(cx, self.lint_points.take());
154164
}
155165

156166
fn check_body_post(&mut self, cx: &LateContext<'tcx>, _: &Body<'tcx>) {
157-
self.lint_if_finish(cx, Default::default());
167+
emit_lints(cx, self.lint_points.take());
158168
}
159169

160170
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
161-
self.lint_if_finish(cx, Default::default());
171+
emit_lints(cx, self.lint_points.take());
162172
}
163173
}
164174

165-
fn emit_lints(cx: &LateContext<'_>, (span, item): &(Span, Option<LintPoint>)) {
166-
let Some((lint, used_mod, replace_with)) = item else {
175+
fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) {
176+
let Some((krate_span, lint_points)) = lint_points else {
167177
return;
168178
};
169179

170-
span_lint_and_sugg(
171-
cx,
172-
lint,
173-
*span,
174-
format!("used import from `{used_mod}` instead of `{replace_with}`"),
175-
format!("consider importing the item from `{replace_with}`"),
176-
(*replace_with).to_string(),
177-
Applicability::MachineApplicable,
178-
);
180+
let mut lint: Option<(&'static Lint, &'static str, &'static str)> = None;
181+
let mut has_conflict = false;
182+
for lint_point in &lint_points {
183+
match lint_point {
184+
LintPoint::Available(_, l, used_mod, replace_with)
185+
if lint.is_none_or(|(prev_l, ..)| l.name == prev_l.name) =>
186+
{
187+
lint = Some((l, used_mod, replace_with));
188+
},
189+
_ => {
190+
has_conflict = true;
191+
break;
192+
},
193+
}
194+
}
195+
196+
if !has_conflict && let Some((lint, used_mod, replace_with)) = lint {
197+
span_lint_and_sugg(
198+
cx,
199+
lint,
200+
krate_span,
201+
format!("used import from `{used_mod}` instead of `{replace_with}`"),
202+
format!("consider importing the item from `{replace_with}`"),
203+
(*replace_with).to_string(),
204+
Applicability::MachineApplicable,
205+
);
206+
return;
207+
}
208+
209+
for lint_point in lint_points {
210+
let LintPoint::Available(span, lint, used_mod, replace_with) = lint_point else {
211+
continue;
212+
};
213+
span_lint_and_help(
214+
cx,
215+
lint,
216+
span,
217+
format!("used import from `{used_mod}` instead of `{replace_with}`"),
218+
None,
219+
format!("consider importing the item from `{replace_with}`"),
220+
);
221+
}
179222
}
180223

181224
/// Returns the first named segment of a [`Path`].

‎tests/ui/std_instead_of_core.fixed

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ extern crate alloc;
88
#[macro_use]
99
extern crate proc_macro_derive;
1010

11-
#[warn(clippy::std_instead_of_core)]
1211
fn std_instead_of_core() {
1312
// Regular import
1413
use core::hash::Hasher;
@@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {}
9089
#[clippy::msrv = "1.77"]
9190
fn msrv_1_77(_: core::net::IpAddr) {}
9291
//~^ std_instead_of_core
93-
94-
#[warn(clippy::std_instead_of_core)]
95-
#[rustfmt::skip]
96-
fn issue14982() {
97-
use std::{collections::HashMap, hash::Hash};
98-
}

‎tests/ui/std_instead_of_core.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ extern crate alloc;
88
#[macro_use]
99
extern crate proc_macro_derive;
1010

11-
#[warn(clippy::std_instead_of_core)]
1211
fn std_instead_of_core() {
1312
// Regular import
1413
use std::hash::Hasher;
@@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {}
9089
#[clippy::msrv = "1.77"]
9190
fn msrv_1_77(_: std::net::IpAddr) {}
9291
//~^ std_instead_of_core
93-
94-
#[warn(clippy::std_instead_of_core)]
95-
#[rustfmt::skip]
96-
fn issue14982() {
97-
use std::{collections::HashMap, hash::Hash};
98-
}

‎tests/ui/std_instead_of_core.stderr

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: used import from `std` instead of `core`
2-
--> tests/ui/std_instead_of_core.rs:14:9
2+
--> tests/ui/std_instead_of_core.rs:13:9
33
|
44
LL | use std::hash::Hasher;
55
| ^^^ help: consider importing the item from `core`: `core`
@@ -8,61 +8,61 @@ LL | use std::hash::Hasher;
88
= help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]`
99

1010
error: used import from `std` instead of `core`
11-
--> tests/ui/std_instead_of_core.rs:17:11
11+
--> tests/ui/std_instead_of_core.rs:16:11
1212
|
1313
LL | use ::std::hash::Hash;
1414
| ^^^ help: consider importing the item from `core`: `core`
1515

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

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

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

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

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

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

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

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

6464
error: used import from `std` instead of `alloc`
65-
--> tests/ui/std_instead_of_core.rs:59:9
65+
--> tests/ui/std_instead_of_core.rs:58:9
6666
|
6767
LL | use std::vec;
6868
| ^^^ help: consider importing the item from `alloc`: `alloc`
@@ -71,13 +71,13 @@ LL | use std::vec;
7171
= help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]`
7272

7373
error: used import from `std` instead of `alloc`
74-
--> tests/ui/std_instead_of_core.rs:61:9
74+
--> tests/ui/std_instead_of_core.rs:60:9
7575
|
7676
LL | use std::vec::Vec;
7777
| ^^^ help: consider importing the item from `alloc`: `alloc`
7878

7979
error: used import from `alloc` instead of `core`
80-
--> tests/ui/std_instead_of_core.rs:67:9
80+
--> tests/ui/std_instead_of_core.rs:66:9
8181
|
8282
LL | use alloc::slice::from_ref;
8383
| ^^^^^ help: consider importing the item from `core`: `core`
@@ -86,13 +86,13 @@ LL | use alloc::slice::from_ref;
8686
= help: to override `-D warnings` add `#[allow(clippy::alloc_instead_of_core)]`
8787

8888
error: used import from `std` instead of `core`
89-
--> tests/ui/std_instead_of_core.rs:82:9
89+
--> tests/ui/std_instead_of_core.rs:81:9
9090
|
9191
LL | std::intrinsics::copy(a, b, 1);
9292
| ^^^ help: consider importing the item from `core`: `core`
9393

9494
error: used import from `std` instead of `core`
95-
--> tests/ui/std_instead_of_core.rs:91:17
95+
--> tests/ui/std_instead_of_core.rs:90:17
9696
|
9797
LL | fn msrv_1_77(_: std::net::IpAddr) {}
9898
| ^^^ help: consider importing the item from `core`: `core`

‎tests/ui/std_instead_of_core_unfixable.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//@no-rustfix
2+
3+
#![warn(clippy::std_instead_of_core)]
4+
#![warn(clippy::std_instead_of_alloc)]
5+
#![allow(unused_imports)]
6+
7+
#[rustfmt::skip]
8+
fn issue14982() {
9+
use std::{collections::HashMap, hash::Hash};
10+
//~^ std_instead_of_core
11+
}
12+
13+
#[rustfmt::skip]
14+
fn issue15143() {
15+
use std::{error::Error, vec::Vec, fs::File};
16+
//~^ std_instead_of_core
17+
//~| std_instead_of_alloc
18+
}

‎tests/ui/std_instead_of_core_unfixable.stderr

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
error: used import from `std` instead of `core`
2+
--> tests/ui/std_instead_of_core_unfixable.rs:9:43
3+
|
4+
LL | use std::{collections::HashMap, hash::Hash};
5+
| ^^^^
6+
|
7+
= help: consider importing the item from `core`
8+
= note: `-D clippy::std-instead-of-core` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]`
10+
11+
error: used import from `std` instead of `core`
12+
--> tests/ui/std_instead_of_core_unfixable.rs:15:22
13+
|
14+
LL | use std::{error::Error, vec::Vec, fs::File};
15+
| ^^^^^
16+
|
17+
= help: consider importing the item from `core`
18+
19+
error: used import from `std` instead of `alloc`
20+
--> tests/ui/std_instead_of_core_unfixable.rs:15:34
21+
|
22+
LL | use std::{error::Error, vec::Vec, fs::File};
23+
| ^^^
24+
|
25+
= help: consider importing the item from `alloc`
26+
= note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
27+
= help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]`
28+
29+
error: aborting due to 3 previous errors
30+

0 commit comments

Comments
 (0)