about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-08-01 01:27:03 +0000
committerbors <bors@rust-lang.org>2022-08-01 01:27:03 +0000
commita591e725a6f5c6097c30d91ec68c992310b7b1bc (patch)
treead5fdf695d6af9d458d45630e651fd27b4192cb9
parenta0ed68776e2f8177198982587bce6685410548d1 (diff)
parent23b4fe6da55291a5937535d09af2bdfed8a9a50d (diff)
downloadrust-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.rs10
-rw-r--r--clippy_lints/src/methods/mod.rs11
-rw-r--r--clippy_lints/src/methods/unwrap_used.rs27
-rw-r--r--tests/ui-toml/expect_used/expect_used.stderr2
-rw-r--r--tests/ui/expect.stderr2
-rw-r--r--tests/ui/unwrap_expect_used.rs10
-rw-r--r--tests/ui/unwrap_expect_used.stderr36
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
+