Add expect

Co-Authored-By: Philipp Krones <hello@philkrones.com>

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index efa283b823d9af3e8f3c3a7e4c5af7fb917faf94..ac8a1dbdac3ff19962f1b833cbad0ebf281cfa0b 100644 (file)
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -73,7 +73,7 @@
     /// **Known problems:** None.
     ///
     /// **Example:**
-    /// Using unwrap on an `Option`:
+    /// Using unwrap on an `Result`:
     ///
     /// ```rust
     /// let res: Result<usize, ()> = Ok(1);
@@ -91,6 +91,63 @@
     "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! {
     /// **What it does:** Checks for methods that should live in a trait
     /// implementation of a `std` trait (see [llogiq's blog
@@ -1037,6 +1094,8 @@
 declare_lint_pass!(Methods => [
     OPTION_UNWRAP_USED,
     RESULT_UNWRAP_USED,
+    OPTION_EXPECT_USED,
+    RESULT_EXPECT_USED,
     SHOULD_IMPLEMENT_TRAIT,
     WRONG_SELF_CONVENTION,
     WRONG_PUB_SELF_CONVENTION,
@@ -1095,6 +1154,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
             ["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
             ["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
             ["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_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]),
@@ -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
 fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
     if_chain! {

diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs
index 6981ecff0d012822d4a6d66081e5c70495c0b618..94efb5acc000da6aae35122d4e3740f3ef6b8122 100644 (file)
--- a/clippy_lints/src/panic_unimplemented.rs
+++ b/clippy_lints/src/panic_unimplemented.rs
@@ -76,7 +76,7 @@
 declare_clippy_lint! {
     /// **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.
     ///

diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs
index 54a58e0c86a800eaf929f4aba48016990ef05255..ca8358754ee9f8b1d79b39b4f9c1eaab16860bf1 100644 (file)
--- a/tests/ui/methods.rs
+++ b/tests/ui/methods.rs
@@ -1,7 +1,13 @@
 // aux-build:option_helpers.rs
 // 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(
     clippy::blacklisted_name,
     clippy::default_trait_access,