From 1ead12c5004cf54c3f0469a19909bf49321926f7 Mon Sep 17 00:00:00 2001 From: Donald Robertson Date: Tue, 29 May 2018 10:20:18 +0200 Subject: [PATCH] Adding handling and tests for custom type with implemented expect method --- clippy_lints/src/methods.rs | 17 +++++++--- tests/ui/methods.rs | 19 +++++++++++ tests/ui/methods.stderr | 64 ++++++++++++++++++------------------- 3 files changed, 63 insertions(+), 37 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 696bd02cf..38413c5aa 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -1006,11 +1006,13 @@ fn lint_expect_fun_call(cx: &LateContext, expr: &hir::Expr, method_span: Span, n return; } - let self_ty = cx.tables.expr_ty(self_expr); - let closure = match match_type(cx, self_ty, &paths::OPTION) { - true => "||", - false => "|_|", - }; + let self_type = cx.tables.expr_ty(self_expr); + let known_types = &[&paths::OPTION, &paths::RESULT]; + + // if not a known type, return early + if known_types.iter().all(|&k| !match_type(cx, self_type, k)) { + return; + } // don't lint for constant values let owner_def = cx.tcx.hir.get_parent_did(arg.id); @@ -1019,6 +1021,11 @@ fn lint_expect_fun_call(cx: &LateContext, expr: &hir::Expr, method_span: Span, n return; } + let closure = match match_type(cx, self_type, &paths::OPTION) { + true => "||", + false => "|_|", + }; + let sugg: Cow<_> = snippet(cx, arg.span, ".."); let span_replace_word = method_span.with_hi(span.hi()); diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index 04e3ec13b..b04c008ba 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -344,6 +344,16 @@ fn or_fun_call() { /// Checks implementation of the `EXPECT_FUN_CALL` lint fn expect_fun_call() { + struct Foo; + + impl Foo { + fn new() -> Self { Foo } + + fn expect(&self, msg: &str) { + panic!("{}", msg) + } + } + let with_some = Some("value"); with_some.expect("error"); @@ -369,6 +379,15 @@ fn expect_fun_call() { let with_err_and_as_str: Result<(), ()> = Err(()); with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); + + let with_dummy_type = Foo::new(); + with_dummy_type.expect("another test string"); + + let with_dummy_type_and_format = Foo::new(); + with_dummy_type_and_format.expect(&format!("Error {}: fake error", error_code)); + + let with_dummy_type_and_as_str = Foo::new(); + with_dummy_type_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); } /// Checks implementation of `ITER_NTH` lint diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index f3c608cb7..65b5589a9 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -424,105 +424,105 @@ error: use of `unwrap_or` followed by a function call | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `expect` followed by a function call - --> $DIR/methods.rs:355:26 + --> $DIR/methods.rs:365:26 | -355 | with_none_and_format.expect(&format!("Error {}: fake error", error_code)); +365 | with_none_and_format.expect(&format!("Error {}: fake error", error_code)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!(&format!("Error {}: fake error", error_code)))` | = note: `-D expect-fun-call` implied by `-D warnings` error: use of `expect` followed by a function call - --> $DIR/methods.rs:358:26 + --> $DIR/methods.rs:368:26 | -358 | with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); +368 | 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()))` error: use of `expect` followed by a function call - --> $DIR/methods.rs:368:25 + --> $DIR/methods.rs:378:25 | -368 | with_err_and_format.expect(&format!("Error {}: fake error", error_code)); +378 | with_err_and_format.expect(&format!("Error {}: fake error", error_code)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!(&format!("Error {}: fake error", error_code)))` error: use of `expect` followed by a function call - --> $DIR/methods.rs:371:25 + --> $DIR/methods.rs:381:25 | -371 | with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); +381 | 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()))` error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable - --> $DIR/methods.rs:382:23 + --> $DIR/methods.rs:401:23 | -382 | let bad_vec = some_vec.iter().nth(3); +401 | let bad_vec = some_vec.iter().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D iter-nth` implied by `-D warnings` error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable - --> $DIR/methods.rs:383:26 + --> $DIR/methods.rs:402:26 | -383 | let bad_slice = &some_vec[..].iter().nth(3); +402 | 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:384:31 + --> $DIR/methods.rs:403:31 | -384 | let bad_boxed_slice = boxed_slice.iter().nth(3); +403 | 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:385:29 + --> $DIR/methods.rs:404:29 | -385 | let bad_vec_deque = some_vec_deque.iter().nth(3); +404 | 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:390:23 + --> $DIR/methods.rs:409:23 | -390 | let bad_vec = some_vec.iter_mut().nth(3); +409 | 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:393:26 + --> $DIR/methods.rs:412:26 | -393 | let bad_slice = &some_vec[..].iter_mut().nth(3); +412 | 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:396:29 + --> $DIR/methods.rs:415:29 | -396 | let bad_vec_deque = some_vec_deque.iter_mut().nth(3); +415 | 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:408:13 + --> $DIR/methods.rs:427:13 | -408 | let _ = some_vec.iter().skip(42).next(); +427 | let _ = some_vec.iter().skip(42).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D 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:409:13 + --> $DIR/methods.rs:428:13 | -409 | let _ = some_vec.iter().cycle().skip(42).next(); +428 | 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:410:13 + --> $DIR/methods.rs:429:13 | -410 | let _ = (1..10).skip(10).next(); +429 | 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:411:14 + --> $DIR/methods.rs:430:14 | -411 | let _ = &some_vec[..].iter().skip(3).next(); +430 | 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:420:13 + --> $DIR/methods.rs:439:13 | -420 | let _ = opt.unwrap(); +439 | let _ = opt.unwrap(); | ^^^^^^^^^^^^ | = note: `-D option-unwrap-used` implied by `-D warnings`