Uplift `clippy::fn_null_check` to rustc

6 files changed, 221 insertions, 0 deletions

diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
index 34b7e09576a..2c92277b50d 100644
--- a/compiler/rustc_lint/messages.ftl
+++ b/compiler/rustc_lint/messages.ftl
@@ -215,6 +215,9 @@ lint_expectation = this lint expectation is unfulfilled
     .note = the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message
     .rationale = {$rationale}
 
+lint_fn_null_check = function pointers are not nullable, so checking them for null will always return false
+    .help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
 lint_for_loops_over_fallibles =
     for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement
     .suggestion = consider using `if let` to clear intent
diff --git a/compiler/rustc_lint/src/fn_null_check.rs b/compiler/rustc_lint/src/fn_null_check.rs
new file mode 100644
index 00000000000..e3b33463ccf
--- /dev/null
+++ b/compiler/rustc_lint/src/fn_null_check.rs
@@ -0,0 +1,112 @@
+use crate::{lints::FnNullCheckDiag, LateContext, LateLintPass, LintContext};
+use rustc_ast::LitKind;
+use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::sym;
+
+declare_lint! {
+    /// The `incorrect_fn_null_checks` lint checks for expression that checks if a
+    /// function pointer is null.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// # fn test() {}
+    /// let fn_ptr: fn() = /* somehow obtained nullable function pointer */
+    /// #   test;
+    ///
+    /// if (fn_ptr as *const ()).is_null() { /* ... */ }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Function pointers are assumed to be non-null, checking them for null will always
+    /// return false.
+    INCORRECT_FN_NULL_CHECKS,
+    Warn,
+    "incorrect checking of null function pointer"
+}
+
+declare_lint_pass!(IncorrectFnNullChecks => [INCORRECT_FN_NULL_CHECKS]);
+
+fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let mut expr = expr.peel_blocks();
+    let mut had_at_least_one_cast = false;
+    while let ExprKind::Cast(cast_expr, cast_ty) = expr.kind
+            && let TyKind::Ptr(_) = cast_ty.kind {
+        expr = cast_expr.peel_blocks();
+        had_at_least_one_cast = true;
+    }
+    had_at_least_one_cast && cx.typeck_results().expr_ty_adjusted(expr).is_fn()
+}
+
+impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        match expr.kind {
+            // Catching:
+            // <*<const/mut> <ty>>::is_null(fn_ptr as *<const/mut> <ty>)
+            ExprKind::Call(path, [arg])
+                if let ExprKind::Path(ref qpath) = path.kind
+                    && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
+                    && matches!(
+                        cx.tcx.get_diagnostic_name(def_id),
+                        Some(sym::ptr_const_is_null | sym::ptr_is_null)
+                    )
+                    && is_fn_ptr_cast(cx, arg) =>
+            {
+                cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+            }
+
+            // Catching:
+            // (fn_ptr as *<const/mut> <ty>).is_null()
+            ExprKind::MethodCall(_, receiver, _, _)
+                if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
+                    && matches!(
+                        cx.tcx.get_diagnostic_name(def_id),
+                        Some(sym::ptr_const_is_null | sym::ptr_is_null)
+                    )
+                    && is_fn_ptr_cast(cx, receiver) =>
+            {
+                cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+            }
+
+            ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
+                let to_check: &Expr<'_>;
+                if is_fn_ptr_cast(cx, left) {
+                    to_check = right;
+                } else if is_fn_ptr_cast(cx, right) {
+                    to_check = left;
+                } else {
+                    return;
+                }
+
+                match to_check.kind {
+                    // Catching:
+                    // (fn_ptr as *<const/mut> <ty>) == (0 as <ty>)
+                    ExprKind::Cast(cast_expr, _)
+                        if let ExprKind::Lit(spanned) = cast_expr.kind
+                            && let LitKind::Int(v, _) = spanned.node && v == 0 =>
+                    {
+                        cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+                    },
+
+                    // Catching:
+                    // (fn_ptr as *<const/mut> <ty>) == std::ptr::null()
+                    ExprKind::Call(path, [])
+                        if let ExprKind::Path(ref qpath) = path.kind
+                            && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
+                            && let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
+                            && (diag_item == sym::ptr_null || diag_item == sym::ptr_null_mut) =>
+                    {
+                        cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+                    },
+
+                    _ => {},
+                }
+            }
+            _ => {}
+        }
+    }
+}
diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
index 5e3f057d428..beb38dbb94c 100644
--- a/compiler/rustc_lint/src/lib.rs
+++ b/compiler/rustc_lint/src/lib.rs
@@ -58,6 +58,7 @@ mod early;
 mod enum_intrinsics_non_enums;
 mod errors;
 mod expect;
+mod fn_null_check;
 mod for_loops_over_fallibles;
 pub mod hidden_unicode_codepoints;
 mod internal;
@@ -102,6 +103,7 @@ use cast_ref_to_mut::*;
 use deref_into_dyn_supertrait::*;
 use drop_forget_useless::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
+use fn_null_check::*;
 use for_loops_over_fallibles::*;
 use hidden_unicode_codepoints::*;
 use internal::*;
@@ -225,6 +227,7 @@ late_lint_methods!(
             // Depends on types used in type definitions
             MissingCopyImplementations: MissingCopyImplementations,
             // Depends on referenced function signatures in expressions
+            IncorrectFnNullChecks: IncorrectFnNullChecks,
             MutableTransmutes: MutableTransmutes,
             TypeAliasBounds: TypeAliasBounds,
             TrivialConstraints: TrivialConstraints,
diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
index 0613ef2c5e9..1dea758bb29 100644
--- a/compiler/rustc_lint/src/lints.rs
+++ b/compiler/rustc_lint/src/lints.rs
@@ -613,6 +613,12 @@ pub struct ExpectationNote {
     pub rationale: Symbol,
 }
 
+// fn_null_check.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_fn_null_check)]
+#[help]
+pub struct FnNullCheckDiag;
+
 // for_loops_over_fallibles.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_for_loops_over_fallibles)]
diff --git a/tests/ui/lint/fn_null_check.rs b/tests/ui/lint/fn_null_check.rs
new file mode 100644
index 00000000000..7f01f2c4283
--- /dev/null
+++ b/tests/ui/lint/fn_null_check.rs
@@ -0,0 +1,30 @@
+// check-pass
+
+fn main() {
+    let fn_ptr = main;
+
+    if (fn_ptr as *mut ()).is_null() {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as *const u8).is_null() {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as *const ()) == std::ptr::null() {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as *mut ()) == std::ptr::null_mut() {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as *const ()) == (0 as *const ()) {}
+    //~^ WARN function pointers are not nullable
+    if <*const _>::is_null(fn_ptr as *const ()) {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {}
+    //~^ WARN function pointers are not nullable
+    if (fn_ptr as fn() as *const ()).is_null() {}
+    //~^ WARN function pointers are not nullable
+
+    const ZPTR: *const () = 0 as *const _;
+    const NOT_ZPTR: *const () = 1 as *const _;
+
+    // unlike the uplifted clippy::fn_null_check lint we do
+    // not lint on them
+    if (fn_ptr as *const ()) == ZPTR {}
+    if (fn_ptr as *const ()) == NOT_ZPTR {}
+}
diff --git a/tests/ui/lint/fn_null_check.stderr b/tests/ui/lint/fn_null_check.stderr
new file mode 100644
index 00000000000..0398c0da50f
--- /dev/null
+++ b/tests/ui/lint/fn_null_check.stderr
@@ -0,0 +1,67 @@
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:6:8
+   |
+LL |     if (fn_ptr as *mut ()).is_null() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+   = note: `#[warn(incorrect_fn_null_checks)]` on by default
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:8:8
+   |
+LL |     if (fn_ptr as *const u8).is_null() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:10:8
+   |
+LL |     if (fn_ptr as *const ()) == std::ptr::null() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:12:8
+   |
+LL |     if (fn_ptr as *mut ()) == std::ptr::null_mut() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:14:8
+   |
+LL |     if (fn_ptr as *const ()) == (0 as *const ()) {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:16:8
+   |
+LL |     if <*const _>::is_null(fn_ptr as *const ()) {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:18:8
+   |
+LL |     if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: function pointers are not nullable, so checking them for null will always return false
+  --> $DIR/fn_null_check.rs:20:8
+   |
+LL |     if (fn_ptr as fn() as *const ()).is_null() {}
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
+
+warning: 8 warnings emitted
+