merged upstream master

Cargo.toml

@@ -19,3 +19,4 @@ plugin = true
 compiletest_rs = "*"
 regex = "*"
 regex_macros = "*"
+lazy_static = "*"

src/collapsible_if.rs

@@ -0,0 +1,80 @@
+//! Checks for if expressions that contain only an if expression.
+//!
+//! For example, the lint would catch:
+//!
+//! ```
+//! if x {
+//!     if y {
+//!         println!("Hello world");
+//!     }
+//! }
+//! ```
+//!
+//! This lint is **warn** by default
+
+use rustc::plugin::Registry;
+use rustc::lint::*;
+use rustc::middle::def::*;
+use syntax::ast::*;
+use syntax::ptr::P;
+use syntax::codemap::{Span, Spanned};
+use syntax::print::pprust::expr_to_string;
+
+declare_lint! {
+    pub COLLAPSIBLE_IF,
+    Warn,
+    "Warn on if expressions that can be collapsed"
+}
+
+#[derive(Copy,Clone)]
+pub struct CollapsibleIf;
+
+impl LintPass for CollapsibleIf {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(COLLAPSIBLE_IF)
+    }
+    
+    fn check_expr(&mut self, cx: &Context, e: &Expr) {
+        if let ExprIf(ref check, ref then_block, None) = e.node {
+            let expr = check_block(then_block);
+            let expr = match expr {
+                Some(e) => e,
+                None => return
+            };
+            if let ExprIf(ref check_inner, _, None) = expr.node {
+                let (check, check_inner) = (check_to_string(check), check_to_string(check_inner));
+                cx.span_lint(COLLAPSIBLE_IF, e.span,
+                             &format!("This if statement can be collapsed. Try: if {} && {}", check, check_inner));
+            }
+		    }
+    }
+}
+
+fn requires_brackets(e: &Expr) -> bool {
+    match e.node {
+        ExprBinary(Spanned {node: n, ..}, _, _) if n == BiEq => false,
+        _ => true
+    }
+}
+
+fn check_to_string(e: &Expr) -> String {
+    if requires_brackets(e) {
+        format!("({})", expr_to_string(e))
+    } else {
+        format!("{}", expr_to_string(e))
+    }
+}
+
+fn check_block(b: &Block) -> Option<&P<Expr>> {
+    if b.stmts.len() == 1 && b.expr.is_none() {
+        let stmt = &b.stmts[0];
+        return match stmt.node {
+            StmtExpr(ref e, _) => Some(e),
+            _ => None
+        };
+    }
+    if let Some(ref e) = b.expr {
+        return Some(e);
+    }
+    None
+}

src/len_zero.rs

@@ -13,7 +13,7 @@ use syntax::ast::*;
 use misc::walk_ty;
 
 declare_lint!(pub LEN_ZERO, Warn,
-              "Warn on usage of double-mut refs, e.g. '&mut &mut ...'");
+              "Warn when .is_empty() could be used instead of checking .len()");
 
 declare_lint!(pub LEN_WITHOUT_IS_EMPTY, Warn,
               "Warn on traits and impls that have .len() but not .is_empty()");

src/lib.rs

@@ -25,6 +25,7 @@ pub mod identity_op;
 pub mod mut_mut;
 pub mod len_zero;
 pub mod attrs;
+pub mod collapsible_if;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -46,6 +47,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box len_zero::LenZero as LintPassObject);
     reg.register_lint_pass(box misc::CmpOwned as LintPassObject);
     reg.register_lint_pass(box attrs::AttrPass as LintPassObject);
+    reg.register_lint_pass(box collapsible_if::CollapsibleIf as LintPassObject);
     
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
@@ -63,5 +65,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            len_zero::LEN_ZERO,
                                            len_zero::LEN_WITHOUT_IS_EMPTY,
                                            attrs::INLINE_ALWAYS,
+                                           collapsible_if::COLLAPSIBLE_IF,
                                            ]);
 }

src/mut_mut.rs

@@ -2,7 +2,7 @@ use syntax::ptr::P;
 use syntax::ast::*;
 use rustc::lint::{Context, LintPass, LintArray, Lint};
 use rustc::middle::ty::{expr_ty, sty, ty_ptr, ty_rptr, mt};
-use syntax::codemap::ExpnInfo;
+use syntax::codemap::{BytePos, ExpnInfo, MacroFormat, Span};
 
 declare_lint!(pub MUT_MUT, Warn,
               "Warn on usage of double-mut refs, e.g. '&mut &mut ...'");
@@ -27,7 +27,7 @@ impl LintPass for MutMut {
 }
 
 fn check_expr_expd(cx: &Context, expr: &Expr, info: Option<&ExpnInfo>) {
-	if in_macro(info) { return; }
+	if in_macro(cx, info) { return; }
 
 	fn unwrap_addr(expr : &Expr) -> Option<&Expr> {
 		match expr.node {
@@ -51,8 +51,14 @@ fn check_expr_expd(cx: &Context, expr: &Expr, info: Option<&ExpnInfo>) {
 	})
 }
 
-fn in_macro(info: Option<&ExpnInfo>) -> bool {
-	info.is_some()
+fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool {
+	opt_info.map_or(false, |info| {
+		info.callee.span.map_or(true, |span| {
+			cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| 
+				!code.starts_with("macro_rules")
+			)
+		})
+	})
 }
 
 fn unwrap_mut(ty : &Ty) -> Option<&Ty> {

tests/compile-fail/collapsible_if.rs

@@ -0,0 +1,37 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[deny(collapsible_if)]
+fn main() {
+    let x = "hello";
+    let y = "world";
+    if x == "hello" { //~ERROR This if statement can be collapsed
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" || x == "world" { //~ERROR This if statement can be collapsed
+        if y == "world" || y == "hello" {
+            println!("Hello world!");
+        }
+    }
+
+    // Works because any if with an else statement cannot be collapsed.
+    if x == "hello" {
+        if y == "world" {
+            println!("Hello world!");
+        }
+    } else {
+        println!("Not Hello world");
+    }
+
+    if x == "hello" {
+        if y == "world" {
+            println!("Hello world!");
+        } else {
+            println!("Hello something else");
+        }
+    }
+
+}

tests/mut_mut_macro.rs

@@ -1,11 +1,31 @@
 #![feature(plugin)]
 #![plugin(clippy, regex_macros)]
 
+#[macro_use]
+extern crate lazy_static;
 extern crate regex;
 
+use std::collections::HashMap;
+
 #[test]
 #[deny(mut_mut)]
 fn test_regex() {
 	let pattern = regex!(r"^(?P<level>[#]+)\s(?P<title>.+)$");
 	assert!(pattern.is_match("# headline"));
 }
+
+#[test]
+#[deny(mut_mut)]
+#[allow(unused_variables, unused_mut)]
+fn test_lazy_static() {
+	lazy_static! {
+		static ref MUT_MAP : HashMap<usize, &'static str> = {
+			let mut m = HashMap::new();
+			let mut zero = &mut &mut "zero";
+			m.insert(0, "zero");
+			m
+		};
+		static ref MUT_COUNT : usize = MUT_MAP.len();
+	}
+	assert!(*MUT_COUNT == 1);
+}