diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 9d829007bca..1c02f73cc1a 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -691,7 +691,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } match expr.node { - hir::ExprMethodCall(ref method_call, _, ref args) => { + hir::ExprMethodCall(ref method_call, ref method_span, ref args) => { // Chain calls // GET_UNWRAP needs to be checked before general `UNWRAP` lints if let Some(arglists) = method_chain_args(expr, &["get", "unwrap"]) { @@ -744,7 +744,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { lint_unnecessary_fold(cx, expr, arglists[0]); } - lint_or_fun_call(cx, expr, &method_call.name.as_str(), args); + lint_or_fun_call(cx, expr, method_span, &method_call.name.as_str(), args); let self_ty = cx.tables.expr_ty_adjusted(&args[0]); if args.len() == 1 && method_call.name == "clone" { @@ -845,7 +845,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } /// Checks for the `OR_FUN_CALL` lint. -fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir::Expr]) { +fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, method_span: &Span, name: &str, args: &[hir::Expr]) { /// Check for `unwrap_or(T::new())` or `unwrap_or(T::default())`. fn check_unwrap_or_default( cx: &LateContext, @@ -894,6 +894,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: fn check_general_case( cx: &LateContext, name: &str, + method_span: &Span, fun_span: Span, self_expr: &hir::Expr, arg: &hir::Expr, @@ -913,8 +914,6 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: return; } - let span_replace_word = self_expr.span.with_lo(span.hi()); - // don't lint for constant values let owner_def = cx.tcx.hir.get_parent_did(arg.id); let promotable = cx.tcx.rvalue_promotable_map(owner_def).contains(&arg.hir_id.local_id); @@ -941,13 +940,14 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: (false, false) => format!("|| {}", snippet(cx, arg.span, "..")).into(), (false, true) => snippet(cx, fun_span, ".."), }; + let span_replace_word = method_span.with_hi(span.hi()); span_lint_and_sugg( cx, OR_FUN_CALL, span_replace_word, &format!("use of `{}` followed by a function call", name), "try this", - format!(".{}_{}({})", name, suffix, sugg), + format!("{}_{}({})", name, suffix, sugg), ); } @@ -956,11 +956,11 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: hir::ExprCall(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, fun.span, &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); } }, hir::ExprMethodCall(_, span, ref or_args) => { - check_general_case(cx, name, span, &args[0], &args[1], !or_args.is_empty(), expr.span) + check_general_case(cx, name, method_span, span, &args[0], &args[1], !or_args.is_empty(), expr.span) }, _ => {}, } diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 952a5ee124e..42cf3d3cbc5 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -350,10 +350,10 @@ error: unnecessary structure name repetition | ^^^ help: use the applicable keyword: `Self` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:307:21 + --> $DIR/methods.rs:307:22 | 307 | with_constructor.unwrap_or(make()); - | ^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(make)` + | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)` | = note: `-D or-fun-call` implied by `-D warnings` @@ -364,22 +364,22 @@ error: use of `unwrap_or` followed by a call to `new` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:313:20 + --> $DIR/methods.rs:313:21 | 313 | with_const_args.unwrap_or(Vec::with_capacity(12)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|| Vec::with_capacity(12))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:316:13 + --> $DIR/methods.rs:316:14 | 316 | with_err.unwrap_or(make()); - | ^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|_| make())` + | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:319:18 + --> $DIR/methods.rs:319:19 | 319 | with_err_args.unwrap_or(Vec::with_capacity(12)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|_| Vec::with_capacity(12))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` --> $DIR/methods.rs:322:5 @@ -394,34 +394,34 @@ error: use of `unwrap_or` followed by a call to `default` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:328:13 + --> $DIR/methods.rs:328:14 | 328 | with_vec.unwrap_or(vec![]); - | ^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|| < [ _ ] > :: into_vec ( box [ $ ( $ x ) , * ] ))` + | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| < [ _ ] > :: into_vec ( box [ $ ( $ x ) , * ] ))` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:333:20 + --> $DIR/methods.rs:333:21 | 333 | without_default.unwrap_or(Foo::new()); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(Foo::new)` + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` error: use of `or_insert` followed by a function call - --> $DIR/methods.rs:336:18 + --> $DIR/methods.rs:336:19 | 336 | map.entry(42).or_insert(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.or_insert_with(String::new)` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/methods.rs:339:20 + --> $DIR/methods.rs:339:21 | 339 | btree.entry(42).or_insert(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.or_insert_with(String::new)` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `unwrap_or` followed by a function call - --> $DIR/methods.rs:342:20 + --> $DIR/methods.rs:342:21 | 342 | let _ = stringy.unwrap_or("".to_owned()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|| "".to_owned())` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable --> $DIR/methods.rs:353:23 diff --git a/tests/ui/unwrap_or.stderr b/tests/ui/unwrap_or.stderr index ec5232d65a0..e4704dd0e43 100644 --- a/tests/ui/unwrap_or.stderr +++ b/tests/ui/unwrap_or.stderr @@ -1,18 +1,16 @@ error: use of `unwrap_or` followed by a function call - --> $DIR/unwrap_or.rs:4:46 + --> $DIR/unwrap_or.rs:4:47 | 4 | let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `.unwrap_or_else(|| "Fail".to_string())` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "Fail".to_string())` | = note: `-D or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a function call - --> $DIR/unwrap_or.rs:8:46 + --> $DIR/unwrap_or.rs:9:10 | -8 | let s = Some(String::from("test string")) - | ______________________________________________^ -9 | | .unwrap_or("Fail".to_string()) - | |______________________________________^ help: try this: `.unwrap_or_else(|| "Fail".to_string())` +9 | .unwrap_or("Fail".to_string()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "Fail".to_string())` error: aborting due to 2 previous errors