diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 48612befc..3bd6a09d3 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -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, diff --git a/tests/ui/suspicious_else_formatting.rs b/tests/ui/suspicious_else_formatting.rs index 226010ec6..547615b10 100644 --- a/tests/ui/suspicious_else_formatting.rs +++ b/tests/ui/suspicious_else_formatting.rs @@ -40,6 +40,7 @@ fn main() { { } + // This is fine, though weird. Allman style braces on the else. if foo() { } else @@ -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 + { + } } diff --git a/tests/ui/suspicious_else_formatting.stderr b/tests/ui/suspicious_else_formatting.stderr index bbc036d37..d8d67b413 100644 --- a/tests/ui/suspicious_else_formatting.stderr +++ b/tests/ui/suspicious_else_formatting.stderr @@ -41,19 +41,8 @@ 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 - | -LL | } - | ______^ -LL | | else -LL | | { - | |____^ - | - = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` - error: this is an `else if` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:50:6 + --> $DIR/suspicious_else_formatting.rs:51:6 | LL | } else | ______^ @@ -63,7 +52,7 @@ 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 + --> $DIR/suspicious_else_formatting.rs:56:6 | LL | } | ______^ @@ -73,5 +62,29 @@ 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: aborting due to 8 previous errors +error: this is an `else {..}` but the formatting might hide it + --> $DIR/suspicious_else_formatting.rs:83:6 + | +LL | } + | ______^ +LL | | +LL | | else +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: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 9 previous errors