From b0994064b03f1f9f8bdb8f594bdaec9dbaa0788a Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Thu, 29 Oct 2020 16:21:01 -0500 Subject: [PATCH 1/2] Make KNOW_TYPES static --- clippy_lints/src/methods/mod.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index a3fa3bdd36a..5309253531b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1803,6 +1803,14 @@ fn check_general_case<'tcx>( or_has_args: bool, span: 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,7 +1835,7 @@ 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); From 9cab08465b5d5b4bb4f5ff406b2e18ca550f7d93 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Thu, 29 Oct 2020 15:30:20 -0500 Subject: [PATCH 2/2] Fix or_fun_call for index operator --- clippy_lints/src/methods/mod.rs | 45 +++++++++++-------------- clippy_lints/src/utils/eager_or_lazy.rs | 7 +++- tests/ui/or_fun_call.fixed | 9 +++++ tests/ui/or_fun_call.rs | 9 +++++ tests/ui/or_fun_call.stderr | 18 ++++++++-- 5 files changed, 58 insertions(+), 30 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5309253531b..b6fb3d06934 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1797,11 +1797,11 @@ 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, ) { // (path, fn_has_argument, methods, suffix) static KNOW_TYPES: [(&[&str], bool, &[&str], &str); 4] = [ @@ -1840,10 +1840,18 @@ fn check_general_case<'tcx>( 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); + }, _ => {}, } } diff --git a/clippy_lints/src/utils/eager_or_lazy.rs b/clippy_lints/src/utils/eager_or_lazy.rs index 4ceea13df37..8fe5ddee1ca 100644 --- a/clippy_lints/src/utils/eager_or_lazy.rs +++ b/clippy_lints/src/utils/eager_or_lazy.rs @@ -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, }; diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index 2045ffdb5f0..20e5016bc17 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -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::::new(); + let _ = Some(1).unwrap_or_else(|| map[&1]); + let map = BTreeMap::::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); diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index 522f31b72d0..e7192deeebb 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -70,6 +70,15 @@ fn make() -> T { let opt = Some(1); let hello = "Hello"; let _ = opt.ok_or(format!("{} world.", hello)); + + // index + let map = HashMap::::new(); + let _ = Some(1).unwrap_or(map[&1]); + let map = BTreeMap::::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); diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index bc5978b538f..d0c4df0e008 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -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