about summary refs log tree commit diff
diff options
context:
space:
mode:
authorKrishna Veera Reddy <veerareddy@email.arizona.edu>2019-12-08 11:46:21 -0800
committerKrishna Veera Reddy <veerareddy@email.arizona.edu>2019-12-31 09:22:34 -0800
commit26812f733daaf65e514c058c51138511f6f5b60c (patch)
treed09811baa85be346a2c9c284c8e5d09605373caa
parent2a75241c1abc54c1becd7eff108160beef4490f8 (diff)
downloadrust-26812f733daaf65e514c058c51138511f6f5b60c.tar.gz
rust-26812f733daaf65e514c058c51138511f6f5b60c.zip
Prevent `mem_replace_with_default` lint within macros
Also added test cases for internal and external macros.
-rw-r--r--clippy_lints/src/mem_replace.rs8
-rw-r--r--tests/ui/auxiliary/macro_rules.rs7
-rw-r--r--tests/ui/mem_replace.fixed14
-rw-r--r--tests/ui/mem_replace.rs14
-rw-r--r--tests/ui/mem_replace.stderr10
5 files changed, 44 insertions, 9 deletions
diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs
index 5ad41a53b89..ab0bdb4d02c 100644
--- a/clippy_lints/src/mem_replace.rs
+++ b/clippy_lints/src/mem_replace.rs
@@ -1,10 +1,10 @@
 use crate::utils::{
-    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_with_applicability, span_help_and_lint, span_lint_and_sugg,
 };
 use if_chain::if_chain;
 use rustc::declare_lint_pass;
 use rustc::hir::{BorrowKind, Expr, ExprKind, HirVec, Mutability, QPath};
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintPass};
 use rustc_errors::Applicability;
 use rustc_session::declare_tool_lint;
 
@@ -163,9 +163,9 @@ fn check_replace_with_uninit(cx: &LateContext<'_, '_>, expr: &'_ Expr, args: &Hi
 }
 
 fn check_replace_with_default(cx: &LateContext<'_, '_>, expr: &'_ Expr, args: &HirVec<Expr>) {
-    if let ExprKind::Call(ref repl_func, ref repl_args) = args[1].kind {
+    if let ExprKind::Call(ref repl_func, _) = args[1].kind {
         if_chain! {
-            if repl_args.is_empty();
+            if !in_macro(expr.span) && !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);
diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs
index 504d6733abf..eafc68bd6bc 100644
--- a/tests/ui/auxiliary/macro_rules.rs
+++ b/tests/ui/auxiliary/macro_rules.rs
@@ -39,3 +39,10 @@ macro_rules! string_add {
         let z = y + "...";
     };
 }
+
+#[macro_export]
+macro_rules! take_external {
+    ($s:expr) => {
+        std::mem::replace($s, Default::default())
+    };
+}
diff --git a/tests/ui/mem_replace.fixed b/tests/ui/mem_replace.fixed
index 58657b934fb..8606e98335d 100644
--- a/tests/ui/mem_replace.fixed
+++ b/tests/ui/mem_replace.fixed
@@ -8,6 +8,7 @@
 // except according to those terms.
 
 // run-rustfix
+// aux-build:macro_rules.rs
 #![allow(unused_imports)]
 #![warn(
     clippy::all,
@@ -16,8 +17,17 @@
     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();
@@ -31,6 +41,10 @@ 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() {
diff --git a/tests/ui/mem_replace.rs b/tests/ui/mem_replace.rs
index eac0ce586a0..c116107a923 100644
--- a/tests/ui/mem_replace.rs
+++ b/tests/ui/mem_replace.rs
@@ -8,6 +8,7 @@
 // except according to those terms.
 
 // run-rustfix
+// aux-build:macro_rules.rs
 #![allow(unused_imports)]
 #![warn(
     clippy::all,
@@ -16,8 +17,17 @@
     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);
@@ -31,6 +41,10 @@ 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() {
diff --git a/tests/ui/mem_replace.stderr b/tests/ui/mem_replace.stderr
index d5dc66873b2..9c925cefb04 100644
--- a/tests/ui/mem_replace.stderr
+++ b/tests/ui/mem_replace.stderr
@@ -1,5 +1,5 @@
 error: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:23:13
+  --> $DIR/mem_replace.rs:33:13
    |
 LL |     let _ = mem::replace(&mut an_option, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
@@ -7,13 +7,13 @@ LL |     let _ = mem::replace(&mut an_option, None);
    = note: `-D clippy::mem-replace-option-with-none` implied by `-D warnings`
 
 error: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:25:13
+  --> $DIR/mem_replace.rs:35: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:30:13
+  --> $DIR/mem_replace.rs:40:13
    |
 LL |     let _ = std::mem::replace(&mut s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
@@ -21,13 +21,13 @@ LL |     let _ = std::mem::replace(&mut s, String::default());
    = 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:32:13
+  --> $DIR/mem_replace.rs:42: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:33:13
+  --> $DIR/mem_replace.rs:43:13
    |
 LL |     let _ = std::mem::replace(s, Default::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`