Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7674,6 +7674,7 @@ Released 2018-09-13
[`allow-large-stack-frames-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-large-stack-frames-in-tests
[`allow-mixed-uninlined-format-args`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-mixed-uninlined-format-args
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
[`allow-panic-in-result-fn-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-result-fn-in-tests
[`allow-panic-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-tests
[`allow-print-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-print-in-tests
[`allow-private-module-inception`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-private-module-inception
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ Whether to allow `r#""#` when `r""` can be used
* [`needless_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_string_hashes)


## `allow-panic-in-result-fn-in-tests`
Whether `panic_in_result_fn` should be allowed in test functions or `#[cfg(test)]`

**Default Value:** `false`

---
**Affected lints:**
* [`panic_in_result_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn)


## `allow-panic-in-tests`
Whether `panic` should be allowed in test functions or `#[cfg(test)]`

Expand Down
3 changes: 3 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ define_Conf! {
/// Whether to allow `r#""#` when `r""` can be used
#[lints(needless_raw_string_hashes)]
allow_one_hash_in_raw_strings("allow-one-hash-in-raw-strings"): bool = false,
/// Whether `panic_in_result_fn` should be allowed in test functions or `#[cfg(test)]`
#[lints(panic_in_result_fn)]
allow_panic_in_result_fn_in_tests("allow-panic-in-result-fn-in-tests"): bool = false,
/// Whether `panic` should be allowed in test functions or `#[cfg(test)]`
#[lints(panic)]
allow_panic_in_tests("allow-panic-in-tests"): bool = false,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ rustc_lint::late_lint_methods!(
IfNotElse: if_not_else::IfNotElse = if_not_else::IfNotElse,
PatternEquality: equatable_if_let::PatternEquality = equatable_if_let::PatternEquality,
ManualAsyncFn: manual_async_fn::ManualAsyncFn = manual_async_fn::ManualAsyncFn,
PanicInResultFn: panic_in_result_fn::PanicInResultFn = panic_in_result_fn::PanicInResultFn,
PanicInResultFn: panic_in_result_fn::PanicInResultFn = panic_in_result_fn::PanicInResultFn::new(conf),
MacroUseImports: macro_use::MacroUseImports = <macro_use::MacroUseImports>::default(),
PatternTypeMismatch: pattern_type_mismatch::PatternTypeMismatch = pattern_type_mismatch::PatternTypeMismatch,
UnwrapInResult: unwrap_in_result::UnwrapInResult = <unwrap_in_result::UnwrapInResult>::default(),
Expand Down
31 changes: 25 additions & 6 deletions clippy_lints/src/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
use clippy_utils::res::MaybeDef as _;
use clippy_utils::visitors::{Descend, for_each_expr};
use clippy_utils::{is_inside_always_const_context, return_ty};
use clippy_utils::{is_in_test, is_inside_always_const_context, return_ty};
use core::ops::ControlFlow;
use rustc_hir as hir;
use rustc_hir::intravisit::FnKind;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, sym};

Expand Down Expand Up @@ -40,7 +41,19 @@ declare_clippy_lint! {
"functions of type `Result<..>` that contain `panic!()` or assertion"
}

declare_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]);
impl_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]);

pub struct PanicInResultFn {
allow_panic_in_result_fn_in_tests: bool,
}

impl PanicInResultFn {
pub fn new(conf: &'static Conf) -> Self {
Self {
allow_panic_in_result_fn_in_tests: conf.allow_panic_in_result_fn_in_tests,
}
}
}

impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
fn check_fn(
Expand All @@ -57,18 +70,24 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
}
let owner = cx.tcx.local_def_id_to_hir_id(def_id).expect_owner();
if return_ty(cx, owner).is_diag_item(cx, sym::Result) {
lint_impl_body(cx, span, body);
lint_impl_body(cx, span, body, self.allow_panic_in_result_fn_in_tests);
}
}
}

fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) {
fn lint_impl_body<'tcx>(
cx: &LateContext<'tcx>,
impl_span: Span,
body: &'tcx hir::Body<'tcx>,
allow_panic_in_result_fn_in_tests: bool,
) {
let mut panics = Vec::new();
let _: Option<!> = for_each_expr(cx.tcx, body.value, |e| {
let Some(macro_call) = root_macro_call_first_node(cx, e) else {
return ControlFlow::Continue(Descend::Yes);
};
if !is_inside_always_const_context(cx.tcx, e.hir_id)
if !(is_inside_always_const_context(cx.tcx, e.hir_id)
|| allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))
Comment on lines +89 to +90

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this bracketed the way you want? Did you not intend this?

likely with some extracting of variables, because the brackets are getting overbearing.

Suggested change
if !(is_inside_always_const_context(cx.tcx, e.hir_id)
|| allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))
if (!(is_inside_always_const_context(cx.tcx, e.hir_id)
|| (allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))))

&& (is_panic(cx, macro_call.def_id)
|| matches!(
cx.tcx.get_diagnostic_name(macro_call.def_id),
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/panic_in_result_fn/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-panic-in-result-fn-in-tests = true
24 changes: 24 additions & 0 deletions tests/ui-toml/panic_in_result_fn/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//@compile-flags: --test
//@check-pass
#![warn(clippy::panic_in_result_fn)]
#![allow(clippy::unnecessary_wraps)]

#[test]
fn test_function() -> Result<(), ()> {
cfg_test_function()?;
assert!(std::hint::black_box(false));
tests::helper()
}

#[cfg(test)]
fn cfg_test_function() -> Result<(), ()> {
panic!();
}

#[cfg(test)]
mod tests {
pub(super) fn helper() -> Result<(), ()> {
assert_eq!(std::hint::black_box(1), 2);
Ok(())
}
}
1 change: 1 addition & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ LL | foobar = 42
allow-large-stack-frames-in-tests
allow-mixed-uninlined-format-args
allow-one-hash-in-raw-strings
allow-panic-in-result-fn-in-tests
allow-panic-in-tests
allow-print-in-tests
allow-private-module-inception
Expand Down