collapsible_if: split collapsible_else_if into its own lint so we can enable/disable it particularly

This splits up clippy::collapsible_if into collapsible_if for if x { if y { } } => if x && y { } and collapsible_else_if for if x { } else { if y { } } => if x { } else if y { } so that we can lint for only the latter but not the first if we desire. changelog: collapsible_if: split up linting for if x {} else { if y {} } into collapsible_else_if lint
2021-01-04 08:25:38 +01:00 · 2021-01-04 08:25:38 +01:00 · 6dcec6ae86
commit 6dcec6ae86
parent 895191628d
8 changed files with 59 additions and 34 deletions
--- 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
--- 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",
--- 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),
--- 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";
--- 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";
--- 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 {
   |  ____________^
--- 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,
--- 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 {
   |  ____________________^