Apply suggestions from code review

Co-authored-by: Eduardo Broto <[email protected]>

Author: korrat

clippy_lints/src/zero_sized_map_values.rs

@@ -1,13 +1,12 @@
 use if_chain::if_chain;
-use rustc_hir::{self as hir, intravisit::FnKind, *};
+use rustc_hir::{self as hir, *};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{Adt, Ty};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::Span;
 use rustc_target::abi::LayoutOf as _;
 use rustc_typeck::hir_ty_to_ty;
 
-use crate::utils::{match_type, paths, span_lint_and_help};
+use crate::utils::{is_type_diagnostic_item, match_type, paths, span_lint_and_help};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for maps with zero-sized value types anywhere in the code.
@@ -41,41 +40,41 @@ declare_clippy_lint! {
 declare_lint_pass!(ZeroSizedMapValues => [ZERO_SIZED_MAP_VALUES]);
 
 impl LateLintPass<'_> for ZeroSizedMapValues {
-    fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
-        let ty = cx.typeck_results().pat_ty(local.pat);
-
-        self.check_ty(cx, ty, local.pat.span);
-    }
-
     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) {
-        let parent_id = cx.tcx.hir().get_parent_item(hir_ty.hir_id);
-        if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(parent_id)) {
-            if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind {
-                return;
-            }
-        }
-
-        // TODO this causes errors in some cases
-        let ty = hir_ty_to_ty(cx.tcx, hir_ty);
-        self.check_ty(cx, ty, hir_ty.span);
-    }
-}
-
-impl ZeroSizedMapValues {
-    fn check_ty<'tcx>(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) {
-        if span.from_expansion() {
-            return;
-        }
-
         if_chain! {
-            if match_type(cx, ty, &paths::HASHMAP) || match_type(cx, ty, &paths::BTREEMAP);
+            if !hir_ty.span.from_expansion();
+            if !in_trait_impl(cx, hir_ty.hir_id);
+            let ty = ty_from_hir_ty(cx, hir_ty);
+            if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) || match_type(cx, ty, &paths::BTREEMAP);
             if let Adt(_, ref substs) = ty.kind();
             let ty = substs.type_at(1);
             if let Ok(layout) = cx.layout_of(ty);
             if layout.is_zst();
             then {
-                span_lint_and_help(cx, ZERO_SIZED_MAP_VALUES, span, "map with zero-sized value type", None, "consider using a set instead");
+                span_lint_and_help(cx, ZERO_SIZED_MAP_VALUES, hir_ty.span, "map with zero-sized value type", None, "consider using a set instead");
             }
         }
     }
 }
+
+fn in_trait_impl(cx: &LateContext<'_>, hir_id: HirId) -> bool {
+    let parent_id = cx.tcx.hir().get_parent_item(hir_id);
+    if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(parent_id)) {
+        if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind {
+            return true;
+        }
+    }
+    false
+}
+
+fn ty_from_hir_ty<'tcx>(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> Ty<'tcx> {
+    cx.maybe_typeck_results()
+        .and_then(|results| {
+            if results.hir_owner == hir_ty.hir_id.owner {
+                results.node_type_opt(hir_ty.hir_id)
+            } else {
+                None
+            }
+        })
+        .unwrap_or_else(|| hir_ty_to_ty(cx.tcx, hir_ty))
+}

tests/ui/zero_sized_map_values.stderr

@@ -80,10 +80,10 @@ LL | fn test(map: HashMap<String, ()>, key: &str) -> HashMap<String, ()> {
    = help: consider using a set instead
 
 error: map with zero-sized value type
-  --> $DIR/zero_sized_map_values.rs:65:9
+  --> $DIR/zero_sized_map_values.rs:65:34
    |
 LL |     let _: HashMap<String, ()> = HashMap::new();
-   |         ^
+   |                                  ^^^^^^^^^^^^
    |
    = help: consider using a set instead
 
@@ -96,10 +96,10 @@ LL |     let _: HashMap<String, ()> = HashMap::new();
    = help: consider using a set instead
 
 error: map with zero-sized value type
-  --> $DIR/zero_sized_map_values.rs:68:9
+  --> $DIR/zero_sized_map_values.rs:68:12
    |
 LL |     let _: HashMap<_, _> = std::iter::empty::<(String, ())>().collect();
-   |         ^
+   |            ^^^^^^^^^^^^^
    |
    = help: consider using a set instead