Auto merge of #86827 - camsteffen:hash-lint-resolved, r=oli-obk

Fix internal `default_hash_types` lint to use resolved path

I run into false positives now and then (mostly in Clippy) when I want to name some util after HashMap.

Author: bors

compiler/rustc_lint/src/internal.rs

@@ -2,15 +2,17 @@
 //! Clippy.
 
 use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
-use rustc_ast::{ImplKind, Item, ItemKind};
-use rustc_data_structures::fx::FxHashMap;
+use rustc_ast as ast;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
-use rustc_hir::{GenericArg, HirId, MutTy, Mutability, Path, PathSegment, QPath, Ty, TyKind};
+use rustc_hir::{
+    GenericArg, HirId, Item, ItemKind, MutTy, Mutability, Node, Path, PathSegment, QPath, Ty,
+    TyKind,
+};
 use rustc_middle::ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::hygiene::{ExpnKind, MacroKind};
-use rustc_span::symbol::{kw, sym, Ident, Symbol};
+use rustc_span::symbol::{kw, sym, Symbol};
 
 declare_tool_lint! {
     pub rustc::DEFAULT_HASH_TYPES,
@@ -19,43 +21,35 @@ declare_tool_lint! {
     report_in_external_macro: true
 }
 
-pub struct DefaultHashTypes {
-    map: FxHashMap<Symbol, Symbol>,
-}
-
-impl DefaultHashTypes {
-    // we are allowed to use `HashMap` and `HashSet` as identifiers for implementing the lint itself
-    #[allow(rustc::default_hash_types)]
-    pub fn new() -> Self {
-        let mut map = FxHashMap::default();
-        map.insert(sym::HashMap, sym::FxHashMap);
-        map.insert(sym::HashSet, sym::FxHashSet);
-        Self { map }
-    }
-}
-
-impl_lint_pass!(DefaultHashTypes => [DEFAULT_HASH_TYPES]);
+declare_lint_pass!(DefaultHashTypes => [DEFAULT_HASH_TYPES]);
 
-impl EarlyLintPass for DefaultHashTypes {
-    fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) {
-        if let Some(replace) = self.map.get(&ident.name) {
-            cx.struct_span_lint(DEFAULT_HASH_TYPES, ident.span, |lint| {
-                // FIXME: We can avoid a copy here. Would require us to take String instead of &str.
-                let msg = format!("Prefer {} over {}, it has better performance", replace, ident);
-                lint.build(&msg)
-                    .span_suggestion(
-                        ident.span,
-                        "use",
-                        replace.to_string(),
-                        Applicability::MaybeIncorrect, // FxHashMap, ... needs another import
-                    )
-                    .note(&format!(
-                        "a `use rustc_data_structures::fx::{}` may be necessary",
-                        replace
-                    ))
-                    .emit();
-            });
+impl LateLintPass<'_> for DefaultHashTypes {
+    fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
+        let def_id = match path.res {
+            Res::Def(rustc_hir::def::DefKind::Struct, id) => id,
+            _ => return,
+        };
+        if matches!(cx.tcx.hir().get(hir_id), Node::Item(Item { kind: ItemKind::Use(..), .. })) {
+            // don't lint imports, only actual usages
+            return;
         }
+        let replace = if cx.tcx.is_diagnostic_item(sym::hashmap_type, def_id) {
+            "FxHashMap"
+        } else if cx.tcx.is_diagnostic_item(sym::hashset_type, def_id) {
+            "FxHashSet"
+        } else {
+            return;
+        };
+        cx.struct_span_lint(DEFAULT_HASH_TYPES, path.span, |lint| {
+            let msg = format!(
+                "prefer `{}` over `{}`, it has better performance",
+                replace,
+                cx.tcx.item_name(def_id)
+            );
+            lint.build(&msg)
+                .note(&format!("a `use rustc_data_structures::fx::{}` may be necessary", replace))
+                .emit();
+        });
     }
 }
 
@@ -242,8 +236,9 @@ declare_tool_lint! {
 declare_lint_pass!(LintPassImpl => [LINT_PASS_IMPL_WITHOUT_MACRO]);
 
 impl EarlyLintPass for LintPassImpl {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
-        if let ItemKind::Impl(box ImplKind { of_trait: Some(lint_pass), .. }) = &item.kind {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
+        if let ast::ItemKind::Impl(box ast::ImplKind { of_trait: Some(lint_pass), .. }) = &item.kind
+        {
             if let Some(last) = lint_pass.path.segments.last() {
                 if last.ident.name == sym::LintPass {
                     let expn_data = lint_pass.path.span.ctxt().outer_expn_data();

compiler/rustc_lint/src/lib.rs

@@ -475,10 +475,10 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
 }
 
 fn register_internals(store: &mut LintStore) {
-    store.register_lints(&DefaultHashTypes::get_lints());
-    store.register_early_pass(|| box DefaultHashTypes::new());
     store.register_lints(&LintPassImpl::get_lints());
     store.register_early_pass(|| box LintPassImpl);
+    store.register_lints(&DefaultHashTypes::get_lints());
+    store.register_late_pass(|| box DefaultHashTypes);
     store.register_lints(&ExistingDocKeyword::get_lints());
     store.register_late_pass(|| box ExistingDocKeyword);
     store.register_lints(&TyTyKind::get_lints());

compiler/rustc_macros/src/symbols.rs

@@ -207,7 +207,7 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
             #keyword_stream
         }
 
-        #[allow(rustc::default_hash_types)]
+        #[cfg_attr(bootstrap, allow(rustc::default_hash_types))]
         #[allow(non_upper_case_globals)]
         #[doc(hidden)]
         pub mod sym_generated {

src/test/ui-fulldeps/internal-lints/default_hash_types.rs

@@ -1,22 +1,29 @@
 // compile-flags: -Z unstable-options
 
 #![feature(rustc_private)]
+#![deny(rustc::default_hash_types)]
 
 extern crate rustc_data_structures;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use std::collections::{HashMap, HashSet};
 
-#[deny(rustc::default_hash_types)]
+mod foo {
+    pub struct HashMap;
+}
+
 fn main() {
     let _map: HashMap<String, String> = HashMap::default();
-    //~^ ERROR Prefer FxHashMap over HashMap, it has better performance
-    //~^^ ERROR Prefer FxHashMap over HashMap, it has better performance
+    //~^ ERROR prefer `FxHashMap` over `HashMap`, it has better performance
+    //~^^ ERROR prefer `FxHashMap` over `HashMap`, it has better performance
     let _set: HashSet<String> = HashSet::default();
-    //~^ ERROR Prefer FxHashSet over HashSet, it has better performance
-    //~^^ ERROR Prefer FxHashSet over HashSet, it has better performance
+    //~^ ERROR prefer `FxHashSet` over `HashSet`, it has better performance
+    //~^^ ERROR prefer `FxHashSet` over `HashSet`, it has better performance
 
     // test that the lint doesn't also match the Fx variants themselves
     let _fx_map: FxHashMap<String, String> = FxHashMap::default();
     let _fx_set: FxHashSet<String> = FxHashSet::default();
+
+    // test another struct of the same name
+    let _ = foo::HashMap;
 }

src/test/ui-fulldeps/internal-lints/default_hash_types.stderr

@@ -1,37 +1,37 @@
-error: Prefer FxHashMap over HashMap, it has better performance
-  --> $DIR/default_hash_types.rs:12:15
+error: prefer `FxHashMap` over `HashMap`, it has better performance
+  --> $DIR/default_hash_types.rs:16:41
    |
 LL |     let _map: HashMap<String, String> = HashMap::default();
-   |               ^^^^^^^ help: use: `FxHashMap`
+   |                                         ^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/default_hash_types.rs:10:8
+  --> $DIR/default_hash_types.rs:4:9
    |
-LL | #[deny(rustc::default_hash_types)]
-   |        ^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #![deny(rustc::default_hash_types)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: a `use rustc_data_structures::fx::FxHashMap` may be necessary
 
-error: Prefer FxHashMap over HashMap, it has better performance
-  --> $DIR/default_hash_types.rs:12:41
+error: prefer `FxHashMap` over `HashMap`, it has better performance
+  --> $DIR/default_hash_types.rs:16:15
    |
 LL |     let _map: HashMap<String, String> = HashMap::default();
-   |                                         ^^^^^^^ help: use: `FxHashMap`
+   |               ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: a `use rustc_data_structures::fx::FxHashMap` may be necessary
 
-error: Prefer FxHashSet over HashSet, it has better performance
-  --> $DIR/default_hash_types.rs:15:15
+error: prefer `FxHashSet` over `HashSet`, it has better performance
+  --> $DIR/default_hash_types.rs:19:33
    |
 LL |     let _set: HashSet<String> = HashSet::default();
-   |               ^^^^^^^ help: use: `FxHashSet`
+   |                                 ^^^^^^^
    |
    = note: a `use rustc_data_structures::fx::FxHashSet` may be necessary
 
-error: Prefer FxHashSet over HashSet, it has better performance
-  --> $DIR/default_hash_types.rs:15:33
+error: prefer `FxHashSet` over `HashSet`, it has better performance
+  --> $DIR/default_hash_types.rs:19:15
    |
 LL |     let _set: HashSet<String> = HashSet::default();
-   |                                 ^^^^^^^ help: use: `FxHashSet`
+   |               ^^^^^^^^^^^^^^^
    |
    = note: a `use rustc_data_structures::fx::FxHashSet` may be necessary
 

src/test/ui/lint/issue-83477.rs

@@ -12,6 +12,5 @@
 //~| SUGGESTION rustc::default_hash_types
 fn main() {
     let _ = std::collections::HashMap::<String, String>::new();
-    //~^ WARN Prefer FxHashMap over HashMap, it has better performance
-    //~| HELP use
+    //~^ WARN prefer `FxHashMap` over `HashMap`, it has better performance
 }

src/test/ui/lint/issue-83477.stderr

@@ -12,11 +12,11 @@ warning: unknown lint: `rustc::foo::default_hash_types`
 LL | #[allow(rustc::foo::default_hash_types)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `rustc::default_hash_types`
 
-warning: Prefer FxHashMap over HashMap, it has better performance
-  --> $DIR/issue-83477.rs:14:31
+warning: prefer `FxHashMap` over `HashMap`, it has better performance
+  --> $DIR/issue-83477.rs:14:13
    |
 LL |     let _ = std::collections::HashMap::<String, String>::new();
-   |                               ^^^^^^^ help: use: `FxHashMap`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/issue-83477.rs:3:9

src/tools/clippy/clippy_lints/src/implicit_hasher.rs

@@ -1,4 +1,4 @@
-#![allow(rustc::default_hash_types)]
+#![cfg_attr(bootstrap, allow(rustc::default_hash_types))]
 
 use std::borrow::Cow;
 use std::collections::BTreeMap;