diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e0ef87938f9d..77c1c5c14992 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -266,6 +266,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>) { reg.register_late_lint_pass(box serde_api::Serde); reg.register_early_lint_pass(box utils::internal_lints::Clippy); reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default()); + reg.register_early_lint_pass(box utils::internal_lints::DefaultHashTypes::default()); reg.register_late_lint_pass(box utils::inspector::Pass); reg.register_late_lint_pass(box utils::author::Pass); reg.register_late_lint_pass(box types::TypePass); @@ -467,6 +468,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>) { reg.register_lint_group("clippy_internal", vec![ utils::internal_lints::CLIPPY_LINTS_INTERNAL, utils::internal_lints::LINT_WITHOUT_LINT_PASS, + utils::internal_lints::DEFAULT_HASH_TYPES, ]); reg.register_lint_group("clippy", vec![ diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 226817c8e51f..260ced14cb43 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -3,9 +3,10 @@ use rustc::{declare_lint, lint_array}; use rustc::hir::*; use rustc::hir; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use crate::utils::{match_qpath, paths, span_lint}; +use rustc_data_structures::fx::FxHashMap; +use crate::utils::{match_qpath, paths, span_lint, span_lint_and_sugg}; use syntax::symbol::LocalInternedString; -use syntax::ast::{Crate as AstCrate, ItemKind, Name}; +use syntax::ast::{Crate as AstCrate, Ident, ItemKind, Name}; use syntax::codemap::Span; use std::collections::{HashMap, HashSet}; @@ -54,6 +55,18 @@ declare_clippy_lint! { } +/// **What it does:** Checks for the presence of the default hash types "HashMap" or "HashSet" +/// and recommends the FxHash* variants. +/// +/// **Why is this bad?** The FxHash variants have better performance +/// and we don't need any collision prevention in clippy. +declare_clippy_lint! { + pub DEFAULT_HASH_TYPES, + internal, + "forbid HashMap and HashSet and suggest the FxHash* variants" +} + + #[derive(Copy, Clone)] pub struct Clippy; @@ -207,3 +220,34 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for LintCollector<'a, 'tcx> { NestedVisitorMap::All(&self.cx.tcx.hir) } } + + + +pub struct DefaultHashTypes { + map: FxHashMap, +} + +impl DefaultHashTypes { + pub fn default() -> Self { + let mut map = FxHashMap::default(); + map.insert("HashMap".to_owned(), "FxHashMap".to_owned()); + map.insert("HashSet".to_owned(), "FxHashSet".to_owned()); + Self { map } + } +} + +impl LintPass for DefaultHashTypes { + fn get_lints(&self) -> LintArray { + lint_array!(DEFAULT_HASH_TYPES) + } +} + +impl EarlyLintPass for DefaultHashTypes { + fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { + let ident_string = ident.to_string(); + if let Some(replace) = self.map.get(&ident_string) { + let msg = format!("Prefer {} over {}, it has better performance and we don't need any collision prevention in clippy", replace, ident_string); + span_lint_and_sugg(cx, DEFAULT_HASH_TYPES, ident.span, &msg, "use", replace.to_owned()); + } + } +} diff --git a/tests/ui/fxhash.rs b/tests/ui/fxhash.rs new file mode 100644 index 000000000000..c6ed9436a51e --- /dev/null +++ b/tests/ui/fxhash.rs @@ -0,0 +1,16 @@ +#![warn(default_hash_types)] +#![feature(rustc_private)] + +extern crate rustc_data_structures; + +use std::collections::{HashMap, HashSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; + +fn main() { + let _map: HashMap = HashMap::default(); + let _set: HashSet = HashSet::default(); + + // test that the lint doesn't also match the Fx variants themselves 😂 + let _fx_map: FxHashMap = FxHashMap::default(); + let _fx_set: FxHashSet = FxHashSet::default(); +} diff --git a/tests/ui/fxhash.stderr b/tests/ui/fxhash.stderr new file mode 100644 index 000000000000..5e90f84f1db0 --- /dev/null +++ b/tests/ui/fxhash.stderr @@ -0,0 +1,40 @@ +error: Prefer FxHashMap over HashMap, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:6:24 + | +6 | use std::collections::{HashMap, HashSet}; + | ^^^^^^^ help: use: `FxHashMap` + | + = note: `-D default-hash-types` implied by `-D warnings` + +error: Prefer FxHashSet over HashSet, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:6:33 + | +6 | use std::collections::{HashMap, HashSet}; + | ^^^^^^^ help: use: `FxHashSet` + +error: Prefer FxHashMap over HashMap, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:10:15 + | +10 | let _map: HashMap = HashMap::default(); + | ^^^^^^^ help: use: `FxHashMap` + +error: Prefer FxHashMap over HashMap, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:10:41 + | +10 | let _map: HashMap = HashMap::default(); + | ^^^^^^^ help: use: `FxHashMap` + +error: Prefer FxHashSet over HashSet, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:11:15 + | +11 | let _set: HashSet = HashSet::default(); + | ^^^^^^^ help: use: `FxHashSet` + +error: Prefer FxHashSet over HashSet, it has better performance and we don't need any collision prevention in clippy + --> $DIR/fxhash.rs:11:33 + | +11 | let _set: HashSet = HashSet::default(); + | ^^^^^^^ help: use: `FxHashSet` + +error: aborting due to 6 previous errors +