From 69af0e13b2662807ba51ffc3a9fe4a9576e100e2 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Sunkara Date: Thu, 16 Feb 2023 17:15:19 +0000 Subject: [PATCH] Recognize `Err` --- clippy_lints/src/methods/mod.rs | 10 ++++- .../src/methods/unnecessary_literal_unwrap.rs | 14 +++---- tests/ui/unnecessary_literal_unwrap.fixed | 11 ++++-- tests/ui/unnecessary_literal_unwrap.rs | 11 ++++-- tests/ui/unnecessary_literal_unwrap.stderr | 38 +++++++++++++++---- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4686adda683..66789e6137a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3666,7 +3666,10 @@ impl Methods { } unnecessary_literal_unwrap::check(cx, expr, recv, name); }, - ("expect_err", [_]) => expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests), + ("expect_err", [_]) => { + unnecessary_literal_unwrap::check(cx, expr, recv, name); + expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests); + }, ("extend", [arg]) => { string_extend_chars::check(cx, expr, recv, arg); extend_with_drain::check(cx, expr, recv, arg); @@ -3874,7 +3877,10 @@ impl Methods { unnecessary_literal_unwrap::check(cx, expr, recv, name); unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests); }, - ("unwrap_err", []) => unwrap_used::check(cx, expr, recv, true, 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) + }, ("unwrap_or", [u_arg]) => { match method_call(recv) { Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => { diff --git a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs index 54160a27cb2..56d3c16a5d2 100644 --- a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs +++ b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs @@ -9,18 +9,16 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr let init = clippy_utils::expr_or_init(cx, recv); if let hir::ExprKind::Call(call, [arg]) = init.kind { - let mess = if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) { - Some("Some") + let constructor = if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) { + "Some" } else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultOk) { - Some("Ok") + "Ok" + } else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultErr) { + "Err" } else { - None + return; }; - let Some(constructor) = mess else { - return; - }; - if init.span == recv.span { span_lint_and_then( cx, diff --git a/tests/ui/unnecessary_literal_unwrap.fixed b/tests/ui/unnecessary_literal_unwrap.fixed index a388a7f83d2..9733993d619 100644 --- a/tests/ui/unnecessary_literal_unwrap.fixed +++ b/tests/ui/unnecessary_literal_unwrap.fixed @@ -7,10 +7,14 @@ fn unwrap_option() { let _val = 1; } -fn unwrap_result() { +fn unwrap_result_ok() { + let _val = 1; + let _val = 1; +} + +fn unwrap_result_err() { let _val = 1; let _val = 1; - // let val = Err(1).unwrap_err(); } fn unwrap_methods_option() { @@ -27,7 +31,8 @@ fn unwrap_methods_result() { fn main() { unwrap_option(); - unwrap_result(); + unwrap_result_ok(); + unwrap_result_err(); unwrap_methods_option(); unwrap_methods_result(); } diff --git a/tests/ui/unnecessary_literal_unwrap.rs b/tests/ui/unnecessary_literal_unwrap.rs index fe557957dd9..775d744c13e 100644 --- a/tests/ui/unnecessary_literal_unwrap.rs +++ b/tests/ui/unnecessary_literal_unwrap.rs @@ -7,10 +7,14 @@ fn unwrap_option() { let _val = Some(1).expect("this never happens"); } -fn unwrap_result() { +fn unwrap_result_ok() { let _val = Ok::(1).unwrap(); let _val = Ok::(1).expect("this never happens"); - // let val = Err(1).unwrap_err(); +} + +fn unwrap_result_err() { + let _val = Err::<(), usize>(1).unwrap_err(); + let _val = Err::<(), usize>(1).expect_err("this never happens"); } fn unwrap_methods_option() { @@ -27,7 +31,8 @@ fn unwrap_methods_result() { fn main() { unwrap_option(); - unwrap_result(); + unwrap_result_ok(); + unwrap_result_err(); unwrap_methods_option(); unwrap_methods_result(); } diff --git a/tests/ui/unnecessary_literal_unwrap.stderr b/tests/ui/unnecessary_literal_unwrap.stderr index 60499fe700b..622743f8c85 100644 --- a/tests/ui/unnecessary_literal_unwrap.stderr +++ b/tests/ui/unnecessary_literal_unwrap.stderr @@ -47,9 +47,33 @@ LL - let _val = Ok::(1).expect("this never happens"); LL + let _val = 1; | -error: used `unwrap_or()` on `Some` value +error: used `unwrap_err()` on `Err` value + --> $DIR/unnecessary_literal_unwrap.rs:16:16 + | +LL | let _val = Err::<(), usize>(1).unwrap_err(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Err` and `unwrap_err()` + | +LL - let _val = Err::<(), usize>(1).unwrap_err(); +LL + let _val = 1; + | + +error: used `expect_err()` on `Err` value --> $DIR/unnecessary_literal_unwrap.rs:17:16 | +LL | let _val = Err::<(), usize>(1).expect_err("this never happens"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Err` and `expect_err()` + | +LL - let _val = Err::<(), usize>(1).expect_err("this never happens"); +LL + let _val = 1; + | + +error: used `unwrap_or()` on `Some` value + --> $DIR/unnecessary_literal_unwrap.rs:21:16 + | LL | let _val = Some(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^ | @@ -60,7 +84,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:18:16 + --> $DIR/unnecessary_literal_unwrap.rs:22:16 | LL | let _val = Some(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +96,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:19:16 + --> $DIR/unnecessary_literal_unwrap.rs:23:16 | LL | let _val = Some(1).unwrap_or_else(|| _val); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -84,7 +108,7 @@ LL + let _val = 1; | error: used `unwrap_or()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:23:16 + --> $DIR/unnecessary_literal_unwrap.rs:27:16 | LL | let _val = Ok::(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -96,7 +120,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:24:16 + --> $DIR/unnecessary_literal_unwrap.rs:28:16 | LL | let _val = Ok::(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -108,7 +132,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:25:16 + --> $DIR/unnecessary_literal_unwrap.rs:29:16 | LL | let _val = Ok::(1).unwrap_or_else(|()| _val); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -119,5 +143,5 @@ LL - let _val = Ok::(1).unwrap_or_else(|()| _val); LL + let _val = 1; | -error: aborting due to 10 previous errors +error: aborting due to 12 previous errors