Auto merge of #3600 - xfix:cast-ref-to-mut, r=flip1995

cast_ref_to_mut lint

I see this pattern way too often, and it's completely wrong. In fact, due to how common this incorrect pattern is, [the Rustonomicon specifically points this out](https://doc.rust-lang.org/nomicon/transmutes.html).

> - Transmuting an & to &mut is UB
>   - Transmuting an & to &mut is always UB
>   - No you can't do it
>   - No you're not special

This is my first lint.

Author: bors

CHANGELOG.md

@@ -634,6 +634,7 @@ All notable changes to this project will be documented in this file.
 [`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap
 [`cast_precision_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_precision_loss
 [`cast_ptr_alignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment
+[`cast_ref_to_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ref_to_mut
 [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss
 [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8
 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp

README.md

@@ -7,7 +7,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -486,6 +486,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box ptr_offset_with_cast::Pass);
     reg.register_late_lint_pass(box redundant_clone::RedundantClone);
     reg.register_late_lint_pass(box slow_vector_initialization::Pass);
+    reg.register_late_lint_pass(box types::RefToMut);
 
     reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -758,6 +759,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         types::BOX_VEC,
         types::CAST_LOSSLESS,
         types::CAST_PTR_ALIGNMENT,
+        types::CAST_REF_TO_MUT,
         types::CHAR_LIT_AS_U8,
         types::FN_TO_NUMERIC_CAST,
         types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
@@ -989,6 +991,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         transmute::WRONG_TRANSMUTE,
         types::ABSURD_EXTREME_COMPARISONS,
         types::CAST_PTR_ALIGNMENT,
+        types::CAST_REF_TO_MUT,
         types::UNIT_CMP,
         unicode::ZERO_WIDTH_SPACE,
         unused_io_amount::UNUSED_IO_AMOUNT,

clippy_lints/src/types.rs

@@ -2240,3 +2240,64 @@ impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'
         NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir())
     }
 }
+
+/// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code.
+///
+/// **Why is this bad?** It’s basically guaranteed to be undefined behaviour.
+/// `UnsafeCell` is the only way to obtain aliasable data that is considered
+/// mutable.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// fn x(r: &i32) {
+///     unsafe {
+///         *(r as *const _ as *mut _) += 1;
+///     }
+/// }
+/// ```
+///
+/// Instead consider using interior mutability types.
+///
+/// ```rust
+/// fn x(r: &UnsafeCell<i32>) {
+///     unsafe {
+///         *r.get() += 1;
+///     }
+/// }
+/// ```
+declare_clippy_lint! {
+    pub CAST_REF_TO_MUT,
+    correctness,
+    "a cast of reference to a mutable pointer"
+}
+
+pub struct RefToMut;
+
+impl LintPass for RefToMut {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(CAST_REF_TO_MUT)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if_chain! {
+            if let ExprKind::Unary(UnOp::UnDeref, e) = &expr.node;
+            if let ExprKind::Cast(e, t) = &e.node;
+            if let TyKind::Ptr(MutTy { mutbl: Mutability::MutMutable, .. }) = t.node;
+            if let ExprKind::Cast(e, t) = &e.node;
+            if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node;
+            if let ty::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty;
+            then {
+                span_lint(
+                    cx,
+                    CAST_REF_TO_MUT,
+                    expr.span,
+                    "casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell",
+                );
+            }
+        }
+    }
+}

tests/ui/cast_ref_to_mut.rs

@@ -0,0 +1,31 @@
+#![warn(clippy::cast_ref_to_mut)]
+#![allow(clippy::no_effect)]
+
+extern "C" {
+    // NB. Mutability can be easily incorrect in FFI calls, as
+    // in C, the default are mutable pointers.
+    fn ffi(c: *mut u8);
+    fn int_ffi(c: *mut i32);
+}
+
+fn main() {
+    let s = String::from("Hello");
+    let a = &s;
+    unsafe {
+        let num = &3i32;
+        let mut_num = &mut 3i32;
+        // Should be warned against
+        (*(a as *const _ as *mut String)).push_str(" world");
+        *(a as *const _ as *mut _) = String::from("Replaced");
+        *(a as *const _ as *mut String) += " world";
+        // Shouldn't be warned against
+        println!("{}", *(num as *const _ as *const i16));
+        println!("{}", *(mut_num as *mut _ as *mut i16));
+        ffi(a.as_ptr() as *mut _);
+        int_ffi(num as *const _ as *mut _);
+        int_ffi(&3 as *const _ as *mut _);
+        let mut value = 3;
+        let value: *const i32 = &mut value;
+        *(value as *const i16 as *mut i16) = 42;
+    }
+}

tests/ui/cast_ref_to_mut.stderr

@@ -0,0 +1,22 @@
+error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell
+  --> $DIR/cast_ref_to_mut.rs:18:9
+   |
+LL |         (*(a as *const _ as *mut String)).push_str(" world");
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::cast-ref-to-mut` implied by `-D warnings`
+
+error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell
+  --> $DIR/cast_ref_to_mut.rs:19:9
+   |
+LL |         *(a as *const _ as *mut _) = String::from("Replaced");
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell
+  --> $DIR/cast_ref_to_mut.rs:20:9
+   |
+LL |         *(a as *const _ as *mut String) += " world";
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+