diff options
| -rw-r--r-- | clippy_lints/src/mem_replace.rs | 29 | ||||
| -rw-r--r-- | tests/ui/mem_replace.rs | 2 | ||||
| -rw-r--r-- | tests/ui/mem_replace.stderr | 8 |
3 files changed, 32 insertions, 7 deletions
diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index f516fc5085c..22460ccb5ea 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -1,7 +1,7 @@ use crate::rustc::hir::{Expr, ExprKind, MutMutable, QPath}; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; -use crate::utils::{match_def_path, match_qpath, match_type, opt_def_id, paths, snippet, span_lint_and_sugg}; +use crate::utils::{match_def_path, match_qpath, opt_def_id, paths, snippet, span_lint_and_sugg}; use if_chain::if_chain; /// **What it does:** Checks for `mem::replace()` on an `Option` with @@ -40,25 +40,42 @@ impl LintPass for MemReplace { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if_chain! { + // Check that `expr` is a call to `mem::replace()` if let ExprKind::Call(ref func, ref func_args) = expr.node; if func_args.len() == 2; if let ExprKind::Path(ref func_qpath) = func.node; if let Some(def_id) = opt_def_id(cx.tables.qpath_def(func_qpath, func.hir_id)); if match_def_path(cx.tcx, def_id, &paths::MEM_REPLACE); - if let ExprKind::AddrOf(MutMutable, ref replaced) = func_args[0].node; - if match_type(cx, cx.tables.expr_ty(replaced), &paths::OPTION); + + // Check that second argument is `Option::None` if let ExprKind::Path(ref replacement_qpath) = func_args[1].node; if match_qpath(replacement_qpath, &paths::OPTION_NONE); - if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node; + then { - let sugg = format!("{}.take()", snippet(cx, replaced_path.span, "")); + // Since this is a late pass (already type-checked), + // and we already know that the second argument is an + // `Option`, we do not need to check if the first + // argument's type. All that's left is to get + // replacee's path. + let replaced_path = match func_args[0].node { + ExprKind::AddrOf(MutMutable, ref replaced) => { + if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node { + replaced_path + } else { + return + } + }, + ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path, + _ => return, + }; + span_lint_and_sugg( cx, MEM_REPLACE_OPTION_WITH_NONE, expr.span, "replacing an `Option` with `None`", "consider `Option::take()` instead", - sugg + format!("{}.take()", snippet(cx, replaced_path.span, "")) ); } } diff --git a/tests/ui/mem_replace.rs b/tests/ui/mem_replace.rs index 14f586e71bf..62df42ef2d2 100644 --- a/tests/ui/mem_replace.rs +++ b/tests/ui/mem_replace.rs @@ -6,4 +6,6 @@ use std::mem; fn main() { let mut an_option = Some(1); let _ = mem::replace(&mut an_option, None); + let an_option = &mut Some(1); + let _ = mem::replace(an_option, None); } diff --git a/tests/ui/mem_replace.stderr b/tests/ui/mem_replace.stderr index 1ce9fc38093..8385fa3cb3c 100644 --- a/tests/ui/mem_replace.stderr +++ b/tests/ui/mem_replace.stderr @@ -6,5 +6,11 @@ error: replacing an `Option` with `None` | = note: `-D clippy::mem-replace-option-with-none` implied by `-D warnings` -error: aborting due to previous error +error: replacing an `Option` with `None` + --> $DIR/mem_replace.rs:10:13 + | +10 | let _ = mem::replace(an_option, None); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()` + +error: aborting due to 2 previous errors |