Auto merge of #4657 - Licenser:additional-restrictions, r=flip1995

Additional restrictions

Add restriction lints for `panic!`, `unreachable!`, `todo!` and `.expect(...)`

changelog: Add 5 new `restriction` lints: `panic`, `unreachable`, `todo`, `option_expect_used`, `result_expect_used`

CHANGELOG.md

@@ -1129,6 +1129,7 @@ Released 2018-09-13
 [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
 [`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
+[`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_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or
@@ -1138,6 +1139,7 @@ Released 2018-09-13
 [`or_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call
 [`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
 [`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
+[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
 [`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
@@ -1167,6 +1169,7 @@ Released 2018-09-13
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`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
+[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
@@ -1198,6 +1201,7 @@ Released 2018-09-13
 [`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
+[`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
 [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
 [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
@@ -1227,6 +1231,7 @@ Released 2018-09-13
 [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
 [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
 [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern
+[`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable
 [`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
 [`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name
 [`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 326 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 331 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -625,13 +625,18 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         mem_forget::MEM_FORGET,
         methods::CLONE_ON_REF_PTR,
         methods::GET_UNWRAP,
+        methods::OPTION_EXPECT_USED,
         methods::OPTION_UNWRAP_USED,
+        methods::RESULT_EXPECT_USED,
         methods::RESULT_UNWRAP_USED,
         methods::WRONG_PUB_SELF_CONVENTION,
         misc::FLOAT_CMP_CONST,
         missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
         missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
+        panic_unimplemented::PANIC,
+        panic_unimplemented::TODO,
         panic_unimplemented::UNIMPLEMENTED,
+        panic_unimplemented::UNREACHABLE,
         shadow::SHADOW_REUSE,
         shadow::SHADOW_SAME,
         strings::STRING_ADD,

clippy_lints/src/methods/mod.rs

@@ -73,7 +73,7 @@ declare_clippy_lint! {
     /// **Known problems:** None.
     ///
     /// **Example:**
-    /// Using unwrap on an `Option`:
+    /// Using unwrap on an `Result`:
     ///
     /// ```rust
     /// let res: Result<usize, ()> = Ok(1);
@@ -91,6 +91,65 @@ declare_clippy_lint! {
     "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:
+    ///
+    /// ```ignore
+    /// let opt = Some(1);
+    /// opt?;
+    /// # Some::<()>(())
+    /// ```
+    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:
+    ///
+    /// ```
+    /// let res: Result<usize, ()> = Ok(1);
+    /// res?;
+    /// # Ok::<(), ()>(())
+    /// ```
+    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 +1096,8 @@ declare_clippy_lint! {
 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 +1156,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["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 +2125,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! {

clippy_lints/src/panic_unimplemented.rs

@@ -25,6 +25,22 @@ declare_clippy_lint! {
     "missing parameters in `panic!` calls"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `panic!`.
+    ///
+    /// **Why is this bad?** `panic!` will stop the execution of the executable
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```no_run
+    /// panic!("even with a good reason");
+    /// ```
+    pub PANIC,
+    restriction,
+    "usage of the `panic!` macro"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `unimplemented!`.
     ///
@@ -41,7 +57,39 @@ declare_clippy_lint! {
     "`unimplemented!` should not be present in production code"
 }
 
-declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED]);
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `todo!`.
+    ///
+    /// **Why is this bad?** This macro should not be present in production code
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```no_run
+    /// todo!();
+    /// ```
+    pub TODO,
+    restriction,
+    "`todo!` should not be present in production code"
+}
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `unreachable!`.
+    ///
+    /// **Why is this bad?** This macro can cause code to panic
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```no_run
+    /// unreachable!();
+    /// ```
+    pub UNREACHABLE,
+    restriction,
+    "`unreachable!` should not be present in production code"
+}
+
+declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHABLE, TODO, PANIC]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@@ -55,7 +103,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented {
                     let span = get_outer_span(expr);
                     span_lint(cx, UNIMPLEMENTED, span,
                               "`unimplemented` should not be present in production code");
-                } else {
+                } else if is_expn_of(expr.span, "todo").is_some() {
+                    let span = get_outer_span(expr);
+                    span_lint(cx, TODO, span,
+                              "`todo` should not be present in production code");
+                } else if is_expn_of(expr.span, "unreachable").is_some() {
+                    let span = get_outer_span(expr);
+                    span_lint(cx, UNREACHABLE, span,
+                              "`unreachable` should not be present in production code");
+                } else if is_expn_of(expr.span, "panic").is_some() {
+                    let span = get_outer_span(expr);
+                    span_lint(cx, PANIC, span,
+                              "`panic` should not be present in production code");
                     match_panic(params, expr, cx);
                 }
             }

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 326] = [
+pub const ALL_LINTS: [Lint; 331] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1393,6 +1393,13 @@ pub const ALL_LINTS: [Lint; 326] = [
         deprecation: None,
         module: "methods",
     },
+    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",
@@ -1456,6 +1463,13 @@ pub const ALL_LINTS: [Lint; 326] = [
         deprecation: None,
         module: "overflow_check_conditional",
     },
+    Lint {
+        name: "panic",
+        group: "restriction",
+        desc: "usage of the `panic!` macro",
+        deprecation: None,
+        module: "panic_unimplemented",
+    },
     Lint {
         name: "panic_params",
         group: "style",
@@ -1652,6 +1666,13 @@ pub const ALL_LINTS: [Lint; 326] = [
         deprecation: None,
         module: "replace_consts",
     },
+    Lint {
+        name: "result_expect_used",
+        group: "restriction",
+        desc: "using `Result.expect()`, which might be better handled",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "result_map_unit_fn",
         group: "complexity",
@@ -1848,6 +1869,13 @@ pub const ALL_LINTS: [Lint; 326] = [
         deprecation: None,
         module: "methods",
     },
+    Lint {
+        name: "todo",
+        group: "restriction",
+        desc: "`todo!` should not be present in production code",
+        deprecation: None,
+        module: "panic_unimplemented",
+    },
     Lint {
         name: "too_many_arguments",
         group: "complexity",
@@ -2051,6 +2079,13 @@ pub const ALL_LINTS: [Lint; 326] = [
         deprecation: None,
         module: "misc_early",
     },
+    Lint {
+        name: "unreachable",
+        group: "restriction",
+        desc: "`unreachable!` should not be present in production code",
+        deprecation: None,
+        module: "panic_unimplemented",
+    },
     Lint {
         name: "unreadable_literal",
         group: "style",

tests/ui/expect.rs

@@ -0,0 +1,16 @@
+#![warn(clippy::option_expect_used, clippy::result_expect_used)]
+
+fn expect_option() {
+    let opt = Some(0);
+    let _ = opt.expect("");
+}
+
+fn expect_result() {
+    let res: Result<u8, ()> = Ok(0);
+    let _ = res.expect("");
+}
+
+fn main() {
+    expect_option();
+    expect_result();
+}

tests/ui/expect.stderr

@@ -0,0 +1,18 @@
+error: used expect() on an Option value. If this value is an None it will panic
+  --> $DIR/expect.rs:5:13
+   |
+LL |     let _ = opt.expect("");
+   |             ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::option-expect-used` implied by `-D warnings`
+
+error: used expect() on a Result value. If this value is an Err it will panic
+  --> $DIR/expect.rs:10:13
+   |
+LL |     let _ = res.expect("");
+   |             ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::result-expect-used` implied by `-D warnings`
+
+error: aborting due to 2 previous errors
+

tests/ui/methods.rs

@@ -1,7 +1,7 @@
 // aux-build:option_helpers.rs
 // compile-flags: --edition 2018
 
-#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)]
+#![warn(clippy::all, clippy::pedantic)]
 #![allow(
     clippy::blacklisted_name,
     clippy::default_trait_access,
@@ -301,8 +301,8 @@ fn search_is_some() {
     let _ = foo.rposition().is_some();
 }
 
-#[allow(clippy::similar_names)]
 fn main() {
-    let opt = Some(0);
+    option_methods();
-    let _ = opt.unwrap();
+    filter_next();
+    search_is_some();
 }

tests/ui/methods.stderr

@@ -206,13 +206,5 @@ LL | |                                }
 LL | |                    ).is_some();
    | |______________________________^
 
-error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
+error: aborting due to 23 previous errors
-  --> $DIR/methods.rs:307:13
-   |
-LL |     let _ = opt.unwrap();
-   |             ^^^^^^^^^^^^
-   |
-   = note: `-D clippy::option-unwrap-used` implied by `-D warnings`
-
-error: aborting due to 24 previous errors
 

tests/ui/panic.rs

@@ -1,4 +1,4 @@
-#![warn(clippy::panic_params, clippy::unimplemented)]
+#![warn(clippy::panic_params)]
 #![allow(clippy::assertions_on_constants)]
 fn missing() {
     if true {
@@ -50,12 +50,6 @@ fn ok_escaped() {
     panic!("{case }}");
 }
 
-fn unimplemented() {
-    let a = 2;
-    unimplemented!();
-    let b = a + 2;
-}
-
 fn main() {
     missing();
     ok_single();
@@ -64,5 +58,4 @@ fn main() {
     ok_inner();
     ok_nomsg();
     ok_escaped();
-    unimplemented();
 }

tests/ui/panic.stderr

@@ -1,5 +1,5 @@
 error: you probably are missing some parameter in your format string
-  --> $DIR/panic_unimplemented.rs:5:16
+  --> $DIR/panic.rs:5:16
    |
 LL |         panic!("{}");
    |                ^^^^
@@ -7,30 +7,22 @@ LL |         panic!("{}");
    = note: `-D clippy::panic-params` implied by `-D warnings`
 
 error: you probably are missing some parameter in your format string
-  --> $DIR/panic_unimplemented.rs:7:16
+  --> $DIR/panic.rs:7:16
    |
 LL |         panic!("{:?}");
    |                ^^^^^^
 
 error: you probably are missing some parameter in your format string
-  --> $DIR/panic_unimplemented.rs:9:23
+  --> $DIR/panic.rs:9:23
    |
 LL |         assert!(true, "here be missing values: {}");
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: you probably are missing some parameter in your format string
-  --> $DIR/panic_unimplemented.rs:12:12
+  --> $DIR/panic.rs:12:12
    |
 LL |     panic!("{{{this}}}");
    |            ^^^^^^^^^^^^
 
-error: `unimplemented` should not be present in production code
+error: aborting due to 4 previous errors
-  --> $DIR/panic_unimplemented.rs:55:5
-   |
-LL |     unimplemented!();
-   |     ^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::unimplemented` implied by `-D warnings`
-
-error: aborting due to 5 previous errors
 

tests/ui/panicking_macros.rs

@@ -0,0 +1,33 @@
+#![warn(clippy::unimplemented, clippy::unreachable, clippy::todo, clippy::panic)]
+#![allow(clippy::assertions_on_constants)]
+
+fn panic() {
+    let a = 2;
+    panic!();
+    let b = a + 2;
+}
+
+fn todo() {
+    let a = 2;
+    todo!();
+    let b = a + 2;
+}
+
+fn unimplemented() {
+    let a = 2;
+    unimplemented!();
+    let b = a + 2;
+}
+
+fn unreachable() {
+    let a = 2;
+    unreachable!();
+    let b = a + 2;
+}
+
+fn main() {
+    panic();
+    todo();
+    unimplemented();
+    unreachable();
+}

tests/ui/panicking_macros.stderr

@@ -0,0 +1,34 @@
+error: `panic` should not be present in production code
+  --> $DIR/panicking_macros.rs:6:5
+   |
+LL |     panic!();
+   |     ^^^^^^^^^
+   |
+   = note: `-D clippy::panic` implied by `-D warnings`
+
+error: `todo` should not be present in production code
+  --> $DIR/panicking_macros.rs:12:5
+   |
+LL |     todo!();
+   |     ^^^^^^^^
+   |
+   = note: `-D clippy::todo` implied by `-D warnings`
+
+error: `unimplemented` should not be present in production code
+  --> $DIR/panicking_macros.rs:18:5
+   |
+LL |     unimplemented!();
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::unimplemented` implied by `-D warnings`
+
+error: `unreachable` should not be present in production code
+  --> $DIR/panicking_macros.rs:24:5
+   |
+LL |     unreachable!();
+   |     ^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::unreachable` implied by `-D warnings`
+
+error: aborting due to 4 previous errors
+

tests/ui/unwrap.rs

@@ -0,0 +1,16 @@
+#![warn(clippy::option_unwrap_used, clippy::result_unwrap_used)]
+
+fn unwrap_option() {
+    let opt = Some(0);
+    let _ = opt.unwrap();
+}
+
+fn unwrap_result() {
+    let res: Result<u8, ()> = Ok(0);
+    let _ = res.unwrap();
+}
+
+fn main() {
+    unwrap_option();
+    unwrap_result();
+}

tests/ui/unwrap.stderr

@@ -0,0 +1,18 @@
+error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
+  --> $DIR/unwrap.rs:5:13
+   |
+LL |     let _ = opt.unwrap();
+   |             ^^^^^^^^^^^^
+   |
+   = note: `-D clippy::option-unwrap-used` implied by `-D warnings`
+
+error: used unwrap() on a Result value. If you don't want to handle the Err case gracefully, consider using expect() to provide a better panic message
+  --> $DIR/unwrap.rs:10:13
+   |
+LL |     let _ = res.unwrap();
+   |             ^^^^^^^^^^^^
+   |
+   = note: `-D clippy::result-unwrap-used` implied by `-D warnings`
+
+error: aborting due to 2 previous errors
+