feat(lint): deref and deref_mut method explicit calls

CHANGELOG.md

@@ -677,6 +677,7 @@ All notable changes to this project will be documented in this file.
 [`expect_fun_call`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#expect_fun_call
 [`expl_impl_clone_on_copy`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
 [`explicit_counter_loop`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#explicit_counter_loop
+[`explicit_deref_method`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#explicit_deref_method
 [`explicit_into_iter_loop`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
 [`explicit_iter_loop`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#explicit_iter_loop
 [`explicit_write`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#explicit_write

README.md

@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 279 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+[There are 280 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/dereference.rs

@@ -0,0 +1,74 @@
+use crate::rustc::hir::{Expr, ExprKind, QPath};
+use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use if_chain::if_chain;
+use crate::utils::{in_macro, span_lint_and_sugg};
+
+/// **What it does:** Checks for explicit deref() or deref_mut() method calls.
+///
+/// **Why is this bad?** Derefencing by &*x or &mut *x is clearer and more concise,
+/// when not part of a method chain.
+///
+/// **Example:**
+/// ```rust
+/// let b = a.deref();
+/// let c = a.deref_mut();
+///
+/// // excludes
+/// let e = d.unwrap().deref();
+/// ```
+declare_clippy_lint! {
+ pub EXPLICIT_DEREF_METHOD,
+ pedantic,
+ "Explicit use of deref or deref_mut method while not in a method chain."
+}
+
+pub struct Pass;
+
+impl LintPass for Pass {
+ fn get_lints(&self) -> LintArray {
+ lint_array!(EXPLICIT_DEREF_METHOD)
+ }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+ fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) {
+ if in_macro(expr.span) {
+ return;
+ }
+
+ if_chain! {
+ // if this is a method call
+ if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.node;
+ // on a Path (i.e. a variable/name, not another method)
+ if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node;
+ then {
+ let name = method_name.ident.as_str();
+ // alter help slightly to account for _mut
+ match &*name {
+ "deref" => {
+ span_lint_and_sugg(
+ cx,
+ EXPLICIT_DEREF_METHOD,
+ expr.span,
+ "explicit deref method call",
+ "try this",
+ format!("&*{}", path),
+ );
+ },
+ "deref_mut" => {
+ span_lint_and_sugg(
+ cx,
+ EXPLICIT_DEREF_METHOD,
+ expr.span,
+ "explicit deref_mut method call",
+ "try this",
+ format!("&mut *{}", path),
+ );
+ },
+ _ => ()
+ };
+ }
+ }
+ }
+}

clippy_lints/src/lib.rs

@@ -88,6 +88,7 @@ pub mod copies;
 pub mod copy_iterator;
 pub mod cyclomatic_complexity;
 pub mod default_trait_access;
+pub mod dereference;
 pub mod derive;
 pub mod doc;
 pub mod double_comparison;
@@ -356,6 +357,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
  reg.register_early_lint_pass(box misc_early::MiscEarly);
  reg.register_late_lint_pass(box panic_unimplemented::Pass);
  reg.register_late_lint_pass(box strings::StringLitAsBytes);
+ reg.register_late_lint_pass(box dereference::Pass);
  reg.register_late_lint_pass(box derive::Derive);
  reg.register_late_lint_pass(box types::CharLitAsU8);
  reg.register_late_lint_pass(box vec::Pass);
@@ -460,6 +462,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
  copies::MATCH_SAME_ARMS,
  copy_iterator::COPY_ITERATOR,
  default_trait_access::DEFAULT_TRAIT_ACCESS,
+ dereference::EXPLICIT_DEREF_METHOD,
  derive::EXPL_IMPL_CLONE_ON_COPY,
  doc::DOC_MARKDOWN,
  empty_enum::EMPTY_ENUM,

tests/ui/dereference.rs

@@ -0,0 +1,55 @@
+#![feature(tool_lints)]
+
+use std::ops::{Deref, DerefMut};
+
+#[allow(clippy::many_single_char_names, clippy::clone_double_ref)]
+#[allow(unused_variables)]
+#[warn(clippy::explicit_deref_method)]
+fn main() {
+ let a: &mut String = &mut String::from("foo");
+
+ // these should require linting
+ {
+ let b: &str = a.deref();
+ }
+
+ {
+ let b: &mut str = a.deref_mut();
+ }
+
+ {
+ let b: String = a.deref().clone();
+ }
+
+ {
+ let b: usize = a.deref_mut().len();
+ }
+
+ {
+ let b: &usize = &a.deref().len();
+ }
+
+ {
+ // only first deref should get linted here
+ let b: &str = a.deref().deref();
+ }
+
+ {
+ // both derefs should get linted here
+ let b: String = format!("{}, {}", a.deref(), a.deref());
+ }
+
+ // these should not require linting
+ {
+ let b: &str = &*a;
+ }
+
+ {
+ let b: &mut str = &mut *a;
+ }
+
+ {
+ macro_rules! expr_deref { ($body:expr) => { $body.deref() } }
+ let b: &str = expr_deref!(a);
+ }
+}

tests/ui/dereference.stderr

@@ -0,0 +1,52 @@
+error: explicit deref method call
+ --> $DIR/dereference.rs:13:23
+ |
+13 | let b: &str = a.deref();
+ | ^^^^^^^^^ help: try this: `&*a`
+ |
+ = note: `-D clippy::explicit-deref-method` implied by `-D warnings`
+
+error: explicit deref_mut method call
+ --> $DIR/dereference.rs:17:27
+ |
+17 | let b: &mut str = a.deref_mut();
+ | ^^^^^^^^^^^^^ help: try this: `&mut *a`
+
+error: explicit deref method call
+ --> $DIR/dereference.rs:21:25
+ |
+21 | let b: String = a.deref().clone();
+ | ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref_mut method call
+ --> $DIR/dereference.rs:25:24
+ |
+25 | let b: usize = a.deref_mut().len();
+ | ^^^^^^^^^^^^^ help: try this: `&mut *a`
+
+error: explicit deref method call
+ --> $DIR/dereference.rs:29:26
+ |
+29 | let b: &usize = &a.deref().len();
+ | ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+ --> $DIR/dereference.rs:34:23
+ |
+34 | let b: &str = a.deref().deref();
+ | ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+ --> $DIR/dereference.rs:39:43
+ |
+39 | let b: String = format!("{}, {}", a.deref(), a.deref());
+ | ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+ --> $DIR/dereference.rs:39:54
+ |
+39 | let b: String = format!("{}, {}", a.deref(), a.deref());
+ | ^^^^^^^^^ help: try this: `&*a`
+
+error: aborting due to 8 previous errors
+