Merge pull request #2471 from bootandy/fix_span2

Fix: point to correct problem part of code, update test

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,
@@ -939,14 +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,
+            span_replace_word,
             &format!("use of `{}` followed by a function call", name),
             "try this",
-            format!("{}.{}_{}({})", snippet(cx, self_expr.span, "_"), name, suffix, sugg),
+            format!("{}_{}({})", name, suffix, sugg),
         );
     }
 
@@ -955,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)
             },
             _ => {},
         }

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:5
+   --> $DIR/methods.rs:307:22
     |
 307 |     with_constructor.unwrap_or(make());
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_constructor.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:5
+   --> $DIR/methods.rs:313:21
     |
 313 |     with_const_args.unwrap_or(Vec::with_capacity(12));
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_const_args.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:5
+   --> $DIR/methods.rs:316:14
     |
 316 |     with_err.unwrap_or(make());
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_err.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:5
+   --> $DIR/methods.rs:319:19
     |
 319 |     with_err_args.unwrap_or(Vec::with_capacity(12));
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_err_args.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:5
+   --> $DIR/methods.rs:328:14
     |
 328 |     with_vec.unwrap_or(vec![]);
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_vec.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:5
+   --> $DIR/methods.rs:333:21
     |
 333 |     without_default.unwrap_or(Foo::new());
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `without_default.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:5
+   --> $DIR/methods.rs:336:19
     |
 336 |     map.entry(42).or_insert(String::new());
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `map.entry(42).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:5
+   --> $DIR/methods.rs:339:21
     |
 339 |     btree.entry(42).or_insert(String::new());
-    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `btree.entry(42).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:13
+   --> $DIR/methods.rs:342:21
     |
 342 |     let _ = stringy.unwrap_or("".to_owned());
-    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `stringy.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

tests/ui/unwrap_or.rs

@@ -0,0 +1,11 @@
+#![warn(clippy)]
+
+fn main() {
+    let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len();
+}
+
+fn new_lines() {
+    let s = Some(String::from("test string"))
+        .unwrap_or("Fail".to_string())
+        .len();
+}

tests/ui/unwrap_or.stderr

@@ -0,0 +1,16 @@
+error: use of `unwrap_or` followed by a function call
+ --> $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())`
+  |
+  = note: `-D or-fun-call` implied by `-D warnings`
+
+error: use of `unwrap_or` followed by a function call
+ --> $DIR/unwrap_or.rs:9:10
+  |
+9 |         .unwrap_or("Fail".to_string())
+  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "Fail".to_string())`
+
+error: aborting due to 2 previous errors
+