Merge pull request #1556 from sinkuu/take_by_value

Lint needless take-by-value

Author: oli-obk

CHANGELOG.md

@@ -381,6 +381,7 @@ All notable changes to this project will be documented in this file.
 [`needless_bool`]: https://github.com/Manishearth/rust-clippy/wiki#needless_bool
 [`needless_borrow`]: https://github.com/Manishearth/rust-clippy/wiki#needless_borrow
 [`needless_lifetimes`]: https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes
+[`needless_pass_by_value`]: https://github.com/Manishearth/rust-clippy/wiki#needless_pass_by_value
 [`needless_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop
 [`needless_return`]: https://github.com/Manishearth/rust-clippy/wiki#needless_return
 [`needless_update`]: https://github.com/Manishearth/rust-clippy/wiki#needless_update

README.md

@@ -180,7 +180,7 @@ transparently:
 
 ## Lints
 
-There are 191 lints included in this crate:
+There are 192 lints included in this crate:
 
 name                                                                                                                   | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -287,6 +287,7 @@ name
 [needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool)                                         | warn    | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
 [needless_borrow](https://github.com/Manishearth/rust-clippy/wiki#needless_borrow)                                     | warn    | taking a reference that is going to be automatically dereferenced
 [needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes)                               | warn    | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
+[needless_pass_by_value](https://github.com/Manishearth/rust-clippy/wiki#needless_pass_by_value)                       | warn    | functions taking arguments by value, but not consuming them in its body
 [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop)                             | warn    | for-looping over a range of indices where an iterator over items would do
 [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return)                                     | warn    | using a return statement like `return expr;` where an expression would suffice
 [needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update)                                     | warn    | using `Foo { ..base }` when there are no missing fields

clippy_lints/src/array_indexing.rs

@@ -83,7 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing {
                         .map(|end| constcx.eval(end, ExprTypeChecked))
                         .map(|v| v.ok());
 
-                    if let Some((start, end)) = to_const_range(start, end, range.limits, size) {
+                    if let Some((start, end)) = to_const_range(&start, &end, range.limits, size) {
                         if start > size || end > size {
                             utils::span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "range is out of bounds");
                         }
@@ -109,18 +109,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing {
 
 /// Returns an option containing a tuple with the start and end (exclusive) of the range.
 fn to_const_range(
-    start: Option<Option<ConstVal>>,
-    end: Option<Option<ConstVal>>,
+    start: &Option<Option<ConstVal>>,
+    end: &Option<Option<ConstVal>>,
     limits: RangeLimits,
     array_size: ConstInt
 ) -> Option<(ConstInt, ConstInt)> {
-    let start = match start {
+    let start = match *start {
         Some(Some(ConstVal::Integral(x))) => x,
         Some(_) => return None,
         None => ConstInt::Infer(0),
     };
 
-    let end = match end {
+    let end = match *end {
         Some(Some(ConstVal::Integral(x))) => {
             if limits == RangeLimits::Closed {
                 (x + ConstInt::Infer(1)).expect("such a big array is not realistic")

clippy_lints/src/consts.rs

@@ -203,9 +203,9 @@ pub fn lit_to_constant(lit: &LitKind) -> Constant {
     }
 }
 
-fn constant_not(o: Constant) -> Option<Constant> {
+fn constant_not(o: &Constant) -> Option<Constant> {
     use self::Constant::*;
-    match o {
+    match *o {
         Bool(b) => Some(Bool(!b)),
         Int(value) => (!value).ok().map(Int),
         _ => None,
@@ -216,12 +216,12 @@ fn constant_negate(o: Constant) -> Option<Constant> {
     use self::Constant::*;
     match o {
         Int(value) => (-value).ok().map(Int),
-        Float(is, ty) => Some(Float(neg_float_str(is), ty)),
+        Float(is, ty) => Some(Float(neg_float_str(&is), ty)),
         _ => None,
     }
 }
 
-fn neg_float_str(s: String) -> String {
+fn neg_float_str(s: &str) -> String {
     if s.starts_with('-') {
         s[1..].to_owned()
     } else {
@@ -271,7 +271,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
             },
             ExprUnary(op, ref operand) => {
                 self.expr(operand).and_then(|o| match op {
-                    UnNot => constant_not(o),
+                    UnNot => constant_not(&o),
                     UnNeg => constant_negate(o),
                     UnDeref => Some(o),
                 })

clippy_lints/src/lib.rs

@@ -107,6 +107,7 @@ pub mod mut_reference;
 pub mod mutex_atomic;
 pub mod needless_bool;
 pub mod needless_borrow;
+pub mod needless_pass_by_value;
 pub mod needless_update;
 pub mod neg_multiply;
 pub mod new_without_default;
@@ -299,6 +300,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount);
     reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold));
     reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq);
+    reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -455,6 +457,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,
         needless_borrow::NEEDLESS_BORROW,
+        needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
         needless_update::NEEDLESS_UPDATE,
         neg_multiply::NEG_MULTIPLY,
         new_without_default::NEW_WITHOUT_DEFAULT,

clippy_lints/src/loops.rs

@@ -549,8 +549,8 @@ fn check_for_loop_range<'a, 'tcx>(
                                        |db| {
                         multispan_sugg(db,
                                        "consider using an iterator".to_string(),
-                                       &[(pat.span, &format!("({}, <item>)", ident.node)),
-                                         (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
+                                       vec![(pat.span, format!("({}, <item>)", ident.node)),
+                                            (arg.span, format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
                     });
                 } else {
                     let repl = if starts_at_zero && take.is_empty() {
@@ -568,7 +568,7 @@ fn check_for_loop_range<'a, 'tcx>(
                                        |db| {
                         multispan_sugg(db,
                                        "consider using an iterator".to_string(),
-                                       &[(pat.span, "<item>"), (arg.span, &repl)]);
+                                       vec![(pat.span, "<item>".to_string()), (arg.span, repl)]);
                     });
                 }
             }
@@ -816,8 +816,8 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
                     let map = sugg::Sugg::hir(cx, arg, "map");
                     multispan_sugg(db,
                                    "use the corresponding method".into(),
-                                   &[(pat_span, &snippet(cx, new_pat_span, kind)),
-                                     (arg_span, &format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
+                                   vec![(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
+                                        (arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
                 });
             }
         }

clippy_lints/src/matches.rs

@@ -395,7 +395,7 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
                                "you don't need to add `&` to both the expression and the patterns",
                                |db| {
                 let inner = Sugg::hir(cx, inner, "..");
-                let template = match_template(expr.span, source, inner);
+                let template = match_template(expr.span, source, &inner);
                 db.span_suggestion(expr.span, "try", template);
             });
         } else {
@@ -405,7 +405,7 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
                                "you don't need to add `&` to all patterns",
                                |db| {
                 let ex = Sugg::hir(cx, ex, "..");
-                let template = match_template(expr.span, source, ex.deref());
+                let template = match_template(expr.span, source, &ex.deref());
                 db.span_suggestion(expr.span,
                                    "instead of prefixing all patterns with `&`, you can dereference the expression",
                                    template);
@@ -509,7 +509,7 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
     mapped.map_or(false, |v| v.iter().any(|el| *el))
 }
 
-fn match_template(span: Span, source: MatchSource, expr: Sugg) -> String {
+fn match_template(span: Span, source: MatchSource, expr: &Sugg) -> String {
     match source {
         MatchSource::Normal => format!("match {} {{ .. }}", expr),
         MatchSource::IfLetDesugar { .. } => format!("if let .. = {} {{ .. }}", expr),