Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
40 changes: 28 additions & 12 deletions src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -343,19 +346,31 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>])
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
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
};

if !poss.contains(&name) {
return
}
else {
return;
};

let sugg = match (is_result, !or_has_args) {
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, "..")),
Expand All @@ -364,13 +379,14 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>])
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) {
Expand Down
2 changes: 2 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ pub type MethodArgs = HirVec<P<Expr>>;

// 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"];
Expand Down
14 changes: 14 additions & 0 deletions tests/compile-fail/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#![allow(unused)]
#![deny(clippy, clippy_pedantic)]

use std::collections::BTreeMap;
use std::collections::HashMap;
use std::ops::Mul;

struct T;
Expand Down Expand Up @@ -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::<u64, String>::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::<u64, String>::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() {
Expand Down