Enforce that the last statement in a loop body has a semicolon #6711
Conversation
There was a problem hiding this comment.
This is properly gated behind the 2027 style edition, and corresponds to style guide guidance (https://doc.rust-lang.org/beta/style-guide/statements.html):
Use a semicolon where an expression has void type, even if it could be propagated.
| // TODO[reviewer-help]: This is roughly "does it end in a | ||
| // curly". There might be a helper for this, or cases I'm | ||
| // missing. | ||
| ast::ExprKind::Loop(..) | ||
| | ast::ExprKind::While(..) | ||
| | ast::ExprKind::ForLoop { .. } | ||
| | ast::ExprKind::Let(..) | ||
| | ast::ExprKind::If(..) | ||
| | ast::ExprKind::Match(..) => false, |
There was a problem hiding this comment.
What about macros that use curly braces?
There was a problem hiding this comment.
An alternative here might be to use span_to_snippet and check if the returned string ends in }.
There was a problem hiding this comment.
What about macros that use curly braces?
This appears to work with the current code as I'd want it to, but I couldn't tell you why...
Before
loop {
dummy! {}
}
loop {
dummy!()
}
loop {
dummy![]
}After
loop {
dummy! {}
}
loop {
dummy!();
}
loop {
dummy![];
}There was a problem hiding this comment.
This appears to work as I'd expect, but I couldn't tell you why...
Probably maybe that the block macro is a statement..?
There was a problem hiding this comment.
might be to use
span_to_snippet
I changed it to this
_ => {
let is_curly = self
.opt_snippet(expr.span())
.is_some_and(|s| s.ends_with("}"));
if is_curly {
return false;
}
// Checking the edition as before 2024 the lack of aAnd the tests still passed, so I think it's up to you to decide which is a more reasonable path.
There was a problem hiding this comment.
Can we add two more test cases:
- One for
rustfmt-edition: 2024andrustfmt-style_edition: 2024 - One for
rustfmt-edition: 2021andrustfmt-style_edition: 2027
In both case I wouldn't expect us to add a ;
| rhs, | ||
| }, | ||
| } | ||
| }; |
| rhs, | ||
| }, | ||
| } | ||
| }; |
There was a problem hiding this comment.
Was this a necessary change? If not I'd prefer we revert it since we try not to change existing test cases.
4 participants
No description provided.