From ff4a85035328e4cff43ac7dbeb1615b2a8cc4621 Mon Sep 17 00:00:00 2001 From: HMPerson1 Date: Fri, 20 Oct 2017 11:39:20 -0400 Subject: [PATCH] Add lint for useless `as_ref` calls --- clippy_lints/src/methods.rs | 55 +++++++++++++++++++- tests/ui/useless_asref.rs | 94 +++++++++++++++++++++++++++++++++++ tests/ui/useless_asref.stderr | 60 ++++++++++++++++++++++ 3 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 tests/ui/useless_asref.rs create mode 100644 tests/ui/useless_asref.stderr diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 849f00244..4a0d1cf19 100644 --- a/clippy_lints/src/methods.rs +++ b/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` type, return its error type (`E`). fn get_error_type<'a>(cx: &LateContext, ty: Ty<'a>) -> Option> { if let ty::TyAdt(_, substs) = ty.sty { diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs new file mode 100644 index 000000000..ef0174ad7 --- /dev/null +++ b/tests/ui/useless_asref.rs @@ -0,0 +1,94 @@ +#![deny(useless_asref)] + +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()); +} + +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()); +} +fn main() { + not_ok(); + ok(); +} diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr new file mode 100644 index 000000000..8cc869ad7 --- /dev/null +++ b/tests/ui/useless_asref.stderr @@ -0,0 +1,60 @@ +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:29:18 + | +29 | 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:31:20 + | +31 | foo_rslice(rslice.as_ref()); + | ^^^^^^^^^^^^^^^ help: try this: `rslice` + +error: this call to `as_mut` does nothing + --> $DIR/useless_asref.rs:35:21 + | +35 | foo_mrslice(mrslice.as_mut()); + | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` + +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:37:20 + | +37 | foo_rslice(mrslice.as_ref()); + | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` + +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:44:20 + | +44 | foo_rslice(rrrrrslice.as_ref()); + | ^^^^^^^^^^^^^^^^^^^ help: try this: `rrrrrslice` + +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:46:18 + | +46 | foo_rstr(rrrrrstr.as_ref()); + | ^^^^^^^^^^^^^^^^^ help: try this: `rrrrrstr` + +error: this call to `as_mut` does nothing + --> $DIR/useless_asref.rs:51:21 + | +51 | foo_mrslice(mrrrrrslice.as_mut()); + | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` + +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:53:20 + | +53 | foo_rslice(mrrrrrslice.as_ref()); + | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` + +error: this call to `as_ref` does nothing + --> $DIR/useless_asref.rs:56:16 + | +56 | foo_rrrrmr((&&&&MoreRef).as_ref()); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&&&&MoreRef)` +