about summary refs log tree commit diff
diff options
context:
space:
mode:
authorlapla-cogito <me@lapla.dev>2025-01-31 10:39:33 +0900
committerlapla-cogito <me@lapla.dev>2025-02-02 04:22:19 +0900
commit3155dabeaa295692fb7664fafcccc161effa2fe1 (patch)
tree62f80b3740ba44c6fb86669a586c54d669465f59
parent88a00a87fa99ef934430b73a6327e0211c79e8a0 (diff)
downloadrust-3155dabeaa295692fb7664fafcccc161effa2fe1.tar.gz
rust-3155dabeaa295692fb7664fafcccc161effa2fe1.zip
add autofix for `cmp_null`
-rw-r--r--clippy_lints/src/ptr.rs30
-rw-r--r--tests/ui/cmp_null.fixed32
-rw-r--r--tests/ui/cmp_null.rs12
-rw-r--r--tests/ui/cmp_null.stderr26
4 files changed, 86 insertions, 14 deletions
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs
index 506adf0f2cc..0b67594a9b1 100644
--- 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,15 +251,24 @@ 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(
-                    cx,
-                    CMP_NULL,
-                    expr.span,
-                    "comparing with null is better expressed by the `.is_null()` method",
-                );
-            }
+        if let ExprKind::Binary(op, l, r) = expr.kind
+            && (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(),
+                (false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
+                _ => 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 {
             check_invalid_ptr_usage(cx, expr);
         }
diff --git a/tests/ui/cmp_null.fixed b/tests/ui/cmp_null.fixed
new file mode 100644
index 00000000000..e5ab765bc86
--- /dev/null
+++ 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
index ef1d93940aa..257f7ba2662 100644
--- 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
index 8362904a5ba..f3b35f3afba 100644
--- 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