Skip to content

Commit a2618b9

Browse files
scottgerringscottgerring
committed
chore: use multipart_suggestions for manual_assert
1 parent 2550530 commit a2618b9

10 files changed

+506
-58
lines changed

‎clippy_lints/src/manual_assert.rs

+20-8
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
6262
};
6363
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
6464
let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" };
65-
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");
65+
let base_sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}");
6666
// we show to the user the suggestion without the comments, but when applying the fix, include the
6767
// comments in the block
6868
span_lint_and_then(
@@ -71,16 +71,28 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
7171
expr.span,
7272
"only a `panic!` in `if`-then statement",
7373
|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,
74+
// comments can be noisy. If we have them, provide a tool-only suggestion that
75+
// preserves them, but hides the code suggestion from the user.
76+
77+
if comments.is_empty() {
78+
// If we _don't_ have comments, we can provide a complete suggestion to the user inline
79+
diag.multipart_suggestion(
80+
"replace `if`-then-`panic!` with `assert!`",
81+
vec![(expr.span, base_sugg.clone())],
82+
applicability,
83+
);
84+
} else {
85+
// If we _do_ have comments, provide a description of the change to the user, and leave
86+
// the complete, comment-preserving suggestion to the tooling
87+
diag.tool_only_multipart_suggestion(
88+
"replace `if`-then-`panic!` with `assert!`",
89+
vec![
90+
(expr.span.shrink_to_lo(), format!("{comments}\n")),
91+
(expr.span, base_sugg.clone()),
92+
],
8093
applicability,
8194
);
8295
}
83-
diag.span_suggestion(expr.span, "try instead", sugg, applicability);
8496
},
8597
);
8698
}

‎tests/ui/manual_assert.edition2018.1.fixed

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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);assert!(a.is_empty(), "qaqaq{:?}", a);
31+
assert!(a.is_empty(), "qwqwq");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");assert!(!b.is_empty(), "panic1");
47+
assert!(!(b.is_empty() && a.is_empty()), "panic2");assert!(!(b.is_empty() && a.is_empty()), "panic2");
48+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");assert!(!(a.is_empty() && !b.is_empty()), "panic3");
49+
assert!(!(b.is_empty() || a.is_empty()), "panic4");assert!(!(b.is_empty() || a.is_empty()), "panic4");
50+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");assert!(!(a.is_empty() || !b.is_empty()), "panic5");
51+
assert!(!a.is_empty(), "with expansion {}", one!());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+
assert!(!(a > 2), "panic with comment");assert!(!(a > 2), "panic with comment");
62+
}
63+
64+
fn issue12505() {
65+
struct Foo<T, const N: usize>(T);
66+
67+
impl<T, const N: usize> Foo<T, N> {
68+
const BAR: () = assert!(!(N == 0), )assert!(!(N == 0), );
69+
}
70+
}

‎tests/ui/manual_assert.edition2018.2.fixed

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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);assert!(a.is_empty(), "qaqaq{:?}", a);
31+
assert!(a.is_empty(), "qwqwq");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");assert!(!b.is_empty(), "panic1");
47+
assert!(!(b.is_empty() && a.is_empty()), "panic2");assert!(!(b.is_empty() && a.is_empty()), "panic2");
48+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");assert!(!(a.is_empty() && !b.is_empty()), "panic3");
49+
assert!(!(b.is_empty() || a.is_empty()), "panic4");assert!(!(b.is_empty() || a.is_empty()), "panic4");
50+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");assert!(!(a.is_empty() || !b.is_empty()), "panic5");
51+
assert!(!a.is_empty(), "with expansion {}", one!());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+
68+
assert!(!(a > 2), "panic with comment");
69+
}
70+
71+
fn issue12505() {
72+
struct Foo<T, const N: usize>(T);
73+
74+
impl<T, const N: usize> Foo<T, N> {
75+
const BAR: () = assert!(!(N == 0), )assert!(!(N == 0), );
76+
}
77+
}

‎tests/ui/manual_assert.edition2018.fixed

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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+
68+
assert!(!(a > 2), "panic with comment");
69+
}
70+
71+
fn issue12505() {
72+
struct Foo<T, const N: usize>(T);
73+
74+
impl<T, const N: usize> Foo<T, N> {
75+
const BAR: () = assert!(!(N == 0), );
76+
}
77+
}

‎tests/ui/manual_assert.edition2018.stderr

+19-24
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
@@ -76,20 +76,15 @@ LL | | multiline
7676
LL | | panic!("panic with comment") // comment after `panic!`
7777
LL | | }
7878
| |_____^
79-
|
80-
help: try instead
81-
|
82-
LL | assert!(!(a > 2), "panic with comment");
83-
|
8479

8580
error: only a `panic!` in `if`-then statement
86-
--> tests/ui/manual_assert.rs:93:25
81+
--> tests/ui/manual_assert.rs:91:25
8782
|
8883
LL | const BAR: () = if N == 0 {
8984
| _________________________^
9085
LL | | panic!()
9186
LL | | };
92-
| |_________^ help: try instead: `assert!(!(N == 0), )`
87+
| |_________^ help: replace `if`-then-`panic!` with `assert!`: `assert!(!(N == 0), )`
9388

9489
error: aborting due to 10 previous errors
9590

0 commit comments

Comments
 (0)