Fix std_instead_of_core for ListStem use

clippy_lints/src/std_instead_of_core.rs

@@ -1,14 +1,21 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::is_from_proc_macro;
+use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::def_id::DefId;
-use rustc_hir::{HirId, Path, PathSegment};
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{
+    Arm, Block, Body, Expr, FieldDef, FnDecl, ForeignItem, GenericParam, Generics, HirId, ImplItem, Item, ItemKind,
+    LetStmt, Mod, Pat, Path, PathSegment, PolyTraitRef, Stmt, TraitItem, Ty, UseKind, Variant, VariantData,
+};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::impl_lint_pass;
+use rustc_span::def_id::LocalDefId;
 use rustc_span::symbol::kw;
 use rustc_span::{sym, Span};
+use std::hash::{Hash, Hasher};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -93,50 +100,339 @@ pub struct StdReexports {
     // twice. First for the mod, second for the macro. This is used to avoid the lint reporting for the macro
     // when the path could be also be used to access the module.
     prev_span: Span,
+    // When inside of a UseKind::ListStem the previous check
+    // isn't enough to ensure correct parsing
+    open_use: Option<OpenUseSpan>,
 }
+
 impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
 
+/// Save all members of a `UseKind::ListStem` `use std::fmt::{Debug, Result};`
+/// Lint when the `ListStem` closes. However, since there's no look-ahead (that I know of).
+/// Close whenever something is encountered that's outside of the container `Span`.
+#[derive(Debug)]
+struct OpenUseSpan {
+    container: Span,
+    // Preserve insert order on iteration, so that lints come out in 'source order'
+    members: FxIndexSet<UseSpanMember>,
+}
+
+#[derive(Debug, Copy, Clone)]
+struct UseSpanMember {
+    inner: Span,
+    hir_id: HirId,
+    first_seg_ident_span: Span,
+    lint_data: LintData,
+}
+
+impl PartialEq for UseSpanMember {
+    fn eq(&self, other: &Self) -> bool {
+        self.inner.eq(&other.inner)
+    }
+}
+
+impl Eq for UseSpanMember {}
+
+impl Hash for UseSpanMember {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.inner.hash(state);
+    }
+}
+
+#[derive(Debug, Copy, Clone)]
+enum LintData {
+    CanReplace(ReplaceLintData),
+    NoReplace,
+}
+
+#[derive(Debug, Copy, Clone)]
+struct ReplaceLintData {
+    lint: &'static crate::Lint,
+    used_mod: &'static str,
+    replace_with: &'static str,
+}
+
+impl StdReexports {
+    fn suggest_for_open_use_item_if_after(&mut self, cx: &LateContext<'_>, span: Span) {
+        if let Some(collected_use) = self.open_use.take() {
+            // Still contains other span, throw it back
+            if collected_use.container.contains(span) {
+                self.open_use = Some(collected_use);
+                return;
+            }
+            // Short circuit
+            if collected_use.members.is_empty() {
+                return;
+            }
+            let mut place_holder_unique_check: Option<(Span, ReplaceLintData)> = None;
+            // If true after checking all members, the lint is 'fixable'.
+            // Otherwise, just warn
+            let mut all_same_valid_lint = true;
+            for member in &collected_use.members {
+                match &member.lint_data {
+                    LintData::CanReplace(lint_data) => {
+                        if let Some((_span, prev_lint_data)) = place_holder_unique_check.take() {
+                            if prev_lint_data.lint.name == lint_data.lint.name
+                                && prev_lint_data.used_mod == lint_data.used_mod
+                                && prev_lint_data.replace_with == lint_data.replace_with
+                            {
+                                place_holder_unique_check = Some((member.first_seg_ident_span, *lint_data));
+                            } else {
+                                // Will have to warn for individual entries
+                                all_same_valid_lint = false;
+                                break;
+                            }
+                        } else {
+                            place_holder_unique_check = Some((member.first_seg_ident_span, *lint_data));
+                        }
+                    },
+                    LintData::NoReplace => {
+                        // Will have to warn for individual entries
+                        all_same_valid_lint = false;
+                        break;
+                    },
+                }
+            }
+            // If they can all be replaced with the same thing, just lint and suggest, then
+            // clippy-fix works as well
+            if all_same_valid_lint {
+                if let Some((
+                    first_segment_ident_span,
+                    ReplaceLintData {
+                        lint,
+                        used_mod,
+                        replace_with,
+                    },
+                )) = place_holder_unique_check
+                {
+                    span_lint_and_sugg(
+                        cx,
+                        lint,
+                        first_segment_ident_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,
+                    );
+                }
+            } else {
+                for member in collected_use.members {
+                    if let LintData::CanReplace(ReplaceLintData {
+                        lint,
+                        used_mod,
+                        replace_with,
+                    }) = member.lint_data
+                    {
+                        // Just lint, don't suggest a change
+                        span_lint_hir_and_then(
+                            cx,
+                            lint,
+                            member.hir_id,
+                            member.inner,
+                            format!("used import from `{used_mod}` instead of `{replace_with}`"),
+                            |diag| {
+                                diag.help(format!("consider importing the item from `{replace_with}`"));
+                            },
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for StdReexports {
-    fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
+    fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, hir_id: HirId) {
+        self.suggest_for_open_use_item_if_after(cx, path.span);
         if let Res::Def(_, def_id) = path.res
             && let Some(first_segment) = get_first_segment(path)
             && is_stable(cx, def_id)
             && !in_external_macro(cx.sess(), path.span)
             && !is_from_proc_macro(cx, &first_segment.ident)
         {
-            let (lint, used_mod, replace_with) = match first_segment.ident.name {
+            let lint_data = 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"),
+                    sym::core => LintData::CanReplace(ReplaceLintData {
+                        lint: STD_INSTEAD_OF_CORE,
+                        used_mod: "std",
+                        replace_with: "core",
+                    }),
+                    sym::alloc => LintData::CanReplace(ReplaceLintData {
+                        lint: STD_INSTEAD_OF_ALLOC,
+                        used_mod: "std",
+                        replace_with: "alloc",
+                    }),
                     _ => {
                         self.prev_span = first_segment.ident.span;
-                        return;
+                        LintData::NoReplace
                     },
                 },
                 sym::alloc => {
                     if cx.tcx.crate_name(def_id.krate) == sym::core {
-                        (ALLOC_INSTEAD_OF_CORE, "alloc", "core")
+                        LintData::CanReplace(ReplaceLintData {
+                            lint: ALLOC_INSTEAD_OF_CORE,
+                            used_mod: "alloc",
+                            replace_with: "core",
+                        })
                     } else {
                         self.prev_span = first_segment.ident.span;
-                        return;
+                        LintData::NoReplace
                     }
                 },
                 _ => return,
             };
-            if first_segment.ident.span != self.prev_span {
-                span_lint_and_sugg(
-                    cx,
-                    lint,
-                    first_segment.ident.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,
-                );
-                self.prev_span = first_segment.ident.span;
+            if let Some(in_use) = self.open_use.as_mut() {
+                in_use.members.insert(UseSpanMember {
+                    inner: path.span,
+                    hir_id,
+                    first_seg_ident_span: first_segment.ident.span,
+                    lint_data,
+                });
+                return;
+            }
+            if let LintData::CanReplace(ReplaceLintData {
+                lint,
+                used_mod,
+                replace_with,
+            }) = lint_data
+            {
+                if first_segment.ident.span != self.prev_span {
+                    span_lint_and_sugg(
+                        cx,
+                        lint,
+                        first_segment.ident.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,
+                    );
+                    self.prev_span = first_segment.ident.span;
+                }
             }
         }
     }
+
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, item.span);
+        if matches!(item.kind, ItemKind::Use(_, UseKind::ListStem)) {
+            self.open_use = Some(OpenUseSpan {
+                container: item.span,
+                members: FxIndexSet::default(),
+            });
+        }
+    }
+
+    // Essentially, check every other parsable thing's start (except for attributes),
+    // If it starts, wrap up the last path lint. Lookahead would be nice but I don't
+    // know if that's possible.
+
+    #[inline]
+    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, body.value.span);
+    }
+
+    #[inline]
+    fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, Span::default());
+    }
+
+    #[inline]
+    fn check_mod(&mut self, cx: &LateContext<'tcx>, _: &'tcx Mod<'tcx>, _: HirId) {
+        self.suggest_for_open_use_item_if_after(cx, Span::default());
+    }
+
+    #[inline]
+    fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, f_item: &'tcx ForeignItem<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, f_item.span);
+    }
+
+    #[inline]
+    fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx LetStmt<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, local.span);
+    }
+
+    #[inline]
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, block.span);
+    }
+
+    #[inline]
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, stmt.span);
+    }
+
+    #[inline]
+    fn check_arm(&mut self, cx: &LateContext<'tcx>, arm: &'tcx Arm<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, arm.span);
+    }
+
+    #[inline]
+    fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, pat.span);
+    }
+
+    #[inline]
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, expr.span);
+    }
+
+    #[inline]
+    fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, ty.span);
+    }
+
+    #[inline]
+    fn check_generic_param(&mut self, cx: &LateContext<'tcx>, gp: &'tcx GenericParam<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, gp.span);
+    }
+
+    #[inline]
+    fn check_generics(&mut self, cx: &LateContext<'tcx>, g: &'tcx Generics<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, g.span);
+    }
+
+    #[inline]
+    fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, ptr: &'tcx PolyTraitRef<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, ptr.span);
+    }
+
+    #[inline]
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'tcx>,
+        _: &'tcx FnDecl<'tcx>,
+        _: &'tcx Body<'tcx>,
+        s: Span,
+        _: LocalDefId,
+    ) {
+        self.suggest_for_open_use_item_if_after(cx, s);
+    }
+
+    #[inline]
+    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, ti: &'tcx TraitItem<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, ti.span);
+    }
+
+    #[inline]
+    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, imp: &'tcx ImplItem<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, imp.span);
+    }
+
+    #[inline]
+    fn check_struct_def(&mut self, cx: &LateContext<'tcx>, _: &'tcx VariantData<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, Span::default());
+    }
+
+    #[inline]
+    fn check_field_def(&mut self, cx: &LateContext<'tcx>, fd: &'tcx FieldDef<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, fd.span);
+    }
+
+    #[inline]
+    fn check_variant(&mut self, cx: &LateContext<'tcx>, v: &'tcx Variant<'tcx>) {
+        self.suggest_for_open_use_item_if_after(cx, v.span);
+    }
 }
 
 /// Returns the first named segment of a [`Path`].

tests/ui/std_instead_of_core.rs

@@ -1,4 +1,5 @@
 //@aux-build:proc_macro_derive.rs
+//@no-rustfix
 
 #![warn(clippy::std_instead_of_core)]
 #![allow(unused_imports)]
@@ -31,6 +32,45 @@ fn std_instead_of_core() {
         ptr::read_unaligned,
     };
 
+    // Multiple import, some not in core/alloc, first pos
+    #[rustfmt::skip]
+    use std::{io::Write as _, fmt::Debug as _, fmt::Alignment as _};
+    //~^ ERROR: used import from `std` instead of `core`
+
+    // Second pos
+    #[rustfmt::skip]
+    use std::{fmt::Alignment as _, io::Write as _, fmt::Debug as _};
+    //~^ ERROR: used import from `std` instead of `core`
+
+    // Third pos
+    #[rustfmt::skip]
+    use std::{fmt::Debug as _, fmt::Alignment as _, io::Write as _};
+    //~^ ERROR: used import from `std` instead of `core`
+
+    // Multiple import, multi-line, same procedure as above
+    #[rustfmt::skip]
+    use std::{
+        io::Write as _,
+        fmt::Debug as _,
+        fmt::Alignment as _,
+    };
+
+    // Second pos
+    #[rustfmt::skip]
+    use std::{
+        fmt::Alignment as _,
+        io::Write as _,
+        fmt::Debug as _,
+    };
+
+    // Third pos
+    #[rustfmt::skip]
+    use std::{
+        fmt::Alignment as _,
+        fmt::Debug as _,
+        io::Write as _,
+    };
+
     // Function calls
     let ptr = std::ptr::null::<u32>();
     //~^ ERROR: used import from `std` instead of `core`
@@ -80,3 +120,6 @@ fn main() {
     std_instead_of_alloc();
     alloc_instead_of_core();
 }
+
+use std::fmt::{Debug as _, Result as _};
+//~^ ERROR: used import from `std` instead of `core`

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:14:9
+  --> tests/ui/std_instead_of_core.rs:15:9
    |
 LL |     use std::hash::Hasher;
    |         ^^^ help: consider importing the item from `core`: `core`
@@ -8,55 +8,151 @@ 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:18: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:24: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:29: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:37:31
+   |
+LL |     use std::{io::Write as _, fmt::Debug as _, fmt::Alignment as _};
+   |                               ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:37:48
+   |
+LL |     use std::{io::Write as _, fmt::Debug as _, fmt::Alignment as _};
+   |                                                ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:42:15
+   |
+LL |     use std::{fmt::Alignment as _, io::Write as _, fmt::Debug as _};
+   |               ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:42:52
+   |
+LL |     use std::{fmt::Alignment as _, io::Write as _, fmt::Debug as _};
+   |                                                    ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:47:15
+   |
+LL |     use std::{fmt::Debug as _, fmt::Alignment as _, io::Write as _};
+   |               ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:47:32
+   |
+LL |     use std::{fmt::Debug as _, fmt::Alignment as _, io::Write as _};
+   |                                ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:54:9
+   |
+LL |         fmt::Debug as _,
+   |         ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:55:9
+   |
+LL |         fmt::Alignment as _,
+   |         ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:61:9
+   |
+LL |         fmt::Alignment as _,
+   |         ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:63:9
+   |
+LL |         fmt::Debug as _,
+   |         ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:69:9
+   |
+LL |         fmt::Alignment as _,
+   |         ^^^^^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:70:9
+   |
+LL |         fmt::Debug as _,
+   |         ^^^^^^^^^^
+   |
+   = help: consider importing the item from `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:75: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:77: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:81: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:83: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:52:9
+  --> tests/ui/std_instead_of_core.rs:92: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:99:9
    |
 LL |     use std::vec;
    |         ^^^ help: consider importing the item from `alloc`: `alloc`
@@ -65,19 +161,25 @@ 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:101: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:107: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 12 previous errors
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:124:5
+   |
+LL | use std::fmt::{Debug as _, Result as _};
+   |     ^^^ help: consider importing the item from `core`: `core`
+
+error: aborting due to 25 previous errors