Skip to content

Commit

Permalink
chore: make manual_assert use multipart suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
scottgerring committed Jan 31, 2025
1 parent d79f862 commit 5e448ee
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 57 deletions.
24 changes: 11 additions & 13 deletions clippy_lints/src/manual_assert.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{is_panic, root_macro_call};
use clippy_utils::source::{indent_of, reindent_multiline};
use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, UnOp};
Expand Down Expand Up @@ -62,25 +63,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
};
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" };
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");
// we show to the user the suggestion without the comments, but when applying the fix, include the
// comments in the block
let base_sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");

let indent = indent_of(cx, expr.span);
let full_sugg = reindent_multiline(format!("{comments}{base_sugg}").into(), true, indent);

span_lint_and_then(
cx,
MANUAL_ASSERT,
expr.span,
"only a `panic!` in `if`-then statement",
|diag| {
// comments can be noisy, do not show them to the user
if !comments.is_empty() {
diag.tool_only_span_suggestion(
expr.span.shrink_to_lo(),
"add comments back",
comments,
applicability,
);
}
diag.span_suggestion(expr.span, "try instead", sugg, applicability);
diag.multipart_suggestion(
"replace `if`-then-`panic!` with `assert!`",
vec![(expr.span, full_sugg.into())],
applicability,
);
},
);
}
Expand Down
76 changes: 76 additions & 0 deletions tests/ui/manual_assert.edition2018.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//@revisions: edition2018 edition2021
//@[edition2018] edition:2018
//@[edition2021] edition:2021

#![warn(clippy::manual_assert)]
#![allow(dead_code, unused_doc_comments)]
#![allow(clippy::nonminimal_bool, clippy::uninlined_format_args, clippy::useless_vec)]

macro_rules! one {
() => {
1
};
}

fn main() {
let a = vec![1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c.is_some()
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
println!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
println!("qwq");
}
let b = vec![1, 2, 3];
assert!(!b.is_empty(), "panic1");
assert!(!(b.is_empty() && a.is_empty()), "panic2");
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
if a.is_empty() {
let _ = 0;
} else if a.len() == 1 {
panic!("panic6");
}
}

fn issue7730(a: u8) {
// Suggestion should preserve comment
// comment
/* this is a
multiline
comment */
/// Doc comment
// comment after `panic!`
assert!(!(a > 2), "panic with comment");
}

fn issue12505() {
struct Foo<T, const N: usize>(T);

impl<T, const N: usize> Foo<T, N> {
const BAR: () = assert!(!(N == 0), );
}
}
48 changes: 27 additions & 21 deletions tests/ui/manual_assert.edition2018.stderr
Original file line number Diff line number Diff line change
@@ -1,72 +1,72 @@
error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:32:5
--> tests/ui/manual_assert.rs:30:5
|
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(a.is_empty(), "qaqaq{:?}", a);`
|
= note: `-D clippy::manual-assert` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_assert)]`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:35:5
--> tests/ui/manual_assert.rs:33:5
|
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(a.is_empty(), "qwqwq");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:52:5
--> tests/ui/manual_assert.rs:50:5
|
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!b.is_empty(), "panic1");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:55:5
--> tests/ui/manual_assert.rs:53:5
|
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:58:5
--> tests/ui/manual_assert.rs:56:5
|
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:61:5
--> tests/ui/manual_assert.rs:59:5
|
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:64:5
--> tests/ui/manual_assert.rs:62:5
|
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:67:5
--> tests/ui/manual_assert.rs:65:5
|
LL | / if a.is_empty() {
LL | | panic!("with expansion {}", one!())
LL | | }
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!a.is_empty(), "with expansion {}", one!());`

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:79:5
--> tests/ui/manual_assert.rs:77:5
|
LL | / if a > 2 {
LL | | // comment
Expand All @@ -77,19 +77,25 @@ LL | | panic!("panic with comment") // comment after `panic!`
LL | | }
| |_____^
|
help: try instead
help: replace `if`-then-`panic!` with `assert!`
|
LL | assert!(!(a > 2), "panic with comment");
LL ~ // comment
LL + /* this is a
LL + multiline
LL + comment */
LL + /// Doc comment
LL + // comment after `panic!`
LL + assert!(!(a > 2), "panic with comment");
|

error: only a `panic!` in `if`-then statement
--> tests/ui/manual_assert.rs:93:25
--> tests/ui/manual_assert.rs:91:25
|
LL | const BAR: () = if N == 0 {
| _________________________^
LL | | panic!()
LL | | };
| |_________^ help: try instead: `assert!(!(N == 0), )`
| |_________^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(N == 0), )`

error: aborting due to 10 previous errors

76 changes: 76 additions & 0 deletions tests/ui/manual_assert.edition2021.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//@revisions: edition2018 edition2021
//@[edition2018] edition:2018
//@[edition2021] edition:2021

#![warn(clippy::manual_assert)]
#![allow(dead_code, unused_doc_comments)]
#![allow(clippy::nonminimal_bool, clippy::uninlined_format_args, clippy::useless_vec)]

macro_rules! one {
() => {
1
};
}

fn main() {
let a = vec![1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c.is_some()
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
println!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
println!("qwq");
}
let b = vec![1, 2, 3];
assert!(!b.is_empty(), "panic1");
assert!(!(b.is_empty() && a.is_empty()), "panic2");
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
if a.is_empty() {
let _ = 0;
} else if a.len() == 1 {
panic!("panic6");
}
}

fn issue7730(a: u8) {
// Suggestion should preserve comment
// comment
/* this is a
multiline
comment */
/// Doc comment
// comment after `panic!`
assert!(!(a > 2), "panic with comment");
}

fn issue12505() {
struct Foo<T, const N: usize>(T);

impl<T, const N: usize> Foo<T, N> {
const BAR: () = assert!(!(N == 0), );
}
}
Loading

0 comments on commit 5e448ee

Please sign in to comment.