Enhance needless continue to detect loop {continue;} #7477
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @giraffate (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
F3real
force-pushed
the
needless_continue
branch
from
c574800 to
24ec35a
Compare
F3real
force-pushed
the
needless_continue
branch
from
f8a62e2 to
b1f5eab
Compare
F3real
force-pushed
the
needless_continue
branch
from
b1f5eab to
9d6127c
Compare
F3real
force-pushed
the
needless_continue
branch
from
3ca8619 to
c3452f3
Compare
| if !loop_block.stmts.is_empty(); | ||
| if let ast::StmtKind::Expr(ref statement) | ||
| | ast::StmtKind::Semi(ref statement) = loop_block.stmts.last().unwrap().kind; | ||
| if let ast::ExprKind::Continue(_) = statement.kind; | ||
| then { | ||
| span_lint_and_help( | ||
| cx, | ||
| NEEDLESS_CONTINUE, | ||
| loop_block.stmts.last().unwrap().span, |
There was a problem hiding this comment.
Formatting looks off here and I don't like the unwrapping. If loop_block.stmts.last() already is Some(_) you don't have to check if it is empty first. So this could be written as:
| if !loop_block.stmts.is_empty(); | |
| if let ast::StmtKind::Expr(ref statement) | |
| | ast::StmtKind::Semi(ref statement) = loop_block.stmts.last().unwrap().kind; | |
| if let ast::ExprKind::Continue(_) = statement.kind; | |
| then { | |
| span_lint_and_help( | |
| cx, | |
| NEEDLESS_CONTINUE, | |
| loop_block.stmts.last().unwrap().span, | |
| if let Some(last_stmt) = loop_block.stmts.last(); | |
| if let ast::StmtKind::Expr(expr) | ast::StmtKind::Semi(expr) = &last_stmt.kind; | |
| if let ast::ExprKind::Continue(_) = expr.kind; | |
| then { | |
| span_lint_and_help( | |
| cx, | |
| NEEDLESS_CONTINUE, | |
| last_stmt.span, |
Also import nvm this should be done for the whole file, but this is something for another time and another PR.StmtKind and ExprKind at the top of the file instead of full qualifying it here.
There was a problem hiding this comment.
I agree it is definitely better without unwrap(), I wasn't sure how to avoid it.
The rust fmt just suggested to break line with pattern matching into two lines, but I also wasn't keen on the end result. Is there some other auto formatter that you use as default?
There was a problem hiding this comment.
Nope, we use rustfmt. But rustfmt (currently) can't format code in macros. Since this code is in an if_chain! macro, rustfmt doesn't know how to format it. What rustfmt does though is to check if a line is too long regardless if it is in a macro or not. You then get a error message something like "this line is too long, but I don't know how to format it" from rustfmt.
|
@bors r+ Thanks for the contribution and implementing my additional suggestions! |
|
📌 Commit 045dbb5 has been approved by |
Enhance needless continue to detect loop {continue;}
Fixes #7417
changelog: Report [`needless_continue`] in `loop { continue; }` case
|
💥 Test timed out |
|
@bors retry |
Enhance needless continue to detect loop {continue;}
Fixes #7417
changelog: Report [`needless_continue`] in `loop { continue; }` case
|
💥 Test timed out |
|
@bors retry |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #7417
changelog: Report [
needless_continue] inloop { continue; }case