]> git.lizzy.rs Git - rust.git/commitdiff
Check for full equality in `type_repetition_in_bounds` rather than just equal hashes
authorJason Newcomb <jsnewcomb@pm.me>
Tue, 4 Jan 2022 19:20:27 +0000 (14:20 -0500)
committerJason Newcomb <jsnewcomb@pm.me>
Tue, 4 Jan 2022 19:28:27 +0000 (14:28 -0500)
clippy_lints/src/trait_bounds.rs
clippy_utils/src/hir_utils.rs
tests/ui/type_repetition_in_bounds.rs

index fb4abceac25e2ccab72372e32f4094c1d8742e81..f2848ad3790936672f49ab70d02740b9036f02ec 100644 (file)
@@ -1,11 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::source::{snippet, snippet_with_applicability};
-use clippy_utils::SpanlessHash;
+use clippy_utils::{SpanlessEq, SpanlessHash};
+use core::hash::{Hash, Hasher};
 use if_chain::if_chain;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::Applicability;
-use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, TyKind, WherePredicate};
+use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, Ty, TyKind, WherePredicate};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::Span;
@@ -94,24 +95,40 @@ fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)
 }
 
 impl TraitBounds {
-    fn check_type_repetition(self, cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
+    fn check_type_repetition<'tcx>(self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) {
+        struct SpanlessTy<'cx, 'tcx> {
+            ty: &'tcx Ty<'tcx>,
+            cx: &'cx LateContext<'tcx>,
+        }
+        impl PartialEq for SpanlessTy<'_, '_> {
+            fn eq(&self, other: &Self) -> bool {
+                let mut eq = SpanlessEq::new(self.cx);
+                eq.inter_expr().eq_ty(self.ty, other.ty)
+            }
+        }
+        impl Hash for SpanlessTy<'_, '_> {
+            fn hash<H: Hasher>(&self, h: &mut H) {
+                let mut t = SpanlessHash::new(self.cx);
+                t.hash_ty(self.ty);
+                h.write_u64(t.finish());
+            }
+        }
+        impl Eq for SpanlessTy<'_, '_> {}
+
         if gen.span.from_expansion() {
             return;
         }
-        let hash = |ty| -> u64 {
-            let mut hasher = SpanlessHash::new(cx);
-            hasher.hash_ty(ty);
-            hasher.finish()
-        };
-        let mut map: UnhashMap<u64, Vec<&GenericBound<'_>>> = UnhashMap::default();
+        let mut map: UnhashMap<SpanlessTy<'_, '_>, Vec<&GenericBound<'_>>> = UnhashMap::default();
         let mut applicability = Applicability::MaybeIncorrect;
         for bound in gen.where_clause.predicates {
             if_chain! {
                 if let WherePredicate::BoundPredicate(ref p) = bound;
                 if p.bounds.len() as u64 <= self.max_trait_bounds;
                 if !p.span.from_expansion();
-                let h = hash(p.bounded_ty);
-                if let Some(ref v) = map.insert(h, p.bounds.iter().collect::<Vec<_>>());
+                if let Some(ref v) = map.insert(
+                    SpanlessTy { ty: p.bounded_ty, cx },
+                    p.bounds.iter().collect::<Vec<_>>()
+                );
 
                 then {
                     let mut hint_string = format!(
index ad50759effaf85d2f65d2ee330986631b674abe9..9df296beb60c98734707551a24803a6f43da73ab 100644 (file)
@@ -388,7 +388,7 @@ pub fn eq_path_segment(&mut self, left: &PathSegment<'_>, right: &PathSegment<'_
     }
 
     #[allow(clippy::similar_names)]
-    fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool {
+    pub fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool {
         match (&left.kind, &right.kind) {
             (&TyKind::Slice(l_vec), &TyKind::Slice(r_vec)) => self.eq_ty(l_vec, r_vec),
             (&TyKind::Array(lt, ref ll_id), &TyKind::Array(rt, ref rl_id)) => {
@@ -845,6 +845,8 @@ pub fn hash_pat(&mut self, pat: &Pat<'_>) {
     pub fn hash_path(&mut self, path: &Path<'_>) {
         match path.res {
             // constant hash since equality is dependant on inter-expression context
+            // e.g. The expressions `if let Some(x) = foo() {}` and `if let Some(y) = foo() {}` are considered equal
+            // even though the binding names are different and they have different `HirId`s.
             Res::Local(_) => 1_usize.hash(&mut self.s),
             _ => {
                 for seg in path.segments {
index 766190f209977fcec1a70bd9f5ae9a4ec467c603..fc740ee11d6ab7b815ab274dd283c76b44055da6 100644 (file)
@@ -69,4 +69,14 @@ struct Bar<S>
     }
 }
 
+// Issue #7360
+struct Foo<T, U>
+where
+    T: Clone,
+    U: Clone,
+{
+    t: T,
+    u: U,
+}
+
 fn main() {}