diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 506adf0f2cc5..0b67594a9b19 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 000000000000..e5ab765bc861 --- /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 ef1d93940aa6..257f7ba26627 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 8362904a5ba9..f3b35f3afba8 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