mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-23 13:13:34 +00:00
Add expect
Co-Authored-By: Philipp Krones <hello@philkrones.com>
This commit is contained in:
parent
98dc3aabea
commit
a7ad78f3eb
3 changed files with 94 additions and 3 deletions
|
@ -73,7 +73,7 @@ declare_clippy_lint! {
|
||||||
/// **Known problems:** None.
|
/// **Known problems:** None.
|
||||||
///
|
///
|
||||||
/// **Example:**
|
/// **Example:**
|
||||||
/// Using unwrap on an `Option`:
|
/// Using unwrap on an `Result`:
|
||||||
///
|
///
|
||||||
/// ```rust
|
/// ```rust
|
||||||
/// let res: Result<usize, ()> = Ok(1);
|
/// let res: Result<usize, ()> = Ok(1);
|
||||||
|
@ -91,6 +91,63 @@ declare_clippy_lint! {
|
||||||
"using `Result.unwrap()`, which might be better handled"
|
"using `Result.unwrap()`, which might be better handled"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
declare_clippy_lint! {
|
||||||
|
/// **What it does:** Checks for `.expect()` calls on `Option`s.
|
||||||
|
///
|
||||||
|
/// **Why is this bad?** Usually it is better to handle the `None` case. Still,
|
||||||
|
/// for a lot of quick-and-dirty code, `expect` is a good choice, which is why
|
||||||
|
/// this lint is `Allow` by default.
|
||||||
|
///
|
||||||
|
/// **Known problems:** None.
|
||||||
|
///
|
||||||
|
/// **Example:**
|
||||||
|
///
|
||||||
|
/// Using expect on an `Option`:
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// let opt = Some(1);
|
||||||
|
/// opt.expect("one");
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Better:
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// let opt = Some(1);
|
||||||
|
/// opt?;
|
||||||
|
/// ```
|
||||||
|
pub OPTION_EXPECT_USED,
|
||||||
|
restriction,
|
||||||
|
"using `Option.expect()`, which might be better handled"
|
||||||
|
}
|
||||||
|
|
||||||
|
declare_clippy_lint! {
|
||||||
|
/// **What it does:** Checks for `.expect()` calls on `Result`s.
|
||||||
|
///
|
||||||
|
/// **Why is this bad?** `result.expect()` will let the thread panic on `Err`
|
||||||
|
/// values. Normally, you want to implement more sophisticated error handling,
|
||||||
|
/// and propagate errors upwards with `try!`.
|
||||||
|
///
|
||||||
|
/// **Known problems:** None.
|
||||||
|
///
|
||||||
|
/// **Example:**
|
||||||
|
/// Using expect on an `Result`:
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// let res: Result<usize, ()> = Ok(1);
|
||||||
|
/// res.expect("one");
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Better:
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// let res: Result<usize, ()> = Ok(1);
|
||||||
|
/// res?;
|
||||||
|
/// ```
|
||||||
|
pub RESULT_EXPECT_USED,
|
||||||
|
restriction,
|
||||||
|
"using `Result.expect()`, which might be better handled"
|
||||||
|
}
|
||||||
|
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// **What it does:** Checks for methods that should live in a trait
|
/// **What it does:** Checks for methods that should live in a trait
|
||||||
/// implementation of a `std` trait (see [llogiq's blog
|
/// implementation of a `std` trait (see [llogiq's blog
|
||||||
|
@ -1037,6 +1094,8 @@ declare_clippy_lint! {
|
||||||
declare_lint_pass!(Methods => [
|
declare_lint_pass!(Methods => [
|
||||||
OPTION_UNWRAP_USED,
|
OPTION_UNWRAP_USED,
|
||||||
RESULT_UNWRAP_USED,
|
RESULT_UNWRAP_USED,
|
||||||
|
OPTION_EXPECT_USED,
|
||||||
|
RESULT_EXPECT_USED,
|
||||||
SHOULD_IMPLEMENT_TRAIT,
|
SHOULD_IMPLEMENT_TRAIT,
|
||||||
WRONG_SELF_CONVENTION,
|
WRONG_SELF_CONVENTION,
|
||||||
WRONG_PUB_SELF_CONVENTION,
|
WRONG_PUB_SELF_CONVENTION,
|
||||||
|
@ -1095,6 +1154,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
|
||||||
["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
|
["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
|
||||||
["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
|
["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
|
||||||
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
|
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
|
||||||
|
["expect", ..] => lint_expect(cx, expr, arg_lists[0]),
|
||||||
["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]),
|
["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]),
|
||||||
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
|
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
|
||||||
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
|
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
|
||||||
|
@ -2063,6 +2123,31 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, unwrap_args: &[hir::E
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// lint use of `expect()` for `Option`s and `Result`s
|
||||||
|
fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, expect_args: &[hir::Expr]) {
|
||||||
|
let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(&expect_args[0]));
|
||||||
|
|
||||||
|
let mess = if match_type(cx, obj_ty, &paths::OPTION) {
|
||||||
|
Some((OPTION_EXPECT_USED, "an Option", "None"))
|
||||||
|
} else if match_type(cx, obj_ty, &paths::RESULT) {
|
||||||
|
Some((RESULT_EXPECT_USED, "a Result", "Err"))
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
|
if let Some((lint, kind, none_value)) = mess {
|
||||||
|
span_lint(
|
||||||
|
cx,
|
||||||
|
lint,
|
||||||
|
expr.span,
|
||||||
|
&format!(
|
||||||
|
"used expect() on {} value. If this value is an {} it will panic",
|
||||||
|
kind, none_value
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// lint use of `ok().expect()` for `Result`s
|
/// lint use of `ok().expect()` for `Result`s
|
||||||
fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
|
fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
|
||||||
if_chain! {
|
if_chain! {
|
||||||
|
|
|
@ -76,7 +76,7 @@ declare_clippy_lint! {
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// **What it does:** Checks for usage of `unreachable!`.
|
/// **What it does:** Checks for usage of `unreachable!`.
|
||||||
///
|
///
|
||||||
/// **Why is this bad?** This macro can cause cause code to panics
|
/// **Why is this bad?** This macro can cause code to panic
|
||||||
///
|
///
|
||||||
/// **Known problems:** None.
|
/// **Known problems:** None.
|
||||||
///
|
///
|
||||||
|
|
|
@ -1,7 +1,13 @@
|
||||||
// aux-build:option_helpers.rs
|
// aux-build:option_helpers.rs
|
||||||
// compile-flags: --edition 2018
|
// compile-flags: --edition 2018
|
||||||
|
|
||||||
#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)]
|
#![warn(
|
||||||
|
clippy::all,
|
||||||
|
clippy::pedantic,
|
||||||
|
clippy::option_unwrap_used,
|
||||||
|
clippy::option_expect_used,
|
||||||
|
clippy::result_expect_used
|
||||||
|
)]
|
||||||
#![allow(
|
#![allow(
|
||||||
clippy::blacklisted_name,
|
clippy::blacklisted_name,
|
||||||
clippy::default_trait_access,
|
clippy::default_trait_access,
|
||||||
|
|
Loading…
Reference in a new issue