Skip to content

Commit e5ae1d1

Browse files
JarchoJarcho
committed
Improve [std|alloc]_instead_of_[alloc|core] lints
* Don't call `TyCtxt::crate_name` unless necessary * Don't lint on `use std::env` * Only lint once on `use std::vec`
1 parent 20e4204 commit e5ae1d1

File tree

4 files changed

+78
-51
lines changed

4 files changed

+78
-51
lines changed

‎clippy_lints/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
916916
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
917917
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
918918
store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
919-
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports));
919+
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
920920
// add lints here, do not remove this comment, it's used in `new_lint`
921921
}
922922

‎clippy_lints/src/std_instead_of_core.rs‎

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use rustc_hir::{def::Res, HirId, Path, PathSegment};
3-
use rustc_lint::{LateContext, LateLintPass, Lint};
4-
use rustc_session::{declare_lint_pass, declare_tool_lint};
5-
use rustc_span::{sym, symbol::kw, Symbol};
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::{declare_tool_lint, impl_lint_pass};
5+
use rustc_span::{sym, symbol::kw, Span};
66

77
declare_clippy_lint! {
88
/// ### What it does
@@ -81,39 +81,54 @@ declare_clippy_lint! {
8181
"type is imported from alloc when available in core"
8282
}
8383

84-
declare_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
84+
#[derive(Default)]
85+
pub struct StdReexports {
86+
// Paths which can be either a module or a macro (e.g. `std::env`) will cause this check to happen
87+
// twice. First for the mod, second for the macro. This is used to avoid the lint reporting for the macro
88+
// when the path could be also be used to access the module.
89+
prev_span: Span,
90+
}
91+
impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
8592

8693
impl<'tcx> LateLintPass<'tcx> for StdReexports {
8794
fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
88-
// std_instead_of_core
89-
check_path(cx, path, sym::std, sym::core, STD_INSTEAD_OF_CORE);
90-
// std_instead_of_alloc
91-
check_path(cx, path, sym::std, sym::alloc, STD_INSTEAD_OF_ALLOC);
92-
// alloc_instead_of_core
93-
check_path(cx, path, sym::alloc, sym::core, ALLOC_INSTEAD_OF_CORE);
94-
}
95-
}
96-
97-
fn check_path(cx: &LateContext<'_>, path: &Path<'_>, krate: Symbol, suggested_crate: Symbol, lint: &'static Lint) {
98-
if_chain! {
99-
// check if path resolves to the suggested crate.
100-
if let Res::Def(_, def_id) = path.res;
101-
if suggested_crate == cx.tcx.crate_name(def_id.krate);
102-
103-
// check if the first segment of the path is the crate we want to identify
104-
if let Some(path_root_segment) = get_first_segment(path);
105-
106-
// check if the path matches the crate we want to suggest the other path for.
107-
if krate == path_root_segment.ident.name;
108-
then {
109-
span_lint_and_help(
110-
cx,
111-
lint,
112-
path.span,
113-
&format!("used import from `{}` instead of `{}`", krate, suggested_crate),
114-
None,
115-
&format!("consider importing the item from `{}`", suggested_crate),
116-
);
95+
if let Res::Def(_, def_id) = path.res
96+
&& let Some(first_segment) = get_first_segment(path)
97+
{
98+
let (lint, msg, help) = match first_segment.ident.name {
99+
sym::std => match cx.tcx.crate_name(def_id.krate) {
100+
sym::core => (
101+
STD_INSTEAD_OF_CORE,
102+
"used import from `std` instead of `core`",
103+
"consider importing the item from `core`",
104+
),
105+
sym::alloc => (
106+
STD_INSTEAD_OF_ALLOC,
107+
"used import from `std` instead of `alloc`",
108+
"consider importing the item from `alloc`",
109+
),
110+
_ => {
111+
self.prev_span = path.span;
112+
return;
113+
}
114+
},
115+
sym::alloc => match cx.tcx.crate_name(def_id.krate) {
116+
sym::core => (
117+
ALLOC_INSTEAD_OF_CORE,
118+
"used import from `alloc` instead of `core`",
119+
"consider importing the item from `core`",
120+
),
121+
_ => {
122+
self.prev_span = path.span;
123+
return;
124+
}
125+
}
126+
_ => return,
127+
};
128+
if path.span != self.prev_span {
129+
span_lint_and_help(cx, lint, path.span, msg, None, help);
130+
self.prev_span = path.span;
131+
}
117132
}
118133
}
119134
}
@@ -123,12 +138,10 @@ fn check_path(cx: &LateContext<'_>, path: &Path<'_>, krate: Symbol, suggested_cr
123138
/// If this is a global path (such as `::std::fmt::Debug`), then the segment after [`kw::PathRoot`]
124139
/// is returned.
125140
fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
126-
let segment = path.segments.first()?;
127-
128-
// A global path will have PathRoot as the first segment. In this case, return the segment after.
129-
if segment.ident.name == kw::PathRoot {
130-
path.segments.get(1)
131-
} else {
132-
Some(segment)
141+
match path.segments {
142+
// A global path will have PathRoot as the first segment. In this case, return the segment after.
143+
[x, y, ..] if x.ident.name == kw::PathRoot => Some(y),
144+
[x, ..] => Some(x),
145+
_ => None,
133146
}
134147
}

‎tests/ui/std_instead_of_core.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ fn std_instead_of_core() {
99
use std::hash::Hasher;
1010
// Absolute path
1111
use ::std::hash::Hash;
12+
// Don't lint on `env` macro
13+
use std::env;
1214

1315
// Multiple imports
1416
use std::fmt::{Debug, Result};
@@ -20,10 +22,14 @@ fn std_instead_of_core() {
2022
// Types
2123
let cell = std::cell::Cell::new(8u32);
2224
let cell_absolute = ::std::cell::Cell::new(8u32);
25+
26+
let _ = std::env!("PATH");
2327
}
2428

2529
#[warn(clippy::std_instead_of_alloc)]
2630
fn std_instead_of_alloc() {
31+
// Only lint once.
32+
use std::vec;
2733
use std::vec::Vec;
2834
}
2935

‎tests/ui/std_instead_of_core.stderr‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,70 +16,78 @@ LL | use ::std::hash::Hash;
1616
= help: consider importing the item from `core`
1717

1818
error: used import from `std` instead of `core`
19-
--> $DIR/std_instead_of_core.rs:14:20
19+
--> $DIR/std_instead_of_core.rs:16:20
2020
|
2121
LL | use std::fmt::{Debug, Result};
2222
| ^^^^^
2323
|
2424
= help: consider importing the item from `core`
2525

2626
error: used import from `std` instead of `core`
27-
--> $DIR/std_instead_of_core.rs:14:27
27+
--> $DIR/std_instead_of_core.rs:16:27
2828
|
2929
LL | use std::fmt::{Debug, Result};
3030
| ^^^^^^
3131
|
3232
= help: consider importing the item from `core`
3333

3434
error: used import from `std` instead of `core`
35-
--> $DIR/std_instead_of_core.rs:17:15
35+
--> $DIR/std_instead_of_core.rs:19:15
3636
|
3737
LL | let ptr = std::ptr::null::<u32>();
3838
| ^^^^^^^^^^^^^^^^^^^^^
3939
|
4040
= help: consider importing the item from `core`
4141

4242
error: used import from `std` instead of `core`
43-
--> $DIR/std_instead_of_core.rs:18:19
43+
--> $DIR/std_instead_of_core.rs:20:19
4444
|
4545
LL | let ptr_mut = ::std::ptr::null_mut::<usize>();
4646
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4747
|
4848
= help: consider importing the item from `core`
4949

5050
error: used import from `std` instead of `core`
51-
--> $DIR/std_instead_of_core.rs:21:16
51+
--> $DIR/std_instead_of_core.rs:23:16
5252
|
5353
LL | let cell = std::cell::Cell::new(8u32);
5454
| ^^^^^^^^^^^^^^^
5555
|
5656
= help: consider importing the item from `core`
5757

5858
error: used import from `std` instead of `core`
59-
--> $DIR/std_instead_of_core.rs:22:25
59+
--> $DIR/std_instead_of_core.rs:24:25
6060
|
6161
LL | let cell_absolute = ::std::cell::Cell::new(8u32);
6262
| ^^^^^^^^^^^^^^^^^
6363
|
6464
= help: consider importing the item from `core`
6565

6666
error: used import from `std` instead of `alloc`
67-
--> $DIR/std_instead_of_core.rs:27:9
67+
--> $DIR/std_instead_of_core.rs:32:9
68+
|
69+
LL | use std::vec;
70+
| ^^^^^^^^
71+
|
72+
= note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
73+
= help: consider importing the item from `alloc`
74+
75+
error: used import from `std` instead of `alloc`
76+
--> $DIR/std_instead_of_core.rs:33:9
6877
|
6978
LL | use std::vec::Vec;
7079
| ^^^^^^^^^^^^^
7180
|
72-
= note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
7381
= help: consider importing the item from `alloc`
7482

7583
error: used import from `alloc` instead of `core`
76-
--> $DIR/std_instead_of_core.rs:32:9
84+
--> $DIR/std_instead_of_core.rs:38:9
7785
|
7886
LL | use alloc::slice::from_ref;
7987
| ^^^^^^^^^^^^^^^^^^^^^^
8088
|
8189
= note: `-D clippy::alloc-instead-of-core` implied by `-D warnings`
8290
= help: consider importing the item from `core`
8391

84-
error: aborting due to 10 previous errors
92+
error: aborting due to 11 previous errors
8593

0 commit comments

Comments
 (0)