Auto merge of #6267 - camsteffen:or-fun-idx, r=flip1995

Fix or_fun_call for index operator

changelog: Fix or_fun_call for index operator

Fixes #6266
This commit is contained in:
bors 2020-11-08 22:03:27 +00:00
commit 2067a01ff1
5 changed files with 68 additions and 40 deletions

View File

@ -1797,12 +1797,20 @@ fn check_general_case<'tcx>(
cx: &LateContext<'tcx>,
name: &str,
method_span: Span,
fun_span: Span,
self_expr: &hir::Expr<'_>,
arg: &'tcx hir::Expr<'_>,
or_has_args: bool,
span: Span,
// None if lambda is required
fun_span: Option<Span>,
) {
// (path, fn_has_argument, methods, suffix)
static KNOW_TYPES: [(&[&str], bool, &[&str], &str); 4] = [
(&paths::BTREEMAP_ENTRY, false, &["or_insert"], "with"),
(&paths::HASHMAP_ENTRY, false, &["or_insert"], "with"),
(&paths::OPTION, false, &["map_or", "ok_or", "or", "unwrap_or"], "else"),
(&paths::RESULT, true, &["or", "unwrap_or"], "else"),
];
if let hir::ExprKind::MethodCall(ref path, _, ref args, _) = &arg.kind {
if path.ident.as_str() == "len" {
let ty = cx.typeck_results().expr_ty(&args[0]).peel_refs();
@ -1818,16 +1826,8 @@ fn check_general_case<'tcx>(
}
}
// (path, fn_has_argument, methods, suffix)
let know_types: &[(&[_], _, &[_], _)] = &[
(&paths::BTREEMAP_ENTRY, false, &["or_insert"], "with"),
(&paths::HASHMAP_ENTRY, false, &["or_insert"], "with"),
(&paths::OPTION, false, &["map_or", "ok_or", "or", "unwrap_or"], "else"),
(&paths::RESULT, true, &["or", "unwrap_or"], "else"),
];
if_chain! {
if know_types.iter().any(|k| k.2.contains(&name));
if KNOW_TYPES.iter().any(|k| k.2.contains(&name));
if is_lazyness_candidate(cx, arg);
if !contains_return(&arg);
@ -1835,15 +1835,23 @@ fn check_general_case<'tcx>(
let self_ty = cx.typeck_results().expr_ty(self_expr);
if let Some(&(_, fn_has_arguments, poss, suffix)) =
know_types.iter().find(|&&i| match_type(cx, self_ty, i.0));
KNOW_TYPES.iter().find(|&&i| match_type(cx, self_ty, i.0));
if poss.contains(&name);
then {
let sugg: Cow<'_, _> = match (fn_has_arguments, !or_has_args) {
(true, _) => format!("|_| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
(false, false) => format!("|| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
(false, true) => snippet_with_macro_callsite(cx, fun_span, ".."),
let sugg: Cow<'_, str> = {
let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
(false, Some(fun_span)) => (fun_span, false),
_ => (arg.span, true),
};
let snippet = snippet_with_macro_callsite(cx, snippet_span, "..");
if use_lambda {
let l_arg = if fn_has_arguments { "_" } else { "" };
format!("|{}| {}", l_arg, snippet).into()
} else {
snippet
}
};
let span_replace_word = method_span.with_hi(span.hi());
span_lint_and_sugg(
@ -1864,28 +1872,13 @@ fn check_general_case<'tcx>(
hir::ExprKind::Call(ref fun, ref or_args) => {
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) {
check_general_case(
cx,
name,
method_span,
fun.span,
&args[0],
&args[1],
or_has_args,
expr.span,
);
let fun_span = if or_has_args { None } else { Some(fun.span) };
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, fun_span);
}
},
hir::ExprKind::MethodCall(_, span, ref or_args, _) => check_general_case(
cx,
name,
method_span,
span,
&args[0],
&args[1],
!or_args.is_empty(),
expr.span,
),
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
},
_ => {},
}
}

View File

@ -9,7 +9,7 @@
//! - or-fun-call
//! - option-if-let-else
use crate::utils::is_ctor_or_promotable_const_function;
use crate::utils::{is_ctor_or_promotable_const_function, is_type_diagnostic_item, match_type, paths};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit;
@ -96,6 +96,11 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
let call_found = match &expr.kind {
// ignore enum and struct constructors
ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr),
ExprKind::Index(obj, _) => {
let ty = self.cx.typeck_results().expr_ty(obj);
is_type_diagnostic_item(self.cx, ty, sym!(hashmap_type))
|| match_type(self.cx, ty, &paths::BTREEMAP)
},
ExprKind::MethodCall(..) => true,
_ => false,
};

View File

@ -70,6 +70,15 @@ fn or_fun_call() {
let opt = Some(1);
let hello = "Hello";
let _ = opt.ok_or(format!("{} world.", hello));
// index
let map = HashMap::<u64, u64>::new();
let _ = Some(1).unwrap_or_else(|| map[&1]);
let map = BTreeMap::<u64, u64>::new();
let _ = Some(1).unwrap_or_else(|| map[&1]);
// don't lint index vec
let vec = vec![1];
let _ = Some(1).unwrap_or(vec[1]);
}
struct Foo(u8);

View File

@ -70,6 +70,15 @@ fn make<T>() -> T {
let opt = Some(1);
let hello = "Hello";
let _ = opt.ok_or(format!("{} world.", hello));
// index
let map = HashMap::<u64, u64>::new();
let _ = Some(1).unwrap_or(map[&1]);
let map = BTreeMap::<u64, u64>::new();
let _ = Some(1).unwrap_or(map[&1]);
// don't lint index vec
let vec = vec![1];
let _ = Some(1).unwrap_or(vec[1]);
}
struct Foo(u8);

View File

@ -78,17 +78,29 @@ error: use of `unwrap_or` followed by a function call
LL | let _ = stringy.unwrap_or("".to_owned());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`
error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:76:21
|
LL | let _ = Some(1).unwrap_or(map[&1]);
| ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])`
error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:78:21
|
LL | let _ = Some(1).unwrap_or(map[&1]);
| ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])`
error: use of `or` followed by a function call
--> $DIR/or_fun_call.rs:93:35
--> $DIR/or_fun_call.rs:102:35
|
LL | let _ = Some("a".to_string()).or(Some("b".to_string()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))`
error: use of `or` followed by a function call
--> $DIR/or_fun_call.rs:97:10
--> $DIR/or_fun_call.rs:106:10
|
LL | .or(Some(Bar(b, Duration::from_secs(2))));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))`
error: aborting due to 15 previous errors
error: aborting due to 17 previous errors