diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 9022617ebfc1..d11614d489d2 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,4 +1,6 @@ -use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg}; +use crate::utils::{ + get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg, +}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; @@ -15,18 +17,19 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// let b = a.deref(); - /// let c = a.deref_mut(); + /// use std::ops::Deref; + /// let a: &mut String = &mut String::from("foo"); + /// let b: &str = a.deref(); /// ``` /// Could be written as: /// ```rust + /// let a: &mut String = &mut String::from("foo"); /// let b = &*a; - /// let c = &mut *a; /// ``` /// /// This lint excludes - /// ```rust - /// let e = d.unwrap().deref(); + /// ```rust,ignore + /// let _ = d.unwrap().deref(); /// ``` pub EXPLICIT_DEREF_METHOD, pedantic, @@ -49,7 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { for arg in args { if_chain! { // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ex: &*a.len()) + // otherwise it can lead to error prone suggestions (ie: &*a.len()) let (method_names, arg_list, _) = method_calls(arg, 2); if method_names.len() == 1; // Caller must be a variable @@ -59,7 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { then { let name = method_names[0].as_str(); - lint_deref(cx, &*name, variables[0].span, arg.span); + lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span); } } } @@ -72,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { if args.len() == 1; if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; // Caller must call only one other function (deref or deref_mut) - // otherwise it can lead to error prone suggestions (ex: &*a.len()) + // otherwise it can lead to error prone suggestions (ie: &*a.len()) let (method_names, arg_list, _) = method_calls(init, 2); if method_names.len() == 1; // Caller must be a variable @@ -82,7 +85,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { then { let name = method_name.ident.as_str(); - lint_deref(cx, &*name, args[0].span, init.span); + lint_deref(cx, &*name, init, args[0].span, init.span); } } } @@ -96,16 +99,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { if_chain! { if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; if args.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind; if let Some(parent) = get_parent_expr(cx, &expr); then { - // Call and MethodCall exprs are better reported using statements match parent.kind { - ExprKind::Call(_, _) => return, - ExprKind::MethodCall(_, _, _) => return, + // Already linted using statements + ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (), _ => { let name = method_name.ident.as_str(); - lint_deref(cx, &*name, args[0].span, expr.span); + lint_deref(cx, &*name, &args[0], args[0].span, expr.span); } } } @@ -113,29 +116,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { } } -fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) { +fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { match fn_name { "deref" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr_span, - "explicit deref method call", - "try this", - format!("&*{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); + if get_trait_def_id(cx, &paths::DEREF_TRAIT) + .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) + { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref method call", + "try this", + format!("&*{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } }, "deref_mut" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr_span, - "explicit deref_mut method call", - "try this", - format!("&mut *{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); + if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT) + .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[])) + { + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHOD, + expr_span, + "explicit deref_mut method call", + "try this", + format!("&mut *{}", &snippet(cx, var_span, "..")), + Applicability::MachineApplicable, + ); + } }, _ => (), } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 96337e42b544..a77f07aec84e 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -21,7 +21,9 @@ pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; +pub const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; +pub const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"]; pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"]; diff --git a/tests/ui/dereference.fixed b/tests/ui/dereference.fixed new file mode 100644 index 000000000000..292324eeacba --- /dev/null +++ b/tests/ui/dereference.fixed @@ -0,0 +1,79 @@ +// run-rustfix + +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_method)] + +use std::ops::{Deref, DerefMut}; + +fn concat(deref_str: &str) -> String { + format!("{}bar", deref_str) +} + +fn just_return(deref_str: &str) -> &str { + deref_str +} + +fn main() { + let a: &mut String = &mut String::from("foo"); + + // these should require linting + + let b: &str = &*a; + + let b: &mut str = &mut *a; + + // both derefs should get linted here + let b: String = format!("{}, {}", &*a, &*a); + + println!("{}", &*a); + + #[allow(clippy::match_single_binding)] + match &*a { + _ => (), + } + + let b: String = concat(&*a); + + // following should not require linting + + let b = just_return(a).deref(); + + let b: String = concat(just_return(a).deref()); + + let b: String = a.deref().clone(); + + let b: usize = a.deref_mut().len(); + + let b: &usize = &a.deref().len(); + + let b: &str = a.deref().deref(); + + let b: &str = &*a; + + let b: &mut str = &mut *a; + + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; + } + let b: &str = expr_deref!(a); + + let opt_a = Some(a); + let b = opt_a.unwrap().deref(); + + // The struct does not implement Deref trait + #[derive(Copy, Clone)] + struct NoLint(u32); + impl NoLint { + pub fn deref(self) -> u32 { + self.0 + } + pub fn deref_mut(self) -> u32 { + self.0 + } + } + let no_lint = NoLint(42); + let b = no_lint.deref(); + let b = no_lint.deref_mut(); +} diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 5de201fb22f0..25e1c29e7fa5 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] #![warn(clippy::explicit_deref_method)] @@ -59,4 +61,19 @@ fn main() { let opt_a = Some(a); let b = opt_a.unwrap().deref(); + + // The struct does not implement Deref trait + #[derive(Copy, Clone)] + struct NoLint(u32); + impl NoLint { + pub fn deref(self) -> u32 { + self.0 + } + pub fn deref_mut(self) -> u32 { + self.0 + } + } + let no_lint = NoLint(42); + let b = no_lint.deref(); + let b = no_lint.deref_mut(); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index 7e10add40b18..97fab3a34e01 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,5 +1,5 @@ error: explicit deref method call - --> $DIR/dereference.rs:19:19 + --> $DIR/dereference.rs:21:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,37 +7,37 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-method` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:21:23 + --> $DIR/dereference.rs:23:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:24:39 + --> $DIR/dereference.rs:26:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:50 + --> $DIR/dereference.rs:26:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:26:20 + --> $DIR/dereference.rs:28:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:29:11 + --> $DIR/dereference.rs:31:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:33:28 + --> $DIR/dereference.rs:35:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a`