Skip to content

Commit

Permalink
Auto merge of #9223 - sgued:unwrap-expect-used, r=giraffate
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
bors committed Aug 1, 2022
2 parents a0ed687 + 23b4fe6 commit a591e72
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 18 deletions.
10 changes: 5 additions & 5 deletions clippy_lints/src/methods/expect_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand All @@ -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"),
);
}
}
11 changes: 11 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 16 additions & 11 deletions clippy_lints/src/methods/unwrap_used.rs
Original file line number Diff line number Diff line change
@@ -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
};
Expand All @@ -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,
);
}
}
2 changes: 1 addition & 1 deletion tests/ui-toml/expect_used/expect_used.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/expect.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/unwrap_expect_used.rs
Original file line number Diff line number Diff line change
@@ -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!");
}
36 changes: 36 additions & 0 deletions tests/ui/unwrap_expect_used.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a591e72

Please sign in to comment.