New cmp_null lint (fixes #1184) (#1186)

* new cmp_null lint (fixes #1184)

* adressed comments (still fails)

* fixed tests, dogfood, ran update_lints

3 files changed, 58 insertions, 14 deletions

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 8c38ea7baeb..07d504f75ad 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -112,7 +112,7 @@ pub mod overflow_check_conditional;
 pub mod panic;
 pub mod precedence;
 pub mod print;
-pub mod ptr_arg;
+pub mod ptr;
 pub mod ranges;
 pub mod regex;
 pub mod returns;
@@ -182,7 +182,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box enum_glob_use::EnumGlobUse);
     reg.register_late_lint_pass(box enum_clike::UnportableVariant);
     reg.register_late_lint_pass(box bit_mask::BitMask);
-    reg.register_late_lint_pass(box ptr_arg::PtrArg);
+    reg.register_late_lint_pass(box ptr::PointerPass);
     reg.register_late_lint_pass(box needless_bool::NeedlessBool);
     reg.register_late_lint_pass(box needless_bool::BoolComparison);
     reg.register_late_lint_pass(box approx_const::Pass);
@@ -405,7 +405,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         panic::PANIC_PARAMS,
         precedence::PRECEDENCE,
         print::PRINT_WITH_NEWLINE,
-        ptr_arg::PTR_ARG,
+        ptr::CMP_NULL,
+        ptr::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,
         regex::INVALID_REGEX,
diff --git a/clippy_lints/src/ptr_arg.rs b/clippy_lints/src/ptr.rs
index f985b2a0031..dea950ac063 100644
--- a/clippy_lints/src/ptr_arg.rs
+++ b/clippy_lints/src/ptr.rs
@@ -5,14 +5,14 @@ use rustc::hir::map::NodeItem;
 use rustc::lint::*;
 use rustc::ty;
 use syntax::ast::NodeId;
-use utils::{match_type, paths, span_lint};
+use utils::{match_path, match_type, paths, span_lint};
 
-/// **What it does:** Checks for function arguments of type `&String` or `&Vec`
-/// unless the references are mutable.
+/// **What it does:** This lint checks for function arguments of type `&String` or `&Vec` unless
+/// the references are mutable.
 ///
-/// **Why is this bad?** Requiring the argument to be of the specific size makes
-/// the function less useful for no benefit; slices in the form of `&[T]` or
-/// `&str` usually suffice and can be obtained from other types, too.
+/// **Why is this bad?** Requiring the argument to be of the specific size makes the function less
+/// useful for no benefit; slices in the form of `&[T]` or `&str` usually suffice and can be
+/// obtained from other types, too.
 ///
 /// **Known problems:** None.
 ///
@@ -23,19 +23,38 @@ use utils::{match_type, paths, span_lint};
 declare_lint! {
     pub PTR_ARG,
     Warn,
-    "arguments of the type `&Vec<...>` (instead of `&[...]`) or `&String` (instead of `&str`)"
+    "fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` \
+     instead, respectively"
 }
 
+/// **What it does:** This lint checks for equality comparisons with `ptr::null`
+///
+/// **Why is this bad?** It's easier and more readable to use the inherent `.is_null()`
+/// method instead
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// if x == ptr::null { .. }
+/// ```
+declare_lint! {
+    pub CMP_NULL,
+    Warn,
+    "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead."
+}
+
+
 #[derive(Copy,Clone)]
-pub struct PtrArg;
+pub struct PointerPass;
 
-impl LintPass for PtrArg {
+impl LintPass for PointerPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(PTR_ARG)
+        lint_array!(PTR_ARG, CMP_NULL)
     }
 }
 
-impl LateLintPass for PtrArg {
+impl LateLintPass for PointerPass {
     fn check_item(&mut self, cx: &LateContext, item: &Item) {
         if let ItemFn(ref decl, _, _, _, _, _) = item.node {
             check_fn(cx, decl, item.id);
@@ -58,6 +77,17 @@ impl LateLintPass for PtrArg {
             check_fn(cx, &sig.decl, item.id);
         }
     }
+    
+    fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+        if let ExprBinary(ref op, ref l, ref r) = expr.node {
+            if (op.node == BiEq || op.node == BiNe) && (is_null_path(l) || is_null_path(r)) {
+                span_lint(cx,
+                          CMP_NULL,
+                          expr.span,
+                          "Comparing with null is better expressed by the .is_null() method");
+            }
+        }
+    }
 }
 
 fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) {
@@ -81,3 +111,14 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) {
         }
     }
 }
+
+fn is_null_path(expr: &Expr) -> bool {
+    if let ExprCall(ref pathexp, ref args) = expr.node {
+        if args.is_empty() {
+            if let ExprPath(_, ref path) = pathexp.node {
+                return match_path(path, &paths::PTR_NULL) || match_path(path, &paths::PTR_NULL_MUT)
+            }
+        }
+    }
+    false
+}
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
index 51071aed7a9..a652d0f42f8 100644
--- a/clippy_lints/src/utils/paths.rs
+++ b/clippy_lints/src/utils/paths.rs
@@ -31,6 +31,8 @@ pub const MUTEX: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
 pub const OPEN_OPTIONS: [&'static str; 3] = ["std", "fs", "OpenOptions"];
 pub const OPS_MODULE: [&'static str; 2] = ["core", "ops"];
 pub const OPTION: [&'static str; 3] = ["core", "option", "Option"];
+pub const PTR_NULL: [&'static str; 2] = ["ptr", "null"];
+pub const PTR_NULL_MUT: [&'static str; 2] = ["ptr", "null_mut"];
 pub const RANGE: [&'static str; 3] = ["core", "ops", "Range"];
 pub const RANGE_FROM: [&'static str; 3] = ["core", "ops", "RangeFrom"];
 pub const RANGE_FROM_STD: [&'static str; 3] = ["std", "ops", "RangeFrom"];