Add lint for mem::replace(.., None).

Suggest `Option::take()` as an alternative.

Author: JayKickliter

clippy_lints/src/lib.rs

@@ -133,6 +133,7 @@ pub mod map_clone;
 pub mod map_unit_fn;
 pub mod matches;
 pub mod mem_forget;
+pub mod mem_replace;
 pub mod methods;
 pub mod minmax;
 pub mod misc;
@@ -380,6 +381,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box neg_multiply::NegMultiply);
     reg.register_early_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval);
     reg.register_late_lint_pass(box mem_forget::MemForget);
+    reg.register_late_lint_pass(box mem_replace::MemReplace);
     reg.register_late_lint_pass(box arithmetic::Arithmetic::default());
     reg.register_late_lint_pass(box assign_ops::AssignOps);
     reg.register_late_lint_pass(box let_if_seq::LetIfSeq);
@@ -748,6 +750,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         matches::MATCH_REF_PATS,
         matches::MATCH_WILD_ERR_ARM,
         matches::SINGLE_MATCH,
+        mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
         methods::CHARS_LAST_CMP,
         methods::GET_UNWRAP,
         methods::ITER_CLONED_COLLECT,

clippy_lints/src/mem_replace.rs

@@ -0,0 +1,66 @@
+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 if_chain::if_chain;
+
+/// **What it does:** Checks for `mem::replace()` on an `Option` with
+/// `None`.
+///
+/// **Why is this bad?** `Option` already has the method `take()` for
+/// taking its current value (Some(..) or None) and replacing it with
+/// `None`.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// let an_option = Some(0);
+/// let replaced = mem::replace(&mut an_option, None);
+/// ```
+/// Is better expressed with:
+/// ```rust
+/// let an_option = Some(0);
+/// let taken = an_option.take();
+/// ```
+declare_clippy_lint! {
+    pub MEM_REPLACE_OPTION_WITH_NONE,
+    style,
+    "replacing an `Option` with `None` instead of `take()`"
+}
+
+pub struct MemReplace;
+
+impl LintPass for MemReplace {
+    fn get_lints(&self) -> LintArray {
+        lint_array![MEM_REPLACE_OPTION_WITH_NONE]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if_chain! {
+            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);
+            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, ""));
+                span_lint_and_sugg(
+                    cx,
+                    MEM_REPLACE_OPTION_WITH_NONE,
+                    expr.span,
+                    "replacing an `Option` with `None`",
+                    "consider `Option::take()` instead",
+                    sugg
+                );
+            }
+        }
+    }
+}

clippy_lints/src/utils/paths.rs

@@ -47,6 +47,7 @@ pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "Link
 pub const LINT: [&str; 3] = ["rustc", "lint", "Lint"];
 pub const LINT_ARRAY: [&str; 3] = ["rustc", "lint", "LintArray"];
 pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
+pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"];
 pub const MEM_UNINIT: [&str; 3] = ["core", "mem", "uninitialized"];
 pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"];
 pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"];

tests/ui/mem_replace.rs

@@ -0,0 +1,9 @@
+#![feature(tool_lints)]
+#![warn(clippy::all, clippy::style, clippy::mem_replace_option_with_none)]
+
+use std::mem;
+
+fn main() {
+    let mut an_option = Some(1);
+    let _ = mem::replace(&mut an_option, None);
+}

tests/ui/mem_replace.stderr

@@ -0,0 +1,10 @@
+error: replacing an `Option` with `None`
+ --> $DIR/mem_replace.rs:8:13
+  |
+8 |     let _ = mem::replace(&mut an_option, None);
+  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
+  |
+  = note: `-D clippy::mem-replace-option-with-none` implied by `-D warnings`
+
+error: aborting due to previous error
+