diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 66789e6137a..912cce39e2e 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3664,10 +3664,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv), _ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests), } - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, ("expect_err", [_]) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests); }, ("extend", [arg]) => { @@ -3874,12 +3874,12 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }, _ => {}, } - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests); }, ("unwrap_err", []) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name); - unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests) + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); + unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests); }, ("unwrap_or", [u_arg]) => { match method_call(recv) { @@ -3894,10 +3894,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }, _ => {}, } - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, ("unwrap_or_default", []) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); } ("unwrap_or_else", [u_arg]) => { match method_call(recv) { @@ -3908,7 +3908,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); }, } - unnecessary_literal_unwrap::check(cx, expr, recv, name); + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, ("zip", [arg]) => { if let ExprKind::MethodCall(name, iter_recv, [], _) = recv.kind diff --git a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs index 56d3c16a5d2..fe6b86b34f5 100644 --- a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs +++ b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs @@ -5,49 +5,68 @@ use super::UNNECESSARY_LITERAL_UNWRAP; -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, name: &str) { +pub(super) fn check( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + recv: &hir::Expr<'_>, + method: &str, + args: &[hir::Expr<'_>], +) { let init = clippy_utils::expr_or_init(cx, recv); - if let hir::ExprKind::Call(call, [arg]) = init.kind { - let constructor = if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) { - "Some" + let (constructor, call_args) = if let hir::ExprKind::Call(call, call_args) = init.kind { + if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) { + ("Some", call_args) } else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultOk) { - "Ok" + ("Ok", call_args) } else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultErr) { - "Err" + ("Err", call_args) } else { return; - }; - - if init.span == recv.span { - span_lint_and_then( - cx, - UNNECESSARY_LITERAL_UNWRAP, - expr.span, - &format!("used `{name}()` on `{constructor}` value"), - |diag| { - let suggestions = vec![ - (recv.span.with_hi(arg.span.lo()), String::new()), - (expr.span.with_lo(arg.span.hi()), String::new()), - ]; - - diag.multipart_suggestion( - format!("remove the `{constructor}` and `{name}()`"), - suggestions, - Applicability::MachineApplicable, - ); - }, - ); - } else { - span_lint_and_then( - cx, - UNNECESSARY_LITERAL_UNWRAP, - expr.span, - &format!("used `{name}()` on `{constructor}` value"), - |diag| { - diag.span_help(init.span, format!("remove the `{constructor}` and `{name}()`")); - }, - ); } + } else if is_res_lang_ctor(cx, path_res(cx, init), hir::LangItem::OptionNone) { + let call_args: &[hir::Expr<'_>] = &[]; + ("None", call_args) + } else { + return; + }; + + let help_message = format!("used `{method}()` on `{constructor}` value"); + let suggestion_message = format!("remove the `{constructor}` and `{method}()`"); + + if init.span == recv.span { + span_lint_and_then(cx, UNNECESSARY_LITERAL_UNWRAP, expr.span, &help_message, |diag| { + let suggestions = match (constructor, method) { + ("None", "unwrap") => vec![(expr.span, "panic!()".to_string())], + ("None", "expect") => vec![ + (expr.span.with_hi(args[0].span.lo()), "panic!(".to_string()), + (expr.span.with_lo(args[0].span.hi()), ")".to_string()), + ], + ("Ok", "unwrap_err") | ("Err", "unwrap") => vec![ + ( + recv.span.with_hi(call_args[0].span.lo()), + "panic!(\"{:?}\", ".to_string(), + ), + (expr.span.with_lo(call_args[0].span.hi()), ")".to_string()), + ], + ("Ok", "expect_err") | ("Err", "expect") => vec![ + ( + recv.span.with_hi(call_args[0].span.lo()), + "panic!(\"{1}: {:?}\", ".to_string(), + ), + (call_args[0].span.with_lo(args[0].span.lo()), ", ".to_string()), + ], + _ => vec![ + (recv.span.with_hi(call_args[0].span.lo()), String::new()), + (expr.span.with_lo(call_args[0].span.hi()), String::new()), + ], + }; + + diag.multipart_suggestion(suggestion_message, suggestions, Applicability::MachineApplicable); + }); + } else { + span_lint_and_then(cx, UNNECESSARY_LITERAL_UNWRAP, expr.span, &help_message, |diag| { + diag.span_help(init.span, suggestion_message); + }); } } diff --git a/tests/ui/unnecessary_literal_unwrap.fixed b/tests/ui/unnecessary_literal_unwrap.fixed index 9733993d619..d622c984c52 100644 --- a/tests/ui/unnecessary_literal_unwrap.fixed +++ b/tests/ui/unnecessary_literal_unwrap.fixed @@ -1,20 +1,30 @@ //run-rustfix #![warn(clippy::unnecessary_literal_unwrap)] #![allow(clippy::unnecessary_lazy_evaluations)] +#![allow(unreachable_code)] -fn unwrap_option() { +fn unwrap_option_some() { let _val = 1; let _val = 1; } +fn unwrap_option_none() { + panic!(); + panic!("this always happens"); +} + fn unwrap_result_ok() { let _val = 1; let _val = 1; + panic!("{:?}", 1); + panic!("{1}: {:?}", 1, "this always happens"); } fn unwrap_result_err() { let _val = 1; let _val = 1; + panic!("{:?}", 1); + panic!("{1}: {:?}", 1, "this always happens"); } fn unwrap_methods_option() { @@ -30,7 +40,8 @@ fn unwrap_methods_result() { } fn main() { - unwrap_option(); + unwrap_option_some(); + unwrap_option_none(); unwrap_result_ok(); unwrap_result_err(); unwrap_methods_option(); diff --git a/tests/ui/unnecessary_literal_unwrap.rs b/tests/ui/unnecessary_literal_unwrap.rs index 775d744c13e..926265a3471 100644 --- a/tests/ui/unnecessary_literal_unwrap.rs +++ b/tests/ui/unnecessary_literal_unwrap.rs @@ -1,20 +1,30 @@ //run-rustfix #![warn(clippy::unnecessary_literal_unwrap)] #![allow(clippy::unnecessary_lazy_evaluations)] +#![allow(unreachable_code)] -fn unwrap_option() { +fn unwrap_option_some() { let _val = Some(1).unwrap(); let _val = Some(1).expect("this never happens"); } +fn unwrap_option_none() { + None::.unwrap(); + None::.expect("this always happens"); +} + fn unwrap_result_ok() { let _val = Ok::(1).unwrap(); let _val = Ok::(1).expect("this never happens"); + Ok::(1).unwrap_err(); + Ok::(1).expect_err("this always happens"); } fn unwrap_result_err() { let _val = Err::<(), usize>(1).unwrap_err(); let _val = Err::<(), usize>(1).expect_err("this never happens"); + Err::<(), usize>(1).unwrap(); + Err::<(), usize>(1).expect("this always happens"); } fn unwrap_methods_option() { @@ -30,7 +40,8 @@ fn unwrap_methods_result() { } fn main() { - unwrap_option(); + unwrap_option_some(); + unwrap_option_none(); unwrap_result_ok(); unwrap_result_err(); unwrap_methods_option(); diff --git a/tests/ui/unnecessary_literal_unwrap.stderr b/tests/ui/unnecessary_literal_unwrap.stderr index 622743f8c85..1d23bb22bdd 100644 --- a/tests/ui/unnecessary_literal_unwrap.stderr +++ b/tests/ui/unnecessary_literal_unwrap.stderr @@ -1,5 +1,5 @@ error: used `unwrap()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:6:16 + --> $DIR/unnecessary_literal_unwrap.rs:7:16 | LL | let _val = Some(1).unwrap(); | ^^^^^^^^^^^^^^^^ @@ -12,7 +12,7 @@ LL + let _val = 1; | error: used `expect()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:7:16 + --> $DIR/unnecessary_literal_unwrap.rs:8:16 | LL | let _val = Some(1).expect("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,8 +23,25 @@ LL - let _val = Some(1).expect("this never happens"); LL + let _val = 1; | +error: used `unwrap()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:12:5 + | +LL | None::.unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap()`: `panic!()` + +error: used `expect()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:13:5 + | +LL | None::.expect("this always happens"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `expect()` + | +LL | panic!("this always happens"); + | ~~~~~~~ ~ + error: used `unwrap()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:11:16 + --> $DIR/unnecessary_literal_unwrap.rs:17:16 | LL | let _val = Ok::(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +53,7 @@ LL + let _val = 1; | error: used `expect()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:12:16 + --> $DIR/unnecessary_literal_unwrap.rs:18:16 | LL | let _val = Ok::(1).expect("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -47,8 +64,30 @@ LL - let _val = Ok::(1).expect("this never happens"); LL + let _val = 1; | +error: used `unwrap_err()` on `Ok` value + --> $DIR/unnecessary_literal_unwrap.rs:19:5 + | +LL | Ok::(1).unwrap_err(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Ok` and `unwrap_err()` + | +LL | panic!("{:?}", 1); + | ~~~~~~~~~~~~~~ ~ + +error: used `expect_err()` on `Ok` value + --> $DIR/unnecessary_literal_unwrap.rs:20:5 + | +LL | Ok::(1).expect_err("this always happens"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Ok` and `expect_err()` + | +LL | panic!("{1}: {:?}", 1, "this always happens"); + | ~~~~~~~~~~~~~~~~~~~ ~ + error: used `unwrap_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:16:16 + --> $DIR/unnecessary_literal_unwrap.rs:24:16 | LL | let _val = Err::<(), usize>(1).unwrap_err(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -60,7 +99,7 @@ LL + let _val = 1; | error: used `expect_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:17:16 + --> $DIR/unnecessary_literal_unwrap.rs:25:16 | LL | let _val = Err::<(), usize>(1).expect_err("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -71,8 +110,30 @@ LL - let _val = Err::<(), usize>(1).expect_err("this never happens"); LL + let _val = 1; | +error: used `unwrap()` on `Err` value + --> $DIR/unnecessary_literal_unwrap.rs:26:5 + | +LL | Err::<(), usize>(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Err` and `unwrap()` + | +LL | panic!("{:?}", 1); + | ~~~~~~~~~~~~~~ ~ + +error: used `expect()` on `Err` value + --> $DIR/unnecessary_literal_unwrap.rs:27:5 + | +LL | Err::<(), usize>(1).expect("this always happens"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Err` and `expect()` + | +LL | panic!("{1}: {:?}", 1, "this always happens"); + | ~~~~~~~~~~~~~~~~~~~ ~ + error: used `unwrap_or()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:21:16 + --> $DIR/unnecessary_literal_unwrap.rs:31:16 | LL | let _val = Some(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^ @@ -84,7 +145,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:22:16 + --> $DIR/unnecessary_literal_unwrap.rs:32:16 | LL | let _val = Some(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -96,7 +157,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:23:16 + --> $DIR/unnecessary_literal_unwrap.rs:33:16 | LL | let _val = Some(1).unwrap_or_else(|| _val); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -108,7 +169,7 @@ LL + let _val = 1; | error: used `unwrap_or()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:27:16 + --> $DIR/unnecessary_literal_unwrap.rs:37:16 | LL | let _val = Ok::(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -120,7 +181,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:28:16 + --> $DIR/unnecessary_literal_unwrap.rs:38:16 | LL | let _val = Ok::(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -132,7 +193,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:29:16 + --> $DIR/unnecessary_literal_unwrap.rs:39:16 | LL | let _val = Ok::(1).unwrap_or_else(|()| _val); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -143,5 +204,5 @@ LL - let _val = Ok::(1).unwrap_or_else(|()| _val); LL + let _val = 1; | -error: aborting due to 12 previous errors +error: aborting due to 18 previous errors