squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Auto merge of #7060 - daxpedda:debug-assert-panic-in-result-fn, r=flip1995

Browse source 
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: https://github.com/rust-lang/rust-clippy/issues/6082

r? `@flip1995`

changelog: Change `panic_in_result_fn` to ignore `debug_assert` and co macros
This commit is contained in:
bors 2021-04-10 19:15:37 +00:00
commit fd5cf4e82d
3 changed files with 12 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
clippy_lints/src/panic_in_result_fn.rs
View file

@ -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};
@ -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",
@ -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,

23
tests/ui/panic_in_result_fn_debug_assertions.rs
View file

@ -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)

57
tests/ui/panic_in_result_fn_debug_assertions.stderr
View file

@ -1,57 +0,0 @@
error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_debug_assertions.rs:7:5
|
LL | / fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint
LL | | {
LL | | debug_assert!(x == 5, "wrong argument");
LL | | Ok(true)
LL | | }
| |_____^
|
= note: `-D clippy::panic-in-result-fn` implied by `-D warnings`
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_debug_assertions.rs:9:9
|
LL | debug_assert!(x == 5, "wrong argument");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_debug_assertions.rs:13:5
|
LL | / fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint
LL | | {
LL | | debug_assert_eq!(x, 5);
LL | | Ok(true)
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_debug_assertions.rs:15:9
|
LL | debug_assert_eq!(x, 5);
| ^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_debug_assertions.rs:19:5
|
LL | / fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint
LL | | {
LL | | debug_assert_ne!(x, 1);
LL | | Ok(true)
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_debug_assertions.rs:21:9
|
LL | debug_assert_ne!(x, 1);
| ^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 3 previous errors