diff --git a/CHANGELOG.md b/CHANGELOG.md index 137b561028a..7af3b666cca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1651,6 +1651,7 @@ Released 2018-09-13 [`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_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result [`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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 577ce6523b4..b70d126af5b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -267,6 +267,7 @@ mod open_options; mod option_env_unwrap; mod option_if_let_else; mod overflow_check_conditional; +mod panic_in_result; mod panic_unimplemented; mod partialeq_ne_impl; mod path_buf_push_overwrite; @@ -747,6 +748,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &option_env_unwrap::OPTION_ENV_UNWRAP, &option_if_let_else::OPTION_IF_LET_ELSE, &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, + &panic_in_result::PANIC_IN_RESULT, &panic_unimplemented::PANIC, &panic_unimplemented::PANIC_PARAMS, &panic_unimplemented::TODO, @@ -1086,6 +1088,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box manual_async_fn::ManualAsyncFn); store.register_early_pass(|| box redundant_field_names::RedundantFieldNames); store.register_late_pass(|| box vec_resize_to_zero::VecResizeToZero); + store.register_late_pass(|| box panic_in_result::PanicInResult); + let single_char_binding_names_threshold = conf.single_char_binding_names_threshold; store.register_early_pass(move || box non_expressive_names::NonExpressiveNames { single_char_binding_names_threshold, @@ -1128,6 +1132,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS), LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS), LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC), + LintId::of(&panic_in_result::PANIC_IN_RESULT), LintId::of(&panic_unimplemented::PANIC), LintId::of(&panic_unimplemented::TODO), LintId::of(&panic_unimplemented::UNIMPLEMENTED), diff --git a/clippy_lints/src/panic_in_result.rs b/clippy_lints/src/panic_in_result.rs new file mode 100644 index 00000000000..3a71a0db6fe --- /dev/null +++ b/clippy_lints/src/panic_in_result.rs @@ -0,0 +1,100 @@ +use crate::utils::{is_expn_of, is_type_diagnostic_item, return_ty, span_lint_and_then}; +use if_chain::if_chain; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// **What it does:** Checks for usage of `panic!`, `unimplemented!` or `unreachable!` in a function of type result/option. + /// + /// **Why is this bad?** For some codebases, + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// fn option_with_panic() -> Option // should emit lint + /// { + /// panic!("error"); + /// } + /// ``` + + pub PANIC_IN_RESULT, + restriction, + "functions of type `Result<..>` / `Option`<...> that contain `panic!()` or `unreachable()` or `unimplemented()` " +} + +declare_lint_pass!(PanicInResult => [PANIC_IN_RESULT]); + +impl<'tcx> LateLintPass<'tcx> for PanicInResult { + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) { + if_chain! { + // first check if it's a method or function + if let hir::ImplItemKind::Fn(ref _signature, _) = impl_item.kind; + // checking if its return type is `result` or `option` + if is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(result_type)) + || is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(option_type)); + then { + lint_impl_body(cx, impl_item.span, impl_item); + } + } + } +} + +use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; +use rustc_hir::{Expr, ImplItemKind}; + +struct FindPanicUnimplementedUnreachable { + result: Vec, +} + +impl<'tcx> Visitor<'tcx> for FindPanicUnimplementedUnreachable { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if is_expn_of(expr.span, "unimplemented").is_some() { + self.result.push(expr.span); + } else if is_expn_of(expr.span, "unreachable").is_some() { + self.result.push(expr.span); + } else if is_expn_of(expr.span, "panic").is_some() { + self.result.push(expr.span); + } + + // and check sub-expressions + intravisit::walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } +} + +fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { + if_chain! { + if let ImplItemKind::Fn(_, body_id) = impl_item.kind; + then { + let body = cx.tcx.hir().body(body_id); + let mut fpu = FindPanicUnimplementedUnreachable { + result: Vec::new(), + }; + fpu.visit_expr(&body.value); + + // if we've found one, lint + if !fpu.result.is_empty() { + span_lint_and_then( + cx, + PANIC_IN_RESULT, + impl_span, + "used unimplemented, unreachable or panic in a function that returns result or option", + move |diag| { + diag.help( + "unimplemented, unreachable or panic should not be used in a function that returns result or option" ); + diag.span_note(fpu.result, "will cause the application to crash."); + }); + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 687fac7baa8..ad57146048e 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1704,6 +1704,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "panic_unimplemented", }, + Lint { + name: "panic_in_result", + group: "restriction", + desc: "default lint description", + deprecation: None, + module: "panic_in_result", + }, Lint { name: "panic_params", group: "style", diff --git a/tests/ui/panic_in_result.rs b/tests/ui/panic_in_result.rs new file mode 100644 index 00000000000..21e9efca87b --- /dev/null +++ b/tests/ui/panic_in_result.rs @@ -0,0 +1,62 @@ +#![warn(clippy::panic_in_result)] + +struct A; + +impl A { + fn result_with_panic() -> Result // should emit lint + { + panic!("error"); + } + + fn result_with_unimplemented() -> Result // should emit lint + { + unimplemented!(); + } + + fn result_with_unreachable() -> Result // should emit lint + { + unreachable!(); + } + + fn option_with_unreachable() -> Option // should emit lint + { + unreachable!(); + } + + fn option_with_unimplemented() -> Option // should emit lint + { + unimplemented!(); + } + + fn option_with_panic() -> Option // should emit lint + { + panic!("error"); + } + + fn other_with_panic() // should not emit lint + { + panic!(""); + } + + fn other_with_unreachable() // should not emit lint + { + unreachable!(); + } + + fn other_with_unimplemented() // should not emit lint + { + unimplemented!(); + } + + fn result_without_banned_functions() -> Result // should not emit lint + { + Ok(true) + } + + fn option_without_banned_functions() -> Option // should not emit lint + { + Some(true) + } +} + +fn main() {} diff --git a/tests/ui/panic_in_result.stderr b/tests/ui/panic_in_result.stderr new file mode 100644 index 00000000000..74273bd9abb --- /dev/null +++ b/tests/ui/panic_in_result.stderr @@ -0,0 +1,105 @@ +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:6:5 + | +LL | / fn result_with_panic() -> Result // should emit lint +LL | | { +LL | | panic!("error"); +LL | | } + | |_____^ + | + = note: `-D clippy::panic-in-result` implied by `-D warnings` + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:8:9 + | +LL | panic!("error"); + | ^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:11:5 + | +LL | / fn result_with_unimplemented() -> Result // should emit lint +LL | | { +LL | | unimplemented!(); +LL | | } + | |_____^ + | + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:13:9 + | +LL | unimplemented!(); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:16:5 + | +LL | / fn result_with_unreachable() -> Result // should emit lint +LL | | { +LL | | unreachable!(); +LL | | } + | |_____^ + | + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:18:9 + | +LL | unreachable!(); + | ^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:21:5 + | +LL | / fn option_with_unreachable() -> Option // should emit lint +LL | | { +LL | | unreachable!(); +LL | | } + | |_____^ + | + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:23:9 + | +LL | unreachable!(); + | ^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:26:5 + | +LL | / fn option_with_unimplemented() -> Option // should emit lint +LL | | { +LL | | unimplemented!(); +LL | | } + | |_____^ + | + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:28:9 + | +LL | unimplemented!(); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used unimplemented, unreachable or panic in a function that returns result or option + --> $DIR/panic_in_result.rs:31:5 + | +LL | / fn option_with_panic() -> Option // should emit lint +LL | | { +LL | | panic!("error"); +LL | | } + | |_____^ + | + = help: unimplemented, unreachable or panic should not be used in a function that returns result or option +note: will cause the application to crash. + --> $DIR/panic_in_result.rs:33:9 + | +LL | panic!("error"); + | ^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 6 previous errors +