diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs new file mode 100644 index 00000000000..c29c0d466d1 --- /dev/null +++ b/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 crate::utils::{in_macro, span_lint_and_sugg}; +use if_chain::if_chain; + +/// **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), + ); + }, + _ => () + }; + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cb9fcfca8a1..a6ddd6573a8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -391,7 +391,7 @@ pub fn read_conf(args: &[rustc_ast::ast::NestedMetaItem], sess: &Session) -> Con } conf - }, + } Err((err, span)) => { sess.struct_span_err(span, err) .span_note(span, "Clippy will use default configuration") @@ -513,6 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ©_iterator::COPY_ITERATOR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, + &dereference::DEREF_METHOD_EXPLICIT, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -1039,6 +1040,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); + store.register_late_pass(|| box dereference::DerefMethodExplicit); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1089,6 +1091,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION), LintId::of(©_iterator::COPY_ITERATOR), LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS), + LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY), LintId::of(&doc::DOC_MARKDOWN), LintId::of(&doc::MISSING_ERRORS_DOC), @@ -1178,6 +1181,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), + LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::DERIVE_HASH_XOR_EQ), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs new file mode 100644 index 00000000000..7800cd84c24 --- /dev/null +++ b/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); + } +} diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr new file mode 100644 index 00000000000..a4c2487d06b --- /dev/null +++ b/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 +