Merge pull request #2298 from goodmanjonathan/else_if_without_else

add new lint else_if_without_else

clippy_lints/src/else_if_without_else.rs

@@ -0,0 +1,70 @@
+//! lint on if expressions with an else if, but without a final else branch
+
+use rustc::lint::*;
+use syntax::ast::*;
+
+use utils::{in_external_macro, span_lint_and_sugg};
+
+/// **What it does:** Checks for usage of if expressions with an `else if` branch,
+/// but without a final `else` branch.
+///
+/// **Why is this bad?** Some coding guidelines require this (e.g. MISRA-C:2004 Rule 14.10).
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// if x.is_positive() {
+///     a();
+/// } else if x.is_negative() {
+///     b();
+/// }
+/// ```
+///
+/// Could be written:
+///
+/// ```rust
+/// if x.is_positive() {
+///     a();
+/// } else if x.is_negative() {
+///     b();
+/// } else {
+///     // we don't care about zero
+/// }
+/// ```
+declare_restriction_lint! {
+    pub ELSE_IF_WITHOUT_ELSE,
+    "if expression with an `else if`, but without a final `else` branch"
+}
+
+#[derive(Copy, Clone)]
+pub struct ElseIfWithoutElse;
+
+impl LintPass for ElseIfWithoutElse {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(ELSE_IF_WITHOUT_ELSE)
+    }
+}
+
+impl EarlyLintPass for ElseIfWithoutElse {
+    fn check_expr(&mut self, cx: &EarlyContext, mut item: &Expr) {
+        if in_external_macro(cx, item.span) {
+            return;
+        }
+
+        while let ExprKind::If(_, _, Some(ref els)) = item.node {
+            if let ExprKind::If(_, _, None) = els.node {
+                span_lint_and_sugg(
+                    cx,
+                    ELSE_IF_WITHOUT_ELSE,
+                    els.span,
+                    "if expression with an `else if`, but without a final `else`",
+                    "add an `else` block here",
+                    "".to_string()
+                );
+            }
+
+            item = els;
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -88,6 +88,7 @@ pub mod derive;
 pub mod doc;
 pub mod double_parens;
 pub mod drop_forget_ref;
+pub mod else_if_without_else;
 pub mod empty_enum;
 pub mod entry;
 pub mod enum_clike;
@@ -329,6 +330,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_early_lint_pass(box formatting::Formatting);
     reg.register_late_lint_pass(box swap::Swap);
     reg.register_early_lint_pass(box if_not_else::IfNotElse);
+    reg.register_early_lint_pass(box else_if_without_else::ElseIfWithoutElse);
     reg.register_early_lint_pass(box int_plus_one::IntPlusOne);
     reg.register_late_lint_pass(box overflow_check_conditional::OverflowCheckConditional);
     reg.register_late_lint_pass(box unused_label::UnusedLabel);
@@ -369,6 +371,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         arithmetic::INTEGER_ARITHMETIC,
         array_indexing::INDEXING_SLICING,
         assign_ops::ASSIGN_OPS,
+        else_if_without_else::ELSE_IF_WITHOUT_ELSE,
         misc::FLOAT_CMP_CONST,
     ]);
 

tests/ui/else_if_without_else.rs

@@ -0,0 +1,50 @@
+#![warn(clippy)]
+#![warn(else_if_without_else)]
+
+fn bla1() -> bool { unimplemented!() }
+fn bla2() -> bool { unimplemented!() }
+fn bla3() -> bool { unimplemented!() }
+
+fn main() {
+    if bla1() {
+        println!("if");
+    }
+
+    if bla1() {
+        println!("if");
+    } else {
+        println!("else");
+    }
+
+    if bla1() {
+        println!("if");
+    } else if bla2() {
+        println!("else if");
+    } else {
+        println!("else")
+    }
+
+    if bla1() {
+        println!("if");
+    } else if bla2() {
+        println!("else if 1");
+    } else if bla3() {
+        println!("else if 2");
+    } else {
+        println!("else")
+    }
+
+    if bla1() {
+        println!("if");
+    } else if bla2() { //~ ERROR else if without else
+        println!("else if");
+    }
+
+    if bla1() {
+        println!("if");
+    } else if bla2() {
+        println!("else if 1");
+    } else if bla3() { //~ ERROR else if without else
+        println!("else if 2");
+    }
+}

tests/ui/else_if_without_else.stderr

@@ -0,0 +1,20 @@
+error: if expression with an `else if`, but without a final `else`
+  --> $DIR/else_if_without_else.rs:39:12
+   |
+39 |       } else if bla2() { //~ ERROR else if without else
+   |  ____________^
+40 | |         println!("else if");
+41 | |     }
+   | |_____^ help: add an `else` block here
+   |
+   = note: `-D else-if-without-else` implied by `-D warnings`
+
+error: if expression with an `else if`, but without a final `else`
+  --> $DIR/else_if_without_else.rs:47:12
+   |
+47 |       } else if bla3() { //~ ERROR else if without else
+   |  ____________^
+48 | |         println!("else if 2");
+49 | |     }
+   | |_____^ help: add an `else` block here
+