New mut_from_ref lint

This fixes #1507.

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+* New [`mut_from_ref`] lint
+
 ## 0.0.114 — 2017-02-08
 * Rustup to rustc 1.17.0-nightly (c49d10207 2017-02-07)
 * Tests are now ui tests (testing the exact output of rustc)
@@ -369,6 +371,7 @@ All notable changes to this project will be documented in this file.
 [`mixed_case_hex_literals`]: https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals
 [`module_inception`]: https://github.com/Manishearth/rust-clippy/wiki#module_inception
 [`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one
+[`mut_from_ref`]: https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref
 [`mut_mut`]: https://github.com/Manishearth/rust-clippy/wiki#mut_mut
 [`mutex_atomic`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic
 [`mutex_integer`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_integer

README.md

@@ -180,7 +180,7 @@ transparently:
 
 ## Lints
 
-There are 186 lints included in this crate:
+There are 187 lints included in this crate:
 
 name                                                                                                                   | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -278,6 +278,7 @@ name
 [mixed_case_hex_literals](https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals)                     | warn    | hex literals whose letter digits are not consistently upper- or lowercased
 [module_inception](https://github.com/Manishearth/rust-clippy/wiki#module_inception)                                   | warn    | modules that have the same name as their parent module
 [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one)                                               | warn    | taking a number modulo 1, which always returns 0
+[mut_from_ref](https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref)                                           | warn    | fns that create mutable refs from immutable ref args
 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut)                                                     | allow   | usage of double-mut refs, e.g. `&mut &mut ...`
 [mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic)                                           | warn    | using a mutex where an atomic value could be used instead
 [mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer)                                         | allow   | using a mutex for an integer type

clippy_lints/src/lib.rs

@@ -464,6 +464,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         precedence::PRECEDENCE,
         print::PRINT_WITH_NEWLINE,
         ptr::CMP_NULL,
+        ptr::MUT_FROM_REF,
         ptr::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,

clippy_lints/src/ptr.rs

@@ -44,13 +44,30 @@ declare_lint! {
     "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead."
 }
 
+/// **What it does:** This lint checks for functions that take immutable refs and return
+/// mutable ones.
+///
+/// **Why is this bad?** This is trivially unsound, as one can create two mutable refs
+/// from the same source.
+///
+/// **Known problems:** This lint will overlook functions where input and output lifetimes differ
+///
+/// **Example:**
+/// ```rust
+/// fn foo(&Foo) -> &mut Bar { .. }
+/// ```
+declare_lint! {
+    pub MUT_FROM_REF,
+    Warn,
+    "fns that create mutable refs from immutable ref args"
+}
 
 #[derive(Copy,Clone)]
 pub struct PointerPass;
 
 impl LintPass for PointerPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(PTR_ARG, CMP_NULL)
+        lint_array!(PTR_ARG, CMP_NULL, MUT_FROM_REF)
     }
 }
 
@@ -111,6 +128,28 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) {
             }
         }
     }
+
+    if let FunctionRetTy::Return(ref ty) = decl.output {
+        if let Some((out, MutMutable)) = get_rptr_lm(ty) {
+            if let Some(MutImmutable) = decl.inputs.iter()
+                     .filter_map(|ty| get_rptr_lm(ty))
+                     .filter(|&(lt, _)| lt.name == out.name)
+                     .fold(None, |x, (_, m)| match (x, m) {
+                        (Some(MutMutable), _) |
+                        (_, MutMutable) => Some(MutMutable),
+                        (_, m) => Some(m),
+                     }) {
+                span_lint(cx,
+                          MUT_FROM_REF,
+                          ty.span,
+                          "this function takes an immutable ref to return a mutable one:");
+            }
+        }
+    }
+}
+
+fn get_rptr_lm(ty: &Ty) -> Option<(&Lifetime, Mutability)> {
+    if let Ty_::TyRptr(ref lt, ref m) = ty.node { Some((lt, m.mutbl)) } else { None }
 }
 
 fn is_null_path(expr: &Expr) -> bool {

tests/ui/mut_from_ref.rs

@@ -0,0 +1,40 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+#![allow(unused)]
+#![deny(mut_from_ref)]
+
+struct Foo;
+
+impl Foo {
+    fn this_wont_hurt_a_bit(&self) -> &mut Foo {
+        unimplemented!()
+    }
+}
+
+trait Ouch {
+    fn ouch(x: &Foo) -> &mut Foo;
+}
+
+impl Ouch for Foo {
+    fn ouch(x: &Foo) -> &mut Foo {
+        unimplemented!()
+    }
+}
+
+fn fail(x: &u32) -> &mut u16 {
+    unimplemented!()
+}
+
+// this is OK, because the result borrows y
+fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+// this is also OK, because the result could borrow y
+fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+fn main() {
+    //TODO
+}

tests/ui/mut_from_ref.stderr

@@ -0,0 +1,26 @@
+error: this function takes an immutable ref to return a mutable one:
+ --> $DIR/mut_from_ref.rs:9:39
+  |
+9 |     fn this_wont_hurt_a_bit(&self) -> &mut Foo {
+  |                                       ^^^^^^^^
+  |
+note: lint level defined here
+ --> $DIR/mut_from_ref.rs:4:9
+  |
+4 | #![deny(mut_from_ref)]
+  |         ^^^^^^^^^^^^
+
+error: this function takes an immutable ref to return a mutable one:
+  --> $DIR/mut_from_ref.rs:15:25
+   |
+15 |     fn ouch(x: &Foo) -> &mut Foo;
+   |                         ^^^^^^^^
+
+error: this function takes an immutable ref to return a mutable one:
+  --> $DIR/mut_from_ref.rs:24:21
+   |
+24 | fn fail(x: &u32) -> &mut u16 {
+   |                     ^^^^^^^^
+
+error: aborting due to 3 previous errors
+