diff --git a/README.md b/README.md index 1300c5ad47bf..6915b1bde025 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 358 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 359 lints included in this crate!](https://rust-lang.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: diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index dea00e5aa3b6..9022617ebfc1 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,9 +1,11 @@ -use rustc_hir::{Expr, ExprKind, QPath}; +use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg}; +use if_chain::if_chain; use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::{Expr, ExprKind, QPath, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, declare_lint_pass}; -use crate::utils::{in_macro, span_lint_and_sugg}; -use if_chain::if_chain; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Span; declare_clippy_lint! { /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls. @@ -21,7 +23,7 @@ declare_clippy_lint! { /// let b = &*a; /// let c = &mut *a; /// ``` - /// + /// /// This lint excludes /// ```rust /// let e = d.unwrap().deref(); @@ -36,45 +38,105 @@ declare_lint_pass!(Dereferencing => [ ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { - if in_macro(expr.span) { - return; + fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) { + if_chain! { + if let StmtKind::Local(ref local) = stmt.kind; + if let Some(ref init) = local.init; + + then { + match init.kind { + ExprKind::Call(ref _method, args) => { + 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()) + let (method_names, arg_list, _) = method_calls(arg, 2); + if method_names.len() == 1; + // Caller must be a variable + let variables = arg_list[0]; + if variables.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; + + then { + let name = method_names[0].as_str(); + lint_deref(cx, &*name, variables[0].span, arg.span); + } + } + } + } + ExprKind::MethodCall(ref method_name, _, ref args) => { + if init.span.from_expansion() { + return; + } + if_chain! { + 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()) + let (method_names, arg_list, _) = method_calls(init, 2); + if method_names.len() == 1; + // Caller must be a variable + let variables = arg_list[0]; + if variables.len() == 1; + if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind; + + then { + let name = method_name.ident.as_str(); + lint_deref(cx, &*name, args[0].span, init.span); + } + } + } + _ => () + } + } } + } + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if_chain! { - // if this is a method call if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; - // on a Path (i.e. a variable/name, not another method) - if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; + if args.len() == 1; + if let Some(parent) = get_parent_expr(cx, &expr); + 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), - Applicability::MachineApplicable - ); - }, - "deref_mut" => { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHOD, - expr.span, - "explicit deref_mut method call", - "try this", - format!("&mut *{}", path), - Applicability::MachineApplicable - ); - }, - _ => () - }; + // Call and MethodCall exprs are better reported using statements + match parent.kind { + ExprKind::Call(_, _) => return, + ExprKind::MethodCall(_, _, _) => return, + _ => { + let name = method_name.ident.as_str(); + lint_deref(cx, &*name, args[0].span, expr.span); + } + } } } } } + +fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, 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, + ); + }, + "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, + ); + }, + _ => (), + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index d99a7a3832bd..05bbe6cebd83 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 358] = [ +pub const ALL_LINTS: [Lint; 359] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 07421eb715df..5de201fb22f0 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -3,28 +3,49 @@ 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.deref(); let b: &mut str = a.deref_mut(); + // both derefs should get linted here + let b: String = format!("{}, {}", a.deref(), a.deref()); + + println!("{}", a.deref()); + + #[allow(clippy::match_single_binding)] + match a.deref() { + _ => (), + } + + let b: String = concat(a.deref()); + + // 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(); - // 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; @@ -35,4 +56,7 @@ fn main() { }; } let b: &str = expr_deref!(a); + + let opt_a = Some(a); + let b = opt_a.unwrap().deref(); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index 7169b689a860..7e10add40b18 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,5 +1,5 @@ error: explicit deref method call - --> $DIR/dereference.rs:10:19 + --> $DIR/dereference.rs:19:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,46 +7,40 @@ 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:12:23 + --> $DIR/dereference.rs:21:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:14:21 - | -LL | let b: String = a.deref().clone(); - | ^^^^^^^^^ help: try this: `&*a` - -error: explicit deref_mut method call - --> $DIR/dereference.rs:16:20 + --> $DIR/dereference.rs:24:39 | -LL | let b: usize = a.deref_mut().len(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:18:22 + --> $DIR/dereference.rs:24:50 | -LL | let b: &usize = &a.deref().len(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:21:19 + --> $DIR/dereference.rs:26:20 | -LL | let b: &str = a.deref().deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | println!("{}", a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:39 + --> $DIR/dereference.rs:29:11 | -LL | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | match a.deref() { + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:24:50 + --> $DIR/dereference.rs:33:28 | -LL | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = concat(a.deref()); + | ^^^^^^^^^ help: try this: `&*a` -error: aborting due to 8 previous errors +error: aborting due to 7 previous errors