From 7499cb543dbf111b1d92b808a91be730e761849f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Wed, 10 Oct 2018 07:52:58 +0200 Subject: [PATCH] Fix #2937 --- clippy_lints/src/methods/mod.rs | 24 ++++++++---- tests/ui/methods.rs | 5 ++- tests/ui/methods.stderr | 66 +++++++++++++++++++-------------- 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> { - 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