Skip to content

Commit 0f8c62c

Browse files
scottgerringscottgerring
committed
chore: make manual_assert use multipart suggestions
1 parent 49ae1d4 commit 0f8c62c

File tree

6 files changed

+250
-77
lines changed

6 files changed

+250
-77
lines changed

‎clippy_lints/src/manual_assert.rs‎

Lines changed: 10 additions & 13 deletions
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::{higher, 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};
@@ -57,25 +58,21 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
5758
}
5859
let cond_sugg = !sugg::Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "..", &mut applicability);
5960
let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" };
60-
let sugg = format!("assert!({cond_sugg}, {format_args_snip}){semicolon}");
61-
// we show to the user the suggestion without the comments, but when applying the fix, include the
62-
// comments in the block
61+
let base_sugg = format!("assert!({cond_sugg}, {format_args_snip}){semicolon}");
62+
63+
let indent = indent_of(cx, expr.span);
64+
let full_sugg = reindent_multiline(format!("{comments}{base_sugg}").into(), true, indent);
6365
span_lint_and_then(
6466
cx,
6567
MANUAL_ASSERT,
6668
expr.span,
6769
"only a `panic!` in `if`-then statement",
6870
|diag| {
69-
// comments can be noisy, do not show them to the user
70-
if !comments.is_empty() {
71-
diag.tool_only_span_suggestion(
72-
expr.span.shrink_to_lo(),
73-
"add comments back",
74-
comments,
75-
applicability,
76-
);
77-
}
78-
diag.span_suggestion(expr.span, "try instead", sugg, applicability);
71+
diag.multipart_suggestion(
72+
"replace `if`-then-`panic!` with `assert!`",
73+
vec![(expr.span, full_sugg.into())],
74+
applicability,
75+
);
7976
},
8077
);
8178
}

‎tests/ui/manual_assert.edition2018.fixed‎

Lines changed: 76 additions & 0 deletions
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‎

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
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 | |
@@ -9,7 +9,7 @@ LL | | }
99
|
1010
= note: `-D clippy::manual-assert` implied by `-D warnings`
1111
= help: to override `-D warnings` add `#[allow(clippy::manual_assert)]`
12-
help: try instead
12+
help: replace `if`-then-`panic!` with `assert!`
1313
|
1414
LL - if !a.is_empty() {
1515
LL -
@@ -19,15 +19,15 @@ LL + assert!(a.is_empty(), "qaqaq{:?}", a);
1919
|
2020

2121
error: only a `panic!` in `if`-then statement
22-
--> tests/ui/manual_assert.rs:36:5
22+
--> tests/ui/manual_assert.rs:33:5
2323
|
2424
LL | / if !a.is_empty() {
2525
LL | |
2626
LL | | panic!("qwqwq");
2727
LL | | }
2828
| |_____^
2929
|
30-
help: try instead
30+
help: replace `if`-then-`panic!` with `assert!`
3131
|
3232
LL - if !a.is_empty() {
3333
LL -
@@ -37,15 +37,15 @@ LL + assert!(a.is_empty(), "qwqwq");
3737
|
3838

3939
error: only a `panic!` in `if`-then statement
40-
--> tests/ui/manual_assert.rs:54:5
40+
--> tests/ui/manual_assert.rs:50:5
4141
|
4242
LL | / if b.is_empty() {
4343
LL | |
4444
LL | | panic!("panic1");
4545
LL | | }
4646
| |_____^
4747
|
48-
help: try instead
48+
help: replace `if`-then-`panic!` with `assert!`
4949
|
5050
LL - if b.is_empty() {
5151
LL -
@@ -55,15 +55,31 @@ LL + assert!(!b.is_empty(), "panic1");
5555
|
5656

5757
error: only a `panic!` in `if`-then statement
58-
--> tests/ui/manual_assert.rs:58:5
58+
--> tests/ui/manual_assert.rs:53:5
59+
|
60+
LL | / if b.is_empty() && a.is_empty() {
61+
LL | | panic!("panic2");
62+
LL | | }
63+
| |_____^
64+
|
65+
help: replace `if`-then-`panic!` with `assert!`
66+
|
67+
LL - if b.is_empty() && a.is_empty() {
68+
LL - panic!("panic2");
69+
LL - }
70+
LL + assert!(!(b.is_empty() && a.is_empty()), "panic2");
71+
|
72+
73+
error: only a `panic!` in `if`-then statement
74+
--> tests/ui/manual_assert.rs:56:5
5975
|
6076
LL | / if b.is_empty() && a.is_empty() {
6177
LL | |
6278
LL | | panic!("panic2");
6379
LL | | }
6480
| |_____^
6581
|
66-
help: try instead
82+
help: replace `if`-then-`panic!` with `assert!`
6783
|
6884
LL - if b.is_empty() && a.is_empty() {
6985
LL -
@@ -81,7 +97,7 @@ LL | | panic!("panic3");
8197
LL | | }
8298
| |_____^
8399
|
84-
help: try instead
100+
help: replace `if`-then-`panic!` with `assert!`
85101
|
86102
LL - if a.is_empty() && !b.is_empty() {
87103
LL -
@@ -91,15 +107,15 @@ LL + assert!(!(a.is_empty() && !b.is_empty()), "panic3");
91107
|
92108

93109
error: only a `panic!` in `if`-then statement
94-
--> tests/ui/manual_assert.rs:66:5
110+
--> tests/ui/manual_assert.rs:59:5
95111
|
96112
LL | / if b.is_empty() || a.is_empty() {
97113
LL | |
98114
LL | | panic!("panic4");
99115
LL | | }
100116
| |_____^
101117
|
102-
help: try instead
118+
help: replace `if`-then-`panic!` with `assert!`
103119
|
104120
LL - if b.is_empty() || a.is_empty() {
105121
LL -
@@ -109,15 +125,15 @@ LL + assert!(!(b.is_empty() || a.is_empty()), "panic4");
109125
|
110126

111127
error: only a `panic!` in `if`-then statement
112-
--> tests/ui/manual_assert.rs:70:5
128+
--> tests/ui/manual_assert.rs:62:5
113129
|
114130
LL | / if a.is_empty() || !b.is_empty() {
115131
LL | |
116132
LL | | panic!("panic5");
117133
LL | | }
118134
| |_____^
119135
|
120-
help: try instead
136+
help: replace `if`-then-`panic!` with `assert!`
121137
|
122138
LL - if a.is_empty() || !b.is_empty() {
123139
LL -
@@ -127,15 +143,15 @@ LL + assert!(!(a.is_empty() || !b.is_empty()), "panic5");
127143
|
128144

129145
error: only a `panic!` in `if`-then statement
130-
--> tests/ui/manual_assert.rs:74:5
146+
--> tests/ui/manual_assert.rs:65:5
131147
|
132148
LL | / if a.is_empty() {
133149
LL | |
134150
LL | | panic!("with expansion {}", one!())
135151
LL | | }
136152
| |_____^
137153
|
138-
help: try instead
154+
help: replace `if`-then-`panic!` with `assert!`
139155
|
140156
LL - if a.is_empty() {
141157
LL -
@@ -145,7 +161,7 @@ LL + assert!(!a.is_empty(), "with expansion {}", one!());
145161
|
146162

147163
error: only a `panic!` in `if`-then statement
148-
--> tests/ui/manual_assert.rs:87:5
164+
--> tests/ui/manual_assert.rs:77:5
149165
|
150166
LL | / if a > 2 {
151167
LL | |
@@ -156,22 +172,19 @@ LL | | panic!("panic with comment") // comment after `panic!`
156172
LL | | }
157173
| |_____^
158174
|
159-
help: try instead
175+
help: replace `if`-then-`panic!` with `assert!`
160176
|
161-
LL - if a > 2 {
162-
LL -
163-
LL - // comment
164-
LL - /* this is a
165-
LL - multiline
166-
LL - comment */
167-
LL - /// Doc comment
168-
LL - panic!("panic with comment") // comment after `panic!`
169-
LL - }
170-
LL + assert!(a <= 2, "panic with comment");
177+
LL ~ // comment
178+
LL + /* this is a
179+
LL + multiline
180+
LL + comment */
181+
LL + /// Doc comment
182+
LL + // comment after `panic!`
183+
LL + assert!(!(a > 2), "panic with comment");
171184
|
172185

173186
error: only a `panic!` in `if`-then statement
174-
--> tests/ui/manual_assert.rs:102:25
187+
--> tests/ui/manual_assert.rs:91:25
175188
|
176189
LL | const BAR: () = if N == 0 {
177190
| _________________________^
@@ -180,13 +193,13 @@ LL | | panic!()
180193
LL | | };
181194
| |_________^
182195
|
183-
help: try instead
196+
help: replace `if`-then-`panic!` with `assert!`
184197
|
185198
LL - const BAR: () = if N == 0 {
186199
LL -
187200
LL - panic!()
188201
LL - };
189-
LL + const BAR: () = assert!(N != 0, );
202+
LL + const BAR: () = assert!(!(N == 0), );
190203
|
191204

192205
error: only a `panic!` in `if`-then statement
@@ -198,7 +211,7 @@ LL | | panic!("SSSE3 is not supported");
198211
LL | | }
199212
| |_____^
200213
|
201-
help: try instead
214+
help: replace `if`-then-`panic!` with `assert!`
202215
|
203216
LL - if !is_x86_feature_detected!("ssse3") {
204217
LL -

0 commit comments

Comments
 (0)