Skip to content

Commit 5e448ee

Browse files
scottgerringscottgerring
committed
chore: make manual_assert use multipart suggestions
1 parent d79f862 commit 5e448ee

6 files changed

+217
-57
lines changed

‎clippy_lints/src/manual_assert.rs

+11-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::macros::{is_panic, root_macro_call};
3+
use clippy_utils::source::{indent_of, reindent_multiline};
34
use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg};
45
use rustc_errors::Applicability;
56
use rustc_hir::{Expr, ExprKind, UnOp};
@@ -62,25 +63,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
6263
};
6364
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
6465
let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" };
65-
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");
66-
// we show to the user the suggestion without the comments, but when applying the fix, include the
67-
// comments in the block
66+
let base_sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");
67+
68+
let indent = indent_of(cx, expr.span);
69+
let full_sugg = reindent_multiline(format!("{comments}{base_sugg}").into(), true, indent);
70+
6871
span_lint_and_then(
6972
cx,
7073
MANUAL_ASSERT,
7174
expr.span,
7275
"only a `panic!` in `if`-then statement",
7376
|diag| {
74-
// comments can be noisy, do not show them to the user
75-
if !comments.is_empty() {
76-
diag.tool_only_span_suggestion(
77-
expr.span.shrink_to_lo(),
78-
"add comments back",
79-
comments,
80-
applicability,
81-
);
82-
}
83-
diag.span_suggestion(expr.span, "try instead", sugg, applicability);
77+
diag.multipart_suggestion(
78+
"replace `if`-then-`panic!` with `assert!`",
79+
vec![(expr.span, full_sugg.into())],
80+
applicability,
81+
);
8482
},
8583
);
8684
}

‎tests/ui/manual_assert.edition2018.fixed

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//@revisions: edition2018 edition2021
2+
//@[edition2018] edition:2018
3+
//@[edition2021] edition:2021
4+
5+
#![warn(clippy::manual_assert)]
6+
#![allow(dead_code, unused_doc_comments)]
7+
#![allow(clippy::nonminimal_bool, clippy::uninlined_format_args, clippy::useless_vec)]
8+
9+
macro_rules! one {
10+
() => {
11+
1
12+
};
13+
}
14+
15+
fn main() {
16+
let a = vec![1, 2, 3];
17+
let c = Some(2);
18+
if !a.is_empty()
19+
&& a.len() == 3
20+
&& c.is_some()
21+
&& !a.is_empty()
22+
&& a.len() == 3
23+
&& !a.is_empty()
24+
&& a.len() == 3
25+
&& !a.is_empty()
26+
&& a.len() == 3
27+
{
28+
panic!("qaqaq{:?}", a);
29+
}
30+
assert!(a.is_empty(), "qaqaq{:?}", a);
31+
assert!(a.is_empty(), "qwqwq");
32+
if a.len() == 3 {
33+
println!("qwq");
34+
println!("qwq");
35+
println!("qwq");
36+
}
37+
if let Some(b) = c {
38+
panic!("orz {}", b);
39+
}
40+
if a.len() == 3 {
41+
panic!("qaqaq");
42+
} else {
43+
println!("qwq");
44+
}
45+
let b = vec![1, 2, 3];
46+
assert!(!b.is_empty(), "panic1");
47+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
48+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
49+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
50+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
51+
assert!(!a.is_empty(), "with expansion {}", one!());
52+
if a.is_empty() {
53+
let _ = 0;
54+
} else if a.len() == 1 {
55+
panic!("panic6");
56+
}
57+
}
58+
59+
fn issue7730(a: u8) {
60+
// Suggestion should preserve comment
61+
// comment
62+
/* this is a
63+
multiline
64+
comment */
65+
/// Doc comment
66+
// comment after `panic!`
67+
assert!(!(a > 2), "panic with comment");
68+
}
69+
70+
fn issue12505() {
71+
struct Foo<T, const N: usize>(T);
72+
73+
impl<T, const N: usize> Foo<T, N> {
74+
const BAR: () = assert!(!(N == 0), );
75+
}
76+
}

‎tests/ui/manual_assert.edition2018.stderr

+27-21
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,72 @@
11
error: only a `panic!` in `if`-then statement
2-
--> tests/ui/manual_assert.rs:32:5
2+
--> tests/ui/manual_assert.rs:30:5
33
|
44
LL | / if !a.is_empty() {
55
LL | | panic!("qaqaq{:?}", a);
66
LL | | }
7-
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
7+
| |_____^ help: replace `if`-then-`panic!` with `assert!`: `assert!(a.is_empty(), "qaqaq{:?}", a);`
88
|
99
= note: `-D clippy::manual-assert` implied by `-D warnings`
1010
= help: to override `-D warnings` add `#[allow(clippy::manual_assert)]`
1111

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

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

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

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

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

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

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

6868
error: only a `panic!` in `if`-then statement
69-
--> tests/ui/manual_assert.rs:79:5
69+
--> tests/ui/manual_assert.rs:77:5
7070
|
7171
LL | / if a > 2 {
7272
LL | | // comment
@@ -77,19 +77,25 @@ LL | | panic!("panic with comment") // comment after `panic!`
7777
LL | | }
7878
| |_____^
7979
|
80-
help: try instead
80+
help: replace `if`-then-`panic!` with `assert!`
8181
|
82-
LL | assert!(!(a > 2), "panic with comment");
82+
LL ~ // comment
83+
LL + /* this is a
84+
LL + multiline
85+
LL + comment */
86+
LL + /// Doc comment
87+
LL + // comment after `panic!`
88+
LL + assert!(!(a > 2), "panic with comment");
8389
|
8490

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

94100
error: aborting due to 10 previous errors
95101

‎tests/ui/manual_assert.edition2021.fixed

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//@revisions: edition2018 edition2021
2+
//@[edition2018] edition:2018
3+
//@[edition2021] edition:2021
4+
5+
#![warn(clippy::manual_assert)]
6+
#![allow(dead_code, unused_doc_comments)]
7+
#![allow(clippy::nonminimal_bool, clippy::uninlined_format_args, clippy::useless_vec)]
8+
9+
macro_rules! one {
10+
() => {
11+
1
12+
};
13+
}
14+
15+
fn main() {
16+
let a = vec![1, 2, 3];
17+
let c = Some(2);
18+
if !a.is_empty()
19+
&& a.len() == 3
20+
&& c.is_some()
21+
&& !a.is_empty()
22+
&& a.len() == 3
23+
&& !a.is_empty()
24+
&& a.len() == 3
25+
&& !a.is_empty()
26+
&& a.len() == 3
27+
{
28+
panic!("qaqaq{:?}", a);
29+
}
30+
assert!(a.is_empty(), "qaqaq{:?}", a);
31+
assert!(a.is_empty(), "qwqwq");
32+
if a.len() == 3 {
33+
println!("qwq");
34+
println!("qwq");
35+
println!("qwq");
36+
}
37+
if let Some(b) = c {
38+
panic!("orz {}", b);
39+
}
40+
if a.len() == 3 {
41+
panic!("qaqaq");
42+
} else {
43+
println!("qwq");
44+
}
45+
let b = vec![1, 2, 3];
46+
assert!(!b.is_empty(), "panic1");
47+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
48+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
49+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
50+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
51+
assert!(!a.is_empty(), "with expansion {}", one!());
52+
if a.is_empty() {
53+
let _ = 0;
54+
} else if a.len() == 1 {
55+
panic!("panic6");
56+
}
57+
}
58+
59+
fn issue7730(a: u8) {
60+
// Suggestion should preserve comment
61+
// comment
62+
/* this is a
63+
multiline
64+
comment */
65+
/// Doc comment
66+
// comment after `panic!`
67+
assert!(!(a > 2), "panic with comment");
68+
}
69+
70+
fn issue12505() {
71+
struct Foo<T, const N: usize>(T);
72+
73+
impl<T, const N: usize> Foo<T, N> {
74+
const BAR: () = assert!(!(N == 0), );
75+
}
76+
}

0 commit comments

Comments
 (0)