Merge pull request #3127 from mikerite/fix-2937

Fix 2937

3 files changed, 59 insertions, 36 deletions

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index a5102824b1c..7c15eb677cc 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1069,12 +1069,19 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa
 /// Checks for the `EXPECT_FUN_CALL` lint.
 fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) {
     fn extract_format_args(arg: &hir::Expr) -> Option<&hir::HirVec<hir::Expr>> {
-        if let hir::ExprKind::AddrOf(_, ref addr_of) = arg.node {
-            if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = addr_of.node {
-                if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1 {
-                    if let hir::ExprKind::Call(_, ref format_args) = inner_args[0].node {
-                        return Some(format_args);
-                    }
+        let arg  = match &arg.node {
+            hir::ExprKind::AddrOf(_, expr)=> expr,
+            hir::ExprKind::MethodCall(method_name, _, args)
+                if method_name.ident.name == "as_str" ||
+                   method_name.ident.name == "as_ref"
+                => &args[0],
+            _ => arg,
+        };
+
+        if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = arg.node {
+            if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1 {
+                if let hir::ExprKind::Call(_, ref format_args) = inner_args[0].node {
+                    return Some(format_args);
                 }
             }
         }
@@ -1111,7 +1118,8 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
                 | hir::ExprKind::MethodCall(..)
                 // These variants are debatable or require further examination
                 | hir::ExprKind::If(..)
-                | hir::ExprKind::Match(..) => true,
+                | hir::ExprKind::Match(..)
+                | hir::ExprKind::Block{ .. } => true,
                 _ => false,
             }
         }
@@ -1165,7 +1173,7 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
             span_replace_word,
             &format!("use of `{}` followed by a function call", name),
             "try this",
-            format!("unwrap_or_else({} panic!({}))", closure, sugg),
+            format!("unwrap_or_else({} {{ let msg = {}; panic!(msg) }}))", closure, sugg),
         );
     }
 
diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs
index 5bf52c740fe..e247a3d6450 100644
--- a/tests/ui/methods.rs
+++ b/tests/ui/methods.rs
@@ -14,7 +14,7 @@
 #![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)]
 #![allow(clippy::blacklisted_name, unused, clippy::print_stdout, clippy::non_ascii_literal, clippy::new_without_default,
     clippy::new_without_default_derive, clippy::missing_docs_in_private_items, clippy::needless_pass_by_value,
-    clippy::default_trait_access, clippy::use_self)]
+    clippy::default_trait_access, clippy::use_self, clippy::useless_format)]
 
 use std::collections::BTreeMap;
 use std::collections::HashMap;
@@ -403,6 +403,9 @@ fn expect_fun_call() {
     //Issue #2979 - this should not lint
     let msg = "bar";
     Some("foo").expect(msg);
+
+    Some("foo").expect({ &format!("error") });
+    Some("foo").expect(format!("error").as_ref());
 }
 
 /// Checks implementation of `ITER_NTH` lint
diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr
index 4b8c0403702..124edee6a52 100644
--- a/tests/ui/methods.stderr
+++ b/tests/ui/methods.stderr
@@ -361,7 +361,7 @@ error: use of `expect` followed by a function call
    --> $DIR/methods.rs:379:26
     |
 379 |     with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
-    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!(format!("Error {}: fake error", error_code).as_str()))`
+    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))`
 
 error: use of `expect` followed by a function call
    --> $DIR/methods.rs:389:25
@@ -373,85 +373,97 @@ error: use of `expect` followed by a function call
    --> $DIR/methods.rs:392:25
     |
 392 |     with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
-    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!(format!("Error {}: fake error", error_code).as_str()))`
+    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))`
+
+error: use of `expect` followed by a function call
+   --> $DIR/methods.rs:407:17
+    |
+407 |     Some("foo").expect({ &format!("error") });
+    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { let msg = { &format!("error") }; panic!(msg) }))`
+
+error: use of `expect` followed by a function call
+   --> $DIR/methods.rs:408:17
+    |
+408 |     Some("foo").expect(format!("error").as_ref());
+    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("error"))`
 
 error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable
-   --> $DIR/methods.rs:416:23
+   --> $DIR/methods.rs:419:23
     |
-416 |         let bad_vec = some_vec.iter().nth(3);
+419 |         let bad_vec = some_vec.iter().nth(3);
     |                       ^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `-D clippy::iter-nth` implied by `-D warnings`
 
 error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
-   --> $DIR/methods.rs:417:26
+   --> $DIR/methods.rs:420:26
     |
-417 |         let bad_slice = &some_vec[..].iter().nth(3);
+420 |         let bad_slice = &some_vec[..].iter().nth(3);
     |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
-   --> $DIR/methods.rs:418:31
+   --> $DIR/methods.rs:421:31
     |
-418 |         let bad_boxed_slice = boxed_slice.iter().nth(3);
+421 |         let bad_boxed_slice = boxed_slice.iter().nth(3);
     |                               ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `.iter().nth()` on a VecDeque. Calling `.get()` is both faster and more readable
-   --> $DIR/methods.rs:419:29
+   --> $DIR/methods.rs:422:29
     |
-419 |         let bad_vec_deque = some_vec_deque.iter().nth(3);
+422 |         let bad_vec_deque = some_vec_deque.iter().nth(3);
     |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `.iter_mut().nth()` on a Vec. Calling `.get_mut()` is both faster and more readable
-   --> $DIR/methods.rs:424:23
+   --> $DIR/methods.rs:427:23
     |
-424 |         let bad_vec = some_vec.iter_mut().nth(3);
+427 |         let bad_vec = some_vec.iter_mut().nth(3);
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `.iter_mut().nth()` on a slice. Calling `.get_mut()` is both faster and more readable
-   --> $DIR/methods.rs:427:26
+   --> $DIR/methods.rs:430:26
     |
-427 |         let bad_slice = &some_vec[..].iter_mut().nth(3);
+430 |         let bad_slice = &some_vec[..].iter_mut().nth(3);
     |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `.iter_mut().nth()` on a VecDeque. Calling `.get_mut()` is both faster and more readable
-   --> $DIR/methods.rs:430:29
+   --> $DIR/methods.rs:433:29
     |
-430 |         let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
+433 |         let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
     |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
-   --> $DIR/methods.rs:442:13
+   --> $DIR/methods.rs:445:13
     |
-442 |     let _ = some_vec.iter().skip(42).next();
+445 |     let _ = some_vec.iter().skip(42).next();
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `-D clippy::iter-skip-next` implied by `-D warnings`
 
 error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
-   --> $DIR/methods.rs:443:13
+   --> $DIR/methods.rs:446:13
     |
-443 |     let _ = some_vec.iter().cycle().skip(42).next();
+446 |     let _ = some_vec.iter().cycle().skip(42).next();
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
-   --> $DIR/methods.rs:444:13
+   --> $DIR/methods.rs:447:13
     |
-444 |     let _ = (1..10).skip(10).next();
+447 |     let _ = (1..10).skip(10).next();
     |             ^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`
-   --> $DIR/methods.rs:445:14
+   --> $DIR/methods.rs:448:14
     |
-445 |     let _ = &some_vec[..].iter().skip(3).next();
+448 |     let _ = &some_vec[..].iter().skip(3).next();
     |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
-   --> $DIR/methods.rs:454:13
+   --> $DIR/methods.rs:457:13
     |
-454 |     let _ = opt.unwrap();
+457 |     let _ = opt.unwrap();
     |             ^^^^^^^^^^^^
     |
     = note: `-D clippy::option-unwrap-used` implied by `-D warnings`
 
-error: aborting due to 56 previous errors
+error: aborting due to 58 previous errors