From 6ee03e2b014fc98e1bbfdbf5c39872b8308cd15d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 21 Jul 2022 22:45:12 +0200 Subject: [PATCH] unwrap_used: Stop recommending using `expect` when the `expect_used` lint is not allowed --- clippy_lints/src/methods/unwrap_used.rs | 21 +++++++++------ tests/ui/unwrap_expect_used.rs | 10 +++++++ tests/ui/unwrap_expect_used.stderr | 36 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 tests/ui/unwrap_expect_used.rs create mode 100644 tests/ui/unwrap_expect_used.stderr diff --git a/clippy_lints/src/methods/unwrap_used.rs b/clippy_lints/src/methods/unwrap_used.rs index 5c761014927..eae6ba693a2 100644 --- a/clippy_lints/src/methods/unwrap_used.rs +++ b/clippy_lints/src/methods/unwrap_used.rs @@ -1,11 +1,11 @@ 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) { @@ -24,17 +24,22 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr } if let Some((lint, kind, none_value)) = 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 an `{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/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 = 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..a44aed4cb38 --- /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 an `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 an `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 +