Extend incorrect_fn_null_checks to incorrect_non_null_checks

compiler/rustc_lint/messages.ftl

@@ -215,9 +215,6 @@ 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
@@ -399,6 +396,9 @@ lint_non_fmt_panic_unused =
     }
     .add_fmt_suggestion = or add a "{"{"}{"}"}" format string to use the message literally
 
+lint_non_null_check = {$ty_desc}s can never be null, so checking them for null will always return false
+    .help = wrap the {$ty_desc} inside an `Option` and use `Option::is_none` to check for null pointer values
+
 lint_non_snake_case = {$sort} `{$name}` should have a snake case name
     .rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier
     .cannot_convert_note = `{$sc}` cannot be used as a raw identifier

compiler/rustc_lint/src/lib.rs

@@ -58,7 +58,6 @@ 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;
@@ -72,6 +71,7 @@ mod methods;
 mod multiple_supertrait_upcastable;
 mod non_ascii_idents;
 mod non_fmt_panic;
+mod non_null_check;
 mod nonstandard_style;
 mod noop_method_call;
 mod opaque_hidden_inferred_bound;
@@ -103,7 +103,6 @@ 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::*;
@@ -114,6 +113,7 @@ use methods::*;
 use multiple_supertrait_upcastable::*;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
+use non_null_check::*;
 use nonstandard_style::*;
 use noop_method_call::*;
 use opaque_hidden_inferred_bound::*;
@@ -227,7 +227,7 @@ late_lint_methods!(
             // Depends on types used in type definitions
             MissingCopyImplementations: MissingCopyImplementations,
             // Depends on referenced function signatures in expressions
-            IncorrectFnNullChecks: IncorrectFnNullChecks,
+            IncorrectNonNullChecks: IncorrectNonNullChecks,
             MutableTransmutes: MutableTransmutes,
             TypeAliasBounds: TypeAliasBounds,
             TrivialConstraints: TrivialConstraints,

compiler/rustc_lint/src/lints.rs

@@ -1,5 +1,6 @@
 #![allow(rustc::untranslatable_diagnostic)]
 #![allow(rustc::diagnostic_outside_of_impl)]
+use std::borrow::Cow;
 use std::num::NonZeroU32;
 
 use crate::fluent_generated as fluent;
@@ -613,11 +614,13 @@ pub struct ExpectationNote {
     pub rationale: Symbol,
 }
 
-// fn_null_check.rs
+// non_null_check.rs
 #[derive(LintDiagnostic)]
-#[diag(lint_fn_null_check)]
+#[diag(lint_non_null_check)]
 #[help]
-pub struct FnNullCheckDiag;
+pub struct NonNullCheckDiag {
+    pub ty_desc: Cow<'static, str>,
+}
 
 // for_loops_over_fallibles.rs
 #[derive(LintDiagnostic)]

compiler/rustc_lint/src/fn_null_check.rs → compiler/rustc_lint/src/non_null_check.rs

@@ -1,12 +1,13 @@
-use crate::{lints::FnNullCheckDiag, LateContext, LateLintPass, LintContext};
+use crate::{lints::NonNullCheckDiag, LateContext, LateLintPass, LintContext};
 use rustc_ast::LitKind;
 use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
+use rustc_middle::ty::Ty;
 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.
+    /// The `incorrect_non_null_checks` lint checks for expressions that check if a
+    /// non-nullable type is null.
     ///
     /// ### Example
     ///
@@ -22,85 +23,108 @@ declare_lint! {
     ///
     /// ### Explanation
     ///
-    /// Function pointers are assumed to be non-null, checking them for null will always
-    /// return false.
-    INCORRECT_FN_NULL_CHECKS,
+    /// A non-nullable type is assumed to never be null, and therefore having an actual
+    /// non-null pointer is ub.
+    INCORRECT_NON_NULL_CHECKS,
     Warn,
-    "incorrect checking of null function pointer"
+    "incorrect checking of non null pointers"
 }
 
-declare_lint_pass!(IncorrectFnNullChecks => [INCORRECT_FN_NULL_CHECKS]);
+declare_lint_pass!(IncorrectNonNullChecks => [INCORRECT_NON_NULL_CHECKS]);
 
-fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+/// Is the cast to a nonnull type?
+/// If yes, return (ty, nullable_version) where former is the nonnull type while latter
+/// is a nullable version (e.g. (fn, Option<fn>) or (&u8, *const u8)).
+fn is_nonnull_cast<'a>(cx: &LateContext<'a>, expr: &Expr<'_>) -> Option<Ty<'a>> {
     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()
+    if !had_at_least_one_cast {
+        return None;
+    }
+    let ty = cx.typeck_results().expr_ty_adjusted(expr);
+    if ty.is_fn() || ty.is_ref() {
+        return Some(ty);
+    }
+    // Usually, references get coerced to pointers in a casting situation.
+    // Therefore, we give also give a look to the original type.
+    let ty_unadjusted = cx.typeck_results().expr_ty_opt(expr);
+    if let Some(ty_unadjusted) = ty_unadjusted && ty_unadjusted.is_ref() {
+        return Some(ty_unadjusted);
+    }
+    None
 }
 
-impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks {
+impl<'tcx> LateLintPass<'tcx> for IncorrectNonNullChecks {
     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>)
+            // <*<const/mut> <ty>>::is_null(test_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) =>
+                    && let Some(ty) = is_nonnull_cast(cx, arg) =>
             {
-                cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+                let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) };
+                cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag)
             }
 
             // Catching:
-            // (fn_ptr as *<const/mut> <ty>).is_null()
+            // (test_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) =>
+                    && let Some(ty) = is_nonnull_cast(cx, receiver) =>
             {
-                cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag)
+                let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) };
+                cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag)
             }
 
             ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
                 let to_check: &Expr<'_>;
-                if is_fn_ptr_cast(cx, left) {
+                let ty: Ty<'_>;
+                if let Some(ty_) = is_nonnull_cast(cx, left) {
                     to_check = right;
-                } else if is_fn_ptr_cast(cx, right) {
+                    ty = ty_;
+                } else if let Some(ty_) = is_nonnull_cast(cx, right) {
                     to_check = left;
+                    ty = ty_;
                 } else {
                     return;
                 }
 
                 match to_check.kind {
                     // Catching:
-                    // (fn_ptr as *<const/mut> <ty>) == (0 as <ty>)
+                    // (test_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)
+                        let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) };
+                        cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag)
                     },
 
                     // Catching:
-                    // (fn_ptr as *<const/mut> <ty>) == std::ptr::null()
+                    // (test_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)
+                        let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) };
+                        cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag)
                     },
 
                     _ => {},

tests/ui/lint/non_null_check.rs

@@ -0,0 +1,45 @@
+// check-pass
+
+fn main() {
+    let fn_ptr = main;
+
+    if (fn_ptr as *mut ()).is_null() {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as *const u8).is_null() {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as *const ()) == std::ptr::null() {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as *mut ()) == std::ptr::null_mut() {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as *const ()) == (0 as *const ()) {}
+    //~^ WARN can never be null, so checking
+    if <*const _>::is_null(fn_ptr as *const ()) {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {}
+    //~^ WARN can never be null, so checking
+    if (fn_ptr as fn() as *const ()).is_null() {}
+    //~^ WARN can never be null, so checking
+
+    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 {}
+
+    // Non fn pointers
+
+    let tup_ref: &_ = &(10u8, 10u8);
+    if (tup_ref as *const (u8, u8)).is_null() {}
+    //~^ WARN can never be null, so checking
+    if (&mut (10u8, 10u8) as *mut (u8, u8)).is_null() {}
+    //~^ WARN can never be null, so checking
+
+    // We could warn on these too, but don't:
+    if Box::into_raw(Box::new("hi")).is_null() {}
+
+    let ptr = &mut () as *mut ();
+    if core::ptr::NonNull::new(ptr).unwrap().as_ptr().is_null() {}
+
+}