Merge pull request #801 from mcarton/transmute

Lint transmute from ptr to ref

README.md

@@ -14,7 +14,7 @@ Table of contents:
 * [License](#license)
 
 ##Lints
-There are 135 lints included in this crate:
+There are 136 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -133,6 +133,7 @@ name
 [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment)                         | warn    | assignments to temporaries
 [too_many_arguments](https://github.com/Manishearth/rust-clippy/wiki#too_many_arguments)                             | warn    | functions with too many arguments
 [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                                 | warn    | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`.
+[transmute_ptr_to_ref](https://github.com/Manishearth/rust-clippy/wiki#transmute_ptr_to_ref)                         | warn    | transmutes from a pointer to a reference type
 [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex)                                       | warn    | finds trivial regular expressions in `Regex::new(_)` invocations
 [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity)                                   | warn    | usage of very complex types; recommends factoring out parts into `type` definitions
 [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc)                                   | allow   | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information)

src/lib.rs

@@ -195,8 +195,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box no_effect::NoEffectPass);
     reg.register_late_lint_pass(box map_clone::MapClonePass);
     reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass);
-    reg.register_late_lint_pass(box transmute::CrosspointerTransmute);
+    reg.register_late_lint_pass(box transmute::Transmute);
-    reg.register_late_lint_pass(box transmute::UselessTransmute);
     reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold));
     reg.register_late_lint_pass(box escape::EscapePass);
     reg.register_early_lint_pass(box misc_early::MiscEarly);
@@ -352,6 +351,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         swap::MANUAL_SWAP,
         temporary_assignment::TEMPORARY_ASSIGNMENT,
         transmute::CROSSPOINTER_TRANSMUTE,
+        transmute::TRANSMUTE_PTR_TO_REF,
         transmute::USELESS_TRANSMUTE,
         types::ABSURD_EXTREME_COMPARISONS,
         types::BOX_VEC,

src/misc.rs

@@ -312,7 +312,7 @@ impl LateLintPass for ModuloOne {
         if let ExprBinary(ref cmp, _, ref right) = expr.node {
             if let Spanned {node: BinOp_::BiRem, ..} = *cmp {
                 if is_integer_literal(right, 1) {
-                    cx.span_lint(MODULO_ONE, expr.span, "any number modulo 1 will be 0");
+                    span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
                 }
             }
         }
@@ -347,11 +347,12 @@ impl LateLintPass for PatternPass {
     fn check_pat(&mut self, cx: &LateContext, pat: &Pat) {
         if let PatKind::Ident(_, ref ident, Some(ref right)) = pat.node {
             if right.node == PatKind::Wild {
-                cx.span_lint(REDUNDANT_PATTERN,
+                span_lint(cx,
-                             pat.span,
+                          REDUNDANT_PATTERN,
-                             &format!("the `{} @ _` pattern can be written as just `{}`",
+                          pat.span,
-                                      ident.node.name,
+                          &format!("the `{} @ _` pattern can be written as just `{}`",
-                                      ident.node.name));
+                                   ident.node.name,
+                                   ident.node.name));
             }
         }
     }
@@ -408,10 +409,11 @@ impl LateLintPass for UsedUnderscoreBinding {
             _ => false,
         };
         if needs_lint {
-            cx.span_lint(USED_UNDERSCORE_BINDING,
+            span_lint(cx,
-                         expr.span,
+                      USED_UNDERSCORE_BINDING,
-                         "used binding which is prefixed with an underscore. A leading underscore signals that a \
+                      expr.span,
-                          binding will not be used.");
+                      "used binding which is prefixed with an underscore. A leading underscore signals that a \
+                       binding will not be used.");
         }
     }
 }

src/ranges.rs

@@ -1,7 +1,7 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
 use syntax::codemap::Spanned;
-use utils::{is_integer_literal, match_type, snippet, unsugar_range, UnsugaredRange};
+use utils::{is_integer_literal, match_type, snippet, span_lint, unsugar_range, UnsugaredRange};
 
 /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates.
 ///
@@ -41,10 +41,11 @@ impl LateLintPass for StepByZero {
             // Range with step_by(0).
             if name.as_str() == "step_by" && args.len() == 2 && is_range(cx, &args[0]) &&
                is_integer_literal(&args[1], 0) {
-                cx.span_lint(RANGE_STEP_BY_ZERO,
+                span_lint(cx,
-                             expr.span,
+                          RANGE_STEP_BY_ZERO,
-                             "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \
+                          expr.span,
-                              instead")
+                          "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \
+                           instead");
             } else if name.as_str() == "zip" && args.len() == 2 {
                 let iter = &args[0].node;
                 let zip_arg = &args[1];
@@ -64,9 +65,11 @@ impl LateLintPass for StepByZero {
                         let ExprPath(_, Path { segments: ref len_path, .. }) = len_args[0].node,
                         iter_path == len_path
                      ], {
-                        cx.span_lint(RANGE_ZIP_WITH_LEN, expr.span,
+                        span_lint(cx,
-                                     &format!("It is more idiomatic to use {}.iter().enumerate()",
+                                  RANGE_ZIP_WITH_LEN,
-                                              snippet(cx, iter_args[0].span, "_")));
+                                  expr.span,
+                                  &format!("It is more idiomatic to use {}.iter().enumerate()",
+                                           snippet(cx, iter_args[0].span, "_")));
                     }
                 }
             }

src/transmute.rs

@@ -1,9 +1,9 @@
 use rustc::lint::*;
+use rustc::ty::TypeVariants::{TyRawPtr, TyRef};
+use rustc::ty;
 use rustc_front::hir::*;
-use rustc::ty::TyS;
-use rustc::ty::TypeVariants::TyRawPtr;
-use utils;
 use utils::TRANSMUTE_PATH;
+use utils::{match_def_path, snippet_opt, span_lint, span_lint_and_then};
 
 /// **What it does:** This lint checks for transmutes to the original type of the object.
 ///
@@ -31,28 +31,63 @@ declare_lint! {
     "transmutes that have to or from types that are a pointer to the other"
 }
 
-pub struct UselessTransmute;
+/// **What it does:*** This lint checks for transmutes from a pointer to a reference.
+///
+/// **Why is this bad?** This can always be rewritten with `&` and `*`.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// let _: &T = std::mem::transmute(p); // where p: *const T
+/// // can be written:
+/// let _: &T = &*p;
+/// ```
+declare_lint! {
+    pub TRANSMUTE_PTR_TO_REF,
+    Warn,
+    "transmutes from a pointer to a reference type"
+}
 
-impl LintPass for UselessTransmute {
+pub struct Transmute;
+
+impl LintPass for Transmute {
     fn get_lints(&self) -> LintArray {
-        lint_array!(USELESS_TRANSMUTE)
+        lint_array! [
+            CROSSPOINTER_TRANSMUTE,
+            TRANSMUTE_PTR_TO_REF,
+            USELESS_TRANSMUTE
+        ]
     }
 }
 
-impl LateLintPass for UselessTransmute {
+impl LateLintPass for Transmute {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
         if let ExprCall(ref path_expr, ref args) = e.node {
             if let ExprPath(None, _) = path_expr.node {
                 let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id();
 
-                if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) {
+                if match_def_path(cx, def_id, &TRANSMUTE_PATH) {
                     let from_ty = cx.tcx.expr_ty(&args[0]);
                     let to_ty = cx.tcx.expr_ty(e);
 
                     if from_ty == to_ty {
-                        cx.span_lint(USELESS_TRANSMUTE,
+                        span_lint(cx,
-                                     e.span,
+                                  USELESS_TRANSMUTE,
-                                     &format!("transmute from a type (`{}`) to itself", from_ty));
+                                  e.span,
+                                  &format!("transmute from a type (`{}`) to itself", from_ty));
+                    } else if is_ptr_to(to_ty, from_ty) {
+                        span_lint(cx,
+                                  CROSSPOINTER_TRANSMUTE,
+                                  e.span,
+                                  &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty));
+                    } else if is_ptr_to(from_ty, to_ty) {
+                        span_lint(cx,
+                                  CROSSPOINTER_TRANSMUTE,
+                                  e.span,
+                                  &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty));
+                    } else {
+                        check_ptr_to_ref(cx, from_ty, to_ty, e, &args[0]);
                     }
                 }
             }
@@ -60,15 +95,7 @@ impl LateLintPass for UselessTransmute {
     }
 }
 
-pub struct CrosspointerTransmute;
+fn is_ptr_to(from: ty::Ty, to: ty::Ty) -> bool {
-
-impl LintPass for CrosspointerTransmute {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(CROSSPOINTER_TRANSMUTE)
-    }
-}
-
-fn is_ptr_to(from: &TyS, to: &TyS) -> bool {
     if let TyRawPtr(from_ptr) = from.sty {
         from_ptr.ty == to
     } else {
@@ -76,27 +103,34 @@ fn is_ptr_to(from: &TyS, to: &TyS) -> bool {
     }
 }
 
-impl LateLintPass for CrosspointerTransmute {
+fn check_ptr_to_ref<'tcx>(cx: &LateContext,
-    fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+                          from_ty: ty::Ty<'tcx>,
-        if let ExprCall(ref path_expr, ref args) = e.node {
+                          to_ty: ty::Ty<'tcx>,
-            if let ExprPath(None, _) = path_expr.node {
+                          e: &Expr, arg: &Expr) {
-                let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id();
+    if let TyRawPtr(ref from_pty) = from_ty.sty {
+        if let TyRef(_, ref to_rty) = to_ty.sty {
+            let mess = format!("transmute from a pointer type (`{}`) to a reference type (`{}`)",
+                               from_ty,
+                               to_ty);
+            span_lint_and_then(cx, TRANSMUTE_PTR_TO_REF, e.span, &mess, |db| {
+                if let Some(arg) = snippet_opt(cx, arg.span) {
+                    let (deref, cast) = if to_rty.mutbl == Mutability::MutMutable {
+                        ("&mut *", "*mut")
+                    } else {
+                        ("&*", "*const")
+                    };
 
-                if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) {
-                    let from_ty = cx.tcx.expr_ty(&args[0]);
-                    let to_ty = cx.tcx.expr_ty(e);
 
-                    if is_ptr_to(to_ty, from_ty) {
+                    let sugg = if from_pty.ty == to_rty.ty {
-                        cx.span_lint(CROSSPOINTER_TRANSMUTE,
+                        format!("{}{}", deref, arg)
-                                     e.span,
-                                     &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty));
-                    } else if is_ptr_to(from_ty, to_ty) {
-                        cx.span_lint(CROSSPOINTER_TRANSMUTE,
-                                     e.span,
-                                     &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty));
                     }
+                    else {
+                        format!("{}({} as {} {})", deref, arg, cast, to_rty.ty)
+                    };
+
+                    db.span_suggestion(e.span, "try", sugg);
                 }
-            }
+            });
         }
     }
 }

tests/compile-fail/transmute.rs

@@ -19,6 +19,45 @@ unsafe fn _generic<'a, T, U: 'a>(t: &'a T) {
     let _: &'a U = core::intrinsics::transmute(t);
 }
 
+#[deny(transmute_ptr_to_ref)]
+unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
+    let _: &T = std::mem::transmute(p);
+    //~^ ERROR transmute from a pointer type (`*const T`) to a reference type (`&T`)
+    //~| HELP try
+    //~| SUGGESTION = &*p;
+    let _: &T = &*p;
+
+    let _: &mut T = std::mem::transmute(m);
+    //~^ ERROR transmute from a pointer type (`*mut T`) to a reference type (`&mut T`)
+    //~| HELP try
+    //~| SUGGESTION = &mut *m;
+    let _: &mut T = &mut *m;
+
+    let _: &T = std::mem::transmute(m);
+    //~^ ERROR transmute from a pointer type (`*mut T`) to a reference type (`&T`)
+    //~| HELP try
+    //~| SUGGESTION = &*m;
+    let _: &T = &*m;
+
+    let _: &T = std::mem::transmute(o);
+    //~^ ERROR transmute from a pointer type (`*const U`) to a reference type (`&T`)
+    //~| HELP try
+    //~| SUGGESTION = &*(o as *const T);
+    let _: &T = &*(o as *const T);
+
+    let _: &mut T = std::mem::transmute(om);
+    //~^ ERROR transmute from a pointer type (`*mut U`) to a reference type (`&mut T`)
+    //~| HELP try
+    //~| SUGGESTION = &mut *(om as *mut T);
+    let _: &mut T = &mut *(om as *mut T);
+
+    let _: &T = std::mem::transmute(om);
+    //~^ ERROR transmute from a pointer type (`*mut U`) to a reference type (`&T`)
+    //~| HELP try
+    //~| SUGGESTION = &*(om as *const T);
+    let _: &T = &*(om as *const T);
+}
+
 #[deny(useless_transmute)]
 fn useless() {
     unsafe {