diff --git a/src/methods.rs b/src/methods.rs index 254b68deb..7be2e0ee1 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -9,7 +9,10 @@ use syntax::codemap::Span; use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method, walk_ptrs_ty_depth, walk_ptrs_ty, get_trait_def_id, implements_trait}; -use utils::{DEFAULT_TRAIT_PATH, OPTION_PATH, RESULT_PATH, STRING_PATH}; +use utils::{ + BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, + RESULT_PATH, STRING_PATH +}; use utils::MethodArgs; use rustc::middle::cstore::CrateStore; @@ -343,19 +346,31 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P]) or_has_args: bool, span: Span ) { + // (path, fn_has_argument, methods) + let know_types : &[(&[_], _, &[_], _)] = &[ + (&BTREEMAP_ENTRY_PATH, false, &["or_insert"], "with"), + (&HASHMAP_ENTRY_PATH, false, &["or_insert"], "with"), + (&OPTION_PATH, false, &["map_or", "ok_or", "or", "unwrap_or"], "else"), + (&RESULT_PATH, true, &["or", "unwrap_or"], "else"), + ]; + let self_ty = cx.tcx.expr_ty(self_expr); - let is_result = if match_type(cx, self_ty, &RESULT_PATH) { - true - } - else if match_type(cx, self_ty, &OPTION_PATH) { - false - } - else { - return; - }; + let (fn_has_arguments, poss, suffix) = + if let Some(&(_, fn_has_arguments, poss, suffix)) = know_types.iter().find(|&&i| { + match_type(cx, self_ty, i.0) + }) { + (fn_has_arguments, poss, suffix) + } + else { + return + }; - let sugg = match (is_result, !or_has_args) { + if !poss.contains(&name) { + return + } + + let sugg = match (fn_has_arguments, !or_has_args) { (true, _) => format!("|_| {}", snippet(cx, arg.span, "..")), (false, false) => format!("|| {}", snippet(cx, arg.span, "..")), (false, true) => format!("{}", snippet(cx, fun.span, "..")), @@ -364,13 +379,14 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P]) span_lint(cx, OR_FUN_CALL, span, &format!("use of `{}` followed by a function call", name)) .span_suggestion(span, "try this", - format!("{}.{}_else({})", + format!("{}.{}_{}({})", snippet(cx, self_expr.span, "_"), name, + suffix, sugg)); } - if args.len() == 2 && ["map_or", "ok_or", "or", "unwrap_or"].contains(&name) { + if args.len() == 2 { if let ExprCall(ref fun, ref or_args) = args[1].node { let or_has_args = !or_args.is_empty(); if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) { diff --git a/src/utils.rs b/src/utils.rs index 65a7bc860..97d0d2ecf 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -20,10 +20,12 @@ pub type MethodArgs = HirVec>; // module DefPaths for certain structs/enums we check for pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"]; +pub const BTREEMAP_ENTRY_PATH: [&'static str; 4] = ["collections", "btree", "map", "Entry"]; pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"]; pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"]; pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"]; pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"]; +pub const HASHMAP_ENTRY_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index f8642cb3e..1e2e881c3 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -4,6 +4,8 @@ #![allow(unused)] #![deny(clippy, clippy_pedantic)] +use std::collections::BTreeMap; +use std::collections::HashMap; use std::ops::Mul; struct T; @@ -238,6 +240,18 @@ fn or_fun_call() { //~^ERROR use of `unwrap_or` //~|HELP try this //~|SUGGESTION without_default.unwrap_or_else(Foo::new); + + let mut map = HashMap::::new(); + map.entry(42).or_insert(String::new()); + //~^ERROR use of `or_insert` followed by a function call + //~|HELP try this + //~|SUGGESTION map.entry(42).or_insert_with(String::new); + + let mut btree = BTreeMap::::new(); + btree.entry(42).or_insert(String::new()); + //~^ERROR use of `or_insert` followed by a function call + //~|HELP try this + //~|SUGGESTION btree.entry(42).or_insert_with(String::new); } fn main() {