Skip to content
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

lint vec.clone().into_iter().stuff().collect() to suggest vec.iter().stuff().cloned().collect() #3302

Open
oli-obk opened this issue Oct 12, 2018 · 3 comments
Labels
A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group L-perf Lint: Belongs in the perf lint group

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2018

Found the following code in the wild

 let w: Vec<String> = vec_of_string
        .clone()
        .into_iter()
        .filter(|s| s.len() < 10)
        .collect();

Not sure what other things should be linted, but at least filter and filter_map after a clone+into_iter

@oli-obk oli-obk added A-lint Area: New lints L-perf Lint: Belongs in the perf lint group L-complexity Lint: Belongs in the complexity lint group labels Oct 12, 2018
@scottmcm
Copy link
Member

scottmcm commented Nov 5, 2018

I think this can be split into two parts:

  • lint .clone().into_iter(), suggesting .iter().cloned() instead (to skip the container allocation)
  • lint .cloned().filter(), suggesting .filter().cloned() instead (to skip filtered-out things)

@hcpl
Copy link

hcpl commented Nov 5, 2018

Throwing a wild idea into the mix: sort adaptors by their individual performance impact and then find a global optimum of the combination. I believe this will allow for a more generic lint. As for an implementation, a guided brute-force should do if the adaptor chain is short, say, < 10-15 elements; if not, it's too long anyway (is there a lint for that already?).

@GnomedDev
Copy link

I feel like that would be way too hard to implement and seems to have caused this idea to get lost for a couple years. A lint for .clone().into_iter() seems like quite an easy thing to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group L-perf Lint: Belongs in the perf lint group
Projects
None yet
Development

No branches or pull requests

4 participants