diff --git a/CHANGELOG.md b/CHANGELOG.md index ec424068a54..b643627c2e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1896,6 +1896,7 @@ Released 2018-09-13 [`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity +[`collapsible_else_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if [`collapsible_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 42bff564de0..93ccc76d0c9 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -23,9 +23,7 @@ use rustc_errors::Applicability; declare_clippy_lint! { /// **What it does:** Checks for nested `if` statements which can be collapsed - /// by `&&`-combining their conditions and for `else { if ... }` expressions - /// that - /// can be collapsed to `else if ...`. + /// by `&&`-combining their conditions. /// /// **Why is this bad?** Each `if`-statement adds one level of nesting, which /// makes code look more complex than it really is. @@ -40,7 +38,31 @@ declare_clippy_lint! { /// } /// } /// - /// // or + /// ``` + /// + /// Should be written: + /// + /// ```rust.ignore + /// if x && y { + /// … + /// } + /// ``` + pub COLLAPSIBLE_IF, + style, + "nested `if`s that can be collapsed (e.g., `if x { if y { ... } }`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for collapsible `else { if ... }` expressions + /// that can be collapsed to `else if ...`. + /// + /// **Why is this bad?** Each `if`-statement adds one level of nesting, which + /// makes code look more complex than it really is. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore /// /// if x { /// … @@ -54,24 +76,18 @@ declare_clippy_lint! { /// Should be written: /// /// ```rust.ignore - /// if x && y { - /// … - /// } - /// - /// // or - /// /// if x { /// … /// } else if y { /// … /// } /// ``` - pub COLLAPSIBLE_IF, + pub COLLAPSIBLE_ELSE_IF, style, - "`if`s that can be collapsed (e.g., `if x { if y { ... } }` and `else { if x { ... } }`)" + "nested `else`-`if` expressions that can be collapsed (e.g., `else { if x { ... } }`)" } -declare_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF]); +declare_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]); impl EarlyLintPass for CollapsibleIf { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { @@ -112,7 +128,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( cx, - COLLAPSIBLE_IF, + COLLAPSIBLE_ELSE_IF, block.span, "this `else { if .. }` block can be collapsed", "collapse nested if block", diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 35b057d7b6a..4f41b6c77e2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -556,6 +556,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &cargo_common_metadata::CARGO_COMMON_METADATA, &checked_conversions::CHECKED_CONVERSIONS, &cognitive_complexity::COGNITIVE_COMPLEXITY, + &collapsible_if::COLLAPSIBLE_ELSE_IF, &collapsible_if::COLLAPSIBLE_IF, &collapsible_match::COLLAPSIBLE_MATCH, &comparison_chain::COMPARISON_CHAIN, @@ -1384,6 +1385,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&booleans::LOGIC_BUG), LintId::of(&booleans::NONMINIMAL_BOOL), LintId::of(&bytecount::NAIVE_BYTECOUNT), + LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&collapsible_match::COLLAPSIBLE_MATCH), LintId::of(&comparison_chain::COMPARISON_CHAIN), @@ -1653,6 +1655,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS), LintId::of(&blacklisted_name::BLACKLISTED_NAME), LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), + LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&collapsible_match::COLLAPSIBLE_MATCH), LintId::of(&comparison_chain::COMPARISON_CHAIN), diff --git a/tests/ui/collapsible_else_if.fixed b/tests/ui/collapsible_else_if.fixed index ce2a1c28c8a..fa4bc30e933 100644 --- a/tests/ui/collapsible_else_if.fixed +++ b/tests/ui/collapsible_else_if.fixed @@ -3,6 +3,8 @@ #[rustfmt::skip] #[warn(clippy::collapsible_if)] +#[warn(clippy::collapsible_else_if)] + fn main() { let x = "hello"; let y = "world"; diff --git a/tests/ui/collapsible_else_if.rs b/tests/ui/collapsible_else_if.rs index 99c40b8d38e..bf6c1d1f894 100644 --- a/tests/ui/collapsible_else_if.rs +++ b/tests/ui/collapsible_else_if.rs @@ -3,6 +3,8 @@ #[rustfmt::skip] #[warn(clippy::collapsible_if)] +#[warn(clippy::collapsible_else_if)] + fn main() { let x = "hello"; let y = "world"; diff --git a/tests/ui/collapsible_else_if.stderr b/tests/ui/collapsible_else_if.stderr index 3d1c458879e..ee3e11ae565 100644 --- a/tests/ui/collapsible_else_if.stderr +++ b/tests/ui/collapsible_else_if.stderr @@ -1,5 +1,5 @@ error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:12:12 + --> $DIR/collapsible_else_if.rs:14:12 | LL | } else { | ____________^ @@ -9,7 +9,7 @@ LL | | } LL | | } | |_____^ | - = note: `-D clippy::collapsible-if` implied by `-D warnings` + = note: `-D clippy::collapsible-else-if` implied by `-D warnings` help: collapse nested if block | LL | } else if y == "world" { @@ -18,7 +18,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:20:12 + --> $DIR/collapsible_else_if.rs:22:12 | LL | } else { | ____________^ @@ -36,7 +36,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:28:12 + --> $DIR/collapsible_else_if.rs:30:12 | LL | } else { | ____________^ @@ -59,7 +59,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:39:12 + --> $DIR/collapsible_else_if.rs:41:12 | LL | } else { | ____________^ @@ -82,7 +82,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:50:12 + --> $DIR/collapsible_else_if.rs:52:12 | LL | } else { | ____________^ @@ -105,7 +105,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:61:12 + --> $DIR/collapsible_else_if.rs:63:12 | LL | } else { | ____________^ @@ -128,7 +128,7 @@ LL | } | error: this `else { if .. }` block can be collapsed - --> $DIR/collapsible_else_if.rs:72:12 + --> $DIR/collapsible_else_if.rs:74:12 | LL | } else { | ____________^ diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs index 8d54f75b5d1..e83ce47e563 100644 --- a/tests/ui/if_same_then_else2.rs +++ b/tests/ui/if_same_then_else2.rs @@ -1,6 +1,7 @@ #![warn(clippy::if_same_then_else)] #![allow( clippy::blacklisted_name, + clippy::collapsible_else_if, clippy::collapsible_if, clippy::ifs_same_cond, clippy::needless_return, diff --git a/tests/ui/if_same_then_else2.stderr b/tests/ui/if_same_then_else2.stderr index da2be6c8aa5..f98e30fa376 100644 --- a/tests/ui/if_same_then_else2.stderr +++ b/tests/ui/if_same_then_else2.stderr @@ -1,5 +1,5 @@ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:20:12 + --> $DIR/if_same_then_else2.rs:21:12 | LL | } else { | ____________^ @@ -13,7 +13,7 @@ LL | | } | = note: `-D clippy::if-same-then-else` implied by `-D warnings` note: same as this - --> $DIR/if_same_then_else2.rs:11:13 + --> $DIR/if_same_then_else2.rs:12:13 | LL | if true { | _____________^ @@ -26,7 +26,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:34:12 + --> $DIR/if_same_then_else2.rs:35:12 | LL | } else { | ____________^ @@ -36,7 +36,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:32:13 + --> $DIR/if_same_then_else2.rs:33:13 | LL | if true { | _____________^ @@ -45,7 +45,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:41:12 + --> $DIR/if_same_then_else2.rs:42:12 | LL | } else { | ____________^ @@ -55,7 +55,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:39:13 + --> $DIR/if_same_then_else2.rs:40:13 | LL | if true { | _____________^ @@ -64,7 +64,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:91:12 + --> $DIR/if_same_then_else2.rs:92:12 | LL | } else { | ____________^ @@ -74,7 +74,7 @@ LL | | }; | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:89:21 + --> $DIR/if_same_then_else2.rs:90:21 | LL | let _ = if true { | _____________________^ @@ -83,7 +83,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:98:12 + --> $DIR/if_same_then_else2.rs:99:12 | LL | } else { | ____________^ @@ -93,7 +93,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:96:13 + --> $DIR/if_same_then_else2.rs:97:13 | LL | if true { | _____________^ @@ -102,7 +102,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:123:12 + --> $DIR/if_same_then_else2.rs:124:12 | LL | } else { | ____________^ @@ -112,7 +112,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:120:20 + --> $DIR/if_same_then_else2.rs:121:20 | LL | } else if true { | ____________________^