Skip to content
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

Add an internal lint for FxHashMap/FxHashSet #3026

Merged
merged 3 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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![
Expand Down
48 changes: 46 additions & 2 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 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);
span_lint_and_sugg(cx, DEFAULT_HASH_TYPES, ident.span, &msg, "use", replace.to_owned());
}
}
}
16 changes: 16 additions & 0 deletions tests/ui/fxhash.rs
Original file line number Diff line number Diff line change
@@ -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();
}
40 changes: 40 additions & 0 deletions tests/ui/fxhash.stderr
Original file line number Diff line number Diff line change
@@ -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