From 8fc425b67681e4beaf0c34dc802f56c7b97bf658 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 10 Aug 2018 18:58:23 +0100 Subject: [PATCH 1/3] Add an internal lint for FxHashMap/FxHashSet --- clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/utils/internal_lints.rs | 46 +++++++++++++++++++++++- tests/ui/fxhash.rs | 16 +++++++++ tests/ui/fxhash.stderr | 40 +++++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/ui/fxhash.rs create mode 100644 tests/ui/fxhash.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e0ef87938f9d..82f974b724f7 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::new()); 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..2e948ee60778 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 rustc_data_structures::fx::FxHashMap; use crate::utils::{match_qpath, paths, span_lint}; 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 new() -> 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); + cx.span_lint(DEFAULT_HASH_TYPES, ident.span, &msg); + } + } +} 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..dc08ab88baca --- /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}; + | ^^^^^^^ + | + = 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}; + | ^^^^^^^ + +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(); + | ^^^^^^^ + +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(); + | ^^^^^^^ + +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(); + | ^^^^^^^ + +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(); + | ^^^^^^^ + +error: aborting due to 6 previous errors + From 1812707d39a9ec62581509638f438cb48e2af13f Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 13 Aug 2018 08:23:07 +0100 Subject: [PATCH 2/3] Use utils::span_lint_and_sugg in default_hash_types --- clippy_lints/src/utils/internal_lints.rs | 4 ++-- tests/ui/fxhash.stderr | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 2e948ee60778..0da3ad83b847 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -4,7 +4,7 @@ use rustc::hir::*; use rustc::hir; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_data_structures::fx::FxHashMap; -use crate::utils::{match_qpath, paths, span_lint}; +use crate::utils::{match_qpath, paths, span_lint, span_lint_and_sugg}; use syntax::symbol::LocalInternedString; use syntax::ast::{Crate as AstCrate, Ident, ItemKind, Name}; use syntax::codemap::Span; @@ -247,7 +247,7 @@ impl EarlyLintPass for DefaultHashTypes { 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); - cx.span_lint(DEFAULT_HASH_TYPES, ident.span, &msg); + span_lint_and_sugg(cx, DEFAULT_HASH_TYPES, ident.span, &msg, "use", replace.to_owned()); } } } diff --git a/tests/ui/fxhash.stderr b/tests/ui/fxhash.stderr index dc08ab88baca..5e90f84f1db0 100644 --- a/tests/ui/fxhash.stderr +++ b/tests/ui/fxhash.stderr @@ -2,7 +2,7 @@ error: Prefer FxHashMap over HashMap, it has better performance and we don't nee --> $DIR/fxhash.rs:6:24 | 6 | use std::collections::{HashMap, HashSet}; - | ^^^^^^^ + | ^^^^^^^ help: use: `FxHashMap` | = note: `-D default-hash-types` implied by `-D warnings` @@ -10,31 +10,31 @@ error: Prefer FxHashSet over HashSet, it has better performance and we don't nee --> $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 From 22ff5a3ef11202b6c21462d8d7dfbeeae31a12ed Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 14 Aug 2018 09:25:09 +0100 Subject: [PATCH 3/3] Avoid new_without_default_derive in DefaultHashTypes --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/utils/internal_lints.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 82f974b724f7..77c1c5c14992 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -266,7 +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::new()); + 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); diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 0da3ad83b847..260ced14cb43 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -228,7 +228,7 @@ pub struct DefaultHashTypes { } impl DefaultHashTypes { - pub fn new() -> Self { + 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());