Skip to content

Commit b8d0b16

Browse files
authored
autofix for cmp_null (#14122)
changelog: [`cmp_null`]: add autofix
2 parents c607f78 + 3155dab commit b8d0b16

File tree

4 files changed

+86
-14
lines changed

4 files changed

+86
-14
lines changed

clippy_lints/src/ptr.rs

+20-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
1+
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
22
use clippy_utils::source::SpanRangeExt;
3+
use clippy_utils::sugg::Sugg;
34
use clippy_utils::visitors::contains_unsafe_block;
45
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, path_def_id, path_to_local, std_or_core};
56
use hir::LifetimeName;
@@ -250,15 +251,24 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
250251
}
251252

252253
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
253-
if let ExprKind::Binary(ref op, l, r) = expr.kind {
254-
if (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) && (is_null_path(cx, l) || is_null_path(cx, r)) {
255-
span_lint(
256-
cx,
257-
CMP_NULL,
258-
expr.span,
259-
"comparing with null is better expressed by the `.is_null()` method",
260-
);
261-
}
254+
if let ExprKind::Binary(op, l, r) = expr.kind
255+
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
256+
{
257+
let non_null_path_snippet = match (is_null_path(cx, l), is_null_path(cx, r)) {
258+
(true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
259+
(false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
260+
_ => return,
261+
};
262+
263+
span_lint_and_sugg(
264+
cx,
265+
CMP_NULL,
266+
expr.span,
267+
"comparing with null is better expressed by the `.is_null()` method",
268+
"try",
269+
format!("{non_null_path_snippet}.is_null()"),
270+
Applicability::MachineApplicable,
271+
);
262272
} else {
263273
check_invalid_ptr_usage(cx, expr);
264274
}

tests/ui/cmp_null.fixed

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![warn(clippy::cmp_null)]
2+
#![allow(unused_mut)]
3+
4+
use std::ptr;
5+
6+
fn main() {
7+
let x = 0;
8+
let p: *const usize = &x;
9+
if p.is_null() {
10+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
11+
//~| NOTE: `-D clippy::cmp-null` implied by `-D warnings`
12+
println!("This is surprising!");
13+
}
14+
if p.is_null() {
15+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
16+
println!("This is surprising!");
17+
}
18+
19+
let mut y = 0;
20+
let mut m: *mut usize = &mut y;
21+
if m.is_null() {
22+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
23+
println!("This is surprising, too!");
24+
}
25+
if m.is_null() {
26+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
27+
println!("This is surprising, too!");
28+
}
29+
30+
let _ = (x as *const ()).is_null();
31+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
32+
}

tests/ui/cmp_null.rs

+12
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,22 @@ fn main() {
1111
//~| NOTE: `-D clippy::cmp-null` implied by `-D warnings`
1212
println!("This is surprising!");
1313
}
14+
if ptr::null() == p {
15+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
16+
println!("This is surprising!");
17+
}
18+
1419
let mut y = 0;
1520
let mut m: *mut usize = &mut y;
1621
if m == ptr::null_mut() {
1722
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
1823
println!("This is surprising, too!");
1924
}
25+
if ptr::null_mut() == m {
26+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
27+
println!("This is surprising, too!");
28+
}
29+
30+
let _ = x as *const () == ptr::null();
31+
//~^ ERROR: comparing with null is better expressed by the `.is_null()` method
2032
}

tests/ui/cmp_null.stderr

+22-4
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,34 @@ error: comparing with null is better expressed by the `.is_null()` method
22
--> tests/ui/cmp_null.rs:9:8
33
|
44
LL | if p == ptr::null() {
5-
| ^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
66
|
77
= note: `-D clippy::cmp-null` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::cmp_null)]`
99

1010
error: comparing with null is better expressed by the `.is_null()` method
11-
--> tests/ui/cmp_null.rs:16:8
11+
--> tests/ui/cmp_null.rs:14:8
12+
|
13+
LL | if ptr::null() == p {
14+
| ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
15+
16+
error: comparing with null is better expressed by the `.is_null()` method
17+
--> tests/ui/cmp_null.rs:21:8
1218
|
1319
LL | if m == ptr::null_mut() {
14-
| ^^^^^^^^^^^^^^^^^^^^
20+
| ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
21+
22+
error: comparing with null is better expressed by the `.is_null()` method
23+
--> tests/ui/cmp_null.rs:25:8
24+
|
25+
LL | if ptr::null_mut() == m {
26+
| ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
27+
28+
error: comparing with null is better expressed by the `.is_null()` method
29+
--> tests/ui/cmp_null.rs:30:13
30+
|
31+
LL | let _ = x as *const () == ptr::null();
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(x as *const ()).is_null()`
1533

16-
error: aborting due to 2 previous errors
34+
error: aborting due to 5 previous errors
1735

0 commit comments

Comments
 (0)