diff --git a/clippy_lints/src/implicit_hasher.rs b/clippy_lints/src/implicit_hasher.rs index 344a04e6e7e82..e56f33f8dcfe2 100644 --- a/clippy_lints/src/implicit_hasher.rs +++ b/clippy_lints/src/implicit_hasher.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use std::collections::BTreeMap; -use rustc_errors::Diag; +use rustc_errors::{Applicability, Diag}; use rustc_hir as hir; use rustc_hir::intravisit::{walk_body, walk_expr, walk_inf, walk_ty, Visitor}; use rustc_hir::{Body, Expr, ExprKind, GenericArg, Item, ItemKind, QPath, TyKind}; @@ -13,7 +13,7 @@ use rustc_session::declare_lint_pass; use rustc_span::symbol::sym; use rustc_span::Span; -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{snippet, IntoSpan, SpanRangeExt}; use clippy_utils::ty::is_type_diagnostic_item; @@ -77,33 +77,32 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { &generics_snip[1..generics_snip.len() - 1] }; - multispan_sugg( - diag, - "consider adding a type parameter", - vec![ - ( - generics_suggestion_span, - format!( - "<{generics_snip}{}S: ::std::hash::BuildHasher{}>", - if generics_snip.is_empty() { "" } else { ", " }, - if vis.suggestions.is_empty() { - "" - } else { - // request users to add `Default` bound so that generic constructors can be used - " + Default" - }, - ), - ), - ( - target.span(), - format!("{}<{}, S>", target.type_name(), target.type_arguments(),), + let mut suggestions = vec![ + ( + generics_suggestion_span, + format!( + "<{generics_snip}{}S: ::std::hash::BuildHasher{}>", + if generics_snip.is_empty() { "" } else { ", " }, + if vis.suggestions.is_empty() { + "" + } else { + // request users to add `Default` bound so that generic constructors can be used + " + Default" + }, ), - ], + ), + ( + target.span(), + format!("{}<{}, S>", target.type_name(), target.type_arguments(),), + ), + ]; + suggestions.extend(vis.suggestions); + + diag.multipart_suggestion( + "add a type parameter for `BuildHasher`", + suggestions, + Applicability::MaybeIncorrect, ); - - if !vis.suggestions.is_empty() { - multispan_sugg(diag, "...and use generic constructor", vis.suggestions); - } } if !cx.effective_visibilities.is_exported(item.owner_id.def_id) { diff --git a/tests/ui/crashes/ice-3717.stderr b/tests/ui/crashes/ice-3717.stderr index 01627ff5b94b9..4b4618ee1bbd0 100644 --- a/tests/ui/crashes/ice-3717.stderr +++ b/tests/ui/crashes/ice-3717.stderr @@ -9,14 +9,13 @@ note: the lint level is defined here | LL | #![deny(clippy::implicit_hasher)] | ^^^^^^^^^^^^^^^^^^^^^^^ -help: consider adding a type parameter +help: add a type parameter for `BuildHasher` | -LL | pub fn ice_3717(_: &HashSet) { - | +++++++++++++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~ -help: ...and use generic constructor +LL ~ pub fn ice_3717(_: &HashSet) { +LL | +LL | let _ = [0u8; 0]; +LL ~ let _: HashSet = HashSet::default(); | -LL | let _: HashSet = HashSet::default(); - | ~~~~~~~~~~~~~~~~~~ error: aborting due to 1 previous error diff --git a/tests/ui/implicit_hasher.fixed b/tests/ui/implicit_hasher.fixed new file mode 100644 index 0000000000000..2d6dc0274cf2a --- /dev/null +++ b/tests/ui/implicit_hasher.fixed @@ -0,0 +1,102 @@ +//@aux-build:proc_macros.rs +#![deny(clippy::implicit_hasher)] + +#[macro_use] +extern crate proc_macros; + +use std::cmp::Eq; +use std::collections::{HashMap, HashSet}; +use std::hash::{BuildHasher, Hash}; + +pub trait Foo: Sized { + fn make() -> (Self, Self); +} + +impl Foo for HashMap { + fn make() -> (Self, Self) { + // OK, don't suggest to modify these + let _: HashMap = HashMap::new(); + let _: HashSet = HashSet::new(); + + (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) + } +} +impl Foo for (HashMap,) { + fn make() -> (Self, Self) { + ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),)) + } +} +impl Foo for HashMap { + fn make() -> (Self, Self) { + (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) + } +} + +impl Foo for HashMap { + fn make() -> (Self, Self) { + (HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default())) + } +} +impl Foo for HashMap { + fn make() -> (Self, Self) { + (HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default())) + } +} + +impl Foo for HashSet { + fn make() -> (Self, Self) { + (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) + } +} +impl Foo for HashSet { + fn make() -> (Self, Self) { + (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) + } +} + +impl Foo for HashSet { + fn make() -> (Self, Self) { + (HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default())) + } +} +impl Foo for HashSet { + fn make() -> (Self, Self) { + (HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default())) + } +} + +pub fn map(map: &mut HashMap) {} + +pub fn set(set: &mut HashSet) {} + +#[inline_macros] +pub mod gen { + use super::*; + inline! { + impl Foo for HashMap { + fn make() -> (Self, Self) { + (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) + } + } + + pub fn bar(_map: &mut HashMap, _set: &mut HashSet) {} + } +} + +// When the macro is in a different file, the suggestion spans can't be combined properly +// and should not cause an ICE +// See #2707 +#[macro_use] +#[path = "auxiliary/test_macro.rs"] +pub mod test_macro; +__implicit_hasher_test_macro!(impl for HashMap where V: test_macro::A); + +// #4260 +external! { + pub fn f(input: &HashMap) {} +} + +// #7712 +pub async fn election_vote(_data: HashMap) {} + +fn main() {} diff --git a/tests/ui/implicit_hasher.rs b/tests/ui/implicit_hasher.rs index f7cd541741b11..0a334357bd1b8 100644 --- a/tests/ui/implicit_hasher.rs +++ b/tests/ui/implicit_hasher.rs @@ -1,11 +1,8 @@ //@aux-build:proc_macros.rs -//@no-rustfix #![deny(clippy::implicit_hasher)] -#![allow(unused)] #[macro_use] extern crate proc_macros; -use proc_macros::external; use std::cmp::Eq; use std::collections::{HashMap, HashSet}; @@ -68,9 +65,11 @@ impl Foo for HashSet { } } -pub fn foo(_map: &mut HashMap, _set: &mut HashSet) {} +pub fn map(map: &mut HashMap) {} -#[proc_macros::inline_macros] +pub fn set(set: &mut HashSet) {} + +#[inline_macros] pub mod gen { use super::*; inline! { diff --git a/tests/ui/implicit_hasher.stderr b/tests/ui/implicit_hasher.stderr index a3df8edf389be..48c6ebc209cf3 100644 --- a/tests/ui/implicit_hasher.stderr +++ b/tests/ui/implicit_hasher.stderr @@ -1,40 +1,121 @@ -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:14:1 +error: impl for `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:15:35 + | +LL | impl Foo for HashMap { + | ^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> tests/ui/implicit_hasher.rs:2:9 + | +LL | #![deny(clippy::implicit_hasher)] + | ^^^^^^^^^^^^^^^^^^^^^^^ +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for HashMap { +LL | fn make() -> (Self, Self) { +... +LL | +LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) + | + +error: impl for `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:24:36 + | +LL | impl Foo for (HashMap,) { + | ^^^^^^^^^^^^^ + | +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for (HashMap,) { +LL | fn make() -> (Self, Self) { +LL ~ ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),)) + | + +error: impl for `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:29:19 + | +LL | impl Foo for HashMap { + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for HashMap { +LL | fn make() -> (Self, Self) { +LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) + | + +error: impl for `HashSet` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:46:32 + | +LL | impl Foo for HashSet { + | ^^^^^^^^^^ + | +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for HashSet { +LL | fn make() -> (Self, Self) { +LL ~ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) | -LL | pub trait Foo: Sized { - | ^^^^^^^^^^^^^^^^^^^^^^^ -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:71:1 +error: impl for `HashSet` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:51:19 + | +LL | impl Foo for HashSet { + | ^^^^^^^^^^^^^^^ + | +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for HashSet { +LL | fn make() -> (Self, Self) { +LL ~ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) | -LL | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:74:1 +error: parameter of type `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:68:22 + | +LL | pub fn map(map: &mut HashMap) {} + | ^^^^^^^^^^^^^^^^^ | -LL | pub mod gen { - | ^^^^^^^^^^^ +help: add a type parameter for `BuildHasher` + | +LL | pub fn map(map: &mut HashMap) {} + | +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~~~~ -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:92:1 +error: parameter of type `HashSet` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:70:22 + | +LL | pub fn set(set: &mut HashSet) {} + | ^^^^^^^^^^^^ | -LL | pub mod test_macro; - | ^^^^^^^^^^^^^^^^^^^ +help: add a type parameter for `BuildHasher` + | +LL | pub fn set(set: &mut HashSet) {} + | +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~ -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:96:1 +error: impl for `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:76:43 + | +LL | impl Foo for HashMap { + | ^^^^^^^^^^^^^ | -LL | external! { - | ^^^^^^^^^ + = note: this error originates in the macro `__inline_mac_mod_gen` (in Nightly builds, run with -Z macro-backtrace for more info) +help: add a type parameter for `BuildHasher` + | +LL ~ impl Foo for HashMap { +LL | fn make() -> (Self, Self) { +LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) | - = note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info) -error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]` - --> tests/ui/implicit_hasher.rs:101:1 +error: parameter of type `HashMap` should be generalized over different hashers + --> tests/ui/implicit_hasher.rs:100:35 | LL | pub async fn election_vote(_data: HashMap) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ + | +help: add a type parameter for `BuildHasher` + | +LL | pub async fn election_vote(_data: HashMap) {} + | +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 6 previous errors +error: aborting due to 9 previous errors