diff options
| author | bors <bors@rust-lang.org> | 2022-08-01 01:27:03 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-08-01 01:27:03 +0000 |
| commit | a591e725a6f5c6097c30d91ec68c992310b7b1bc (patch) | |
| tree | ad5fdf695d6af9d458d45630e651fd27b4192cb9 | |
| parent | a0ed68776e2f8177198982587bce6685410548d1 (diff) | |
| parent | 23b4fe6da55291a5937535d09af2bdfed8a9a50d (diff) | |
| download | rust-a591e725a6f5c6097c30d91ec68c992310b7b1bc.tar.gz rust-a591e725a6f5c6097c30d91ec68c992310b7b1bc.zip | |
Auto merge of #9223 - sgued:unwrap-expect-used, r=giraffate
unwrap_used: Don't recommend using `expect` when the `expect_used` lint is not allowed Fixes #9222 ``` changelog: [`unwrap_used`]: Don't recommend using `expect` when the `expect_used` lint is not allowed ```
| -rw-r--r-- | clippy_lints/src/methods/expect_used.rs | 10 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 11 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unwrap_used.rs | 27 | ||||
| -rw-r--r-- | tests/ui-toml/expect_used/expect_used.stderr | 2 | ||||
| -rw-r--r-- | tests/ui/expect.stderr | 2 | ||||
| -rw-r--r-- | tests/ui/unwrap_expect_used.rs | 10 | ||||
| -rw-r--r-- | tests/ui/unwrap_expect_used.stderr | 36 |
7 files changed, 80 insertions, 18 deletions
diff --git a/clippy_lints/src/methods/expect_used.rs b/clippy_lints/src/methods/expect_used.rs index fbc3348f185..5ef08ca6290 100644 --- a/clippy_lints/src/methods/expect_used.rs +++ b/clippy_lints/src/methods/expect_used.rs @@ -12,9 +12,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs(); let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) { - Some((EXPECT_USED, "an Option", "None")) + Some((EXPECT_USED, "an Option", "None", "")) } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) { - Some((EXPECT_USED, "a Result", "Err")) + Some((EXPECT_USED, "a Result", "Err", "an ")) } else { None }; @@ -23,14 +23,14 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr return; } - if let Some((lint, kind, none_value)) = mess { + if let Some((lint, kind, none_value, none_prefix)) = mess { span_lint_and_help( cx, lint, expr.span, - &format!("used `expect()` on `{}` value", kind,), + &format!("used `expect()` on `{kind}` value"), None, - &format!("if this value is an `{}`, it will panic", none_value,), + &format!("if this value is {none_prefix}`{none_value}`, it will panic"), ); } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 202fbc1f7f6..5ac6b09f0aa 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -204,6 +204,17 @@ declare_clippy_lint! { /// option.expect("more helpful message"); /// result.expect("more helpful message"); /// ``` + /// + /// If [expect_used](#expect_used) is enabled, instead: + /// ```rust,ignore + /// # let option = Some(1); + /// # let result: Result<usize, ()> = Ok(1); + /// option?; + /// + /// // or + /// + /// result?; + /// ``` #[clippy::version = "1.45.0"] pub UNWRAP_USED, restriction, diff --git a/clippy_lints/src/methods/unwrap_used.rs b/clippy_lints/src/methods/unwrap_used.rs index 5c761014927..ce1a52e5480 100644 --- a/clippy_lints/src/methods/unwrap_used.rs +++ b/clippy_lints/src/methods/unwrap_used.rs @@ -1,20 +1,20 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::is_in_test_function; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_in_test_function, is_lint_allowed}; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::sym; -use super::UNWRAP_USED; +use super::{EXPECT_USED, UNWRAP_USED}; /// lint use of `unwrap()` for `Option`s and `Result`s pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, allow_unwrap_in_tests: bool) { let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs(); let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) { - Some((UNWRAP_USED, "an Option", "None")) + Some((UNWRAP_USED, "an Option", "None", "")) } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) { - Some((UNWRAP_USED, "a Result", "Err")) + Some((UNWRAP_USED, "a Result", "Err", "an ")) } else { None }; @@ -23,18 +23,23 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr return; } - if let Some((lint, kind, none_value)) = mess { + if let Some((lint, kind, none_value, none_prefix)) = mess { + let help = if is_lint_allowed(cx, EXPECT_USED, expr.hir_id) { + format!( + "if you don't want to handle the `{none_value}` case gracefully, consider \ + using `expect()` to provide a better panic message" + ) + } else { + format!("if this value is {none_prefix}`{none_value}`, it will panic") + }; + span_lint_and_help( cx, lint, expr.span, - &format!("used `unwrap()` on `{}` value", kind,), + &format!("used `unwrap()` on `{kind}` value"), None, - &format!( - "if you don't want to handle the `{}` case gracefully, consider \ - using `expect()` to provide a better panic message", - none_value, - ), + &help, ); } } diff --git a/tests/ui-toml/expect_used/expect_used.stderr b/tests/ui-toml/expect_used/expect_used.stderr index 9cb2199ed21..c5d95cb8a14 100644 --- a/tests/ui-toml/expect_used/expect_used.stderr +++ b/tests/ui-toml/expect_used/expect_used.stderr @@ -5,7 +5,7 @@ LL | let _ = opt.expect(""); | ^^^^^^^^^^^^^^ | = note: `-D clippy::expect-used` implied by `-D warnings` - = help: if this value is an `None`, it will panic + = help: if this value is `None`, it will panic error: used `expect()` on `a Result` value --> $DIR/expect_used.rs:11:13 diff --git a/tests/ui/expect.stderr b/tests/ui/expect.stderr index 9d3fc7df15c..ab28aac4556 100644 --- a/tests/ui/expect.stderr +++ b/tests/ui/expect.stderr @@ -5,7 +5,7 @@ LL | let _ = opt.expect(""); | ^^^^^^^^^^^^^^ | = note: `-D clippy::expect-used` implied by `-D warnings` - = help: if this value is an `None`, it will panic + = help: if this value is `None`, it will panic error: used `expect()` on `a Result` value --> $DIR/expect.rs:10:13 diff --git a/tests/ui/unwrap_expect_used.rs b/tests/ui/unwrap_expect_used.rs new file mode 100644 index 00000000000..0d4a0504a6e --- /dev/null +++ b/tests/ui/unwrap_expect_used.rs @@ -0,0 +1,10 @@ +#![warn(clippy::unwrap_used, clippy::expect_used)] + +fn main() { + Some(3).unwrap(); + Some(3).expect("Hello world!"); + + let a: Result<i32, i32> = Ok(3); + a.unwrap(); + a.expect("Hello world!"); +} diff --git a/tests/ui/unwrap_expect_used.stderr b/tests/ui/unwrap_expect_used.stderr new file mode 100644 index 00000000000..f54bfd617c4 --- /dev/null +++ b/tests/ui/unwrap_expect_used.stderr @@ -0,0 +1,36 @@ +error: used `unwrap()` on `an Option` value + --> $DIR/unwrap_expect_used.rs:4:5 + | +LL | Some(3).unwrap(); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unwrap-used` implied by `-D warnings` + = help: if this value is `None`, it will panic + +error: used `expect()` on `an Option` value + --> $DIR/unwrap_expect_used.rs:5:5 + | +LL | Some(3).expect("Hello world!"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::expect-used` implied by `-D warnings` + = help: if this value is `None`, it will panic + +error: used `unwrap()` on `a Result` value + --> $DIR/unwrap_expect_used.rs:8:5 + | +LL | a.unwrap(); + | ^^^^^^^^^^ + | + = help: if this value is an `Err`, it will panic + +error: used `expect()` on `a Result` value + --> $DIR/unwrap_expect_used.rs:9:5 + | +LL | a.expect("Hello world!"); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: if this value is an `Err`, it will panic + +error: aborting due to 4 previous errors + |
