Merge option_expect_used and result_expect_used lints into expect_used lint

This commit is contained in:
ThibsG 2020-05-03 13:32:17 +02:00 committed by Thibaud Genty
parent bcf61666bd
commit 0e8be599cd
6 changed files with 38 additions and 66 deletions

View File

@ -1337,6 +1337,7 @@ Released 2018-09-13
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision
[`exit`]: https://rust-lang.github.io/rust-clippy/master/index.html#exit
[`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call
[`expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_used
[`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
@ -1497,7 +1498,6 @@ Released 2018-09-13
[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
[`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
[`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
[`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
[`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option
@ -1537,7 +1537,6 @@ Released 2018-09-13
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used

View File

@ -657,6 +657,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::CLONE_ON_COPY,
&methods::CLONE_ON_REF_PTR,
&methods::EXPECT_FUN_CALL,
&methods::EXPECT_USED,
&methods::FILETYPE_IS_FILE,
&methods::FILTER_MAP,
&methods::FILTER_MAP_NEXT,
@ -678,10 +679,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::OK_EXPECT,
&methods::OPTION_AND_THEN_SOME,
&methods::OPTION_AS_REF_DEREF,
&methods::OPTION_EXPECT_USED,
&methods::OPTION_MAP_OR_NONE,
&methods::OR_FUN_CALL,
&methods::RESULT_EXPECT_USED,
&methods::RESULT_MAP_OR_INTO_OPTION,
&methods::SEARCH_IS_SOME,
&methods::SHOULD_IMPLEMENT_TRAIT,
@ -1086,10 +1085,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
LintId::of(&mem_forget::MEM_FORGET),
LintId::of(&methods::CLONE_ON_REF_PTR),
LintId::of(&methods::EXPECT_USED),
LintId::of(&methods::FILETYPE_IS_FILE),
LintId::of(&methods::GET_UNWRAP),
LintId::of(&methods::OPTION_EXPECT_USED),
LintId::of(&methods::RESULT_EXPECT_USED),
LintId::of(&methods::UNWRAP_USED),
LintId::of(&methods::WRONG_PUB_SELF_CONVENTION),
LintId::of(&misc::FLOAT_CMP_CONST),

View File

@ -78,61 +78,45 @@ declare_clippy_lint! {
}
declare_clippy_lint! {
/// **What it does:** Checks for `.expect()` calls on `Option`s.
/// **What it does:** Checks for `.expect()` calls on `Option`s and `Result`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.
/// **Why is this bad?** Usually it is better to handle the `None` or `Err` 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,ignore
/// 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`
/// `result.expect()` will let the thread panic on `Err`
/// values. Normally, you want to implement more sophisticated error handling,
/// and propagate errors upwards with `?` operator.
///
/// **Known problems:** None.
///
/// **Example:**
/// Using expect on an `Result`:
/// **Examples:**
/// ```rust,ignore
/// # let opt = Some(1);
///
/// ```rust
/// let res: Result<usize, ()> = Ok(1);
/// res.expect("one");
/// // Bad
/// opt.expect("one");
///
/// // Good
/// let opt = Some(1);
/// opt?;
/// ```
///
/// Better:
/// // or
///
/// ```rust
/// let res: Result<usize, ()> = Ok(1);
/// # let res: Result<usize, ()> = Ok(1);
///
/// // Bad
/// res.expect("one");
///
/// // Good
/// res?;
/// # Ok::<(), ()>(())
/// ```
pub RESULT_EXPECT_USED,
pub EXPECT_USED,
restriction,
"using `Result.expect()`, which might be better handled"
"using `.expect()` on `Result` or `Option`, which might be better handled"
}
declare_clippy_lint! {
@ -1251,8 +1235,7 @@ declare_clippy_lint! {
declare_lint_pass!(Methods => [
UNWRAP_USED,
OPTION_EXPECT_USED,
RESULT_EXPECT_USED,
EXPECT_USED,
SHOULD_IMPLEMENT_TRAIT,
WRONG_SELF_CONVENTION,
WRONG_PUB_SELF_CONVENTION,
@ -2407,9 +2390,9 @@ fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, expect_args: &[hi
let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(&expect_args[0]));
let mess = if is_type_diagnostic_item(cx, obj_ty, sym!(option_type)) {
Some((OPTION_EXPECT_USED, "an Option", "None"))
Some((EXPECT_USED, "an Option", "None"))
} else if is_type_diagnostic_item(cx, obj_ty, sym!(result_type)) {
Some((RESULT_EXPECT_USED, "a Result", "Err"))
Some((EXPECT_USED, "a Result", "Err"))
} else {
None
};

View File

@ -514,6 +514,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "methods",
},
Lint {
name: "expect_used",
group: "restriction",
desc: "using `.expect()` on `Result` or `Option`, which might be better handled",
deprecation: None,
module: "methods",
},
Lint {
name: "expl_impl_clone_on_copy",
group: "pedantic",
@ -1599,13 +1606,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "option_env_unwrap",
},
Lint {
name: "option_expect_used",
group: "restriction",
desc: "using `Option.expect()`, which might be better handled",
deprecation: None,
module: "methods",
},
Lint {
name: "option_map_or_none",
group: "style",
@ -1865,13 +1865,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "matches",
},
Lint {
name: "result_expect_used",
group: "restriction",
desc: "using `Result.expect()`, which might be better handled",
deprecation: None,
module: "methods",
},
Lint {
name: "result_map_or_into_option",
group: "style",

View File

@ -1,4 +1,4 @@
#![warn(clippy::option_expect_used, clippy::result_expect_used)]
#![warn(clippy::expect_used)]
fn expect_option() {
let opt = Some(0);

View File

@ -4,7 +4,7 @@ error: used `expect()` on `an Option` value
LL | let _ = opt.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::option-expect-used` implied by `-D warnings`
= note: `-D clippy::expect-used` implied by `-D warnings`
= help: if this value is an `None`, it will panic
error: used `expect()` on `a Result` value
@ -13,7 +13,6 @@ error: used `expect()` on `a Result` value
LL | let _ = res.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::result-expect-used` implied by `-D warnings`
= help: if this value is an `Err`, it will panic
error: aborting due to 2 previous errors