Merge pull request #3026 from dwijnand/fxhash

Add an internal lint for FxHashMap/FxHashSet

Author: flip1995

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![

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<String, String>,
+}
+
+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());
+        }
+    }
+}

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<String, String> = HashMap::default();
+    let _set: HashSet<String> = HashSet::default();
+
+    // 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();
+}

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<String, String> = 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<String, String> = 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<String> = 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<String> = HashSet::default();
+   |                                 ^^^^^^^ help: use: `FxHashSet`
+
+error: aborting due to 6 previous errors
+