Skip to content

Commit f6d2bf6

Browse files
UrgauUrgau
committed
Uplift clippy::fn_null_check to rustc
1 parent 1e377c1 commit f6d2bf6

File tree

6 files changed

+221
-0
lines changed

6 files changed

+221
-0
lines changed

‎compiler/rustc_lint/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ lint_expectation = this lint expectation is unfulfilled
215215
.note = the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message
216216
.rationale = {$rationale}
217217
218+
lint_fn_null_check = function pointers are not nullable, so checking them for null will always return false
219+
.help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
220+
218221
lint_for_loops_over_fallibles =
219222
for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement
220223
.suggestion = consider using `if let` to clear intent

‎compiler/rustc_lint/src/fn_null_check.rs

+112
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
use crate::{lints::FnNullCheckDiag, LateContext, LateLintPass, LintContext};
2+
use rustc_ast::LitKind;
3+
use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
4+
use rustc_session::{declare_lint, declare_lint_pass};
5+
use rustc_span::sym;
6+
7+
declare_lint! {
8+
/// The `incorrect_fn_null_checks` lint checks for expression that checks if a
9+
/// function pointer is null.
10+
///
11+
/// ### Example
12+
///
13+
/// ```rust
14+
/// # fn test() {}
15+
/// let fn_ptr: fn() = /* somehow obtained nullable function pointer */
16+
/// # test;
17+
///
18+
/// if (fn_ptr as *const ()).is_null() { /* ... */ }
19+
/// ```
20+
///
21+
/// {{produces}}
22+
///
23+
/// ### Explanation
24+
///
25+
/// Function pointers are assumed to be non-null, checking them for null will always
26+
/// return false.
27+
INCORRECT_FN_NULL_CHECKS,
28+
Warn,
29+
"incorrect checking of null function pointer"
30+
}
31+
32+
declare_lint_pass!(IncorrectFnNullChecks => [INCORRECT_FN_NULL_CHECKS]);
33+
34+
fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
35+
let mut expr = expr.peel_blocks();
36+
let mut had_at_least_one_cast = false;
37+
while let ExprKind::Cast(cast_expr, cast_ty) = expr.kind
38+
&& let TyKind::Ptr(_) = cast_ty.kind {
39+
expr = cast_expr.peel_blocks();
40+
had_at_least_one_cast = true;
41+
}
42+
had_at_least_one_cast && cx.typeck_results().expr_ty_adjusted(expr).is_fn()
43+
}
44+
45+
impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks {
46+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
47+
match expr.kind {
48+
// Catching:
49+
// <*<const/mut> <ty>>::is_null(fn_ptr as *<const/mut> <ty>)
50+
ExprKind::Call(path, [arg])
51+
if let ExprKind::Path(ref qpath) = path.kind
52+
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
53+
&& matches!(
54+
cx.tcx.get_diagnostic_name(def_id),
55+
Some(sym::ptr_const_is_null | sym::ptr_is_null)
56+
)
57+
&& is_fn_ptr_cast(cx, arg) =>
58+
{
59+
cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
60+
}
61+
62+
// Catching:
63+
// (fn_ptr as *<const/mut> <ty>).is_null()
64+
ExprKind::MethodCall(_, receiver, _, _)
65+
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
66+
&& matches!(
67+
cx.tcx.get_diagnostic_name(def_id),
68+
Some(sym::ptr_const_is_null | sym::ptr_is_null)
69+
)
70+
&& is_fn_ptr_cast(cx, receiver) =>
71+
{
72+
cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
73+
}
74+
75+
ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
76+
let to_check: &Expr<'_>;
77+
if is_fn_ptr_cast(cx, left) {
78+
to_check = right;
79+
} else if is_fn_ptr_cast(cx, right) {
80+
to_check = left;
81+
} else {
82+
return;
83+
}
84+
85+
match to_check.kind {
86+
// Catching:
87+
// (fn_ptr as *<const/mut> <ty>) == (0 as <ty>)
88+
ExprKind::Cast(cast_expr, _)
89+
if let ExprKind::Lit(spanned) = cast_expr.kind
90+
&& let LitKind::Int(v, _) = spanned.node && v == 0 =>
91+
{
92+
cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
93+
},
94+
95+
// Catching:
96+
// (fn_ptr as *<const/mut> <ty>) == std::ptr::null()
97+
ExprKind::Call(path, [])
98+
if let ExprKind::Path(ref qpath) = path.kind
99+
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
100+
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
101+
&& (diag_item == sym::ptr_null || diag_item == sym::ptr_null_mut) =>
102+
{
103+
cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
104+
},
105+
106+
_ => {},
107+
}
108+
}
109+
_ => {}
110+
}
111+
}
112+
}

‎compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ mod early;
5858
mod enum_intrinsics_non_enums;
5959
mod errors;
6060
mod expect;
61+
mod fn_null_check;
6162
mod for_loops_over_fallibles;
6263
pub mod hidden_unicode_codepoints;
6364
mod internal;
@@ -102,6 +103,7 @@ use cast_ref_to_mut::*;
102103
use deref_into_dyn_supertrait::*;
103104
use drop_forget_useless::*;
104105
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
106+
use fn_null_check::*;
105107
use for_loops_over_fallibles::*;
106108
use hidden_unicode_codepoints::*;
107109
use internal::*;
@@ -225,6 +227,7 @@ late_lint_methods!(
225227
// Depends on types used in type definitions
226228
MissingCopyImplementations: MissingCopyImplementations,
227229
// Depends on referenced function signatures in expressions
230+
IncorrectFnNullChecks: IncorrectFnNullChecks,
228231
MutableTransmutes: MutableTransmutes,
229232
TypeAliasBounds: TypeAliasBounds,
230233
TrivialConstraints: TrivialConstraints,

‎compiler/rustc_lint/src/lints.rs

+6
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,12 @@ pub struct ExpectationNote {
613613
pub rationale: Symbol,
614614
}
615615

616+
// fn_null_check.rs
617+
#[derive(LintDiagnostic)]
618+
#[diag(lint_fn_null_check)]
619+
#[help]
620+
pub struct FnNullCheckDiag;
621+
616622
// for_loops_over_fallibles.rs
617623
#[derive(LintDiagnostic)]
618624
#[diag(lint_for_loops_over_fallibles)]

‎tests/ui/lint/fn_null_check.rs

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// check-pass
2+
3+
fn main() {
4+
let fn_ptr = main;
5+
6+
if (fn_ptr as *mut ()).is_null() {}
7+
//~^ WARN function pointers are not nullable
8+
if (fn_ptr as *const u8).is_null() {}
9+
//~^ WARN function pointers are not nullable
10+
if (fn_ptr as *const ()) == std::ptr::null() {}
11+
//~^ WARN function pointers are not nullable
12+
if (fn_ptr as *mut ()) == std::ptr::null_mut() {}
13+
//~^ WARN function pointers are not nullable
14+
if (fn_ptr as *const ()) == (0 as *const ()) {}
15+
//~^ WARN function pointers are not nullable
16+
if <*const _>::is_null(fn_ptr as *const ()) {}
17+
//~^ WARN function pointers are not nullable
18+
if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {}
19+
//~^ WARN function pointers are not nullable
20+
if (fn_ptr as fn() as *const ()).is_null() {}
21+
//~^ WARN function pointers are not nullable
22+
23+
const ZPTR: *const () = 0 as *const _;
24+
const NOT_ZPTR: *const () = 1 as *const _;
25+
26+
// unlike the uplifted clippy::fn_null_check lint we do
27+
// not lint on them
28+
if (fn_ptr as *const ()) == ZPTR {}
29+
if (fn_ptr as *const ()) == NOT_ZPTR {}
30+
}

‎tests/ui/lint/fn_null_check.stderr

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
warning: function pointers are not nullable, so checking them for null will always return false
2+
--> $DIR/fn_null_check.rs:6:8
3+
|
4+
LL | if (fn_ptr as *mut ()).is_null() {}
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
8+
= note: `#[warn(incorrect_fn_null_checks)]` on by default
9+
10+
warning: function pointers are not nullable, so checking them for null will always return false
11+
--> $DIR/fn_null_check.rs:8:8
12+
|
13+
LL | if (fn_ptr as *const u8).is_null() {}
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
17+
18+
warning: function pointers are not nullable, so checking them for null will always return false
19+
--> $DIR/fn_null_check.rs:10:8
20+
|
21+
LL | if (fn_ptr as *const ()) == std::ptr::null() {}
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
25+
26+
warning: function pointers are not nullable, so checking them for null will always return false
27+
--> $DIR/fn_null_check.rs:12:8
28+
|
29+
LL | if (fn_ptr as *mut ()) == std::ptr::null_mut() {}
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
33+
34+
warning: function pointers are not nullable, so checking them for null will always return false
35+
--> $DIR/fn_null_check.rs:14:8
36+
|
37+
LL | if (fn_ptr as *const ()) == (0 as *const ()) {}
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
41+
42+
warning: function pointers are not nullable, so checking them for null will always return false
43+
--> $DIR/fn_null_check.rs:16:8
44+
|
45+
LL | if <*const _>::is_null(fn_ptr as *const ()) {}
46+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47+
|
48+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
49+
50+
warning: function pointers are not nullable, so checking them for null will always return false
51+
--> $DIR/fn_null_check.rs:18:8
52+
|
53+
LL | if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {}
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
55+
|
56+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
57+
58+
warning: function pointers are not nullable, so checking them for null will always return false
59+
--> $DIR/fn_null_check.rs:20:8
60+
|
61+
LL | if (fn_ptr as fn() as *const ()).is_null() {}
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
63+
|
64+
= help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
65+
66+
warning: 8 warnings emitted
67+

0 commit comments

Comments
 (0)