X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fcollapsible_if.rs;h=ae613d70240c39f7d94ca396fe5f15768f539524;hb=4d60841205d10293d6759df1948acda4c607c7eb;hp=9d23d32c81ae33a4dc3280284c9446210ed0108c;hpb=3b5ff0f813f8846677905fcf355221d4862d24db;p=rust.git diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 9d23d32c81a..ae613d70240 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -1,8 +1,17 @@ +// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + //! Checks for if expressions that contain only an if expression. //! //! For example, the lint would catch: //! -//! ```rust +//! ```rust,ignore //! if x { //! if y { //! println!("Hello world"); @@ -12,14 +21,18 @@ //! //! This lint is **warn** by default -use rustc::lint::*; +use if_chain::if_chain; +use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; use syntax::ast; -use utils::{in_macro, snippet_block, span_lint_and_then}; -use utils::sugg::Sugg; +use crate::utils::sugg::Sugg; +use crate::utils::{in_macro, snippet_block, snippet_block_with_applicability, span_lint_and_sugg, span_lint_and_then}; +use rustc_errors::Applicability; /// **What it does:** Checks for nested `if` statements which can be collapsed -/// by `&&`-combining their conditions and for `else { if ... }` expressions that +/// by `&&`-combining their conditions and for `else { if ... }` expressions +/// that /// can be collapsed to `else if ...`. /// /// **Why is this bad?** Each `if`-statement adds one level of nesting, which @@ -28,7 +41,7 @@ /// **Known problems:** None. /// /// **Example:** -/// ```rust +/// ```rust,ignore /// if x { /// if y { /// … @@ -48,7 +61,7 @@ /// /// Should be written: /// -/// ```rust +/// ```rust.ignore /// if x && y { /// … /// } @@ -61,13 +74,13 @@ /// … /// } /// ``` -declare_lint! { +declare_clippy_lint! { pub COLLAPSIBLE_IF, - Warn, + style, "`if`s that can be collapsed (e.g. `if x { if y { ... } }` and `else { if x { ... } }`)" } -#[derive(Copy,Clone)] +#[derive(Copy, Clone)] pub struct CollapsibleIf; impl LintPass for CollapsibleIf { @@ -77,14 +90,14 @@ fn get_lints(&self) -> LintArray { } impl EarlyLintPass for CollapsibleIf { - fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) { - if !in_macro(cx, expr.span) { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { + if !in_macro(expr.span) { check_if(cx, expr) } } } -fn check_if(cx: &EarlyContext, expr: &ast::Expr) { +fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) { match expr.node { ast::ExprKind::If(ref check, ref then, ref else_) => { if let Some(ref else_) = *else_ { @@ -92,60 +105,76 @@ fn check_if(cx: &EarlyContext, expr: &ast::Expr) { } else { check_collapsible_no_if_let(cx, expr, check, then); } - } + }, ast::ExprKind::IfLet(_, _, _, Some(ref else_)) => { check_collapsible_maybe_if_let(cx, else_); - } + }, _ => (), } } -fn check_collapsible_maybe_if_let(cx: &EarlyContext, else_: &ast::Expr) { - if_let_chain! {[ - let ast::ExprKind::Block(ref block) = else_.node, - let Some(ref else_) = expr_block(block), - !in_macro(cx, else_.span), - ], { - match else_.node { - ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { - span_lint_and_then(cx, - COLLAPSIBLE_IF, - block.span, - "this `else { if .. }` block can be collapsed", |db| { - db.span_suggestion(block.span, "try", snippet_block(cx, else_.span, "..").into_owned()); - }); +fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool { + // We trim all opening braces and whitespaces and then check if the next string is a comment. + let trimmed_block_text = snippet_block(cx, expr.span, "..") + .trim_start_matches(|c: char| c.is_whitespace() || c == '{') + .to_owned(); + trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*") +} + +fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) { + if_chain! { + if let ast::ExprKind::Block(ref block, _) = else_.node; + if !block_starts_with_comment(cx, block); + if let Some(else_) = expr_block(block); + if !in_macro(else_.span); + then { + match else_.node { + ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + COLLAPSIBLE_IF, + block.span, + "this `else { if .. }` block can be collapsed", + "try", + snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(), + applicability, + ); + } + _ => (), } - _ => (), } - }} + } } -fn check_collapsible_no_if_let( - cx: &EarlyContext, - expr: &ast::Expr, - check: &ast::Expr, - then: &ast::Block, -) { - if_let_chain! {[ - let Some(inner) = expr_block(then), - let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node, - ], { - if expr.span.expn_id != inner.span.expn_id { - return; +fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) { + if_chain! { + if !block_starts_with_comment(cx, then); + if let Some(inner) = expr_block(then); + if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node; + then { + if expr.span.ctxt() != inner.span.ctxt() { + return; + } + span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| { + let lhs = Sugg::ast(cx, check, ".."); + let rhs = Sugg::ast(cx, check_inner, ".."); + db.span_suggestion_with_applicability( + expr.span, + "try", + format!( + "if {} {}", + lhs.and(&rhs), + snippet_block(cx, content.span, ".."), + ), + Applicability::MachineApplicable, // snippet + ); + }); } - span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| { - let lhs = Sugg::ast(cx, check, ".."); - let rhs = Sugg::ast(cx, check_inner, ".."); - db.span_suggestion(expr.span, - "try", - format!("if {} {}", - lhs.and(rhs), - snippet_block(cx, content.span, ".."))); - }); - }} + } } -/// If the block contains only one expression, returns it. +/// If the block contains only one expression, return it. fn expr_block(block: &ast::Block) -> Option<&ast::Expr> { let mut it = block.stmts.iter();