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

@@ -984,6 +984,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

@@ -839,6 +839,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!(
         ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq,
         WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero,
         RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
+        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,91 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::res::{MaybeDef, MaybeResPath};
+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, QPath, 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()
+            // Parse `NonNull::new_unchecked`
+            && let ExprKind::Call(nonnull_new_unchecked, [arg]) = expr.kind
+            && let ExprKind::Path(QPath::TypeRelative(nonnnull, new_unchecked)) = nonnull_new_unchecked.kind
+            && new_unchecked.ident.name == sym::new_unchecked
+            && nonnnull.basic_res().is_diag_item(cx, sym::NonNull)
+            // Parse `Box::into_raw`
+            && let ExprKind::Call(box_into_raw, [arg]) = arg.kind
+            && let ExprKind::Path(QPath::TypeRelative(r#box, into_raw)) = box_into_raw.kind
+            && into_raw.ident.name == sym::into_raw
+            && r#box.basic_res().is_lang_item(cx, LangItem::OwnedBox)
+            && self.msrv.meets(cx, msrvs::NONNULL_FROM_MUT)
+        {
+            let ctxt = expr.span.ctxt();
+            let span = match cx.tcx.parent_hir_node(expr.hir_id) {
+                Node::Block(
+                    block @ &Block {
+                        rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+                        span: unsafe_span,
+                        ..
+                    },
+                ) if unsafe_span.ctxt() == ctxt && !is_expr_unsafe(cx, arg) && block.stmts.is_empty() => unsafe_span,
+                _ => expr.span,
+            };
+
+            let mut app = Applicability::MachineApplicable;
+            let arg_name = snippet_with_context(cx, arg.span, ctxt, "_", &mut app).0;
+
+            span_lint_and_sugg(
+                cx,
+                NONNULL_UNCHECKED_ON_BOX_PTR,
+                span,
+                "use of `NonNull::new_unchecked` with `Box::into_raw`",
+                "try",
+                format!("NonNull::from_mut(Box::leak({arg_name}))"),
+                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 }

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,152 @@
+#![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));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(identity(one)));
+        //~^ nonnull_unchecked_on_box_ptr
+    }
+
+    fn qualifiers() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        use Box as Box2;
+        use NonNull as NonNull2;
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+    }
+
+    fn macros() {
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = identity!(NonNull::from_mut(Box::leak(one)));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(one));
+        //~^ nonnull_unchecked_on_box_ptr
+
+        let one = Box::new(1);
+        let _ = NonNull::from_mut(Box::leak(weird!(one)));
+        //~^ nonnull_unchecked_on_box_ptr
+    }
+
+    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.88"]
+fn msrv_1_88() {
+    let one = Box::new(1);
+    let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
+}
+
+#[clippy::msrv = "1.89"]
+fn msrv_1_89() {
+    let one = Box::new(1);
+    let _ = NonNull::from_mut(Box::leak(one));
+    //~^ nonnull_unchecked_on_box_ptr
+}
+
+fn main() {}