Handle let ref in toplevel_ref_arg as well

README.md

@@ -9,7 +9,7 @@ A collection of lints that give helpful tips to newbies and catch oversights.
 There are 58 lints included in this crate:
 
 name                                                                                                   | default | meaning
--------------------------------------------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+-------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant)                     | warn    | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
 [bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask)                           | warn    | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
 [box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec)                                     | warn    | usage of `Box<Vec<T>>`, vector elements are already on the heap
@@ -59,7 +59,7 @@ name
 [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add)                               | allow   | using `x + ..` where x is a `String`; suggests using `push_str()` instead
 [string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign)                 | allow   | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
 [string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string)                   | warn    | calling `String.to_string()` which is a no-op
-[toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                   | warn    | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
+[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 `&`.
 [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)
 [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp)                                   | warn    | comparing unit values (which is always `true` or `false`, respectively)

src/misc.rs

@@ -8,11 +8,13 @@ use rustc_front::visit::FnKind;
 use rustc::middle::ty;
 
 use utils::{get_item_name, match_path, snippet, span_lint, walk_ptrs_ty, is_integer_literal};
+use utils::span_help_and_lint;
 use consts::constant;
 
 declare_lint!(pub TOPLEVEL_REF_ARG, Warn,
-              "a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not \
+              "An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), \
-               `fn foo((ref x, ref y): (u8, u8))`)");
+               or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take \
+               references with `&`.");
 
 #[allow(missing_copy_implementations)]
 pub struct TopLevelRefPass;
@@ -39,6 +41,29 @@ impl LateLintPass for TopLevelRefPass {
             }
         }
     }
+    fn check_stmt(&mut self, cx: &LateContext, s: &Stmt) {
+        if_let_chain! {
+            [
+            let StmtDecl(ref d, _) = s.node,
+            let DeclLocal(ref l) = d.node,
+            let PatIdent(BindByRef(_), i, None) = l.pat.node,
+            let Some(ref init) = l.init
+            ], {
+                let tyopt = if let Some(ref ty) = l.ty {
+                    format!(": {:?} ", ty)
+                } else {
+                    "".to_owned()
+                };
+                span_help_and_lint(cx,
+                    TOPLEVEL_REF_ARG,
+                    l.pat.span,
+                    "`ref` on an entire `let` pattern is discouraged, take a reference with & instead",
+                    &format!("try `let {} {}= &{};`", snippet(cx, i.span, "_"),
+                             tyopt, snippet(cx, init.span, "_"))
+                );
+            }
+        };
+    }
 }
 
 declare_lint!(pub CMP_NAN, Deny,

tests/compile-fail/matches.rs

@@ -40,7 +40,7 @@ fn single_match(){
 
 fn ref_pats() {
     {
-        let ref v = Some(0);
+        let v = &Some(0);
         match v {  //~ERROR instead of prefixing all patterns with `&`
             &Some(v) => println!("{:?}", v),
             &None => println!("none"),
@@ -50,7 +50,7 @@ fn ref_pats() {
             other => println!("other"),
         }
     }
-    let ref tup = (1, 2);
+    let tup =& (1, 2);
     match tup {  //~ERROR instead of prefixing all patterns with `&`
         &(v, 1) => println!("{}", v),
         _ => println!("none"),

tests/compile-fail/toplevel_ref_arg.rs

@@ -14,5 +14,13 @@ fn main() {
   // Closures should not warn
   let y = |ref x| { println!("{:?}", x) };
   y(1u8);
+
+  let ref x = 1; //~ ERROR `ref` on an entire `let` pattern is discouraged
+                 //~^ HELP try `let x = &1;`
+
+  let ref y = (&1, 2); //~ ERROR `ref` on an entire `let` pattern is discouraged
+                 //~^ HELP try `let y = &(&1, 2);`
+
+  let (ref x, _) = (1,2); // okay, not top level
   println!("The answer is {}.", x);
 }