Auto merge of rust-lang#6542 - rail-rain:ptr_as_ptr, r=flip1995

Add a new lint `ptr_as_ptr`

This PR adds a new lint `ptr_as_ptr` which checks for `as` casts between raw pointers without changing its mutability and suggest replacing it with `pointer::cast`. Closes rust-lang#5890.

Open question: should this lint be `pedantic` or `style`? I set it `pedantic` for now because the original post suggests using it, but I think the lint also fits well to `style`.

---

changelog: New lint `ptr_as_ptr`

CHANGELOG.md

@@ -2141,6 +2141,7 @@ Released 2018-09-13
 [`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
 [`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string
 [`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
+[`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
 [`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
 [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
 [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names

clippy_lints/src/lib.rs

@@ -912,6 +912,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &types::LET_UNIT_VALUE,
         &types::LINKEDLIST,
         &types::OPTION_OPTION,
+        &types::PTR_AS_PTR,
         &types::RC_BUFFER,
         &types::REDUNDANT_ALLOCATION,
         &types::TYPE_COMPLEXITY,
@@ -1222,6 +1223,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box strings::StringToString);
     store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
     store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
+    store.register_late_pass(move || box types::PtrAsPtr::new(msrv));
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1348,6 +1350,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&types::LET_UNIT_VALUE),
         LintId::of(&types::LINKEDLIST),
         LintId::of(&types::OPTION_OPTION),
+        LintId::of(&types::PTR_AS_PTR),
         LintId::of(&unicode::NON_ASCII_LITERAL),
         LintId::of(&unicode::UNICODE_NOT_NFC),
         LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),

clippy_lints/src/types.rs

@@ -19,7 +19,8 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::TypeFoldable;
-use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeckResults};
+use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeAndMut, TypeckResults};
+use rustc_semver::RustcVersion;
 use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::Span;
@@ -30,11 +31,13 @@ use rustc_typeck::hir_ty_to_ty;
 
 use crate::consts::{constant, Constant};
 use crate::utils::paths;
+use crate::utils::sugg::Sugg;
 use crate::utils::{
     clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item,
-    last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral,
-    qpath_res, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite,
-    span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
+    last_path_segment, match_def_path, match_path, meets_msrv, method_chain_args, multispan_sugg,
+    numeric_literal::NumericLiteral, qpath_res, reindent_multiline, sext, snippet, snippet_opt,
+    snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
+    span_lint_and_then, unsext,
 };
 
 declare_clippy_lint! {
@@ -2878,3 +2881,93 @@ impl<'tcx> LateLintPass<'tcx> for RefToMut {
         }
     }
 }
+
+const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0);
+
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Checks for `as` casts between raw pointers without changing its mutability,
+    /// namely `*const T` to `*const U` and `*mut T` to `*mut U`.
+    ///
+    /// **Why is this bad?**
+    /// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because
+    /// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let ptr: *const u32 = &42_u32;
+    /// let mut_ptr: *mut u32 = &mut 42_u32;
+    /// let _ = ptr as *const i32;
+    /// let _ = mut_ptr as *mut i32;
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let ptr: *const u32 = &42_u32;
+    /// let mut_ptr: *mut u32 = &mut 42_u32;
+    /// let _ = ptr.cast::<i32>();
+    /// let _ = mut_ptr.cast::<i32>();
+    /// ```
+    pub PTR_AS_PTR,
+    pedantic,
+    "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
+}
+
+pub struct PtrAsPtr {
+    msrv: Option<RustcVersion>,
+}
+
+impl PtrAsPtr {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
+
+impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]);
+
+impl<'tcx> LateLintPass<'tcx> for PtrAsPtr {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if !meets_msrv(self.msrv.as_ref(), &PTR_AS_PTR_MSRV) {
+            return;
+        }
+
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        if_chain! {
+            if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind;
+            let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr));
+            if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
+            if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind();
+            if matches!((from_mutbl, to_mutbl),
+                (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut));
+            // The `U` in `pointer::cast` have to be `Sized`
+            // as explained here: https://github.com/rust-lang/rust/issues/60602.
+            if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env);
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability);
+                let turbofish = match &cast_to_hir_ty.kind {
+                        TyKind::Infer => Cow::Borrowed(""),
+                        TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""),
+                        _ => Cow::Owned(format!("::<{}>", to_pointee_ty)),
+                    };
+                span_lint_and_sugg(
+                    cx,
+                    PTR_AS_PTR,
+                    expr.span,
+                    "`as` casting between raw pointers without changing its mutability",
+                    "try `pointer::cast`, a safer alternative",
+                    format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish),
+                    applicability,
+                );
+            }
+        }
+    }
+
+    extract_msrv_attr!(LateContext);
+}

tests/ui/ptr_as_ptr.fixed

@@ -0,0 +1,50 @@
+// run-rustfix
+
+#![warn(clippy::ptr_as_ptr)]
+#![feature(custom_inner_attributes)]
+
+fn main() {
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    let _ = ptr.cast::<i32>();
+    let _ = mut_ptr.cast::<i32>();
+
+    // Make sure the lint can handle the difference in their operator precedences.
+    unsafe {
+        let ptr_ptr: *const *const u32 = &ptr;
+        let _ = (*ptr_ptr).cast::<i32>();
+    }
+
+    // Changes in mutability. Do not lint this.
+    let _ = ptr as *mut i32;
+    let _ = mut_ptr as *const i32;
+
+    // `pointer::cast` cannot perform unsized coercions unlike `as`. Do not lint this.
+    let ptr_of_array: *const [u32; 4] = &[1, 2, 3, 4];
+    let _ = ptr_of_array as *const [u32];
+    let _ = ptr_of_array as *const dyn std::fmt::Debug;
+
+    // Ensure the lint doesn't produce unnecessary turbofish for inferred types.
+    let _: *const i32 = ptr.cast();
+    let _: *mut i32 = mut_ptr.cast();
+}
+
+fn _msrv_1_37() {
+    #![clippy::msrv = "1.37"]
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    // `pointer::cast` was stabilized in 1.38. Do not lint this
+    let _ = ptr as *const i32;
+    let _ = mut_ptr as *mut i32;
+}
+
+fn _msrv_1_38() {
+    #![clippy::msrv = "1.38"]
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    let _ = ptr.cast::<i32>();
+    let _ = mut_ptr.cast::<i32>();
+}

tests/ui/ptr_as_ptr.rs

@@ -0,0 +1,50 @@
+// run-rustfix
+
+#![warn(clippy::ptr_as_ptr)]
+#![feature(custom_inner_attributes)]
+
+fn main() {
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    let _ = ptr as *const i32;
+    let _ = mut_ptr as *mut i32;
+
+    // Make sure the lint can handle the difference in their operator precedences.
+    unsafe {
+        let ptr_ptr: *const *const u32 = &ptr;
+        let _ = *ptr_ptr as *const i32;
+    }
+
+    // Changes in mutability. Do not lint this.
+    let _ = ptr as *mut i32;
+    let _ = mut_ptr as *const i32;
+
+    // `pointer::cast` cannot perform unsized coercions unlike `as`. Do not lint this.
+    let ptr_of_array: *const [u32; 4] = &[1, 2, 3, 4];
+    let _ = ptr_of_array as *const [u32];
+    let _ = ptr_of_array as *const dyn std::fmt::Debug;
+
+    // Ensure the lint doesn't produce unnecessary turbofish for inferred types.
+    let _: *const i32 = ptr as *const _;
+    let _: *mut i32 = mut_ptr as _;
+}
+
+fn _msrv_1_37() {
+    #![clippy::msrv = "1.37"]
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    // `pointer::cast` was stabilized in 1.38. Do not lint this
+    let _ = ptr as *const i32;
+    let _ = mut_ptr as *mut i32;
+}
+
+fn _msrv_1_38() {
+    #![clippy::msrv = "1.38"]
+    let ptr: *const u32 = &42_u32;
+    let mut_ptr: *mut u32 = &mut 42_u32;
+
+    let _ = ptr as *const i32;
+    let _ = mut_ptr as *mut i32;
+}

tests/ui/ptr_as_ptr.stderr

@@ -0,0 +1,46 @@
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:10:13
+   |
+LL |     let _ = ptr as *const i32;
+   |             ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::<i32>()`
+   |
+   = note: `-D clippy::ptr-as-ptr` implied by `-D warnings`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:11:13
+   |
+LL |     let _ = mut_ptr as *mut i32;
+   |             ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::<i32>()`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:16:17
+   |
+LL |         let _ = *ptr_ptr as *const i32;
+   |                 ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::<i32>()`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:29:25
+   |
+LL |     let _: *const i32 = ptr as *const _;
+   |                         ^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast()`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:30:23
+   |
+LL |     let _: *mut i32 = mut_ptr as _;
+   |                       ^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast()`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:48:13
+   |
+LL |     let _ = ptr as *const i32;
+   |             ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::<i32>()`
+
+error: `as` casting between raw pointers without changing its mutability
+  --> $DIR/ptr_as_ptr.rs:49:13
+   |
+LL |     let _ = mut_ptr as *mut i32;
+   |             ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::<i32>()`
+
+error: aborting due to 7 previous errors
+