Auto merge of rust-lang#13369 - samueltardieu:issue-13361, r=y21

bors · bors · commit 131681b4b9c2 · 2024-09-11T15:43:48.000Z
Special-case suggestions for null pointers constness cast This implements the suggestions from rust-lang#13361. It fits into the existing `ptr_cast_constness` lint, as this is a specialized version. However, 1. I have not modified the lint MSRV, so the documentation for this lint will still show that it applies only from Rust 1.72.0. This is true in the general case, but the lint for null pointers will trigger even before this version as `null()` and `null_mut()` were already present in Rust 1.0 and there is no reason not to apply this lint. I guess this is only a minor documentation issue that can be ignored. 2. I have not covered the `core::ptr::null::<T>().cast_mut()` (could be made into `core::ptr::null_mut::<T>()`) and `cotr::ptr::null_mut::<T>().cast_const()` (could be made into `core::ptr::null::<T>()`) cases. Should they be covered? If they should, here or in a separate PR? changelog: [`ptr_cast_constness`]: special-case suggestions for null pointers constness cast Fix rust-lang#13361
diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs
@@ -410,19 +410,27 @@ declare_clippy_lint! {
     /// ### Why is this bad?
     /// Though `as` casts between raw pointers are not terrible, `pointer::cast_mut` and
     /// `pointer::cast_const` are safer because they cannot accidentally cast the pointer to another
-    /// type.
+    /// type. Or, when null pointers are involved, `null()` and `null_mut()` can be used directly.
     ///
     /// ### Example
     /// ```no_run
     /// let ptr: *const u32 = &42_u32;
     /// let mut_ptr = ptr as *mut u32;
     /// let ptr = mut_ptr as *const u32;
+    /// let ptr1 = std::ptr::null::<u32>() as *mut u32;
+    /// let ptr2 = std::ptr::null_mut::<u32>() as *const u32;
+    /// let ptr3 = std::ptr::null::<u32>().cast_mut();
+    /// let ptr4 = std::ptr::null_mut::<u32>().cast_const();
     /// ```
     /// Use instead:
     /// ```no_run
     /// let ptr: *const u32 = &42_u32;
     /// let mut_ptr = ptr.cast_mut();
     /// let ptr = mut_ptr.cast_const();
+    /// let ptr1 = std::ptr::null_mut::<u32>();
+    /// let ptr2 = std::ptr::null::<u32>();
+    /// let ptr3 = std::ptr::null_mut::<u32>();
+    /// let ptr4 = std::ptr::null::<u32>();
     /// ```
     #[clippy::version = "1.72.0"]
     pub PTR_CAST_CONSTNESS,
@@ -809,6 +817,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
         char_lit_as_u8::check(cx, expr);
         ptr_as_ptr::check(cx, expr, &self.msrv);
         cast_slice_different_sizes::check(cx, expr, &self.msrv);
+        ptr_cast_constness::check_null_ptr_cast_method(cx, expr);
     }
 
     extract_msrv_attr!(LateContext);
diff --git a/clippy_lints/src/casts/ptr_cast_constness.rs b/clippy_lints/src/casts/ptr_cast_constness.rs
@@ -1,10 +1,12 @@
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::std_or_core;
 use clippy_utils::sugg::Sugg;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, Mutability};
+use rustc_hir::{Expr, ExprKind, Mutability, QPath};
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, Ty, TypeVisitableExt};
+use rustc_span::sym;
 
 use super::PTR_CAST_CONSTNESS;
 
@@ -16,8 +18,7 @@ pub(super) fn check<'tcx>(
     cast_to: Ty<'tcx>,
     msrv: &Msrv,
 ) {
-    if msrv.meets(msrvs::POINTER_CAST_CONSTNESS)
-        && let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
+    if let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
         && let ty::RawPtr(to_ty, to_mutbl) = cast_to.kind()
         && matches!(
             (from_mutbl, to_mutbl),
@@ -26,20 +27,74 @@ pub(super) fn check<'tcx>(
         && from_ty == to_ty
         && !from_ty.has_erased_regions()
     {
-        let sugg = Sugg::hir(cx, cast_expr, "_");
-        let constness = match *to_mutbl {
-            Mutability::Not => "const",
-            Mutability::Mut => "mut",
-        };
+        if let ExprKind::Call(func, []) = cast_expr.kind
+            && let ExprKind::Path(QPath::Resolved(None, path)) = func.kind
+            && let Some(defid) = path.res.opt_def_id()
+            && let Some(prefix) = std_or_core(cx)
+            && let mut app = Applicability::MachineApplicable
+            && let sugg = format!("{}", Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app))
+            && let Some((_, after_lt)) = sugg.split_once("::<")
+            && let Some((source, target, target_func)) = match cx.tcx.get_diagnostic_name(defid) {
+                Some(sym::ptr_null) => Some(("const", "mutable", "null_mut")),
+                Some(sym::ptr_null_mut) => Some(("mutable", "const", "null")),
+                _ => None,
+            }
+        {
+            span_lint_and_sugg(
+                cx,
+                PTR_CAST_CONSTNESS,
+                expr.span,
+                format!("`as` casting to make a {source} null pointer into a {target} null pointer"),
+                format!("use `{target_func}()` directly instead"),
+                format!("{prefix}::ptr::{target_func}::<{after_lt}"),
+                app,
+            );
+            return;
+        }
 
+        if msrv.meets(msrvs::POINTER_CAST_CONSTNESS) {
+            let sugg = Sugg::hir(cx, cast_expr, "_");
+            let constness = match *to_mutbl {
+                Mutability::Not => "const",
+                Mutability::Mut => "mut",
+            };
+
+            span_lint_and_sugg(
+                cx,
+                PTR_CAST_CONSTNESS,
+                expr.span,
+                "`as` casting between raw pointers while changing only its constness",
+                format!("try `pointer::cast_{constness}`, a safer alternative"),
+                format!("{}.cast_{constness}()", sugg.maybe_par()),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
+pub(super) fn check_null_ptr_cast_method(cx: &LateContext<'_>, expr: &Expr<'_>) {
+    if let ExprKind::MethodCall(method, cast_expr, [], _) = expr.kind
+        && let ExprKind::Call(func, []) = cast_expr.kind
+        && let ExprKind::Path(QPath::Resolved(None, path)) = func.kind
+        && let Some(defid) = path.res.opt_def_id()
+        && let method = match (cx.tcx.get_diagnostic_name(defid), method.ident.as_str()) {
+            (Some(sym::ptr_null), "cast_mut") => "null_mut",
+            (Some(sym::ptr_null_mut), "cast_const") => "null",
+            _ => return,
+        }
+        && let Some(prefix) = std_or_core(cx)
+        && let mut app = Applicability::MachineApplicable
+        && let sugg = format!("{}", Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app))
+        && let Some((_, after_lt)) = sugg.split_once("::<")
+    {
         span_lint_and_sugg(
             cx,
             PTR_CAST_CONSTNESS,
             expr.span,
-            "`as` casting between raw pointers while changing only its constness",
-            format!("try `pointer::cast_{constness}`, a safer alternative"),
-            format!("{}.cast_{constness}()", sugg.maybe_par()),
-            Applicability::MachineApplicable,
+            "changing constness of a null pointer",
+            format!("use `{method}()` directly instead"),
+            format!("{prefix}::ptr::{method}::<{after_lt}"),
+            app,
         );
     }
 }
diff --git a/tests/ui/ptr_cast_constness.fixed b/tests/ui/ptr_cast_constness.fixed
@@ -68,3 +68,20 @@ fn _msrv_1_65() {
     let _ = ptr.cast_mut();
     let _ = mut_ptr.cast_const();
 }
+
+#[inline_macros]
+fn null_pointers() {
+    use std::ptr;
+    let _ = std::ptr::null_mut::<String>();
+    let _ = std::ptr::null::<u32>();
+    let _ = std::ptr::null_mut::<u32>();
+    let _ = std::ptr::null::<u32>();
+
+    // Make sure the lint is triggered inside a macro
+    let _ = inline!(std::ptr::null_mut::<u32>());
+    let _ = inline!(std::ptr::null_mut::<u32>());
+
+    // Do not lint inside macros from external crates
+    let _ = external!(ptr::null::<u32>() as *mut u32);
+    let _ = external!(ptr::null::<u32>().cast_mut());
+}
diff --git a/tests/ui/ptr_cast_constness.rs b/tests/ui/ptr_cast_constness.rs
@@ -68,3 +68,20 @@ fn _msrv_1_65() {
     let _ = ptr as *mut u32;
     let _ = mut_ptr as *const u32;
 }
+
+#[inline_macros]
+fn null_pointers() {
+    use std::ptr;
+    let _ = ptr::null::<String>() as *mut String;
+    let _ = ptr::null_mut::<u32>() as *const u32;
+    let _ = ptr::null::<u32>().cast_mut();
+    let _ = ptr::null_mut::<u32>().cast_const();
+
+    // Make sure the lint is triggered inside a macro
+    let _ = inline!(ptr::null::<u32>() as *mut u32);
+    let _ = inline!(ptr::null::<u32>().cast_mut());
+
+    // Do not lint inside macros from external crates
+    let _ = external!(ptr::null::<u32>() as *mut u32);
+    let _ = external!(ptr::null::<u32>().cast_mut());
+}
diff --git a/tests/ui/ptr_cast_constness.stderr b/tests/ui/ptr_cast_constness.stderr
@@ -43,5 +43,45 @@ error: `as` casting between raw pointers while changing only its constness
 LL |     let _ = mut_ptr as *const u32;
    |             ^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast_const`, a safer alternative: `mut_ptr.cast_const()`
 
-error: aborting due to 7 previous errors
+error: `as` casting to make a const null pointer into a mutable null pointer
+  --> tests/ui/ptr_cast_constness.rs:75:13
+   |
+LL |     let _ = ptr::null::<String>() as *mut String;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<String>()`
+
+error: `as` casting to make a mutable null pointer into a const null pointer
+  --> tests/ui/ptr_cast_constness.rs:76:13
+   |
+LL |     let _ = ptr::null_mut::<u32>() as *const u32;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null()` directly instead: `std::ptr::null::<u32>()`
+
+error: changing constness of a null pointer
+  --> tests/ui/ptr_cast_constness.rs:77:13
+   |
+LL |     let _ = ptr::null::<u32>().cast_mut();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<u32>()`
+
+error: changing constness of a null pointer
+  --> tests/ui/ptr_cast_constness.rs:78:13
+   |
+LL |     let _ = ptr::null_mut::<u32>().cast_const();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null()` directly instead: `std::ptr::null::<u32>()`
+
+error: `as` casting to make a const null pointer into a mutable null pointer
+  --> tests/ui/ptr_cast_constness.rs:81:21
+   |
+LL |     let _ = inline!(ptr::null::<u32>() as *mut u32);
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<u32>()`
+   |
+   = note: this error originates in the macro `__inline_mac_fn_null_pointers` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: changing constness of a null pointer
+  --> tests/ui/ptr_cast_constness.rs:82:21
+   |
+LL |     let _ = inline!(ptr::null::<u32>().cast_mut());
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<u32>()`
+   |
+   = note: this error originates in the macro `__inline_mac_fn_null_pointers` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 13 previous errors
 
