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.

Sign up for GitHub

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

Lint needless take-by-value #1556

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d81d961
Lint needless take-by-value
sinkuu Feb 18, 2017
d2f73e7
Add explanation
sinkuu Feb 19, 2017
0a6bc60
Rename lint to needless_take_by_value
sinkuu Feb 20, 2017
f1b0b77
Support non-moving usages at `match`
sinkuu Feb 20, 2017
263e60c
Run update_lints.py
sinkuu Feb 20, 2017
627d24c
Fix suggestion for `let <pat>`
sinkuu Feb 20, 2017
3516d45
Use `multispan_sugg`
sinkuu Feb 21, 2017
cb86c57
Integrate suggestion spans
sinkuu Feb 21, 2017
bf21c84
Ensure deref_span is empty for Vec and String
sinkuu Feb 21, 2017
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
20 changes: 9 additions & 11 deletions clippy_lints/src/needless_pass_by_value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,27 +132,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
db.span_suggestion(input.span,
&format!("consider changing the type to `{}`", slice_ty),
slice_ty);
return;
return; // `Vec` and `String` cannot be destructured - no need for `*` suggestion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add assertions that span_need_deref is empty as a canary for when matching on vec/string becomes possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}}

if match_type(cx, ty, &paths::STRING) {
db.span_suggestion(input.span,
"consider changing the type to `&str`",
"&str".to_string());
} else {
db.span_suggestion(input.span,
"consider taking a reference instead",
format!("&{}", snippet(cx, input.span, "_")));
return;
}

// Suggests adding `*` to dereference the added reference if needed.
if let Some(spans) = spans_need_deref.get(&defid) {
let mut spans: Vec<_> = spans.iter().cloned()
.map(|span| (span, format!("*{}", snippet(cx, span, "<expr>"))))
.collect();
let mut spans = vec![(input.span, format!("&{}", snippet(cx, input.span, "_")))];

// Suggests adding `*` to dereference the added reference.
if let Some(deref_span) = spans_need_deref.get(&defid) {
spans.extend(deref_span.iter().cloned()
.map(|span| (span, format!("*{}", snippet(cx, span, "<expr>")))));
spans.sort_by_key(|&(span, _)| span);
multispan_sugg(db, "...and dereference it here".to_string(), spans);
}
multispan_sugg(db, "consider taking a reference instead".to_string(), spans);
};

span_lint_and_then(cx,
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/needless_pass_by_value.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ error: this argument is passed by value, but not consumed in the function body
|
help: consider taking a reference instead
| fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
help: ...and dereference it here
| match *x {

error: this argument is passed by value, but not consumed in the function body
Expand All @@ -67,7 +66,7 @@ error: this argument is passed by value, but not consumed in the function body
|
help: consider taking a reference instead
| fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
help: ...and dereference it here
| let Wrapper(s) = z; // moved
| let Wrapper(ref t) = *y; // not moved
| let Wrapper(_) = *y; // still not moved

Expand Down