Skip to content

Commit

Permalink
Auto merge of rust-lang#12406 - MarcusGrass:fix-duplicate-std-instead…
Browse files Browse the repository at this point in the history
…-of-core, r=Alexendoo

Dedup std_instead_of_core by using first segment span for uniqueness

Relates to rust-lang#12379.

Instead of checking that the paths have an identical span, it checks that the relevant `std` part of the path segment's span is identical. Added a multiline test, because my first implementation was worse and failed that, then I realized that you could grab the span off the first_segment `Ident`.

I did find another bug that isn't addressed by this, and that exists on master as well.

The path:
```Rust
use std::{io::Write, fmt::Display};
```

Will get fixed into:
```Rust
use core::{io::Write, fmt::Display};
```

Which doesn't compile since `io::Write` isn't in `core`, if any of those paths are present in `core` it'll do the replace and cause a miscompilation. Do you think I should file a separate bug for that? Since `rustfmt` default splits those up it isn't that big of a deal.

Rustfmt:
```Rust
// Pre
use std::{io::Write, fmt::Display};
// Post
use std::fmt::Display;
use std::io::Write;
```
---

changelog: [`std_instead_of_core`]: Fix duplicated output on multiple imports
  • Loading branch information
bors committed Mar 3, 2024
2 parents 28e11b3 + 3735bf9 commit 3064211
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 19 deletions.
6 changes: 2 additions & 4 deletions clippy_lints/src/std_instead_of_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,21 +109,19 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
sym::core => (STD_INSTEAD_OF_CORE, "std", "core"),
sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"),
_ => {
self.prev_span = path.span;
return;
},
},
sym::alloc => {
if cx.tcx.crate_name(def_id.krate) == sym::core {
(ALLOC_INSTEAD_OF_CORE, "alloc", "core")
} else {
self.prev_span = path.span;
return;
}
},
_ => return,
};
if path.span != self.prev_span {
if first_segment.ident.span != self.prev_span {
span_lint_and_sugg(
cx,
lint,
Expand All @@ -133,7 +131,7 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
replace_with.to_string(),
Applicability::MachineApplicable,
);
self.prev_span = path.span;
self.prev_span = first_segment.ident.span;
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions tests/ui/std_instead_of_core.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@aux-build:proc_macro_derive.rs
//@compile-flags: -Zdeduplicate-diagnostics=yes

#![warn(clippy::std_instead_of_core)]
#![allow(unused_imports)]
Expand All @@ -18,12 +17,20 @@ fn std_instead_of_core() {
use ::core::hash::Hash;
//~^ ERROR: used import from `std` instead of `core`
// Don't lint on `env` macro
use std::env;
use core::env;

// Multiple imports
use core::fmt::{Debug, Result};
//~^ ERROR: used import from `std` instead of `core`

// Multiple imports multiline
#[rustfmt::skip]
use core::{
//~^ ERROR: used import from `std` instead of `core`
fmt::Write as _,
ptr::read_unaligned,
};

// Function calls
let ptr = core::ptr::null::<u32>();
//~^ ERROR: used import from `std` instead of `core`
Expand Down
9 changes: 8 additions & 1 deletion tests/ui/std_instead_of_core.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@aux-build:proc_macro_derive.rs
//@compile-flags: -Zdeduplicate-diagnostics=yes

#![warn(clippy::std_instead_of_core)]
#![allow(unused_imports)]
Expand All @@ -24,6 +23,14 @@ fn std_instead_of_core() {
use std::fmt::{Debug, Result};
//~^ ERROR: used import from `std` instead of `core`

// Multiple imports multiline
#[rustfmt::skip]
use std::{
//~^ ERROR: used import from `std` instead of `core`
fmt::Write as _,
ptr::read_unaligned,
};

// Function calls
let ptr = std::ptr::null::<u32>();
//~^ ERROR: used import from `std` instead of `core`
Expand Down
36 changes: 24 additions & 12 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:15:9
--> tests/ui/std_instead_of_core.rs:14:9
|
LL | use std::hash::Hasher;
| ^^^ help: consider importing the item from `core`: `core`
Expand All @@ -8,49 +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:18:11
--> tests/ui/std_instead_of_core.rs:17: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:24:9
--> tests/ui/std_instead_of_core.rs:20:9
|
LL | use std::env;
| ^^^ 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
|
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:15
--> tests/ui/std_instead_of_core.rs:28: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
|
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:30:21
--> tests/ui/std_instead_of_core.rs:37: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:34:16
--> tests/ui/std_instead_of_core.rs:41: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:36:27
--> tests/ui/std_instead_of_core.rs:43: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:45:9
--> tests/ui/std_instead_of_core.rs:52: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:52:9
--> tests/ui/std_instead_of_core.rs:59:9
|
LL | use std::vec;
| ^^^ help: consider importing the item from `alloc`: `alloc`
Expand All @@ -59,19 +71,19 @@ 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:54:9
--> tests/ui/std_instead_of_core.rs:61: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:60:9
--> tests/ui/std_instead_of_core.rs:67:9
|
LL | use alloc::slice::from_ref;
| ^^^^^ help: consider importing the item from `core`: `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: aborting due to 11 previous errors
error: aborting due to 13 previous errors

0 comments on commit 3064211

Please sign in to comment.