implement nonnull_unchecked_on_box_ptr lint

Author: ArhanChaudhary

CHANGELOG.md

@@ -7168,6 +7168,7 @@ Released 2018-09-13
 [`non_std_lazy_statics`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_std_lazy_statics
 [`non_zero_suggestions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_zero_suggestions
 [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
+[`nonnull_unchecked_on_box_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonnull_unchecked_on_box_ptr
 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
 [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref

book/src/lint_configuration.md

@@ -985,6 +985,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)
 * [`needless_borrow`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow)
 * [`non_std_lazy_statics`](https://rust-lang.github.io/rust-clippy/master/index.html#non_std_lazy_statics)
+* [`nonnull_unchecked_on_box_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#nonnull_unchecked_on_box_ptr)
 * [`option_as_ref_deref`](https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref)
 * [`or_fun_call`](https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call)
 * [`ptr_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr)

clippy_config/src/conf.rs

@@ -840,6 +840,7 @@ define_Conf! {
         missing_const_for_fn,
         needless_borrow,
         non_std_lazy_statics,
+        nonnull_unchecked_on_box_ptr,
         option_as_ref_deref,
         or_fun_call,
         ptr_as_ptr,

clippy_lints/src/declared_lints.rs

@@ -590,6 +590,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO,
     crate::non_std_lazy_statics::NON_STD_LAZY_STATICS_INFO,
     crate::non_zero_suggestions::NON_ZERO_SUGGESTIONS_INFO,
+    crate::nonnull_unchecked_on_box_ptr::NONNULL_UNCHECKED_ON_BOX_PTR_INFO,
     crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
     crate::octal_escapes::OCTAL_ESCAPES_INFO,
     crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,

clippy_lints/src/lib.rs

@@ -280,6 +280,7 @@ mod non_octal_unix_permissions;
 mod non_send_fields_in_send_ty;
 mod non_std_lazy_statics;
 mod non_zero_suggestions;
+mod nonnull_unchecked_on_box_ptr;
 mod nonstandard_macro_braces;
 mod octal_escapes;
 mod only_used_in_recursion;
@@ -862,6 +863,7 @@ rustc_lint::late_lint_methods!(
         WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero,
         RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
         RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
+        NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf),
         // add late passes here, used by `cargo dev new_lint`
     ]]
 );

clippy_lints/src/nonnull_unchecked_on_box_ptr.rs

@@ -0,0 +1,98 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::res::{MaybeDef, MaybeQPath};
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::sym;
+use clippy_utils::visitors::is_expr_unsafe;
+use rustc_errors::Applicability;
+use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, LangItem, Node, UnsafeSource};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for unsafe usage of `NonNull::new_unchecked(Box::into_raw(x))`, and suggests calling `NonNull::from_mut(Box::leak(x))` instead.
+    ///
+    /// ### Why is this bad?
+    /// `NonNull::new_unchecked` is an unsafe function, which we don't need to call at all if we can instead use a mutable reference.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use std::ptr::NonNull;
+    /// let one = Box::new(1);
+    /// let ptr = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// use std::ptr::NonNull;
+    /// let one = Box::new(1);
+    /// let ptr = NonNull::from_mut(Box::leak(one));
+    /// ```
+    #[clippy::version = "1.98.0"]
+    pub NONNULL_UNCHECKED_ON_BOX_PTR,
+    complexity,
+    "using `NonNull::new_unchecked` with `Box::into_raw`, while `NonNull::from_mut` with `Box::leak` can be used instead"
+}
+
+impl_lint_pass!(NonnullUncheckedOnBoxPtr => [NONNULL_UNCHECKED_ON_BOX_PTR]);
+
+pub struct NonnullUncheckedOnBoxPtr {
+    msrv: Msrv,
+}
+
+impl NonnullUncheckedOnBoxPtr {
+    pub fn new(conf: &Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for NonnullUncheckedOnBoxPtr {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if !expr.span.from_expansion()
+            && let ExprKind::Call(nonnull_new_unchecked, [arg]) = expr.kind
+            && let ExprKind::Call(box_into_raw, [arg]) = arg.kind
+            && nonnull_new_unchecked
+                .ty_rel_def_if_named(cx, sym::new_unchecked)
+                .opt_parent(cx)
+                .opt_impl_ty(cx)
+                .is_diag_item(cx, sym::NonNull)
+            && box_into_raw
+                .ty_rel_def_if_named(cx, sym::into_raw)
+                .opt_parent(cx)
+                .opt_impl_ty(cx)
+                .is_lang_item(cx, LangItem::OwnedBox)
+            && self.msrv.meets(cx, msrvs::BOX_LEAK)
+        {
+            let ctxt = expr.span.ctxt();
+            let span = match cx.tcx.parent_hir_node(expr.hir_id) {
+                Node::Block(&Block {
+                    rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+                    span: unsafe_span,
+                    stmts,
+                    ..
+                }) if unsafe_span.ctxt() == ctxt && !is_expr_unsafe(cx, arg) && stmts.is_empty() => unsafe_span,
+                _ => expr.span,
+            };
+
+            span_lint_and_then(
+                cx,
+                NONNULL_UNCHECKED_ON_BOX_PTR,
+                span,
+                "use of `NonNull::new_unchecked` with `Box::into_raw`",
+                |diag| {
+                    let mut app = Applicability::MachineApplicable;
+                    let arg_name = snippet_with_context(cx, arg.span, ctxt, "_", &mut app).0;
+
+                    let sugg = if self.msrv.meets(cx, msrvs::NONNULL_FROM_MUT) {
+                        format!("NonNull::from_mut(Box::leak({arg_name}))")
+                    } else {
+                        format!("NonNull::from(Box::leak({arg_name}))")
+                    };
+
+                    diag.span_suggestion(span, "try", sugg, app);
+                },
+            );
+        }
+    }
+}

clippy_utils/src/msrvs.rs

@@ -27,6 +27,7 @@ msrv_aliases! {
     1,97,0 { ISOLATE_LOWEST_ONE }
     1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF }
     1,91,0 { DURATION_FROM_MINUTES_HOURS }
+    1,89,0 { NONNULL_FROM_MUT }
     1,88,0 { LET_CHAINS, AS_CHUNKS }
     1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST }
     1,86,0 { VEC_POP_IF }
@@ -80,7 +81,7 @@ msrv_aliases! {
     1,29,0 { ITER_FLATTEN }
     1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
     1,27,0 { ITERATOR_TRY_FOLD, DOUBLE_ENDED_ITERATOR_RFIND, DURATION_FROM_NANOS_MICROS }
-    1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
+    1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS, BOX_LEAK }
     1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
     1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
     1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }

clippy_utils/src/sym.rs

@@ -363,6 +363,7 @@ generate! {
     into_bytes,
     into_ok,
     into_owned,
+    into_raw,
     intrinsics_unaligned_volatile_load,
     intrinsics_unaligned_volatile_store,
     io,

tests/ui/nonnull_unchecked_on_box_ptr.fixed

@@ -0,0 +1,156 @@
+#![warn(clippy::nonnull_unchecked_on_box_ptr)]
+
+use std::ptr::NonNull;
+
+macro_rules! identity {
+    ($x:expr) => {
+        $x
+    };
+}
+
+macro_rules! weird {
+    ($x:expr) => {{
+        let y = 1;
+        $x
+    }};
+}
+
+macro_rules! from_macro {
+    ($x:expr) => {
+        unsafe { NonNull::new_unchecked(Box::into_raw($x)) }
+    };
+}
+
+fn identity<T>(x: T) -> T {
+    x
+}
+
+unsafe fn unsafe_identity<T>(x: T) -> T {
+    x
+}
+
+fn lint() {
+    fn basic() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(identity(one)));
+    }
+
+    fn qualifiers() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        {
+            use Box as Box2;
+            use NonNull as NonNull2;
+            let one = Box::new(1);
+            let _ = NonNull::from_mut(Box::leak(one));
+        }
+
+        {
+            type Box2<T> = Box<T>;
+            type NonNull2<T> = NonNull<T>;
+            let one = Box::new(1);
+            let _ = NonNull::from_mut(Box::leak(one));
+        }
+    }
+
+    fn macros() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = identity!(NonNull::from_mut(Box::leak(one)));
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(weird!(one)));
+    }
+
+    fn keep_unsafe_block() {
+        let one = Box::new(1);
+        let _ = unsafe {
+            NonNull::from_mut(Box::leak(unsafe_identity(one)))
+            //~^ nonnull_unchecked_on_box_ptr
+        };
+
+        let one = Box::new(1);
+        let _ = unsafe {
+            identity(NonNull::from_mut(Box::leak(one)))
+            //~^ nonnull_unchecked_on_box_ptr
+        };
+
+        let one = Box::new(1);
+        let _ = unsafe {
+            unsafe_identity(NonNull::from_mut(Box::leak(one)))
+            //~^ nonnull_unchecked_on_box_ptr
+        };
+
+        let _ = unsafe {
+            NonNull::from_mut(Box::leak(Box::new(std::num::NonZeroI32::new_unchecked(1))))
+            //~^ nonnull_unchecked_on_box_ptr
+        };
+
+        let _ = unsafe {
+            let one = Box::new(1);
+            NonNull::from_mut(Box::leak(one))
+            //~^ nonnull_unchecked_on_box_ptr
+        };
+    }
+}
+
+fn no_lint() {
+    fn basic() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+
+        let one = Box::new(1);
+        let _ = unsafe { NonNull::new_unchecked(identity(Box::into_raw(one))) };
+    }
+
+    // TODO?
+    fn does_not_check_expr_init() {
+        let one = Box::new(1);
+        let leaked = Box::into_raw(one);
+        let _ = unsafe { NonNull::new_unchecked(leaked) };
+
+        let one = Box::new(1);
+        let leaked = Box::leak(one);
+        let _ = NonNull::from_mut(leaked);
+    }
+
+    fn macros() {
+        let one = Box::new(1);
+        let _ = from_macro!(one);
+    }
+}
+
+#[clippy::msrv = "1.25"]
+fn msrv_1_25() {
+    let one = Box::new(1);
+    let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
+}
+
+#[clippy::msrv = "1.26"]
+fn msrv_1_26() {
+    let one = Box::new(1);
+    let _ = NonNull::from(Box::leak(one));
+}
+
+#[clippy::msrv = "1.89"]
+fn msrv_1_89() {
+    let one = Box::new(1);
+    let _ = NonNull::from_mut(Box::leak(one));
+}
+
+fn main() {}