Fix or_fun_call for index operator

clippy_lints/src/methods/mod.rs

@@ -1797,12 +1797,20 @@ fn lint_or_fun_call<'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,32 +1826,32 @@ fn lint_or_fun_call<'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);
 
  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 lint_or_fun_call<'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);
+ },
  _ => {},
  }
  }

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 identify_some_potentially_expensive_patterns<'tcx>(cx: &LateContext<'tcx>, ex
  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,
  };

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::<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);

tests/ui/or_fun_call.rs

@@ -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(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);

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