Skip to content

Commit 276e895

Browse files
borsbors
committed
Auto merge of #7851 - nbdd0121:master, r=flip1995
Fix manual_assert and match_wild_err_arm for `#![no_std]` and Rust 2021 Rust 2015 `std::panic!` has a wrapping block while `core::panic!` and Rust 2021 `std::panic!` does not. See rust-lang/rust#88919 for details. Note that the test won't pass until clippy changes in rust-lang/rust#88860 is synced. --- changelog: Fix [`manual_assert`] and [`match_wild_err_arm`] for `#![no_std]` and Rust 2021. Fixes #7723
2 parents 38d8025 + 14e0390 commit 276e895

13 files changed

+254
-54
lines changed

‎clippy_lints/src/manual_assert.rs

+13-12
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,24 @@ impl LateLintPass<'_> for ManualAssert {
5454
if !cx.tcx.sess.source_map().is_multiline(cond.span);
5555

5656
then {
57-
let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
57+
let call = if_chain! {
58+
if let ExprKind::Block(block, _) = semi.kind;
59+
if let Some(init) = block.expr;
60+
then {
61+
init
62+
} else {
63+
semi
64+
}
65+
};
66+
let span = if let Some(panic_expn) = PanicExpn::parse(call) {
5867
match *panic_expn.format_args.value_args {
5968
[] => panic_expn.format_args.format_string_span,
6069
[.., last] => panic_expn.format_args.format_string_span.to(last.span),
6170
}
71+
} else if let ExprKind::Call(_, [format_args]) = call.kind {
72+
format_args.span
6273
} else {
63-
if_chain! {
64-
if let ExprKind::Block(block, _) = semi.kind;
65-
if let Some(init) = block.expr;
66-
if let ExprKind::Call(_, [format_args]) = init.kind;
67-
68-
then {
69-
format_args.span
70-
} else {
71-
return
72-
}
73-
}
74+
return
7475
};
7576
let mut applicability = Applicability::MachineApplicable;
7677
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);

‎clippy_lints/src/matches.rs

+14-10
Original file line numberDiff line numberDiff line change
@@ -967,8 +967,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
967967
}
968968
if_chain! {
969969
if matching_wild;
970-
if let ExprKind::Block(block, _) = arm.body.kind;
971-
if is_panic_block(block);
970+
if is_panic_call(arm.body);
972971
then {
973972
// `Err(_)` or `Err(_e)` arm with `panic!` found
974973
span_lint_and_note(cx,
@@ -1171,14 +1170,19 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
11711170
}
11721171

11731172
// If the block contains only a `panic!` macro (as expression or statement)
1174-
fn is_panic_block(block: &Block<'_>) -> bool {
1175-
match (&block.expr, block.stmts.len(), block.stmts.first()) {
1176-
(&Some(exp), 0, _) => is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none(),
1177-
(&None, 1, Some(stmt)) => {
1178-
is_expn_of(stmt.span, "panic").is_some() && is_expn_of(stmt.span, "unreachable").is_none()
1179-
},
1180-
_ => false,
1181-
}
1173+
fn is_panic_call(expr: &Expr<'_>) -> bool {
1174+
// Unwrap any wrapping blocks
1175+
let span = if let ExprKind::Block(block, _) = expr.kind {
1176+
match (&block.expr, block.stmts.len(), block.stmts.first()) {
1177+
(&Some(exp), 0, _) => exp.span,
1178+
(&None, 1, Some(stmt)) => stmt.span,
1179+
_ => return false,
1180+
}
1181+
} else {
1182+
expr.span
1183+
};
1184+
1185+
is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none()
11821186
}
11831187

11841188
fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>)

‎clippy_utils/src/higher.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -718,9 +718,7 @@ impl PanicExpn<'tcx> {
718718
/// Parses an expanded `panic!` invocation
719719
pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
720720
if_chain! {
721-
if let ExprKind::Block(block, _) = expr.kind;
722-
if let Some(init) = block.expr;
723-
if let ExprKind::Call(_, [format_args]) = init.kind;
721+
if let ExprKind::Call(_, [format_args]) = expr.kind;
724722
let expn_data = expr.span.ctxt().outer_expn_data();
725723
if let Some(format_args) = FormatArgsExpn::parse(format_args);
726724
then {

‎tests/missing-test-files.rs

+24-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
22
#![warn(rust_2018_idioms, unused_lifetimes)]
33
#![allow(clippy::assertions_on_constants)]
4+
#![feature(path_file_prefix)]
45

6+
use std::cmp::Ordering;
7+
use std::ffi::OsStr;
58
use std::fs::{self, DirEntry};
69
use std::path::Path;
710

@@ -21,29 +24,39 @@ fn test_missing_tests() {
2124
}
2225
}
2326

24-
/*
25-
Test for missing files.
26-
27-
Since rs files are alphabetically before stderr/stdout, we can sort by the full name
28-
and iter in that order. If we've seen the file stem for the first time and it's not
29-
a rust file, it means the rust file has to be missing.
30-
*/
27+
// Test for missing files.
3128
fn explore_directory(dir: &Path) -> Vec<String> {
3229
let mut missing_files: Vec<String> = Vec::new();
3330
let mut current_file = String::new();
3431
let mut files: Vec<DirEntry> = fs::read_dir(dir).unwrap().filter_map(Result::ok).collect();
35-
files.sort_by_key(std::fs::DirEntry::path);
32+
files.sort_by(|x, y| {
33+
match x.path().file_prefix().cmp(&y.path().file_prefix()) {
34+
Ordering::Equal => (),
35+
ord => return ord,
36+
}
37+
// Sort rs files before the others if they share the same prefix. So when we see
38+
// the file prefix for the first time and it's not a rust file, it means the rust
39+
// file has to be missing.
40+
match (
41+
x.path().extension().and_then(OsStr::to_str),
42+
y.path().extension().and_then(OsStr::to_str),
43+
) {
44+
(Some("rs"), _) => Ordering::Less,
45+
(_, Some("rs")) => Ordering::Greater,
46+
_ => Ordering::Equal,
47+
}
48+
});
3649
for entry in &files {
3750
let path = entry.path();
3851
if path.is_dir() {
3952
missing_files.extend(explore_directory(&path));
4053
} else {
41-
let file_stem = path.file_stem().unwrap().to_str().unwrap().to_string();
54+
let file_prefix = path.file_prefix().unwrap().to_str().unwrap().to_string();
4255
if let Some(ext) = path.extension() {
4356
match ext.to_str().unwrap() {
44-
"rs" => current_file = file_stem.clone(),
57+
"rs" => current_file = file_prefix.clone(),
4558
"stderr" | "stdout" => {
46-
if file_stem != current_file {
59+
if file_prefix != current_file {
4760
missing_files.push(path.to_str().unwrap().to_string());
4861
}
4962
},

‎tests/ui/manual_assert.edition2018.fixed

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// revisions: edition2018 edition2021
2+
// [edition2018] edition:2018
3+
// [edition2021] edition:2021
4+
// run-rustfix
5+
#![warn(clippy::manual_assert)]
6+
7+
fn main() {
8+
let a = vec![1, 2, 3];
9+
let c = Some(2);
10+
if !a.is_empty()
11+
&& a.len() == 3
12+
&& c != None
13+
&& !a.is_empty()
14+
&& a.len() == 3
15+
&& !a.is_empty()
16+
&& a.len() == 3
17+
&& !a.is_empty()
18+
&& a.len() == 3
19+
{
20+
panic!("qaqaq{:?}", a);
21+
}
22+
assert!(a.is_empty(), "qaqaq{:?}", a);
23+
assert!(a.is_empty(), "qwqwq");
24+
if a.len() == 3 {
25+
println!("qwq");
26+
println!("qwq");
27+
println!("qwq");
28+
}
29+
if let Some(b) = c {
30+
panic!("orz {}", b);
31+
}
32+
if a.len() == 3 {
33+
panic!("qaqaq");
34+
} else {
35+
println!("qwq");
36+
}
37+
let b = vec![1, 2, 3];
38+
assert!(!b.is_empty(), "panic1");
39+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
40+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
41+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
42+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
43+
}

‎tests/ui/manual_assert.stderr renamed to ‎tests/ui/manual_assert.edition2018.stderr

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: only a `panic!` in `if`-then statement
2-
--> $DIR/manual_assert.rs:21:5
2+
--> $DIR/manual_assert.rs:22:5
33
|
44
LL | / if !a.is_empty() {
55
LL | | panic!("qaqaq{:?}", a);
@@ -9,47 +9,47 @@ LL | | }
99
= note: `-D clippy::manual-assert` implied by `-D warnings`
1010

1111
error: only a `panic!` in `if`-then statement
12-
--> $DIR/manual_assert.rs:24:5
12+
--> $DIR/manual_assert.rs:25:5
1313
|
1414
LL | / if !a.is_empty() {
1515
LL | | panic!("qwqwq");
1616
LL | | }
1717
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
1818

1919
error: only a `panic!` in `if`-then statement
20-
--> $DIR/manual_assert.rs:41:5
20+
--> $DIR/manual_assert.rs:42:5
2121
|
2222
LL | / if b.is_empty() {
2323
LL | | panic!("panic1");
2424
LL | | }
2525
| |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
2626

2727
error: only a `panic!` in `if`-then statement
28-
--> $DIR/manual_assert.rs:44:5
28+
--> $DIR/manual_assert.rs:45:5
2929
|
3030
LL | / if b.is_empty() && a.is_empty() {
3131
LL | | panic!("panic2");
3232
LL | | }
3333
| |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
3434

3535
error: only a `panic!` in `if`-then statement
36-
--> $DIR/manual_assert.rs:47:5
36+
--> $DIR/manual_assert.rs:48:5
3737
|
3838
LL | / if a.is_empty() && !b.is_empty() {
3939
LL | | panic!("panic3");
4040
LL | | }
4141
| |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
4242

4343
error: only a `panic!` in `if`-then statement
44-
--> $DIR/manual_assert.rs:50:5
44+
--> $DIR/manual_assert.rs:51:5
4545
|
4646
LL | / if b.is_empty() || a.is_empty() {
4747
LL | | panic!("panic4");
4848
LL | | }
4949
| |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
5050

5151
error: only a `panic!` in `if`-then statement
52-
--> $DIR/manual_assert.rs:53:5
52+
--> $DIR/manual_assert.rs:54:5
5353
|
5454
LL | / if a.is_empty() || !b.is_empty() {
5555
LL | | panic!("panic5");

‎tests/ui/manual_assert.edition2021.fixed

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// revisions: edition2018 edition2021
2+
// [edition2018] edition:2018
3+
// [edition2021] edition:2021
4+
// run-rustfix
5+
#![warn(clippy::manual_assert)]
6+
7+
fn main() {
8+
let a = vec![1, 2, 3];
9+
let c = Some(2);
10+
if !a.is_empty()
11+
&& a.len() == 3
12+
&& c != None
13+
&& !a.is_empty()
14+
&& a.len() == 3
15+
&& !a.is_empty()
16+
&& a.len() == 3
17+
&& !a.is_empty()
18+
&& a.len() == 3
19+
{
20+
panic!("qaqaq{:?}", a);
21+
}
22+
assert!(a.is_empty(), "qaqaq{:?}", a);
23+
assert!(a.is_empty(), "qwqwq");
24+
if a.len() == 3 {
25+
println!("qwq");
26+
println!("qwq");
27+
println!("qwq");
28+
}
29+
if let Some(b) = c {
30+
panic!("orz {}", b);
31+
}
32+
if a.len() == 3 {
33+
panic!("qaqaq");
34+
} else {
35+
println!("qwq");
36+
}
37+
let b = vec![1, 2, 3];
38+
assert!(!b.is_empty(), "panic1");
39+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
40+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
41+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
42+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
43+
}

‎tests/ui/manual_assert.edition2021.stderr

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
error: only a `panic!` in `if`-then statement
2+
--> $DIR/manual_assert.rs:22:5
3+
|
4+
LL | / if !a.is_empty() {
5+
LL | | panic!("qaqaq{:?}", a);
6+
LL | | }
7+
| |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
8+
|
9+
= note: `-D clippy::manual-assert` implied by `-D warnings`
10+
11+
error: only a `panic!` in `if`-then statement
12+
--> $DIR/manual_assert.rs:25:5
13+
|
14+
LL | / if !a.is_empty() {
15+
LL | | panic!("qwqwq");
16+
LL | | }
17+
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
18+
19+
error: only a `panic!` in `if`-then statement
20+
--> $DIR/manual_assert.rs:42:5
21+
|
22+
LL | / if b.is_empty() {
23+
LL | | panic!("panic1");
24+
LL | | }
25+
| |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
26+
27+
error: only a `panic!` in `if`-then statement
28+
--> $DIR/manual_assert.rs:45:5
29+
|
30+
LL | / if b.is_empty() && a.is_empty() {
31+
LL | | panic!("panic2");
32+
LL | | }
33+
| |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
34+
35+
error: only a `panic!` in `if`-then statement
36+
--> $DIR/manual_assert.rs:48:5
37+
|
38+
LL | / if a.is_empty() && !b.is_empty() {
39+
LL | | panic!("panic3");
40+
LL | | }
41+
| |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
42+
43+
error: only a `panic!` in `if`-then statement
44+
--> $DIR/manual_assert.rs:51:5
45+
|
46+
LL | / if b.is_empty() || a.is_empty() {
47+
LL | | panic!("panic4");
48+
LL | | }
49+
| |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
50+
51+
error: only a `panic!` in `if`-then statement
52+
--> $DIR/manual_assert.rs:54:5
53+
|
54+
LL | / if a.is_empty() || !b.is_empty() {
55+
LL | | panic!("panic5");
56+
LL | | }
57+
| |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
58+
59+
error: aborting due to 7 previous errors
60+

‎tests/ui/manual_assert.fixed

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
// edition:2018
1+
// revisions: edition2018 edition2021
2+
// [edition2018] edition:2018
3+
// [edition2021] edition:2021
24
// run-rustfix
3-
//FIXME: This does not correctly match in edition 2021, see #7843
45
#![warn(clippy::manual_assert)]
56

67
fn main() {

‎tests/ui/manual_assert.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
// edition:2018
1+
// revisions: edition2018 edition2021
2+
// [edition2018] edition:2018
3+
// [edition2021] edition:2021
24
// run-rustfix
3-
//FIXME: This does not correctly match in edition 2021, see #7843
45
#![warn(clippy::manual_assert)]
56

67
fn main() {

0 commit comments

Comments
 (0)