diff options
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 6 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 63 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 21 | ||||
| -rw-r--r-- | tests/ui/expect.rs | 2 | ||||
| -rw-r--r-- | tests/ui/expect.stderr | 3 |
6 files changed, 35 insertions, 63 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 78f98bba2b4..4eeb71fa5c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bb3bc0b4545..eaef1f543d3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 1af4d03c7a2..2e75de019b6 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -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. + /// `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. /// - /// **Example:** + /// **Known problems:** None. /// - /// Using expect on an `Option`: + /// **Examples:** + /// ```rust,ignore + /// # let opt = Some(1); /// - /// ```rust - /// let opt = Some(1); + /// // Bad /// opt.expect("one"); - /// ``` - /// - /// Better: /// - /// ```rust,ignore + /// // Good /// 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 `?` operator. - /// - /// **Known problems:** None. /// - /// **Example:** - /// Using expect on an `Result`: + /// // or /// /// ```rust - /// let res: Result<usize, ()> = Ok(1); - /// res.expect("one"); - /// ``` + /// # let res: Result<usize, ()> = Ok(1); /// - /// Better: + /// // Bad + /// res.expect("one"); /// - /// ```rust - /// let res: Result<usize, ()> = Ok(1); + /// // 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 }; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 5cbf3ef028c..4e79ce96bb5 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -515,6 +515,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ 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", desc: "implementing `Clone` explicitly on `Copy` types", @@ -1600,13 +1607,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ 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", desc: "using `Option.map_or(None, f)`, which is more succinctly expressed as `and_then(f)`", @@ -1866,13 +1866,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ 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", desc: "using `Result.map_or(None, Some)`, which is more succinctly expressed as `ok()`", diff --git a/tests/ui/expect.rs b/tests/ui/expect.rs index 0bd4252c49a..1073acf6f0c 100644 --- a/tests/ui/expect.rs +++ b/tests/ui/expect.rs @@ -1,4 +1,4 @@ -#![warn(clippy::option_expect_used, clippy::result_expect_used)] +#![warn(clippy::expect_used)] fn expect_option() { let opt = Some(0); diff --git a/tests/ui/expect.stderr b/tests/ui/expect.stderr index adf9f4f1921..9d3fc7df15c 100644 --- a/tests/ui/expect.stderr +++ b/tests/ui/expect.stderr @@ -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 |