|
| 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 | + /// |
| 13 | + /// This lint is concerned with the semantics of `Borrow` and `Hash` for a |
| 14 | + /// type that implements all three of `Hash`, `Borrow<str>` and `Borrow<[u8]>` |
| 15 | + /// as it is impossible to satisfy the semantics of Borrow and `Hash` for |
| 16 | + /// both `Borrow<str>` and `Borrow<[u8]>`. |
| 17 | + /// |
| 18 | + /// ### Why is this bad? |
| 19 | + /// |
| 20 | + /// When providing implementations for `Borrow<T>`, one should consider whether the different |
| 21 | + /// implementations should act as facets or representations of the underlying type. Generic code |
| 22 | + /// typically uses `Borrow<T>` when it relies on the identical behavior of these additional trait |
| 23 | + /// implementations. These traits will likely appear as additional trait bounds. |
| 24 | + /// |
| 25 | + /// In particular `Eq`, `Ord` and `Hash` must be equivalent for borrowed and owned values: |
| 26 | + /// `x.borrow() == y.borrow()` should give the same result as `x == y`. |
| 27 | + /// It follows then that the following equivalence must hold: |
| 28 | + /// `hash(x) == hash((x as Borrow<[u8]>).borrow()) == hash((x as Borrow<str>).borrow())` |
| 29 | + /// |
| 30 | + /// Unfortunately it doesn't hold as `hash("abc") != hash("abc".as_bytes())`. |
| 31 | + /// This happens because the `Hash` impl for str passes an additional `0xFF` byte to |
| 32 | + /// the hasher to avoid collisions. For example, given the tuples `("a", "bc")`, and `("ab", "c")`, |
| 33 | + /// the two tuples would have the same hash value if the `0xFF` byte was not added. |
| 34 | + /// |
| 35 | + /// ### Example |
| 36 | + /// |
| 37 | + /// ``` |
| 38 | + /// use std::borrow::Borrow; |
| 39 | + /// use std::hash::{Hash, Hasher}; |
| 40 | + /// |
| 41 | + /// struct ExampleType { |
| 42 | + /// data: String |
| 43 | + /// } |
| 44 | + /// |
| 45 | + /// impl Hash for ExampleType { |
| 46 | + /// fn hash<H: Hasher>(&self, state: &mut H) { |
| 47 | + /// self.data.hash(state); |
| 48 | + /// } |
| 49 | + /// } |
| 50 | + /// |
| 51 | + /// impl Borrow<str> for ExampleType { |
| 52 | + /// fn borrow(&self) -> &str { |
| 53 | + /// &self.data |
| 54 | + /// } |
| 55 | + /// } |
| 56 | + /// |
| 57 | + /// impl Borrow<[u8]> for ExampleType { |
| 58 | + /// fn borrow(&self) -> &[u8] { |
| 59 | + /// self.data.as_bytes() |
| 60 | + /// } |
| 61 | + /// } |
| 62 | + /// ``` |
| 63 | + /// As a consequence, hashing a `&ExampleType` and hashing the result of the two |
| 64 | + /// borrows will result in different values. |
| 65 | + /// |
| 66 | + #[clippy::version = "1.76.0"] |
| 67 | + pub IMPL_HASH_BORROW_WITH_STR_AND_BYTES, |
| 68 | + correctness, |
| 69 | + "ensures that the semantics of `Borrow` for `Hash` are satisfied when `Borrow<str>` and `Borrow<[u8]>` are implemented" |
| 70 | +} |
| 71 | + |
| 72 | +declare_lint_pass!(ImplHashWithBorrowStrBytes => [IMPL_HASH_BORROW_WITH_STR_AND_BYTES]); |
| 73 | + |
| 74 | +impl LateLintPass<'_> for ImplHashWithBorrowStrBytes { |
| 75 | + /// We are emitting this lint at the Hash impl of a type that implements all |
| 76 | + /// three of `Hash`, `Borrow<str>` and `Borrow<[u8]>`. |
| 77 | + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { |
| 78 | + if let ItemKind::Impl(imp) = item.kind |
| 79 | + && let Some(TraitRef {path: Path {span, res, ..}, ..}) = imp.of_trait |
| 80 | + && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity() |
| 81 | + && let Some(hash_id) = cx.tcx.get_diagnostic_item(sym::Hash) |
| 82 | + && Res::Def(DefKind::Trait, hash_id) == *res |
| 83 | + && let Some(borrow_id) = cx.tcx.get_diagnostic_item(sym::Borrow) |
| 84 | + // since we are in the `Hash` impl, we don't need to check for that. |
| 85 | + // we need only to check for `Borrow<str>` and `Borrow<[u8]>` |
| 86 | + && implements_trait(cx, ty, borrow_id, &[cx.tcx.types.str_.into()]) |
| 87 | + && implements_trait(cx, ty, borrow_id, &[Ty::new_slice(cx.tcx, cx.tcx.types.u8).into()]) |
| 88 | + { |
| 89 | + span_lint_and_then( |
| 90 | + cx, |
| 91 | + IMPL_HASH_BORROW_WITH_STR_AND_BYTES, |
| 92 | + *span, |
| 93 | + "the semantics of `Borrow<T>` around `Hash` can't be satisfied when both `Borrow<str>` and `Borrow<[u8]>` are implemented", |
| 94 | + |diag| { |
| 95 | + diag.note("the `Borrow` semantics require that `Hash` must behave the same for all implementations of Borrow<T>"); |
| 96 | + diag.note( |
| 97 | + "however, the hash implementations of a string (`str`) and the bytes of a string `[u8]` do not behave the same ..." |
| 98 | + ); |
| 99 | + diag.note("... as (`hash(\"abc\") != hash(\"abc\".as_bytes())`"); |
| 100 | + diag.help("consider either removing one of the `Borrow` implementations (`Borrow<str>` or `Borrow<[u8]>`) ..."); |
| 101 | + diag.help("... or not implementing `Hash` for this type"); |
| 102 | + }, |
| 103 | + ); |
| 104 | + } |
| 105 | + } |
| 106 | +} |
0 commit comments