Skip to content

Commit c6f45df

Browse files
borsbors
committed
Auto merge of #13181 - Alexendoo:implicit-hasher-suggestion, r=llogiq
Use a single multipart suggestion for `implicit_hasher` The second commit individually shows the diagnostic change ---- changelog: none
2 parents 8f3cfb4 + 33b16d3 commit c6f45df

File tree

5 files changed

+243
-63
lines changed

5 files changed

+243
-63
lines changed

‎clippy_lints/src/implicit_hasher.rs

+26-27
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::borrow::Cow;
22
use std::collections::BTreeMap;
33

4-
use rustc_errors::Diag;
4+
use rustc_errors::{Applicability, Diag};
55
use rustc_hir as hir;
66
use rustc_hir::intravisit::{walk_body, walk_expr, walk_inf, walk_ty, Visitor};
77
use rustc_hir::{Body, Expr, ExprKind, GenericArg, Item, ItemKind, QPath, TyKind};
@@ -13,7 +13,7 @@ use rustc_session::declare_lint_pass;
1313
use rustc_span::symbol::sym;
1414
use rustc_span::Span;
1515

16-
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
16+
use clippy_utils::diagnostics::span_lint_and_then;
1717
use clippy_utils::source::{snippet, IntoSpan, SpanRangeExt};
1818
use clippy_utils::ty::is_type_diagnostic_item;
1919

@@ -77,33 +77,32 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
7777
&generics_snip[1..generics_snip.len() - 1]
7878
};
7979

80-
multispan_sugg(
81-
diag,
82-
"consider adding a type parameter",
83-
vec![
84-
(
85-
generics_suggestion_span,
86-
format!(
87-
"<{generics_snip}{}S: ::std::hash::BuildHasher{}>",
88-
if generics_snip.is_empty() { "" } else { ", " },
89-
if vis.suggestions.is_empty() {
90-
""
91-
} else {
92-
// request users to add `Default` bound so that generic constructors can be used
93-
" + Default"
94-
},
95-
),
96-
),
97-
(
98-
target.span(),
99-
format!("{}<{}, S>", target.type_name(), target.type_arguments(),),
80+
let mut suggestions = vec![
81+
(
82+
generics_suggestion_span,
83+
format!(
84+
"<{generics_snip}{}S: ::std::hash::BuildHasher{}>",
85+
if generics_snip.is_empty() { "" } else { ", " },
86+
if vis.suggestions.is_empty() {
87+
""
88+
} else {
89+
// request users to add `Default` bound so that generic constructors can be used
90+
" + Default"
91+
},
10092
),
101-
],
93+
),
94+
(
95+
target.span(),
96+
format!("{}<{}, S>", target.type_name(), target.type_arguments(),),
97+
),
98+
];
99+
suggestions.extend(vis.suggestions);
100+
101+
diag.multipart_suggestion(
102+
"add a type parameter for `BuildHasher`",
103+
suggestions,
104+
Applicability::MaybeIncorrect,
102105
);
103-
104-
if !vis.suggestions.is_empty() {
105-
multispan_sugg(diag, "...and use generic constructor", vis.suggestions);
106-
}
107106
}
108107

109108
if !cx.effective_visibilities.is_exported(item.owner_id.def_id) {

‎tests/ui/crashes/ice-3717.stderr

+5-6
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ note: the lint level is defined here
99
|
1010
LL | #![deny(clippy::implicit_hasher)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^^
12-
help: consider adding a type parameter
12+
help: add a type parameter for `BuildHasher`
1313
|
14-
LL | pub fn ice_3717<S: ::std::hash::BuildHasher + Default>(_: &HashSet<usize, S>) {
15-
| +++++++++++++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~
16-
help: ...and use generic constructor
14+
LL ~ pub fn ice_3717<S: ::std::hash::BuildHasher + Default>(_: &HashSet<usize, S>) {
15+
LL |
16+
LL | let _ = [0u8; 0];
17+
LL ~ let _: HashSet<usize> = HashSet::default();
1718
|
18-
LL | let _: HashSet<usize> = HashSet::default();
19-
| ~~~~~~~~~~~~~~~~~~
2019

2120
error: aborting due to 1 previous error
2221

‎tests/ui/implicit_hasher.fixed

+102
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//@aux-build:proc_macros.rs
2+
#![deny(clippy::implicit_hasher)]
3+
4+
#[macro_use]
5+
extern crate proc_macros;
6+
7+
use std::cmp::Eq;
8+
use std::collections::{HashMap, HashSet};
9+
use std::hash::{BuildHasher, Hash};
10+
11+
pub trait Foo<T>: Sized {
12+
fn make() -> (Self, Self);
13+
}
14+
15+
impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashMap<K, V, S> {
16+
fn make() -> (Self, Self) {
17+
// OK, don't suggest to modify these
18+
let _: HashMap<i32, i32> = HashMap::new();
19+
let _: HashSet<i32> = HashSet::new();
20+
21+
(HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
22+
}
23+
}
24+
impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for (HashMap<K, V, S>,) {
25+
fn make() -> (Self, Self) {
26+
((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),))
27+
}
28+
}
29+
impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashMap<String, String, S> {
30+
fn make() -> (Self, Self) {
31+
(HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
32+
}
33+
}
34+
35+
impl<K: Hash + Eq, V, S: BuildHasher + Default> Foo<i32> for HashMap<K, V, S> {
36+
fn make() -> (Self, Self) {
37+
(HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default()))
38+
}
39+
}
40+
impl<S: BuildHasher + Default> Foo<i64> for HashMap<String, String, S> {
41+
fn make() -> (Self, Self) {
42+
(HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default()))
43+
}
44+
}
45+
46+
impl<T: Hash + Eq, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashSet<T, S> {
47+
fn make() -> (Self, Self) {
48+
(HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
49+
}
50+
}
51+
impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashSet<String, S> {
52+
fn make() -> (Self, Self) {
53+
(HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
54+
}
55+
}
56+
57+
impl<T: Hash + Eq, S: BuildHasher + Default> Foo<i32> for HashSet<T, S> {
58+
fn make() -> (Self, Self) {
59+
(HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default()))
60+
}
61+
}
62+
impl<S: BuildHasher + Default> Foo<i64> for HashSet<String, S> {
63+
fn make() -> (Self, Self) {
64+
(HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default()))
65+
}
66+
}
67+
68+
pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
69+
70+
pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
71+
72+
#[inline_macros]
73+
pub mod gen {
74+
use super::*;
75+
inline! {
76+
impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<u8> for HashMap<K, V, S> {
77+
fn make() -> (Self, Self) {
78+
(HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
79+
}
80+
}
81+
82+
pub fn bar(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
83+
}
84+
}
85+
86+
// When the macro is in a different file, the suggestion spans can't be combined properly
87+
// and should not cause an ICE
88+
// See #2707
89+
#[macro_use]
90+
#[path = "auxiliary/test_macro.rs"]
91+
pub mod test_macro;
92+
__implicit_hasher_test_macro!(impl<K, V> for HashMap<K, V> where V: test_macro::A);
93+
94+
// #4260
95+
external! {
96+
pub fn f(input: &HashMap<u32, u32>) {}
97+
}
98+
99+
// #7712
100+
pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
101+
102+
fn main() {}

‎tests/ui/implicit_hasher.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
//@aux-build:proc_macros.rs
2-
//@no-rustfix
32
#![deny(clippy::implicit_hasher)]
4-
#![allow(unused)]
53

64
#[macro_use]
75
extern crate proc_macros;
8-
use proc_macros::external;
96

107
use std::cmp::Eq;
118
use std::collections::{HashMap, HashSet};
@@ -68,9 +65,11 @@ impl<S: BuildHasher + Default> Foo<i64> for HashSet<String, S> {
6865
}
6966
}
7067

71-
pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
68+
pub fn map(map: &mut HashMap<i32, i32>) {}
7269

73-
#[proc_macros::inline_macros]
70+
pub fn set(set: &mut HashSet<i32>) {}
71+
72+
#[inline_macros]
7473
pub mod gen {
7574
use super::*;
7675
inline! {

‎tests/ui/implicit_hasher.stderr

+106-25
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,121 @@
1-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
2-
--> tests/ui/implicit_hasher.rs:14:1
1+
error: impl for `HashMap` should be generalized over different hashers
2+
--> tests/ui/implicit_hasher.rs:15:35
3+
|
4+
LL | impl<K: Hash + Eq, V> Foo<i8> for HashMap<K, V> {
5+
| ^^^^^^^^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> tests/ui/implicit_hasher.rs:2:9
9+
|
10+
LL | #![deny(clippy::implicit_hasher)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^
12+
help: add a type parameter for `BuildHasher`
13+
|
14+
LL ~ impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashMap<K, V, S> {
15+
LL | fn make() -> (Self, Self) {
16+
...
17+
LL |
18+
LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
19+
|
20+
21+
error: impl for `HashMap` should be generalized over different hashers
22+
--> tests/ui/implicit_hasher.rs:24:36
23+
|
24+
LL | impl<K: Hash + Eq, V> Foo<i8> for (HashMap<K, V>,) {
25+
| ^^^^^^^^^^^^^
26+
|
27+
help: add a type parameter for `BuildHasher`
28+
|
29+
LL ~ impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for (HashMap<K, V, S>,) {
30+
LL | fn make() -> (Self, Self) {
31+
LL ~ ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),))
32+
|
33+
34+
error: impl for `HashMap` should be generalized over different hashers
35+
--> tests/ui/implicit_hasher.rs:29:19
36+
|
37+
LL | impl Foo<i16> for HashMap<String, String> {
38+
| ^^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
help: add a type parameter for `BuildHasher`
41+
|
42+
LL ~ impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashMap<String, String, S> {
43+
LL | fn make() -> (Self, Self) {
44+
LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
45+
|
46+
47+
error: impl for `HashSet` should be generalized over different hashers
48+
--> tests/ui/implicit_hasher.rs:46:32
49+
|
50+
LL | impl<T: Hash + Eq> Foo<i8> for HashSet<T> {
51+
| ^^^^^^^^^^
52+
|
53+
help: add a type parameter for `BuildHasher`
54+
|
55+
LL ~ impl<T: Hash + Eq, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashSet<T, S> {
56+
LL | fn make() -> (Self, Self) {
57+
LL ~ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
358
|
4-
LL | pub trait Foo<T>: Sized {
5-
| ^^^^^^^^^^^^^^^^^^^^^^^
659

7-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
8-
--> tests/ui/implicit_hasher.rs:71:1
60+
error: impl for `HashSet` should be generalized over different hashers
61+
--> tests/ui/implicit_hasher.rs:51:19
62+
|
63+
LL | impl Foo<i16> for HashSet<String> {
64+
| ^^^^^^^^^^^^^^^
65+
|
66+
help: add a type parameter for `BuildHasher`
67+
|
68+
LL ~ impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashSet<String, S> {
69+
LL | fn make() -> (Self, Self) {
70+
LL ~ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
971
|
10-
LL | pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1272

13-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
14-
--> tests/ui/implicit_hasher.rs:74:1
73+
error: parameter of type `HashMap` should be generalized over different hashers
74+
--> tests/ui/implicit_hasher.rs:68:22
75+
|
76+
LL | pub fn map(map: &mut HashMap<i32, i32>) {}
77+
| ^^^^^^^^^^^^^^^^^
1578
|
16-
LL | pub mod gen {
17-
| ^^^^^^^^^^^
79+
help: add a type parameter for `BuildHasher`
80+
|
81+
LL | pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
82+
| +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~~~~
1883

19-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
20-
--> tests/ui/implicit_hasher.rs:92:1
84+
error: parameter of type `HashSet` should be generalized over different hashers
85+
--> tests/ui/implicit_hasher.rs:70:22
86+
|
87+
LL | pub fn set(set: &mut HashSet<i32>) {}
88+
| ^^^^^^^^^^^^
2189
|
22-
LL | pub mod test_macro;
23-
| ^^^^^^^^^^^^^^^^^^^
90+
help: add a type parameter for `BuildHasher`
91+
|
92+
LL | pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
93+
| +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~
2494

25-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
26-
--> tests/ui/implicit_hasher.rs:96:1
95+
error: impl for `HashMap` should be generalized over different hashers
96+
--> tests/ui/implicit_hasher.rs:76:43
97+
|
98+
LL | impl<K: Hash + Eq, V> Foo<u8> for HashMap<K, V> {
99+
| ^^^^^^^^^^^^^
27100
|
28-
LL | external! {
29-
| ^^^^^^^^^
101+
= note: this error originates in the macro `__inline_mac_mod_gen` (in Nightly builds, run with -Z macro-backtrace for more info)
102+
help: add a type parameter for `BuildHasher`
103+
|
104+
LL ~ impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<u8> for HashMap<K, V, S> {
105+
LL | fn make() -> (Self, Self) {
106+
LL ~ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
30107
|
31-
= note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
32108

33-
error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
34-
--> tests/ui/implicit_hasher.rs:101:1
109+
error: parameter of type `HashMap` should be generalized over different hashers
110+
--> tests/ui/implicit_hasher.rs:100:35
35111
|
36112
LL | pub async fn election_vote(_data: HashMap<i32, i32>) {}
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
113+
| ^^^^^^^^^^^^^^^^^
114+
|
115+
help: add a type parameter for `BuildHasher`
116+
|
117+
LL | pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
118+
| +++++++++++++++++++++++++++++ ~~~~~~~~~~~~~~~~~~~~
38119

39-
error: aborting due to 6 previous errors
120+
error: aborting due to 9 previous errors
40121

0 commit comments

Comments
 (0)