From 96c34e85c4bc23d153ebe4db5e24beba92d4d174 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sun, 14 Apr 2019 11:04:41 +0200 Subject: [PATCH 1/3] Remove `except` in suspicious_else_formatting This was causing two different ICEs in #3741. The first was fixed in #3925. The second one is fixed with this commit: We just don't `expect` anymore. If the snippet doesn't contain an `else`, we stop emitting the lint because it's not a suspiciously formatted else anyway. --- clippy_lints/src/formatting.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 08376bc49d22..9a58d414ae47 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -165,23 +165,23 @@ fn check_else(cx: &EarlyContext<'_>, expr: &ast::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) { - let else_pos = else_snippet.find("else").expect("there must be a `else` here"); + if let Some(else_pos) = else_snippet.find("else") { + if else_snippet[else_pos..].contains('\n') { + let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" }; - if else_snippet[else_pos..].contains('\n') { - let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" }; - - span_note_and_lint( - cx, - SUSPICIOUS_ELSE_FORMATTING, - else_span, - &format!("this is an `else {}` but the formatting might hide it", else_desc), - else_span, - &format!( - "to remove this lint, remove the `else` or remove the new line between \ - `else` and `{}`", - else_desc, - ), - ); + span_note_and_lint( + cx, + SUSPICIOUS_ELSE_FORMATTING, + else_span, + &format!("this is an `else {}` but the formatting might hide it", else_desc), + else_span, + &format!( + "to remove this lint, remove the `else` or remove the new line between \ + `else` and `{}`", + else_desc, + ), + ); + } } } } From ad27e3ff9b47e7afd2e75f969b42222a830feb8b Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sun, 14 Apr 2019 11:12:51 +0200 Subject: [PATCH 2/3] Refactor suspicious_else_formatting using if_chain --- clippy_lints/src/formatting.rs | 67 ++++++++++++++++------------------ 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 9a58d414ae47..7d0266a87316 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -3,6 +3,7 @@ use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, Lin use rustc::{declare_tool_lint, lint_array}; use syntax::ast; use syntax::ptr::P; +use if_chain::if_chain; declare_clippy_lint! { /// **What it does:** Checks for use of the non-existent `=*`, `=!` and `=-` @@ -146,44 +147,40 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &ast::Expr) { /// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`. fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) { - if let Some((then, &Some(ref else_))) = unsugar_if(expr) { - if (is_block(else_) || unsugar_if(else_).is_some()) - && !differing_macro_contexts(then.span, else_.span) - && !in_macro(then.span) - && !in_external_macro(cx.sess, expr.span) - { - // workaround for rust-lang/rust#43081 - if expr.span.lo().0 == 0 && expr.span.hi().0 == 0 { - return; - } + if_chain! { + if let Some((then, &Some(ref else_))) = unsugar_if(expr); + if is_block(else_) || unsugar_if(else_).is_some(); + if !differing_macro_contexts(then.span, else_.span); + if !in_macro(then.span) && !in_external_macro(cx.sess, expr.span); - // this will be a span from the closing ‘}’ of the “then” block (excluding) to - // the - // “if” of the “else if” block (excluding) - let else_span = then.span.between(else_.span); + // workaround for rust-lang/rust#43081 + if expr.span.lo().0 != 0 && expr.span.hi().0 != 0; - // 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') { - let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" }; + // this will be a span from the closing ‘}’ of the “then” block (excluding) to + // the + // “if” of the “else if” block (excluding) + let else_span = then.span.between(else_.span); - span_note_and_lint( - cx, - SUSPICIOUS_ELSE_FORMATTING, - else_span, - &format!("this is an `else {}` but the formatting might hide it", else_desc), - else_span, - &format!( - "to remove this lint, remove the `else` or remove the new line between \ - `else` and `{}`", - else_desc, - ), - ); - } - } - } + // 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'); + let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" }; + + then { + span_note_and_lint( + cx, + SUSPICIOUS_ELSE_FORMATTING, + else_span, + &format!("this is an `else {}` but the formatting might hide it", else_desc), + else_span, + &format!( + "to remove this lint, remove the `else` or remove the new line between \ + `else` and `{}`", + else_desc, + ), + ); } } } From 289a9af8014806aae79547645e4055f867b0cd09 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sun, 14 Apr 2019 13:37:38 +0200 Subject: [PATCH 3/3] cargo fmt --- clippy_lints/src/formatting.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 7d0266a87316..542392c8b29b 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -1,9 +1,9 @@ use crate::utils::{differing_macro_contexts, in_macro, snippet_opt, span_note_and_lint}; +use if_chain::if_chain; use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass}; use rustc::{declare_tool_lint, lint_array}; use syntax::ast; use syntax::ptr::P; -use if_chain::if_chain; declare_clippy_lint! { /// **What it does:** Checks for use of the non-existent `=*`, `=!` and `=-` @@ -157,8 +157,7 @@ fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) { if expr.span.lo().0 != 0 && expr.span.hi().0 != 0; // this will be a span from the closing ‘}’ of the “then” block (excluding) to - // the - // “if” of the “else if” block (excluding) + // the “if” of the “else if” block (excluding) let else_span = then.span.between(else_.span); // the snippet should look like " else \n " with maybe comments anywhere