Skip to content

Commit

Permalink
Auto merge of #10013 - Jarcho:issue_9886, r=Manishearth
Browse files Browse the repository at this point in the history
Don't lint `manual_assert` in `else if`

fixes #9886
changelog: `manual_assert`: Don't lint in `else if`
  • Loading branch information
bors committed Dec 1, 2022
2 parents e16a8b9 + 47fb67f commit fec0057
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 2 deletions.
6 changes: 5 additions & 1 deletion clippy_lints/src/manual_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::rustc_lint::LintContext;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{root_macro_call, FormatArgsExpn};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{peel_blocks_with_stmt, span_extract_comment, sugg};
use clippy_utils::{is_else_clause, peel_blocks_with_stmt, span_extract_comment, sugg};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -47,6 +47,10 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
if cx.tcx.item_name(macro_call.def_id) == sym::panic;
if !cx.tcx.sess.source_map().is_multiline(cond.span);
if let Some(format_args) = FormatArgsExpn::find_nested(cx, then, macro_call.expn);
// Don't change `else if foo { panic!(..) }` to `else { assert!(foo, ..) }` as it just
// shuffles the condition around.
// Should this have a config value?
if !is_else_clause(cx.tcx, expr);
then {
let mut applicability = Applicability::MachineApplicable;
let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability);
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/manual_assert.edition2018.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ fn main() {
panic!("panic5");
}
assert!(!a.is_empty(), "with expansion {}", one!());
if a.is_empty() {
let _ = 0;
} else if a.len() == 1 {
panic!("panic6");
}
}

fn issue7730(a: u8) {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/manual_assert.edition2021.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ fn main() {
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
if a.is_empty() {
let _ = 0;
} else if a.len() == 1 {
panic!("panic6");
}
}

fn issue7730(a: u8) {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/manual_assert.edition2021.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ LL | | }
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`

error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:73:5
--> $DIR/manual_assert.rs:78:5
|
LL | / if a > 2 {
LL | | // comment
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/manual_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ fn main() {
if a.is_empty() {
panic!("with expansion {}", one!())
}
if a.is_empty() {
let _ = 0;
} else if a.len() == 1 {
panic!("panic6");
}
}

fn issue7730(a: u8) {
Expand Down

0 comments on commit fec0057

Please sign in to comment.