From 59fd637ba134396ed41e1e2bb771966da7a721ea Mon Sep 17 00:00:00 2001
From: Andre Bogus <bogusandre@gmail.com>
Date: Fri, 17 Jan 2020 00:18:37 +0100
Subject: [PATCH] Avoid mut_key on types of unknown layout

---
 clippy_lints/src/mut_key.rs | 25 ++++++++++++++-----------
 tests/ui/mut_key.rs         | 22 +++++++++++++++++++++-
 tests/ui/mut_key.stderr     | 14 ++++++++++----
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs
index 8a79786d4fc..d48c4c419ae 100644
--- a/clippy_lints/src/mut_key.rs
+++ b/clippy_lints/src/mut_key.rs
@@ -1,5 +1,5 @@
 use crate::utils::{match_def_path, paths, span_lint, trait_ref_of_method, walk_ptrs_ty};
-use rustc::ty::{Adt, Dynamic, Opaque, Param, RawPtr, Ref, Ty, TypeAndMut};
+use rustc::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut};
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -101,21 +101,24 @@ fn check_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, ty: Ty<'tcx>) {
         if [&paths::HASHMAP, &paths::BTREEMAP, &paths::HASHSET, &paths::BTREESET]
             .iter()
             .any(|path| match_def_path(cx, def.did, &**path))
+            && is_mutable_type(cx, substs.type_at(0), span)
         {
-            let key_type = concrete_type(substs.type_at(0));
-            if let Some(key_type) = key_type {
-                if !key_type.is_freeze(cx.tcx, cx.param_env, span) {
-                    span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
-                }
-            }
+            span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
         }
     }
 }
 
-fn concrete_type(ty: Ty<'_>) -> Option<Ty<'_>> {
+fn is_mutable_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span) -> bool {
     match ty.kind {
-        RawPtr(TypeAndMut { ty: inner_ty, .. }) | Ref(_, inner_ty, _) => concrete_type(inner_ty),
-        Dynamic(..) | Opaque(..) | Param(..) => None,
-        _ => Some(ty),
+        RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => {
+            mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span)
+        },
+        Slice(inner_ty) => is_mutable_type(cx, inner_ty, span),
+        Array(inner_ty, size) => {
+            size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span)
+        },
+        Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)),
+        Adt(..) => cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env, span),
+        _ => false,
     }
 }
diff --git a/tests/ui/mut_key.rs b/tests/ui/mut_key.rs
index 5ec9b05f517..d45cf8278a8 100644
--- a/tests/ui/mut_key.rs
+++ b/tests/ui/mut_key.rs
@@ -1,3 +1,5 @@
+#![allow(clippy::implicit_hasher)]
+
 use std::collections::{HashMap, HashSet};
 use std::hash::{Hash, Hasher};
 use std::sync::atomic::{AtomicUsize, Ordering::Relaxed};
@@ -29,9 +31,27 @@ fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<K
     m.keys().cloned().collect()
 }
 
-fn this_is_ok(m: &mut HashMap<usize, Key>) {}
+fn this_is_ok(_m: &mut HashMap<usize, Key>) {}
+
+#[allow(unused)]
+trait Trait {
+    type AssociatedType;
+
+    fn trait_fn(&self, set: std::collections::HashSet<Self::AssociatedType>);
+}
+
+fn generics_are_ok_too<K>(_m: &mut HashSet<K>) {
+    // nothing to see here, move along
+}
+
+fn tuples<U>(_m: &mut HashMap<((), U), ()>) {}
+
+fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {}
 
 fn main() {
     let _ = should_not_take_this_arg(&mut HashMap::new(), 1);
     this_is_ok(&mut HashMap::new());
+    tuples::<Key>(&mut HashMap::new());
+    tuples::<()>(&mut HashMap::new());
+    tuples_bad::<()>(&mut HashMap::new());
 }
diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr
index ebdbfe99022..5af28f18d3d 100644
--- a/tests/ui/mut_key.stderr
+++ b/tests/ui/mut_key.stderr
@@ -1,5 +1,5 @@
 error: mutable key type
-  --> $DIR/mut_key.rs:27:32
+  --> $DIR/mut_key.rs:29:32
    |
 LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -7,16 +7,22 @@ LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> Hash
    = note: `#[deny(clippy::mutable_key_type)]` on by default
 
 error: mutable key type
-  --> $DIR/mut_key.rs:27:72
+  --> $DIR/mut_key.rs:29:72
    |
 LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
    |                                                                        ^^^^^^^^^^^^
 
 error: mutable key type
-  --> $DIR/mut_key.rs:28:5
+  --> $DIR/mut_key.rs:30:5
    |
 LL |     let _other: HashMap<Key, bool> = HashMap::new();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 3 previous errors
+error: mutable key type
+  --> $DIR/mut_key.rs:49:22
+   |
+LL | fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {}
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors