add autofix for cmp_null

rust-lang · Feb 1, 2025 · 2211aef · 2211aef
1 parent 88a00a8
commit 2211aef
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 8 deletions.
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs
@@ -1,5 +1,6 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::source::SpanRangeExt;
+use clippy_utils::sugg::Sugg;
 use clippy_utils::visitors::contains_unsafe_block;
 use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, path_def_id, path_to_local, std_or_core};
 use hir::LifetimeName;
@@ -250,13 +251,22 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
     }
 
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if let ExprKind::Binary(ref op, l, r) = expr.kind {
-            if (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) && (is_null_path(cx, l) || is_null_path(cx, r)) {
-                span_lint(
+        if let ExprKind::Binary(op, l, r) = expr.kind {
+            if op.node == BinOpKind::Eq || op.node == BinOpKind::Ne {
+                let non_null_path_snippet = match (is_null_path(cx, l), is_null_path(cx, r)) {
+                    (true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par().to_string(),
+                    (false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par().to_string(),
+                    _ => return,
+                };
+
+                span_lint_and_sugg(
                     cx,
                     CMP_NULL,
                     expr.span,
                     "comparing with null is better expressed by the `.is_null()` method",
+                    "try",
+                    format!("{non_null_path_snippet}.is_null()"),
+                    Applicability::MachineApplicable,
                 );
             }
         } else {

diff --git a/tests/ui/cmp_null.fixed b/tests/ui/cmp_null.fixed
@@ -0,0 +1,32 @@
+#![warn(clippy::cmp_null)]
+#![allow(unused_mut)]
+
+use std::ptr;
+
+fn main() {
+    let x = 0;
+    let p: *const usize = &x;
+    if p.is_null() {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        //~| NOTE: `-D clippy::cmp-null` implied by `-D warnings`
+        println!("This is surprising!");
+    }
+    if p.is_null() {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        println!("This is surprising!");
+    }
+
+    let mut y = 0;
+    let mut m: *mut usize = &mut y;
+    if m.is_null() {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        println!("This is surprising, too!");
+    }
+    if m.is_null() {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        println!("This is surprising, too!");
+    }
+
+    let _ = (x as *const ()).is_null();
+    //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+}
diff --git a/tests/ui/cmp_null.rs b/tests/ui/cmp_null.rs
@@ -11,10 +11,22 @@ fn main() {
         //~| NOTE: `-D clippy::cmp-null` implied by `-D warnings`
         println!("This is surprising!");
     }
+    if ptr::null() == p {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        println!("This is surprising!");
+    }
+
     let mut y = 0;
     let mut m: *mut usize = &mut y;
     if m == ptr::null_mut() {
         //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
         println!("This is surprising, too!");
     }
+    if ptr::null_mut() == m {
+        //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
+        println!("This is surprising, too!");
+    }
+
+    let _ = x as *const () == ptr::null();
+    //~^ ERROR: comparing with null is better expressed by the `.is_null()` method
 }
diff --git a/tests/ui/cmp_null.stderr b/tests/ui/cmp_null.stderr
@@ -2,16 +2,34 @@ error: comparing with null is better expressed by the `.is_null()` method
   --> tests/ui/cmp_null.rs:9:8
    |
 LL |     if p == ptr::null() {
-   |        ^^^^^^^^^^^^^^^^
+   |        ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
    |
    = note: `-D clippy::cmp-null` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::cmp_null)]`
 
 error: comparing with null is better expressed by the `.is_null()` method
-  --> tests/ui/cmp_null.rs:16:8
+  --> tests/ui/cmp_null.rs:14:8
+   |
+LL |     if ptr::null() == p {
+   |        ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
+
+error: comparing with null is better expressed by the `.is_null()` method
+  --> tests/ui/cmp_null.rs:21:8
    |
 LL |     if m == ptr::null_mut() {
-   |        ^^^^^^^^^^^^^^^^^^^^
+   |        ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
+
+error: comparing with null is better expressed by the `.is_null()` method
+  --> tests/ui/cmp_null.rs:25:8
+   |
+LL |     if ptr::null_mut() == m {
+   |        ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
+
+error: comparing with null is better expressed by the `.is_null()` method
+  --> tests/ui/cmp_null.rs:30:13
+   |
+LL |     let _ = x as *const () == ptr::null();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(x as *const ()).is_null()`
 
-error: aborting due to 2 previous errors
+error: aborting due to 5 previous errors