Skip to content

Fix internal default_hash_types lint to use resolved path #86827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 37 additions & 42 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
});
}
}

Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,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());
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_macros/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 12 additions & 5 deletions src/test/ui-fulldeps/internal-lints/default_hash_types.rs
Original file line number Diff line number Diff line change
@@ -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;
}
30 changes: 15 additions & 15 deletions src/test/ui-fulldeps/internal-lints/default_hash_types.stderr
Original file line number Diff line number Diff line change
@@ -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

Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/lint/issue-83477.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 3 additions & 3 deletions src/test/ui/lint/issue-83477.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/implicit_hasher.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down