mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Do not lint unwrapping on ! or never-like enums

Browse Source
This commit is contained in:
Catherine Flores 2023-07-29 00:06:08 -05:00
parent 78f5e0d3ec
commit 779e0f4021
6 changed files with 137 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
clippy_lints/src/methods/expect_used.rs
View File

@ -1,44 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_in_cfg_test, is_in_test_function};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::EXPECT_USED;
/// lint use of `expect()` or `expect_err` for `Result` and `expect()` for `Option`.
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
is_err: bool,
allow_expect_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) && !is_err {
Some((EXPECT_USED, "an `Option`", "None", ""))
} else if is_type_diagnostic_item(cx, obj_ty, sym::Result) {
Some((EXPECT_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an "))
} else {
None
};
let method = if is_err { "expect_err" } else { "expect" };
if allow_expect_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
return;
}
if let Some((lint, kind, none_value, none_prefix)) = mess {
span_lint_and_help(
cx,
lint,
expr.span,
&format!("used `{method}()` on {kind} value"),
None,
&format!("if this value is {none_prefix}`{none_value}`, it will panic"),
);
}
}

39
clippy_lints/src/methods/mod.rs
View File

@ -17,7 +17,6 @@
mod drain_collect;
mod err_expect;
mod expect_fun_call;
mod expect_used;
mod extend_with_drain;
mod filetype_is_file;
mod filter_map;
@ -105,7 +104,7 @@
mod unnecessary_literal_unwrap;
mod unnecessary_sort_by;
mod unnecessary_to_owned;
mod unwrap_used;
mod unwrap_expect_used;
mod useless_asref;
mod utils;
mod vec_resize_to_zero;
@ -3948,13 +3947,27 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match method_call(recv) {
Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv),
Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv),
_ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
_ => unwrap_expect_used::check(
cx,
expr,
recv,
false,
self.allow_expect_in_tests,
unwrap_expect_used::Variant::Expect,
),
}
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
},
("expect_err", [_]) => {
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests);
unwrap_expect_used::check(
cx,
expr,
recv,
true,
self.allow_expect_in_tests,
unwrap_expect_used::Variant::Expect,
);
},
("extend", [arg]) => {
string_extend_chars::check(cx, expr, recv, arg);
@ -4180,11 +4193,25 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
_ => {},
}
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests);
unwrap_expect_used::check(
cx,
expr,
recv,
false,
self.allow_unwrap_in_tests,
unwrap_expect_used::Variant::Unwrap,
);
},
("unwrap_err", []) => {
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests);
unwrap_expect_used::check(
cx,
expr,
recv,
true,
self.allow_unwrap_in_tests,
unwrap_expect_used::Variant::Unwrap,
);
},
("unwrap_or", [u_arg]) => {
match method_call(recv) {

87
clippy_lints/src/methods/unwrap_expect_used.rs Normal file
View File

@ -0,0 +1,87 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
use rustc_hir::Expr;
use rustc_lint::{LateContext, Lint};
use rustc_middle::ty;
use rustc_span::sym;
use super::{EXPECT_USED, UNWRAP_USED};
#[derive(Clone, Copy, Eq, PartialEq)]
pub(super) enum Variant {
Unwrap,
Expect,
}
impl Variant {
fn method_name(self, is_err: bool) -> &'static str {
match (self, is_err) {
(Variant::Unwrap, true) => "unwrap_err",
(Variant::Unwrap, false) => "unwrap",
(Variant::Expect, true) => "expect_err",
(Variant::Expect, false) => "expect",
}
}
fn lint(self) -> &'static Lint {
match self {
Variant::Unwrap => UNWRAP_USED,
Variant::Expect => EXPECT_USED,
}
}
}
/// lint use of `unwrap()` or `unwrap_err` for `Result` and `unwrap()` for `Option`.
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
recv: &Expr<'_>,
is_err: bool,
allow_unwrap_in_tests: bool,
variant: Variant,
) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
let (kind, none_value, none_prefix) = if is_type_diagnostic_item(cx, ty, sym::Option) && !is_err {
("an `Option`", "None", "")
} else if is_type_diagnostic_item(cx, ty, sym::Result)
&& let ty::Adt(_, substs) = ty.kind()
&& let Some(t_or_e_ty) = substs[usize::from(!is_err)].as_type()
{
// Issue #11245: Do not lint `!` or never-like enums
if t_or_e_ty.is_never()
|| (t_or_e_ty.is_enum() && t_or_e_ty.ty_adt_def().is_some_and(|def| def.variants().is_empty()))
{
return;
}
("a `Result`", if is_err { "Ok" } else { "Err" }, "an ")
} else {
return;
};
let method_suffix = if is_err { "_err" } else { "" };
if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
return;
}
let help = if variant == Variant::Unwrap && 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{method_suffix}()` to provide a better panic message"
)
} else {
format!("if this value is {none_prefix}`{none_value}`, it will panic")
};
span_lint_and_help(
cx,
variant.lint(),
expr.span,
&format!("used `{}()` on {kind} value", variant.method_name(is_err)),
None,
&help,
);
}

53
clippy_lints/src/methods/unwrap_used.rs
View File

@ -1,53 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::{EXPECT_USED, UNWRAP_USED};
/// lint use of `unwrap()` or `unwrap_err` for `Result` and `unwrap()` for `Option`.
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
is_err: bool,
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) && !is_err {
Some((UNWRAP_USED, "an `Option`", "None", ""))
} else if is_type_diagnostic_item(cx, obj_ty, sym::Result) {
Some((UNWRAP_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an "))
} else {
None
};
let method_suffix = if is_err { "_err" } else { "" };
if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
return;
}
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{method_suffix}()` 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{method_suffix}()` on {kind} value"),
None,
&help,
);
}
}

11
tests/ui/unwrap_expect_used.rs
View File

@ -1,5 +1,8 @@
#![warn(clippy::unwrap_used, clippy::expect_used)]
#![allow(clippy::unnecessary_literal_unwrap)]
#![feature(never_type)]
use std::convert::Infallible;
trait OptionExt {
type Item;
@ -28,6 +31,14 @@ fn main() {
Some(3).unwrap_err();
Some(3).expect_err("Hellow none!");
// Issue #11245: The `Err` variant can never be constructed so do not lint this.
let x: Result<(), !> = Ok(());
x.unwrap();
x.expect("is `!` (never)");
let x: Result<(), Infallible> = Ok(());
x.unwrap();
x.expect("is never-like (0 variants)");
let a: Result<i32, i32> = Ok(3);
a.unwrap();
a.expect("Hello world!");

12
tests/ui/unwrap_expect_used.stderr
View File

@ -1,5 +1,5 @@
error: used `unwrap()` on an `Option` value
--> $DIR/unwrap_expect_used.rs:24:5
--> $DIR/unwrap_expect_used.rs:27:5
|
LL | Some(3).unwrap();
| ^^^^^^^^^^^^^^^^
@ -8,7 +8,7 @@ LL | Some(3).unwrap();
= note: `-D clippy::unwrap-used` implied by `-D warnings`
error: used `expect()` on an `Option` value
--> $DIR/unwrap_expect_used.rs:25:5
--> $DIR/unwrap_expect_used.rs:28:5
|
LL | Some(3).expect("Hello world!");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -17,7 +17,7 @@ LL | Some(3).expect("Hello world!");
= note: `-D clippy::expect-used` implied by `-D warnings`
error: used `unwrap()` on a `Result` value
--> $DIR/unwrap_expect_used.rs:32:5
--> $DIR/unwrap_expect_used.rs:43:5
|
LL | a.unwrap();
| ^^^^^^^^^^
@ -25,7 +25,7 @@ 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:33:5
--> $DIR/unwrap_expect_used.rs:44:5
|
LL | a.expect("Hello world!");
| ^^^^^^^^^^^^^^^^^^^^^^^^
@ -33,7 +33,7 @@ LL | a.expect("Hello world!");
= help: if this value is an `Err`, it will panic
error: used `unwrap_err()` on a `Result` value
--> $DIR/unwrap_expect_used.rs:34:5
--> $DIR/unwrap_expect_used.rs:45:5
|
LL | a.unwrap_err();
| ^^^^^^^^^^^^^^
@ -41,7 +41,7 @@ LL | a.unwrap_err();
= help: if this value is an `Ok`, it will panic
error: used `expect_err()` on a `Result` value
--> $DIR/unwrap_expect_used.rs:35:5
--> $DIR/unwrap_expect_used.rs:46:5
|
LL | a.expect_err("Hello error!");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^