Skip to content

Commit 3e1c75c

Browse files
borsbors
committed
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.
2 parents ca99e3e + 17ebba7 commit 3e1c75c

File tree

8 files changed

+72
-71
lines changed

8 files changed

+72
-71
lines changed

‎compiler/rustc_lint/src/internal.rs

+37-42
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22
//! Clippy.
33
44
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
5-
use rustc_ast::{ImplKind, Item, ItemKind};
6-
use rustc_data_structures::fx::FxHashMap;
5+
use rustc_ast as ast;
76
use rustc_errors::Applicability;
87
use rustc_hir::def::Res;
9-
use rustc_hir::{GenericArg, HirId, MutTy, Mutability, Path, PathSegment, QPath, Ty, TyKind};
8+
use rustc_hir::{
9+
GenericArg, HirId, Item, ItemKind, MutTy, Mutability, Node, Path, PathSegment, QPath, Ty,
10+
TyKind,
11+
};
1012
use rustc_middle::ty;
11-
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
13+
use rustc_session::{declare_lint_pass, declare_tool_lint};
1214
use rustc_span::hygiene::{ExpnKind, MacroKind};
13-
use rustc_span::symbol::{kw, sym, Ident, Symbol};
15+
use rustc_span::symbol::{kw, sym, Symbol};
1416

1517
declare_tool_lint! {
1618
pub rustc::DEFAULT_HASH_TYPES,
@@ -19,43 +21,35 @@ declare_tool_lint! {
1921
report_in_external_macro: true
2022
}
2123

22-
pub struct DefaultHashTypes {
23-
map: FxHashMap<Symbol, Symbol>,
24-
}
25-
26-
impl DefaultHashTypes {
27-
// we are allowed to use `HashMap` and `HashSet` as identifiers for implementing the lint itself
28-
#[allow(rustc::default_hash_types)]
29-
pub fn new() -> Self {
30-
let mut map = FxHashMap::default();
31-
map.insert(sym::HashMap, sym::FxHashMap);
32-
map.insert(sym::HashSet, sym::FxHashSet);
33-
Self { map }
34-
}
35-
}
36-
37-
impl_lint_pass!(DefaultHashTypes => [DEFAULT_HASH_TYPES]);
24+
declare_lint_pass!(DefaultHashTypes => [DEFAULT_HASH_TYPES]);
3825

39-
impl EarlyLintPass for DefaultHashTypes {
40-
fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) {
41-
if let Some(replace) = self.map.get(&ident.name) {
42-
cx.struct_span_lint(DEFAULT_HASH_TYPES, ident.span, |lint| {
43-
// FIXME: We can avoid a copy here. Would require us to take String instead of &str.
44-
let msg = format!("Prefer {} over {}, it has better performance", replace, ident);
45-
lint.build(&msg)
46-
.span_suggestion(
47-
ident.span,
48-
"use",
49-
replace.to_string(),
50-
Applicability::MaybeIncorrect, // FxHashMap, ... needs another import
51-
)
52-
.note(&format!(
53-
"a `use rustc_data_structures::fx::{}` may be necessary",
54-
replace
55-
))
56-
.emit();
57-
});
26+
impl LateLintPass<'_> for DefaultHashTypes {
27+
fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
28+
let def_id = match path.res {
29+
Res::Def(rustc_hir::def::DefKind::Struct, id) => id,
30+
_ => return,
31+
};
32+
if matches!(cx.tcx.hir().get(hir_id), Node::Item(Item { kind: ItemKind::Use(..), .. })) {
33+
// don't lint imports, only actual usages
34+
return;
5835
}
36+
let replace = if cx.tcx.is_diagnostic_item(sym::hashmap_type, def_id) {
37+
"FxHashMap"
38+
} else if cx.tcx.is_diagnostic_item(sym::hashset_type, def_id) {
39+
"FxHashSet"
40+
} else {
41+
return;
42+
};
43+
cx.struct_span_lint(DEFAULT_HASH_TYPES, path.span, |lint| {
44+
let msg = format!(
45+
"prefer `{}` over `{}`, it has better performance",
46+
replace,
47+
cx.tcx.item_name(def_id)
48+
);
49+
lint.build(&msg)
50+
.note(&format!("a `use rustc_data_structures::fx::{}` may be necessary", replace))
51+
.emit();
52+
});
5953
}
6054
}
6155

@@ -242,8 +236,9 @@ declare_tool_lint! {
242236
declare_lint_pass!(LintPassImpl => [LINT_PASS_IMPL_WITHOUT_MACRO]);
243237

244238
impl EarlyLintPass for LintPassImpl {
245-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
246-
if let ItemKind::Impl(box ImplKind { of_trait: Some(lint_pass), .. }) = &item.kind {
239+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
240+
if let ast::ItemKind::Impl(box ast::ImplKind { of_trait: Some(lint_pass), .. }) = &item.kind
241+
{
247242
if let Some(last) = lint_pass.path.segments.last() {
248243
if last.ident.name == sym::LintPass {
249244
let expn_data = lint_pass.path.span.ctxt().outer_expn_data();

‎compiler/rustc_lint/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -475,10 +475,10 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
475475
}
476476

477477
fn register_internals(store: &mut LintStore) {
478-
store.register_lints(&DefaultHashTypes::get_lints());
479-
store.register_early_pass(|| box DefaultHashTypes::new());
480478
store.register_lints(&LintPassImpl::get_lints());
481479
store.register_early_pass(|| box LintPassImpl);
480+
store.register_lints(&DefaultHashTypes::get_lints());
481+
store.register_late_pass(|| box DefaultHashTypes);
482482
store.register_lints(&ExistingDocKeyword::get_lints());
483483
store.register_late_pass(|| box ExistingDocKeyword);
484484
store.register_lints(&TyTyKind::get_lints());

‎compiler/rustc_macros/src/symbols.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
207207
#keyword_stream
208208
}
209209

210-
#[allow(rustc::default_hash_types)]
210+
#[cfg_attr(bootstrap, allow(rustc::default_hash_types))]
211211
#[allow(non_upper_case_globals)]
212212
#[doc(hidden)]
213213
pub mod sym_generated {

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

+12-5
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,29 @@
11
// compile-flags: -Z unstable-options
22

33
#![feature(rustc_private)]
4+
#![deny(rustc::default_hash_types)]
45

56
extern crate rustc_data_structures;
67

78
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
89
use std::collections::{HashMap, HashSet};
910

10-
#[deny(rustc::default_hash_types)]
11+
mod foo {
12+
pub struct HashMap;
13+
}
14+
1115
fn main() {
1216
let _map: HashMap<String, String> = HashMap::default();
13-
//~^ ERROR Prefer FxHashMap over HashMap, it has better performance
14-
//~^^ ERROR Prefer FxHashMap over HashMap, it has better performance
17+
//~^ ERROR prefer `FxHashMap` over `HashMap`, it has better performance
18+
//~^^ ERROR prefer `FxHashMap` over `HashMap`, it has better performance
1519
let _set: HashSet<String> = HashSet::default();
16-
//~^ ERROR Prefer FxHashSet over HashSet, it has better performance
17-
//~^^ ERROR Prefer FxHashSet over HashSet, it has better performance
20+
//~^ ERROR prefer `FxHashSet` over `HashSet`, it has better performance
21+
//~^^ ERROR prefer `FxHashSet` over `HashSet`, it has better performance
1822

1923
// test that the lint doesn't also match the Fx variants themselves
2024
let _fx_map: FxHashMap<String, String> = FxHashMap::default();
2125
let _fx_set: FxHashSet<String> = FxHashSet::default();
26+
27+
// test another struct of the same name
28+
let _ = foo::HashMap;
2229
}

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

+15-15
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,37 @@
1-
error: Prefer FxHashMap over HashMap, it has better performance
2-
--> $DIR/default_hash_types.rs:12:15
1+
error: prefer `FxHashMap` over `HashMap`, it has better performance
2+
--> $DIR/default_hash_types.rs:16:41
33
|
44
LL | let _map: HashMap<String, String> = HashMap::default();
5-
| ^^^^^^^ help: use: `FxHashMap`
5+
| ^^^^^^^
66
|
77
note: the lint level is defined here
8-
--> $DIR/default_hash_types.rs:10:8
8+
--> $DIR/default_hash_types.rs:4:9
99
|
10-
LL | #[deny(rustc::default_hash_types)]
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
10+
LL | #![deny(rustc::default_hash_types)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
1212
= note: a `use rustc_data_structures::fx::FxHashMap` may be necessary
1313

14-
error: Prefer FxHashMap over HashMap, it has better performance
15-
--> $DIR/default_hash_types.rs:12:41
14+
error: prefer `FxHashMap` over `HashMap`, it has better performance
15+
--> $DIR/default_hash_types.rs:16:15
1616
|
1717
LL | let _map: HashMap<String, String> = HashMap::default();
18-
| ^^^^^^^ help: use: `FxHashMap`
18+
| ^^^^^^^^^^^^^^^^^^^^^^^
1919
|
2020
= note: a `use rustc_data_structures::fx::FxHashMap` may be necessary
2121

22-
error: Prefer FxHashSet over HashSet, it has better performance
23-
--> $DIR/default_hash_types.rs:15:15
22+
error: prefer `FxHashSet` over `HashSet`, it has better performance
23+
--> $DIR/default_hash_types.rs:19:33
2424
|
2525
LL | let _set: HashSet<String> = HashSet::default();
26-
| ^^^^^^^ help: use: `FxHashSet`
26+
| ^^^^^^^
2727
|
2828
= note: a `use rustc_data_structures::fx::FxHashSet` may be necessary
2929

30-
error: Prefer FxHashSet over HashSet, it has better performance
31-
--> $DIR/default_hash_types.rs:15:33
30+
error: prefer `FxHashSet` over `HashSet`, it has better performance
31+
--> $DIR/default_hash_types.rs:19:15
3232
|
3333
LL | let _set: HashSet<String> = HashSet::default();
34-
| ^^^^^^^ help: use: `FxHashSet`
34+
| ^^^^^^^^^^^^^^^
3535
|
3636
= note: a `use rustc_data_structures::fx::FxHashSet` may be necessary
3737

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,5 @@
1212
//~| SUGGESTION rustc::default_hash_types
1313
fn main() {
1414
let _ = std::collections::HashMap::<String, String>::new();
15-
//~^ WARN Prefer FxHashMap over HashMap, it has better performance
16-
//~| HELP use
15+
//~^ WARN prefer `FxHashMap` over `HashMap`, it has better performance
1716
}

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ warning: unknown lint: `rustc::foo::default_hash_types`
1212
LL | #[allow(rustc::foo::default_hash_types)]
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `rustc::default_hash_types`
1414

15-
warning: Prefer FxHashMap over HashMap, it has better performance
16-
--> $DIR/issue-83477.rs:14:31
15+
warning: prefer `FxHashMap` over `HashMap`, it has better performance
16+
--> $DIR/issue-83477.rs:14:13
1717
|
1818
LL | let _ = std::collections::HashMap::<String, String>::new();
19-
| ^^^^^^^ help: use: `FxHashMap`
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2020
|
2121
note: the lint level is defined here
2222
--> $DIR/issue-83477.rs:3:9

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(rustc::default_hash_types)]
1+
#![cfg_attr(bootstrap, allow(rustc::default_hash_types))]
22

33
use std::borrow::Cow;
44
use std::collections::BTreeMap;

0 commit comments

Comments
 (0)