Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix manual_assert and match_wild_err_arm for #![no_std] and Rust 2021 #7851

Merged
merged 2 commits into from
Nov 2, 2021
Merged

Fix manual_assert and match_wild_err_arm for #![no_std] and Rust 2021 #7851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e5f99a
Fix manual_assert for `#![no_std]` and Rust 2021
nbdd0121 Oct 20, 2021
14e0390
Account for revisions in missing-test-files
nbdd0121 Oct 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions clippy_lints/src/manual_assert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,24 @@ impl LateLintPass<'_> for ManualAssert {
if !cx.tcx.sess.source_map().is_multiline(cond.span);

then {
let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
let call = if_chain! {
if let ExprKind::Block(block, _) = semi.kind;
if let Some(init) = block.expr;
then {
init
} else {
semi
}
};
let span = if let Some(panic_expn) = PanicExpn::parse(call) {
match *panic_expn.format_args.value_args {
[] => panic_expn.format_args.format_string_span,
[.., last] => panic_expn.format_args.format_string_span.to(last.span),
}
} else if let ExprKind::Call(_, [format_args]) = call.kind {
format_args.span
} else {
if_chain! {
if let ExprKind::Block(block, _) = semi.kind;
if let Some(init) = block.expr;
if let ExprKind::Call(_, [format_args]) = init.kind;

then {
format_args.span
} else {
return
}
}
return
};
let mut applicability = Applicability::MachineApplicable;
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
Expand Down
24 changes: 14 additions & 10 deletions clippy_lints/src/matches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,8 +967,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
}
if_chain! {
if matching_wild;
if let ExprKind::Block(block, _) = arm.body.kind;
if is_panic_block(block);
if is_panic_call(arm.body);
then {
// `Err(_)` or `Err(_e)` arm with `panic!` found
span_lint_and_note(cx,
Expand Down Expand Up @@ -1171,14 +1170,19 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
}

// If the block contains only a `panic!` macro (as expression or statement)
fn is_panic_block(block: &Block<'_>) -> bool {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
(&Some(exp), 0, _) => is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none(),
(&None, 1, Some(stmt)) => {
is_expn_of(stmt.span, "panic").is_some() && is_expn_of(stmt.span, "unreachable").is_none()
},
_ => false,
}
fn is_panic_call(expr: &Expr<'_>) -> bool {
// Unwrap any wrapping blocks
let span = if let ExprKind::Block(block, _) = expr.kind {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
(&Some(exp), 0, _) => exp.span,
(&None, 1, Some(stmt)) => stmt.span,
_ => return false,
}
} else {
expr.span
};

is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none()
}

fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>)
Expand Down
4 changes: 1 addition & 3 deletions clippy_utils/src/higher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,9 +718,7 @@ impl PanicExpn<'tcx> {
/// Parses an expanded `panic!` invocation
pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
if_chain! {
if let ExprKind::Block(block, _) = expr.kind;
if let Some(init) = block.expr;
if let ExprKind::Call(_, [format_args]) = init.kind;
if let ExprKind::Call(_, [format_args]) = expr.kind;
let expn_data = expr.span.ctxt().outer_expn_data();
if let Some(format_args) = FormatArgsExpn::parse(format_args);
then {
Expand Down
35 changes: 24 additions & 11 deletions tests/missing-test-files.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![warn(rust_2018_idioms, unused_lifetimes)]
#![allow(clippy::assertions_on_constants)]
#![feature(path_file_prefix)]

use std::cmp::Ordering;
use std::ffi::OsStr;
use std::fs::{self, DirEntry};
use std::path::Path;

Expand All @@ -21,29 +24,39 @@ fn test_missing_tests() {
}
}

/*
Test for missing files.

Since rs files are alphabetically before stderr/stdout, we can sort by the full name
and iter in that order. If we've seen the file stem for the first time and it's not
a rust file, it means the rust file has to be missing.
*/
// Test for missing files.
fn explore_directory(dir: &Path) -> Vec<String> {
let mut missing_files: Vec<String> = Vec::new();
let mut current_file = String::new();
let mut files: Vec<DirEntry> = fs::read_dir(dir).unwrap().filter_map(Result::ok).collect();
files.sort_by_key(std::fs::DirEntry::path);
files.sort_by(|x, y| {
match x.path().file_prefix().cmp(&y.path().file_prefix()) {
Ordering::Equal => (),
ord => return ord,
}
// Sort rs files before the others if they share the same prefix. So when we see
// the file prefix for the first time and it's not a rust file, it means the rust
// file has to be missing.
match (
x.path().extension().and_then(OsStr::to_str),
y.path().extension().and_then(OsStr::to_str),
) {
(Some("rs"), _) => Ordering::Less,
(_, Some("rs")) => Ordering::Greater,
_ => Ordering::Equal,
}
});
for entry in &files {
let path = entry.path();
if path.is_dir() {
missing_files.extend(explore_directory(&path));
} else {
let file_stem = path.file_stem().unwrap().to_str().unwrap().to_string();
let file_prefix = path.file_prefix().unwrap().to_str().unwrap().to_string();
if let Some(ext) = path.extension() {
match ext.to_str().unwrap() {
"rs" => current_file = file_stem.clone(),
"rs" => current_file = file_prefix.clone(),
"stderr" | "stdout" => {
if file_stem != current_file {
if file_prefix != current_file {
missing_files.push(path.to_str().unwrap().to_string());
}
},
Expand Down
43 changes: 43 additions & 0 deletions tests/ui/manual_assert.edition2018.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// revisions: edition2018 edition2021
// [edition2018] edition:2018
// [edition2021] edition:2021
// run-rustfix
#![warn(clippy::manual_assert)]

fn main() {
let a = vec![1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c != None
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
println!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
println!("qwq");
}
let b = vec![1, 2, 3];
assert!(!b.is_empty(), "panic1");
assert!(!(b.is_empty() && a.is_empty()), "panic2");
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
}
14 changes: 7 additions & 7 deletions tests/ui/manual_assert.stderr → tests/ui/manual_assert.edition2018.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:21:5
--> $DIR/manual_assert.rs:22:5
|
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
Expand All @@ -9,47 +9,47 @@ LL | | }
= note: `-D clippy::manual-assert` implied by `-D warnings`

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

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

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

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

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

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:53:5
--> $DIR/manual_assert.rs:54:5
|
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
Expand Down
43 changes: 43 additions & 0 deletions tests/ui/manual_assert.edition2021.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// revisions: edition2018 edition2021
// [edition2018] edition:2018
// [edition2021] edition:2021
// run-rustfix
#![warn(clippy::manual_assert)]

fn main() {
let a = vec![1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c != None
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
println!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
println!("qwq");
}
let b = vec![1, 2, 3];
assert!(!b.is_empty(), "panic1");
assert!(!(b.is_empty() && a.is_empty()), "panic2");
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
}
60 changes: 60 additions & 0 deletions tests/ui/manual_assert.edition2021.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:22:5
|
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
| |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
|
= note: `-D clippy::manual-assert` implied by `-D warnings`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:25:5
|
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:42:5
|
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
| |_____^ help: try: `assert!(!b.is_empty(), "panic1");`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:45:5
|
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
| |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:48:5
|
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
| |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:51:5
|
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
| |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:54:5
|
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
| |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`

error: aborting due to 7 previous errors

5 changes: 3 additions & 2 deletions tests/ui/manual_assert.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// edition:2018
// revisions: edition2018 edition2021
// [edition2018] edition:2018
// [edition2021] edition:2021
// run-rustfix
//FIXME: This does not correctly match in edition 2021, see #7843
#![warn(clippy::manual_assert)]

fn main() {
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/manual_assert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// edition:2018
// revisions: edition2018 edition2021
// [edition2018] edition:2018
// [edition2021] edition:2021
// run-rustfix
//FIXME: This does not correctly match in edition 2021, see #7843
#![warn(clippy::manual_assert)]

fn main() {
Expand Down
Loading