about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-03 18:22:40 +0000
committerbors <bors@rust-lang.org>2024-03-03 18:22:40 +0000
commit30642113b218f625de8df0e388c720f9a41641be (patch)
tree63a48aa92bbea6f1909b7fbd34e723a4a1a90fbe
parent28e11b31deef5fd0f59a1e289f8f2173ed6e5944 (diff)
parent3735bf93ff62eee7818852a1b31b7d75939c90db (diff)
downloadrust-30642113b218f625de8df0e388c720f9a41641be.tar.gz
rust-30642113b218f625de8df0e388c720f9a41641be.zip
Auto merge of #12406 - MarcusGrass:fix-duplicate-std-instead-of-core, r=Alexendoo
Dedup std_instead_of_core by using first segment span for uniqueness

Relates to #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
-rw-r--r--clippy_lints/src/std_instead_of_core.rs6
-rw-r--r--tests/ui/std_instead_of_core.fixed11
-rw-r--r--tests/ui/std_instead_of_core.rs9
-rw-r--r--tests/ui/std_instead_of_core.stderr36
4 files changed, 43 insertions, 19 deletions
diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs
index 38fd54a0f1e..c0e4f3c368a 100644
--- a/clippy_lints/src/std_instead_of_core.rs
+++ b/clippy_lints/src/std_instead_of_core.rs
@@ -109,7 +109,6 @@ 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;
                     },
                 },
@@ -117,13 +116,12 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
                     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,
@@ -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;
             }
         }
     }
diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed
index 63b30620a5d..0a734a65d29 100644
--- a/tests/ui/std_instead_of_core.fixed
+++ b/tests/ui/std_instead_of_core.fixed
@@ -1,5 +1,4 @@
 //@aux-build:proc_macro_derive.rs
-//@compile-flags: -Zdeduplicate-diagnostics=yes
 
 #![warn(clippy::std_instead_of_core)]
 #![allow(unused_imports)]
@@ -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`
diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs
index 0696d3a18d8..c12c459c7eb 100644
--- a/tests/ui/std_instead_of_core.rs
+++ b/tests/ui/std_instead_of_core.rs
@@ -1,5 +1,4 @@
 //@aux-build:proc_macro_derive.rs
-//@compile-flags: -Zdeduplicate-diagnostics=yes
 
 #![warn(clippy::std_instead_of_core)]
 #![allow(unused_imports)]
@@ -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`
diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr
index 97835908c72..ee42b474a32 100644
--- a/tests/ui/std_instead_of_core.stderr
+++ b/tests/ui/std_instead_of_core.stderr
@@ -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`
@@ -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`
@@ -59,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: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`
@@ -73,5 +85,5 @@ LL |     use alloc::slice::from_ref;
    = 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