Skip to content

Commit 006a4cc

Browse files
borsbors
committed
Auto merge of #10276 - m-ou-se:manual-assert, r=Alexendoo
Don't depend on FormatArgsExpn in ManualAssert. Part of #10233 changelog: none
2 parents a2f85de + ecd98ba commit 006a4cc

File tree

3 files changed

+125
-74
lines changed

3 files changed

+125
-74
lines changed

‎clippy_lints/src/manual_assert.rs

+48-49
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use crate::rustc_lint::LintContext;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::macros::{root_macro_call, FormatArgsExpn};
4-
use clippy_utils::source::snippet_with_applicability;
3+
use clippy_utils::macros::root_macro_call;
54
use clippy_utils::{is_else_clause, peel_blocks_with_stmt, span_extract_comment, sugg};
65
use rustc_errors::Applicability;
76
use rustc_hir::{Expr, ExprKind, UnOp};
@@ -38,57 +37,57 @@ declare_lint_pass!(ManualAssert => [MANUAL_ASSERT]);
3837

3938
impl<'tcx> LateLintPass<'tcx> for ManualAssert {
4039
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
41-
if_chain! {
42-
if let ExprKind::If(cond, then, None) = expr.kind;
43-
if !matches!(cond.kind, ExprKind::Let(_));
44-
if !expr.span.from_expansion();
45-
let then = peel_blocks_with_stmt(then);
46-
if let Some(macro_call) = root_macro_call(then.span);
47-
if cx.tcx.item_name(macro_call.def_id) == sym::panic;
48-
if !cx.tcx.sess.source_map().is_multiline(cond.span);
49-
if let Some(format_args) = FormatArgsExpn::find_nested(cx, then, macro_call.expn);
40+
if let ExprKind::If(cond, then, None) = expr.kind
41+
&& !matches!(cond.kind, ExprKind::Let(_))
42+
&& !expr.span.from_expansion()
43+
&& let then = peel_blocks_with_stmt(then)
44+
&& let Some(macro_call) = root_macro_call(then.span)
45+
&& cx.tcx.item_name(macro_call.def_id) == sym::panic
46+
&& !cx.tcx.sess.source_map().is_multiline(cond.span)
47+
&& let Ok(panic_snippet) = cx.sess().source_map().span_to_snippet(macro_call.span)
48+
&& let Some(panic_snippet) = panic_snippet.strip_suffix(')')
49+
&& let Some((_, format_args_snip)) = panic_snippet.split_once('(')
5050
// Don't change `else if foo { panic!(..) }` to `else { assert!(foo, ..) }` as it just
5151
// shuffles the condition around.
5252
// Should this have a config value?
53-
if !is_else_clause(cx.tcx, expr);
54-
then {
55-
let mut applicability = Applicability::MachineApplicable;
56-
let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability);
57-
let cond = cond.peel_drop_temps();
58-
let mut comments = span_extract_comment(cx.sess().source_map(), expr.span);
59-
if !comments.is_empty() {
60-
comments += "\n";
61-
}
62-
let (cond, not) = match cond.kind {
63-
ExprKind::Unary(UnOp::Not, e) => (e, ""),
64-
_ => (cond, "!"),
65-
};
66-
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
67-
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});");
68-
// we show to the user the suggestion without the comments, but when applicating the fix, include the comments in the block
69-
span_lint_and_then(
70-
cx,
71-
MANUAL_ASSERT,
72-
expr.span,
73-
"only a `panic!` in `if`-then statement",
74-
|diag| {
75-
// comments can be noisy, do not show them to the user
76-
if !comments.is_empty() {
77-
diag.tool_only_span_suggestion(
78-
expr.span.shrink_to_lo(),
79-
"add comments back",
80-
comments,
81-
applicability);
82-
}
83-
diag.span_suggestion(
84-
expr.span,
85-
"try instead",
86-
sugg,
87-
applicability);
88-
}
89-
90-
);
53+
&& !is_else_clause(cx.tcx, expr)
54+
{
55+
let mut applicability = Applicability::MachineApplicable;
56+
let cond = cond.peel_drop_temps();
57+
let mut comments = span_extract_comment(cx.sess().source_map(), expr.span);
58+
if !comments.is_empty() {
59+
comments += "\n";
9160
}
61+
let (cond, not) = match cond.kind {
62+
ExprKind::Unary(UnOp::Not, e) => (e, ""),
63+
_ => (cond, "!"),
64+
};
65+
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
66+
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});");
67+
// we show to the user the suggestion without the comments, but when applicating the fix, include the comments in the block
68+
span_lint_and_then(
69+
cx,
70+
MANUAL_ASSERT,
71+
expr.span,
72+
"only a `panic!` in `if`-then statement",
73+
|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(
84+
expr.span,
85+
"try instead",
86+
sugg,
87+
applicability
88+
);
89+
}
90+
);
9291
}
9392
}
9493
}

‎tests/ui/manual_assert.edition2018.fixed

+11-24
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ fn main() {
2929
panic!("qaqaq{:?}", a);
3030
}
3131
assert!(a.is_empty(), "qaqaq{:?}", a);
32-
if !a.is_empty() {
33-
panic!("qwqwq");
34-
}
32+
assert!(a.is_empty(), "qwqwq");
3533
if a.len() == 3 {
3634
println!("qwq");
3735
println!("qwq");
@@ -46,21 +44,11 @@ fn main() {
4644
println!("qwq");
4745
}
4846
let b = vec![1, 2, 3];
49-
if b.is_empty() {
50-
panic!("panic1");
51-
}
52-
if b.is_empty() && a.is_empty() {
53-
panic!("panic2");
54-
}
55-
if a.is_empty() && !b.is_empty() {
56-
panic!("panic3");
57-
}
58-
if b.is_empty() || a.is_empty() {
59-
panic!("panic4");
60-
}
61-
if a.is_empty() || !b.is_empty() {
62-
panic!("panic5");
63-
}
47+
assert!(!b.is_empty(), "panic1");
48+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
49+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
50+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
51+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
6452
assert!(!a.is_empty(), "with expansion {}", one!());
6553
if a.is_empty() {
6654
let _ = 0;
@@ -71,12 +59,11 @@ fn main() {
7159

7260
fn issue7730(a: u8) {
7361
// Suggestion should preserve comment
74-
if a > 2 {
75-
// comment
76-
/* this is a
62+
// comment
63+
/* this is a
7764
multiline
7865
comment */
79-
/// Doc comment
80-
panic!("panic with comment") // comment after `panic!`
81-
}
66+
/// Doc comment
67+
// comment after `panic!`
68+
assert!(!(a > 2), "panic with comment");
8269
}

‎tests/ui/manual_assert.edition2018.stderr

+66-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,54 @@ LL | | }
88
|
99
= note: `-D clippy::manual-assert` implied by `-D warnings`
1010

11+
error: only a `panic!` in `if`-then statement
12+
--> $DIR/manual_assert.rs:34:5
13+
|
14+
LL | / if !a.is_empty() {
15+
LL | | panic!("qwqwq");
16+
LL | | }
17+
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
18+
19+
error: only a `panic!` in `if`-then statement
20+
--> $DIR/manual_assert.rs:51:5
21+
|
22+
LL | / if b.is_empty() {
23+
LL | | panic!("panic1");
24+
LL | | }
25+
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
26+
27+
error: only a `panic!` in `if`-then statement
28+
--> $DIR/manual_assert.rs:54:5
29+
|
30+
LL | / if b.is_empty() && a.is_empty() {
31+
LL | | panic!("panic2");
32+
LL | | }
33+
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
34+
35+
error: only a `panic!` in `if`-then statement
36+
--> $DIR/manual_assert.rs:57:5
37+
|
38+
LL | / if a.is_empty() && !b.is_empty() {
39+
LL | | panic!("panic3");
40+
LL | | }
41+
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
42+
43+
error: only a `panic!` in `if`-then statement
44+
--> $DIR/manual_assert.rs:60:5
45+
|
46+
LL | / if b.is_empty() || a.is_empty() {
47+
LL | | panic!("panic4");
48+
LL | | }
49+
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
50+
51+
error: only a `panic!` in `if`-then statement
52+
--> $DIR/manual_assert.rs:63:5
53+
|
54+
LL | / if a.is_empty() || !b.is_empty() {
55+
LL | | panic!("panic5");
56+
LL | | }
57+
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
58+
1159
error: only a `panic!` in `if`-then statement
1260
--> $DIR/manual_assert.rs:66:5
1361
|
@@ -16,5 +64,22 @@ LL | | panic!("with expansion {}", one!())
1664
LL | | }
1765
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
1866

19-
error: aborting due to 2 previous errors
67+
error: only a `panic!` in `if`-then statement
68+
--> $DIR/manual_assert.rs:78:5
69+
|
70+
LL | / if a > 2 {
71+
LL | | // comment
72+
LL | | /* this is a
73+
LL | | multiline
74+
... |
75+
LL | | panic!("panic with comment") // comment after `panic!`
76+
LL | | }
77+
| |_____^
78+
|
79+
help: try instead
80+
|
81+
LL | assert!(!(a > 2), "panic with comment");
82+
|
83+
84+
error: aborting due to 9 previous errors
2085

0 commit comments

Comments
 (0)