Merge branch 'master' into option_option_pr

Cargo.toml

@@ -43,7 +43,7 @@ cargo_metadata = "0.2"
 regex = "0.2"
 
 [dev-dependencies]
-compiletest_rs = "0.3"
+compiletest_rs = "0.3.5"
 duct = "0.8.2"
 lazy_static = "1.0"
 serde_derive = "1.0"

clippy_lints/src/map_clone.rs

@@ -10,7 +10,9 @@ use utils::{is_adjusted, iter_input_pats, match_qpath, match_trait_method, match
 /// **Why is this bad?** It makes the code less readable than using the
 /// `.cloned()` adapter.
 ///
-/// **Known problems:** None.
+/// **Known problems:** Sometimes `.cloned()` requires stricter trait
+/// bound than `.map(|e| e.clone())` (which works because of the coercion).
+/// See [#498](https://github.com/rust-lang-nursery/rust-clippy/issues/498).
 ///
 /// **Example:**
 /// ```rust

clippy_lints/src/needless_pass_by_value.rs

@@ -6,6 +6,7 @@ use rustc::ty::{self, RegionKind, TypeFoldable};
 use rustc::traits;
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::mem_categorization as mc;
+use syntax::abi::Abi;
 use syntax::ast::NodeId;
 use syntax_pos::Span;
 use syntax::errors::DiagnosticBuilder;
@@ -71,13 +72,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
         }
 
         match kind {
-            FnKind::ItemFn(.., attrs) => for a in attrs {
+            FnKind::ItemFn(.., abi, _, attrs) => {
-                if_chain! {
+                if abi != Abi::Rust {
-                    if a.meta_item_list().is_some();
+                    return;
-                    if let Some(name) = a.name();
+                }
-                    if name == "proc_macro_derive";
+                for a in attrs {
-                    then {
+                    if_chain! {
-                        return;
+                        if a.meta_item_list().is_some();
+                        if let Some(name) = a.name();
+                        if name == "proc_macro_derive";
+                        then {
+                            return;
+                        }
                     }
                 }
             },
@@ -96,10 +102,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
 
         // Allow `Borrow` or functions to be taken by value
         let borrow_trait = need!(get_trait_def_id(cx, &paths::BORROW_TRAIT));
-        let fn_traits = [
+        let whitelisted_traits = [
             need!(cx.tcx.lang_items().fn_trait()),
             need!(cx.tcx.lang_items().fn_once_trait()),
             need!(cx.tcx.lang_items().fn_mut_trait()),
+            need!(get_trait_def_id(cx, &paths::RANGE_ARGUMENT_TRAIT))
         ];
 
         let sized_trait = need!(cx.tcx.lang_items().sized_trait());
@@ -183,7 +190,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
                 if !is_self(arg);
                 if !ty.is_mutable_pointer();
                 if !is_copy(cx, ty);
-                if !fn_traits.iter().any(|&t| implements_trait(cx, ty, t, &[]));
+                if !whitelisted_traits.iter().any(|&t| implements_trait(cx, ty, t, &[]));
                 if !implements_borrow_trait;
                 if !all_borrowable_trait;
 
@@ -196,6 +203,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
 
                     // Dereference suggestion
                     let sugg = |db: &mut DiagnosticBuilder| {
+                        if let ty::TypeVariants::TyAdt(ref def, ..) = ty.sty {
+                            if let Some(span) = cx.tcx.hir.span_if_local(def.did) {
+                                let param_env = ty::ParamEnv::empty(traits::Reveal::UserFacing);
+                                if param_env.can_type_implement_copy(cx.tcx, ty, span).is_ok() {
+                                    db.span_help(span, "consider marking this type as Copy");
+                                }
+                            }
+                        }
+
                         let deref_span = spans_need_deref.get(&canonical_id);
                         if_chain! {
                             if match_type(cx, ty, &paths::VEC);

clippy_lints/src/utils/paths.rs

@@ -55,6 +55,7 @@ pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"];
 pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
 pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
 pub const RANGE: [&str; 3] = ["core", "ops", "Range"];
+pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["alloc", "range", "RangeArgument"];
 pub const RANGE_FROM: [&str; 3] = ["core", "ops", "RangeFrom"];
 pub const RANGE_FROM_STD: [&str; 3] = ["std", "ops", "RangeFrom"];
 pub const RANGE_FULL: [&str; 3] = ["core", "ops", "RangeFull"];

tests/dogfood.rs

@@ -1,5 +1,8 @@
 #[test]
 fn dogfood() {
+    if option_env!("RUSTC_TEST_SUITE").is_some() {
+        return;
+    }
     let root_dir = std::env::current_dir().unwrap();
     for d in &[".", "clippy_lints"] {
         std::env::set_current_dir(root_dir.join(d)).unwrap();

tests/ui/needless_pass_by_value.rs

@@ -4,6 +4,8 @@
 #![warn(needless_pass_by_value)]
 #![allow(dead_code, single_match, if_let_redundant_pattern_matching, many_single_char_names, option_option)]
 
+#![feature(collections_range)]
+
 use std::borrow::Borrow;
 use std::convert::AsRef;
 
@@ -110,4 +112,29 @@ trait FalsePositive {
     }
 }
 
+// shouldn't warn on extern funcs
+extern "C" fn ext(x: String) -> usize { x.len() }
+
+// whitelist RangeArgument
+fn range<T: ::std::collections::range::RangeArgument<usize>>(range: T) {
+    let _ = range.start();
+}
+
+struct CopyWrapper(u32);
+
+fn bar_copy(x: u32, y: CopyWrapper) {
+    assert_eq!(x, 42);
+    assert_eq!(y.0, 42);
+}
+
+// x and y should be warned, but z is ok
+fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
+    let CopyWrapper(s) = z; // moved
+    let CopyWrapper(ref t) = y; // not moved
+    let CopyWrapper(_) = y; // still not moved
+
+    assert_eq!(x.0, s);
+    println!("{}", t);
+}
+
 fn main() {}

tests/ui/needless_pass_by_value.stderr

@@ -1,128 +1,188 @@
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:12:23
+  --> $DIR/needless_pass_by_value.rs:14:23
    |
-12 | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
+14 | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
    |                       ^^^^^^ help: consider changing the type to: `&[T]`
    |
    = note: `-D needless-pass-by-value` implied by `-D warnings`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:26:11
+  --> $DIR/needless_pass_by_value.rs:28:11
    |
-26 | fn bar(x: String, y: Wrapper) {
+28 | fn bar(x: String, y: Wrapper) {
    |           ^^^^^^ help: consider changing the type to: `&str`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:26:22
+  --> $DIR/needless_pass_by_value.rs:28:22
    |
-26 | fn bar(x: String, y: Wrapper) {
+28 | fn bar(x: String, y: Wrapper) {
    |                      ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:32:71
+  --> $DIR/needless_pass_by_value.rs:34:71
    |
-32 | fn test_borrow_trait<T: Borrow<str>, U: AsRef<str>, V>(t: T, u: U, v: V) {
+34 | fn test_borrow_trait<T: Borrow<str>, U: AsRef<str>, V>(t: T, u: U, v: V) {
    |                                                                       ^ help: consider taking a reference instead: `&V`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:44:18
+  --> $DIR/needless_pass_by_value.rs:46:18
    |
-44 | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
+46 | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
    |                  ^^^^^^^^^^^^^^^^^^^^^^
 help: consider taking a reference instead
    |
-44 | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
+46 | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
-45 |     match *x {
+47 |     match *x {
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:57:24
+  --> $DIR/needless_pass_by_value.rs:59:24
    |
-57 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
+59 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
    |                        ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:57:36
+  --> $DIR/needless_pass_by_value.rs:59:36
    |
-57 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
+59 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
    |                                    ^^^^^^^
 help: consider taking a reference instead
    |
-57 | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
+59 | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
-58 |     let Wrapper(s) = z; // moved
+60 |     let Wrapper(s) = z; // moved
-59 |     let Wrapper(ref t) = *y; // not moved
+61 |     let Wrapper(ref t) = *y; // not moved
-60 |     let Wrapper(_) = *y; // still not moved
+62 |     let Wrapper(_) = *y; // still not moved
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:73:49
+  --> $DIR/needless_pass_by_value.rs:75:49
    |
-73 | fn test_blanket_ref<T: Foo, S: Serialize>(_foo: T, _serializable: S) {}
+75 | fn test_blanket_ref<T: Foo, S: Serialize>(_foo: T, _serializable: S) {}
    |                                                 ^ help: consider taking a reference instead: `&T`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:75:18
+  --> $DIR/needless_pass_by_value.rs:77:18
    |
-75 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
+77 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                  ^^^^^^ help: consider taking a reference instead: `&String`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:75:29
+  --> $DIR/needless_pass_by_value.rs:77:29
    |
-75 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
+77 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                             ^^^^^^
 help: consider changing the type to
    |
-75 | fn issue_2114(s: String, t: &str, u: Vec<i32>, v: Vec<i32>) {
+77 | fn issue_2114(s: String, t: &str, u: Vec<i32>, v: Vec<i32>) {
    |                             ^^^^
 help: change `t.clone()` to
    |
-77 |     let _ = t.to_string();
+79 |     let _ = t.to_string();
    |             ^^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:75:40
+  --> $DIR/needless_pass_by_value.rs:77:40
    |
-75 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
+77 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                                        ^^^^^^^^ help: consider taking a reference instead: `&Vec<i32>`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:75:53
+  --> $DIR/needless_pass_by_value.rs:77:53
    |
-75 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
+77 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                                                     ^^^^^^^^
 help: consider changing the type to
    |
-75 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: &[i32]) {
+77 | fn issue_2114(s: String, t: String, u: Vec<i32>, v: &[i32]) {
    |                                                     ^^^^^^
 help: change `v.clone()` to
    |
-79 |     let _ = v.to_owned();
+81 |     let _ = v.to_owned();
    |             ^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:87:12
+  --> $DIR/needless_pass_by_value.rs:89:12
    |
-87 |         s: String,
+89 |         s: String,
    |            ^^^^^^ help: consider changing the type to: `&str`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:88:12
+  --> $DIR/needless_pass_by_value.rs:90:12
    |
-88 |         t: String,
+90 |         t: String,
    |            ^^^^^^ help: consider taking a reference instead: `&String`
 
 error: this argument is passed by value, but not consumed in the function body
-   --> $DIR/needless_pass_by_value.rs:100:13
+   --> $DIR/needless_pass_by_value.rs:102:13
     |
-100 |         _u: U,
+102 |         _u: U,
     |             ^ help: consider taking a reference instead: `&U`
 
 error: this argument is passed by value, but not consumed in the function body
-   --> $DIR/needless_pass_by_value.rs:101:13
+   --> $DIR/needless_pass_by_value.rs:103:13
     |
-101 |         _s: Self,
+103 |         _s: Self,
     |             ^^^^ help: consider taking a reference instead: `&Self`
 
-error: aborting due to 16 previous errors
+error: this argument is passed by value, but not consumed in the function body
+   --> $DIR/needless_pass_by_value.rs:125:24
+    |
+125 | fn bar_copy(x: u32, y: CopyWrapper) {
+    |                        ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
+    |
+help: consider marking this type as Copy
+   --> $DIR/needless_pass_by_value.rs:123:1
+    |
+123 | struct CopyWrapper(u32);
+    | ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this argument is passed by value, but not consumed in the function body
+   --> $DIR/needless_pass_by_value.rs:131:29
+    |
+131 | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
+    |                             ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
+    |
+help: consider marking this type as Copy
+   --> $DIR/needless_pass_by_value.rs:123:1
+    |
+123 | struct CopyWrapper(u32);
+    | ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this argument is passed by value, but not consumed in the function body
+   --> $DIR/needless_pass_by_value.rs:131:45
+    |
+131 | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
+    |                                             ^^^^^^^^^^^
+    |
+help: consider marking this type as Copy
+   --> $DIR/needless_pass_by_value.rs:123:1
+    |
+123 | struct CopyWrapper(u32);
+    | ^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider taking a reference instead
+    |
+131 | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) {
+132 |     let CopyWrapper(s) = z; // moved
+133 |     let CopyWrapper(ref t) = *y; // not moved
+134 |     let CopyWrapper(_) = *y; // still not moved
+    |
+
+error: this argument is passed by value, but not consumed in the function body
+   --> $DIR/needless_pass_by_value.rs:131:61
+    |
+131 | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
+    |                                                             ^^^^^^^^^^^
+    |
+help: consider marking this type as Copy
+   --> $DIR/needless_pass_by_value.rs:123:1
+    |
+123 | struct CopyWrapper(u32);
+    | ^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider taking a reference instead
+    |
+131 | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) {
+132 |     let CopyWrapper(s) = *z; // moved
+    |
+
+error: aborting due to 20 previous errors