Skip to content

Lint needless take-by-value #1556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/array_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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),
})
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)]);
});
}
}
Expand Down Expand Up @@ -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))]);
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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),
Expand Down
Loading