Skip to content

Commit

Permalink
Auto merge of #7060 - daxpedda:debug-assert-panic-in-result-fn, r=fli…
Browse files Browse the repository at this point in the history
…p1995

Remove `debug_assert` from `panic_in_result_fn`

I couldn't find any documentation on `debug_assert` that should be remove.
In my humble opinion, I would also like to argue that `todo` and `unreachable` shouldn't trigger this lint?

Related: #6082

r? `@flip1995`

changelog: Change `panic_in_result_fn` to ignore `debug_assert` and co macros
  • Loading branch information
bors committed Apr 10, 2021
2 parents f7c2c44 + 271c163 commit fd5cf4e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 76 deletions.
8 changes: 3 additions & 5 deletions clippy_lints/src/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{find_macro_calls, return_ty};
use clippy_utils::{find_macro_calls, is_expn_of, return_ty};
use rustc_hir as hir;
use rustc_hir::intravisit::FnKind;
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -52,7 +52,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
}

fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) {
let panics = find_macro_calls(
let mut panics = find_macro_calls(
&[
"unimplemented",
"unreachable",
Expand All @@ -61,12 +61,10 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir
"assert",
"assert_eq",
"assert_ne",
"debug_assert",
"debug_assert_eq",
"debug_assert_ne",
],
body,
);
panics.retain(|span| is_expn_of(*span, "debug_assert").is_none());
if !panics.is_empty() {
span_lint_and_then(
cx,
Expand Down
23 changes: 9 additions & 14 deletions tests/ui/panic_in_result_fn_debug_assertions.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,39 @@
#![warn(clippy::panic_in_result_fn)]
#![allow(clippy::unnecessary_wraps)]

// debug_assert should never trigger the `panic_in_result_fn` lint

struct A;

impl A {
fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint
{
fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> {
debug_assert!(x == 5, "wrong argument");
Ok(true)
}

fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint
{
fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> {
debug_assert_eq!(x, 5);
Ok(true)
}

fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint
{
fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> {
debug_assert_ne!(x, 1);
Ok(true)
}

fn other_with_debug_assert_with_message(x: i32) // should not emit lint
{
fn other_with_debug_assert_with_message(x: i32) {
debug_assert!(x == 5, "wrong argument");
}

fn other_with_debug_assert_eq(x: i32) // should not emit lint
{
fn other_with_debug_assert_eq(x: i32) {
debug_assert_eq!(x, 5);
}

fn other_with_debug_assert_ne(x: i32) // should not emit lint
{
fn other_with_debug_assert_ne(x: i32) {
debug_assert_ne!(x, 1);
}

fn result_without_banned_functions() -> Result<bool, String> // should not emit lint
{
fn result_without_banned_functions() -> Result<bool, String> {
let debug_assert = "debug_assert!";
println!("No {}", debug_assert);
Ok(true)
Expand Down
57 changes: 0 additions & 57 deletions tests/ui/panic_in_result_fn_debug_assertions.stderr

This file was deleted.

0 comments on commit fd5cf4e

Please sign in to comment.