diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 926c56332cc1..39f9789be48c 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/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, } + 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, +} + +#[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(&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`]. diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed deleted file mode 100644 index ec4ae2ea13c5..000000000000 --- a/tests/ui/std_instead_of_core.fixed +++ /dev/null @@ -1,82 +0,0 @@ -//@aux-build:proc_macro_derive.rs - -#![warn(clippy::std_instead_of_core)] -#![allow(unused_imports)] - -extern crate alloc; - -#[macro_use] -extern crate proc_macro_derive; - -#[warn(clippy::std_instead_of_core)] -fn std_instead_of_core() { - // Regular import - use core::hash::Hasher; - //~^ ERROR: used import from `std` instead of `core` - // Absolute path - use ::core::hash::Hash; - //~^ ERROR: used import from `std` instead of `core` - // Don't lint on `env` macro - use std::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::(); - //~^ ERROR: used import from `std` instead of `core` - let ptr_mut = ::core::ptr::null_mut::(); - //~^ ERROR: used import from `std` instead of `core` - - // Types - let cell = core::cell::Cell::new(8u32); - //~^ ERROR: used import from `std` instead of `core` - let cell_absolute = ::core::cell::Cell::new(8u32); - //~^ ERROR: used import from `std` instead of `core` - - let _ = std::env!("PATH"); - - // do not lint until `error_in_core` is stable - use std::error::Error; - - // lint items re-exported from private modules, `core::iter::traits::iterator::Iterator` - use core::iter::Iterator; - //~^ ERROR: used import from `std` instead of `core` -} - -#[warn(clippy::std_instead_of_alloc)] -fn std_instead_of_alloc() { - // Only lint once. - use alloc::vec; - //~^ ERROR: used import from `std` instead of `alloc` - use alloc::vec::Vec; - //~^ ERROR: used import from `std` instead of `alloc` -} - -#[warn(clippy::alloc_instead_of_core)] -fn alloc_instead_of_core() { - use core::slice::from_ref; - //~^ ERROR: used import from `alloc` instead of `core` -} - -mod std_in_proc_macro_derive { - #[warn(clippy::alloc_instead_of_core)] - #[allow(unused)] - #[derive(ImplStructWithStdDisplay)] - struct B {} -} - -fn main() { - std_instead_of_core(); - std_instead_of_alloc(); - alloc_instead_of_core(); -} diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs index c12c459c7eb4..29691f727074 100644 --- a/tests/ui/std_instead_of_core.rs +++ b/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::(); //~^ 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` diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr index 8f920511cc5d..c40b05fd3d49 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: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::(); | ^^^ 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::(); | ^^^ 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,13 +161,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: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` @@ -79,5 +175,11 @@ 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 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