Skip to content

Commit e3ff20c

Browse files
New Lint [hash_borrow_str_semantics]
Implements a lint to prevent implementation of Hash, Borrow<str> and Borrow<[u8]> as it breaks Borrow<T> "semantics". According to the book, types that implement Borrow<A> and Borrow<B> must ensure equality of borrow results under Eq,Ord and Hash. > In particular Eq, Ord and Hash must be equivalent for borrowed and owned values: x.borrow() == y.borrow() should give the same result as x == y. In the same way, hash(x) == hash(x as Borrow<[u8]>) != hash(x as Borrow<str>). changelog: newlint [`hash_borrow_str_semantics`]
1 parent d487579 commit e3ff20c

6 files changed

+219
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5058,6 +5058,7 @@ Released 2018-09-13
50585058
[`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first
50595059
[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
50605060
[`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
5061+
[`hash_borrow_str_semantics`]: https://rust-lang.github.io/rust-clippy/master/index.html#hash_borrow_str_semantics
50615062
[`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#host_endian_bytes
50625063
[`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
50635064
[`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
199199
crate::functions::TOO_MANY_ARGUMENTS_INFO,
200200
crate::functions::TOO_MANY_LINES_INFO,
201201
crate::future_not_send::FUTURE_NOT_SEND_INFO,
202+
crate::hash_borrow_str_semantics::HASH_BORROW_STR_SEMANTICS_INFO,
202203
crate::if_let_mutex::IF_LET_MUTEX_INFO,
203204
crate::if_not_else::IF_NOT_ELSE_INFO,
204205
crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO,
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::ty::implements_trait;
3+
use rustc_hir::def::{DefKind, Res};
4+
use rustc_hir::{Item, ItemKind, Path, TraitRef};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty::Ty;
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::symbol::sym;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// This lint is concerned with the semantics of Borrow and Hash for a
13+
/// type that implements all three of Hash, Borrow<str> and Borrow<[u8]>
14+
/// as it is impossible to satisfy the semantics of Borrow and Hash for
15+
/// both Borrow<str> and Borrow<[u8]>.
16+
///
17+
/// ### Why is this bad?
18+
///
19+
/// When providing implementations for Borrow<T>, one should consider whether the different
20+
/// implementations should act as facets or representations of the underlying type. Generic code
21+
/// typically uses Borrow<T> when it relies on the identical behavior of these additional trait
22+
/// implementations. These traits will likely appear as additional trait bounds.
23+
///
24+
/// In particular Eq, Ord and Hash must be equivalent for borrowed and owned values:
25+
/// `x.borrow() == y.borrow()` should give the same result as `x == y`.
26+
/// It follows then that the following equivalence must hold:
27+
/// `hash(x) == hash((x as Borrow<[u8]>).borrow()) == hash((x as Borrow<str>).borrow())`
28+
///
29+
/// Unfortunately it doesn't hold as `hash("abc") != hash("abc".as_bytes())`.
30+
///
31+
/// ### Example
32+
///
33+
/// ```
34+
/// use std::borrow::Borrow;
35+
/// use std::hash::{Hash, Hasher};
36+
///
37+
/// struct ExampleType {
38+
/// data: String
39+
/// }
40+
///
41+
/// impl Hash for ExampleType {
42+
/// fn hash<H: Hasher>(&self, state: &mut H) {
43+
/// self.data.hash(state);
44+
/// }
45+
/// }
46+
///
47+
/// impl Borrow<str> for ExampleType {
48+
/// fn borrow(&self) -> &str {
49+
/// &self.data
50+
/// }
51+
/// }
52+
///
53+
/// impl Borrow<[u8]> for ExampleType {
54+
/// fn borrow(&self) -> &[u8] {
55+
/// self.data.as_bytes()
56+
/// }
57+
/// }
58+
/// ```
59+
/// As a consequence, hashing a `&ExampleType` and hashing the result of the two
60+
/// borrows will result in different values.
61+
///
62+
#[clippy::version = "1.75.0"]
63+
pub HASH_BORROW_STR_SEMANTICS,
64+
correctness,
65+
"Ensures that the semantics of Borrow for Hash are satisfied when Borrow<str> and Borrow<[u8]> are implemented"
66+
}
67+
68+
declare_lint_pass!(HashBorrowStrSemantics => [HASH_BORROW_STR_SEMANTICS]);
69+
70+
impl LateLintPass<'_> for HashBorrowStrSemantics {
71+
/// We are emitting this lint at the Hash impl of a type that implements all
72+
/// three of Hash, Borrow<str> and Borrow<[u8]>.
73+
///
74+
/// We check that we are in the Hash impl
75+
/// Then we check that the type implements Borrow<str> and Borrow<[u8]>
76+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
77+
if let ItemKind::Impl(imp) = item.kind
78+
&& let Some(TraitRef {path: Path {span, res, ..}, ..}) = imp.of_trait
79+
&& let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
80+
&& let Some(hash_id) = cx.tcx.get_diagnostic_item(sym::Hash)
81+
&& Res::Def(DefKind::Trait, hash_id) == *res
82+
&& let Some(borrow_id) = cx.tcx.get_diagnostic_item(sym::Borrow)
83+
// since we are in the Hash impl, we don't need to check for that.
84+
// we need only to check for Borrow<str> and Borrow<[u8]>
85+
&& implements_trait(cx, ty, borrow_id, &[cx.tcx.types.str_.into()])
86+
&&implements_trait(cx, ty, borrow_id, &[Ty::new_slice(cx.tcx, cx.tcx.types.u8).into()])
87+
{
88+
span_lint_and_then(
89+
cx,
90+
HASH_BORROW_STR_SEMANTICS,
91+
*span,
92+
"can't satisfy the semantics of `Hash` when both `Borrow<str>` and `Borrow<[u8]>` are implemented",
93+
|diag| {
94+
diag.note("types that implement Hash and Borrow<T> should have equivalent Hash results for borrowed and owned values");
95+
96+
diag.note("this is not the case for Borrow<str> and Borrow<[u8]> as the two types result in different Hash values");
97+
98+
diag.help("consider removing one implementation of Borrow (Borrow<str> or Borrow<[u8]>) or not implementing Hash for this type");
99+
},
100+
);
101+
}
102+
}
103+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ mod from_raw_with_void_ptr;
140140
mod from_str_radix_10;
141141
mod functions;
142142
mod future_not_send;
143+
mod hash_borrow_str_semantics;
143144
mod if_let_mutex;
144145
mod if_not_else;
145146
mod if_then_some_else_none;
@@ -1066,6 +1067,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10661067
});
10671068
store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
10681069
store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
1070+
store.register_late_pass(|_| Box::new(hash_borrow_str_semantics::HashBorrowStrSemantics));
10691071
// add lints here, do not remove this comment, it's used in `new_lint`
10701072
}
10711073

tests/ui/hash_borrow_str_semantics.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
#![warn(clippy::hash_borrow_str_semantics)]
2+
3+
use std::borrow::Borrow;
4+
use std::hash::{Hash, Hasher};
5+
6+
struct ExampleType {
7+
data: String,
8+
}
9+
10+
impl Hash for ExampleType {
11+
fn hash<H: Hasher>(&self, state: &mut H) {
12+
self.data.hash(state);
13+
}
14+
}
15+
16+
impl Borrow<str> for ExampleType {
17+
fn borrow(&self) -> &str {
18+
&self.data
19+
}
20+
}
21+
22+
impl Borrow<[u8]> for ExampleType {
23+
fn borrow(&self) -> &[u8] {
24+
self.data.as_bytes()
25+
}
26+
}
27+
28+
struct ShouldNotRaiseForHash {}
29+
impl Hash for ShouldNotRaiseForHash {
30+
fn hash<H: Hasher>(&self, state: &mut H) {
31+
todo!();
32+
}
33+
}
34+
35+
struct ShouldNotRaiseForBorrow {}
36+
impl Borrow<str> for ShouldNotRaiseForBorrow {
37+
fn borrow(&self) -> &str {
38+
todo!();
39+
}
40+
}
41+
impl Borrow<[u8]> for ShouldNotRaiseForBorrow {
42+
fn borrow(&self) -> &[u8] {
43+
todo!();
44+
}
45+
}
46+
47+
struct ShouldNotRaiseForHashBorrowStr {}
48+
impl Hash for ShouldNotRaiseForHashBorrowStr {
49+
fn hash<H: Hasher>(&self, state: &mut H) {
50+
todo!();
51+
}
52+
}
53+
impl Borrow<str> for ShouldNotRaiseForHashBorrowStr {
54+
fn borrow(&self) -> &str {
55+
todo!();
56+
}
57+
}
58+
59+
struct ShouldNotRaiseForHashBorrowSlice {}
60+
impl Hash for ShouldNotRaiseForHashBorrowSlice {
61+
fn hash<H: Hasher>(&self, state: &mut H) {
62+
todo!();
63+
}
64+
}
65+
66+
impl Borrow<[u8]> for ShouldNotRaiseForHashBorrowSlice {
67+
fn borrow(&self) -> &[u8] {
68+
todo!();
69+
}
70+
}
71+
72+
#[derive(Hash)]
73+
struct Derived {
74+
data: String,
75+
}
76+
77+
impl Borrow<str> for Derived {
78+
fn borrow(&self) -> &str {
79+
self.data.as_str()
80+
}
81+
}
82+
83+
impl Borrow<[u8]> for Derived {
84+
fn borrow(&self) -> &[u8] {
85+
self.data.as_bytes()
86+
}
87+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
error: can't satisfy the semantics of `Hash` when both `Borrow<str>` and `Borrow<[u8]>` are implemented
2+
--> $DIR/hash_borrow_str_semantics.rs:10:6
3+
|
4+
LL | impl Hash for ExampleType {
5+
| ^^^^
6+
|
7+
= note: types that implement Hash and Borrow<T> should have equivalent Hash results for borrowed and owned values
8+
= note: this is not the case for Borrow<str> and Borrow<[u8]> as the two types result in different Hash values
9+
= help: consider removing one implementation of Borrow (Borrow<str> or Borrow<[u8]>) or not implementing Hash for this type
10+
= note: `-D clippy::hash-borrow-str-semantics` implied by `-D warnings`
11+
= help: to override `-D warnings` add `#[allow(clippy::hash_borrow_str_semantics)]`
12+
13+
error: can't satisfy the semantics of `Hash` when both `Borrow<str>` and `Borrow<[u8]>` are implemented
14+
--> $DIR/hash_borrow_str_semantics.rs:72:10
15+
|
16+
LL | #[derive(Hash)]
17+
| ^^^^
18+
|
19+
= note: types that implement Hash and Borrow<T> should have equivalent Hash results for borrowed and owned values
20+
= note: this is not the case for Borrow<str> and Borrow<[u8]> as the two types result in different Hash values
21+
= help: consider removing one implementation of Borrow (Borrow<str> or Borrow<[u8]>) or not implementing Hash for this type
22+
= note: this error originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)
23+
24+
error: aborting due to 2 previous errors
25+

0 commit comments

Comments
 (0)