Lint within internal macros without a suggestion

Author: krishna-veerareddy

clippy_lints/src/mem_replace.rs

@@ -1,5 +1,6 @@
 use crate::utils::{
-    in_macro, match_def_path, match_qpath, paths, snippet_with_applicability, span_help_and_lint, span_lint_and_sugg,
+    in_macro, match_def_path, match_qpath, paths, snippet, snippet_with_applicability, span_help_and_lint,
+    span_lint_and_sugg, span_lint_and_then,
 };
 use if_chain::if_chain;
 use rustc::declare_lint_pass;
@@ -166,24 +167,28 @@ fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr, expr_span: Sp
 fn check_replace_with_default(cx: &LateContext<'_, '_>, src: &Expr, dest: &Expr, expr_span: Span) {
     if let ExprKind::Call(ref repl_func, _) = src.kind {
         if_chain! {
-            if !in_macro(expr_span) && !in_external_macro(cx.tcx.sess, expr_span);
+            if !in_external_macro(cx.tcx.sess, expr_span);
             if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
             if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
             if match_def_path(cx, repl_def_id, &paths::DEFAULT_TRAIT_METHOD);
             then {
-                let mut applicability = Applicability::MachineApplicable;
-
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     MEM_REPLACE_WITH_DEFAULT,
                     expr_span,
                     "replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
-                    "consider using",
-                    format!(
-                        "std::mem::take({})",
-                        snippet_with_applicability(cx, dest.span, "", &mut applicability)
-                    ),
-                    applicability,
+                    |db| {
+                        if !in_macro(expr_span) {
+                            let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));
+
+                            db.span_suggestion(
+                                expr_span,
+                                "consider using",
+                                suggestion,
+                                Applicability::MachineApplicable
+                            );
+                        }
+                    }
                 );
             }
         }

tests/ui/mem_replace.fixed

@@ -8,7 +8,6 @@
 // except according to those terms.
 
 // run-rustfix
-// aux-build:macro_rules.rs
 #![allow(unused_imports)]
 #![warn(
     clippy::all,
@@ -17,17 +16,8 @@
     clippy::mem_replace_with_default
 )]
 
-#[macro_use]
-extern crate macro_rules;
-
 use std::mem;
 
-macro_rules! take {
-    ($s:expr) => {
-        std::mem::replace($s, Default::default())
-    };
-}
-
 fn replace_option_with_none() {
     let mut an_option = Some(1);
     let _ = an_option.take();
@@ -41,10 +31,6 @@ fn replace_with_default() {
     let s = &mut String::from("foo");
     let _ = std::mem::take(s);
     let _ = std::mem::take(s);
-
-    // dont lint within macros
-    take!(s);
-    take_external!(s);
 }
 
 fn main() {

tests/ui/mem_replace.rs

@@ -8,7 +8,6 @@
 // except according to those terms.
 
 // run-rustfix
-// aux-build:macro_rules.rs
 #![allow(unused_imports)]
 #![warn(
     clippy::all,
@@ -17,17 +16,8 @@
     clippy::mem_replace_with_default
 )]
 
-#[macro_use]
-extern crate macro_rules;
-
 use std::mem;
 
-macro_rules! take {
-    ($s:expr) => {
-        std::mem::replace($s, Default::default())
-    };
-}
-
 fn replace_option_with_none() {
     let mut an_option = Some(1);
     let _ = mem::replace(&mut an_option, None);
@@ -41,10 +31,6 @@ fn replace_with_default() {
     let s = &mut String::from("foo");
     let _ = std::mem::replace(s, String::default());
     let _ = std::mem::replace(s, Default::default());
-
-    // dont lint within macros
-    take!(s);
-    take_external!(s);
 }
 
 fn main() {

tests/ui/mem_replace.stderr

@@ -1,33 +1,33 @@
 error: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:33:13
+  --> $DIR/mem_replace.rs:23:13
    |
 LL |     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: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:35:13
+  --> $DIR/mem_replace.rs:25:13
    |
 LL |     let _ = mem::replace(an_option, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:40:13
+  --> $DIR/mem_replace.rs:30:13
    |
 LL |     let _ = std::mem::replace(&mut s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
    |
    = note: `-D clippy::mem-replace-with-default` implied by `-D warnings`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:42:13
+  --> $DIR/mem_replace.rs:32:13
    |
 LL |     let _ = std::mem::replace(s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:43:13
+  --> $DIR/mem_replace.rs:33:13
    |
 LL |     let _ = std::mem::replace(s, Default::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`

tests/ui/mem_replace_macro.rs

@@ -0,0 +1,23 @@
+// aux-build:macro_rules.rs
+#![warn(clippy::mem_replace_with_default)]
+
+#[macro_use]
+extern crate macro_rules;
+
+use std::mem;
+
+macro_rules! take {
+    ($s:expr) => {
+        std::mem::replace($s, Default::default())
+    };
+}
+
+fn replace_with_default() {
+    let s = &mut String::from("foo");
+    take!(s);
+    take_external!(s);
+}
+
+fn main() {
+    replace_with_default();
+}

tests/ui/mem_replace_macro.stderr

@@ -0,0 +1,13 @@
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace_macro.rs:11:9
+   |
+LL |         std::mem::replace($s, Default::default())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     take!(s);
+   |     --------- in this macro invocation
+   |
+   = note: `-D clippy::mem-replace-with-default` implied by `-D warnings`
+
+error: aborting due to previous error
+