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.

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

Allow allman style braces in suspicious_else_formatting #7087

Merged
merged 1 commit into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 15 additions & 2 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,22 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
// the snippet should look like " else \n " with maybe comments anywhere
// it’s bad when there is a ‘\n’ after the “else”
if let Some(else_snippet) = snippet_opt(cx, else_span);
if let Some(else_pos) = else_snippet.find("else");
if else_snippet[else_pos..].contains('\n');
if let Some((pre_else, post_else)) = else_snippet.split_once("else");
if let Some((_, post_else_post_eol)) = post_else.split_once('\n');

then {
// Allow allman style braces `} \n else \n {`
if_chain! {
if is_block(else_);
if let Some((_, pre_else_post_eol)) = pre_else.split_once('\n');
// Exactly one eol before and after the else
if !pre_else_post_eol.contains('\n');
if !post_else_post_eol.contains('\n');
then {
return;
}
}

let else_desc = if is_if(else_) { "if" } else { "{..}" };
span_lint_and_note(
cx,
Expand Down
26 changes: 26 additions & 0 deletions tests/ui/suspicious_else_formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fn main() {
{
}

// This is fine, though weird. Allman style braces on the else.
if foo() {
}
else
Expand Down Expand Up @@ -76,4 +77,29 @@ fn main() {
}
if foo() {
}

// Almost Allman style braces. Lint these.
if foo() {
}

else
{

}

if foo() {
}
else

{

}

// #3864 - Allman style braces
if foo()
{
}
else
{
}
}
39 changes: 26 additions & 13 deletions tests/ui/suspicious_else_formatting.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -41,37 +41,50 @@ LL | | {
|
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`

error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:44:6
error: this is an `else if` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:51:6
|
LL | }
LL | } else
| ______^
LL | | else
LL | | {
LL | | if foo() { // the span of the above error should continue here
| |____^
|
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`
= note: to remove this lint, remove the `else` or remove the new line between `else` and `if`

error: this is an `else if` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:50:6
--> $DIR/suspicious_else_formatting.rs:56:6
|
LL | } else
LL | }
| ______^
LL | | else
LL | | if foo() { // the span of the above error should continue here
| |____^
|
= note: to remove this lint, remove the `else` or remove the new line between `else` and `if`

error: this is an `else if` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:55:6
error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:83:6
|
LL | }
| ______^
LL | |
LL | | else
LL | | if foo() { // the span of the above error should continue here
LL | | {
| |____^
|
= note: to remove this lint, remove the `else` or remove the new line between `else` and `if`
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`

error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:91:6
|
LL | }
| ______^
LL | | else
LL | |
LL | | {
| |____^
|
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`

error: aborting due to 8 previous errors
error: aborting due to 9 previous errors