Merge pull request #2163 from HMPerson1/useless_asref

Add lint for useless `as_ref` calls

clippy_lints/src/methods.rs

@@ -581,6 +581,29 @@ declare_lint! {
     "using `.chars().last()` or `.chars().next_back()` to check if a string ends with a char"
 }
 
+/// **What it does:** Checks for usage of `.as_ref()` or `.as_mut()` where the
+/// types before and after the call are the same.
+///
+/// **Why is this bad?** The call is unnecessary.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// let x: &[i32] = &[1,2,3,4,5];
+/// do_stuff(x.as_ref());
+/// ```
+/// The correct use would be:
+/// ```rust
+/// let x: &[i32] = &[1,2,3,4,5];
+/// do_stuff(x);
+/// ```
+declare_lint! {
+    pub USELESS_ASREF,
+    Warn,
+    "using `as_ref` where the types before and after the call are the same"
+}
+
 impl LintPass for Pass {
     fn get_lints(&self) -> LintArray {
         lint_array!(
@@ -609,8 +632,8 @@ impl LintPass for Pass {
             ITER_SKIP_NEXT,
             GET_UNWRAP,
             STRING_EXTEND_CHARS,
-            ITER_CLONED_COLLECT
-        )
+            ITER_CLONED_COLLECT,
+            USELESS_ASREF)
     }
 }
 
@@ -669,6 +692,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                     lint_iter_skip_next(cx, expr);
                 } else if let Some(arglists) = method_chain_args(expr, &["cloned", "collect"]) {
                     lint_iter_cloned_collect(cx, expr, arglists[0]);
+                } else if let Some(arglists) = method_chain_args(expr, &["as_ref"]) {
+                    lint_asref(cx, expr, "as_ref", arglists[0]);
+                } else if let Some(arglists) = method_chain_args(expr, &["as_mut"]) {
+                    lint_asref(cx, expr, "as_mut", arglists[0]);
                 }
 
                 lint_or_fun_call(cx, expr, &method_call.name.as_str(), args);
@@ -1504,6 +1531,30 @@ fn lint_single_char_pattern<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hi
     }
 }
 
+/// Checks for the `USELESS_ASREF` lint.
+fn lint_asref(cx: &LateContext, expr: &hir::Expr, call_name: &str, as_ref_args: &[hir::Expr]) {
+    // when we get here, we've already checked that the call name is "as_ref" or "as_mut"
+    // check if the call is to the actual `AsRef` or `AsMut` trait
+    if match_trait_method(cx, expr, &paths::ASREF_TRAIT) || match_trait_method(cx, expr, &paths::ASMUT_TRAIT) {
+        // check if the type after `as_ref` or `as_mut` is the same as before
+        let recvr = &as_ref_args[0];
+        let rcv_ty = cx.tables.expr_ty(recvr);
+        let res_ty = cx.tables.expr_ty(expr);
+        let (base_res_ty, res_depth) = walk_ptrs_ty_depth(res_ty);
+        let (base_rcv_ty, rcv_depth) = walk_ptrs_ty_depth(rcv_ty);
+        if base_rcv_ty == base_res_ty && rcv_depth >= res_depth {
+            span_lint_and_sugg(
+                cx,
+                USELESS_ASREF,
+                expr.span,
+                &format!("this call to `{}` does nothing", call_name),
+                "try this",
+                snippet(cx, recvr.span, "_").into_owned(),
+            );
+        }
+    }
+}
+
 /// Given a `Result<T, E>` type, return its error type (`E`).
 fn get_error_type<'a>(cx: &LateContext, ty: Ty<'a>) -> Option<Ty<'a>> {
     if let ty::TyAdt(_, substs) = ty.sty {

tests/ui/useless_asref.rs

@@ -0,0 +1,118 @@
+#![deny(useless_asref)]
+
+use std::fmt::Debug;
+
+struct FakeAsRef;
+
+#[allow(should_implement_trait)]
+impl FakeAsRef {
+    fn as_ref(&self) -> &Self { self }
+}
+
+struct MoreRef;
+
+impl<'a, 'b, 'c> AsRef<&'a &'b &'c MoreRef> for MoreRef {
+    fn as_ref(&self) -> &&'a &'b &'c MoreRef {
+        &&&&MoreRef
+    }
+}
+
+fn foo_rstr(x: &str) { println!("{:?}", x); }
+fn foo_rslice(x: &[i32]) { println!("{:?}", x); }
+fn foo_mrslice(x: &mut [i32]) { println!("{:?}", x); }
+fn foo_rrrrmr(_: &&&&MoreRef) { println!("so many refs"); }
+
+fn not_ok() {
+    let rstr: &str = "hello";
+    let mut mrslice: &mut [i32] = &mut [1,2,3];
+
+    {
+        let rslice: &[i32] = &*mrslice;
+        foo_rstr(rstr.as_ref());
+        foo_rstr(rstr);
+        foo_rslice(rslice.as_ref());
+        foo_rslice(rslice);
+    }
+    {
+        foo_mrslice(mrslice.as_mut());
+        foo_mrslice(mrslice);
+        foo_rslice(mrslice.as_ref());
+        foo_rslice(mrslice);
+    }
+
+    {
+        let rrrrrstr = &&&&rstr;
+        let rrrrrslice = &&&&&*mrslice;
+        foo_rslice(rrrrrslice.as_ref());
+        foo_rslice(rrrrrslice);
+        foo_rstr(rrrrrstr.as_ref());
+        foo_rstr(rrrrrstr);
+    }
+    {
+        let mrrrrrslice = &mut &mut &mut &mut mrslice;
+        foo_mrslice(mrrrrrslice.as_mut());
+        foo_mrslice(mrrrrrslice);
+        foo_rslice(mrrrrrslice.as_ref());
+        foo_rslice(mrrrrrslice);
+    }
+    foo_rrrrmr((&&&&MoreRef).as_ref());
+
+    generic_not_ok(mrslice);
+    generic_ok(mrslice);
+}
+
+fn ok() {
+    let string = "hello".to_owned();
+    let mut arr = [1,2,3];
+    let mut vec = vec![1,2,3];
+
+    {
+        foo_rstr(string.as_ref());
+        foo_rslice(arr.as_ref());
+        foo_rslice(vec.as_ref());
+    }
+    {
+        foo_mrslice(arr.as_mut());
+        foo_mrslice(vec.as_mut());
+    }
+
+    {
+        let rrrrstring = &&&&string;
+        let rrrrarr = &&&&arr;
+        let rrrrvec = &&&&vec;
+        foo_rstr(rrrrstring.as_ref());
+        foo_rslice(rrrrarr.as_ref());
+        foo_rslice(rrrrvec.as_ref());
+    }
+    {
+        let mrrrrarr = &mut &mut &mut &mut arr;
+        let mrrrrvec = &mut &mut &mut &mut vec;
+        foo_mrslice(mrrrrarr.as_mut());
+        foo_mrslice(mrrrrvec.as_mut());
+    }
+    FakeAsRef.as_ref();
+    foo_rrrrmr(MoreRef.as_ref());
+
+    generic_not_ok(arr.as_mut());
+    generic_ok(&mut arr);
+}
+
+fn foo_mrt<T: Debug + ?Sized>(t: &mut T) { println!("{:?}", t); }
+fn foo_rt<T: Debug + ?Sized>(t: &T) { println!("{:?}", t); }
+
+fn generic_not_ok<T: AsMut<T> + AsRef<T> + Debug + ?Sized>(mrt: &mut T) {
+    foo_mrt(mrt.as_mut());
+    foo_mrt(mrt);
+    foo_rt(mrt.as_ref());
+    foo_rt(mrt);
+}
+
+fn generic_ok<U: AsMut<T> + AsRef<T> + ?Sized, T: Debug + ?Sized>(mru: &mut U) {
+    foo_mrt(mru.as_mut());
+    foo_rt(mru.as_ref());
+}
+
+fn main() {
+    not_ok();
+    ok();
+}

tests/ui/useless_asref.stderr

@@ -0,0 +1,72 @@
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:31:18
+   |
+31 |         foo_rstr(rstr.as_ref());
+   |                  ^^^^^^^^^^^^^ help: try this: `rstr`
+   |
+note: lint level defined here
+  --> $DIR/useless_asref.rs:1:9
+   |
+1  | #![deny(useless_asref)]
+   |         ^^^^^^^^^^^^^
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:33:20
+   |
+33 |         foo_rslice(rslice.as_ref());
+   |                    ^^^^^^^^^^^^^^^ help: try this: `rslice`
+
+error: this call to `as_mut` does nothing
+  --> $DIR/useless_asref.rs:37:21
+   |
+37 |         foo_mrslice(mrslice.as_mut());
+   |                     ^^^^^^^^^^^^^^^^ help: try this: `mrslice`
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:39:20
+   |
+39 |         foo_rslice(mrslice.as_ref());
+   |                    ^^^^^^^^^^^^^^^^ help: try this: `mrslice`
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:46:20
+   |
+46 |         foo_rslice(rrrrrslice.as_ref());
+   |                    ^^^^^^^^^^^^^^^^^^^ help: try this: `rrrrrslice`
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:48:18
+   |
+48 |         foo_rstr(rrrrrstr.as_ref());
+   |                  ^^^^^^^^^^^^^^^^^ help: try this: `rrrrrstr`
+
+error: this call to `as_mut` does nothing
+  --> $DIR/useless_asref.rs:53:21
+   |
+53 |         foo_mrslice(mrrrrrslice.as_mut());
+   |                     ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice`
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:55:20
+   |
+55 |         foo_rslice(mrrrrrslice.as_ref());
+   |                    ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice`
+
+error: this call to `as_ref` does nothing
+  --> $DIR/useless_asref.rs:58:16
+   |
+58 |     foo_rrrrmr((&&&&MoreRef).as_ref());
+   |                ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&&&&MoreRef)`
+
+error: this call to `as_mut` does nothing
+   --> $DIR/useless_asref.rs:104:13
+    |
+104 |     foo_mrt(mrt.as_mut());
+    |             ^^^^^^^^^^^^ help: try this: `mrt`
+
+error: this call to `as_ref` does nothing
+   --> $DIR/useless_asref.rs:106:12
+    |
+106 |     foo_rt(mrt.as_ref());
+    |            ^^^^^^^^^^^^ help: try this: `mrt`
+