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

from_iter_instead_of_collect feels like a blanket ban of the FromIterator trait #7213

Open
Nemo157 opened this issue May 12, 2021 · 5 comments · Fixed by #7375
Open

from_iter_instead_of_collect feels like a blanket ban of the FromIterator trait #7213

Nemo157 opened this issue May 12, 2021 · 5 comments · Fixed by #7375
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@Nemo157
Copy link
Member

Nemo157 commented May 12, 2021

I tried this code:

Vec::from_iter(2..5)

I expected to see this happen: no warning

Instead, this happened:

warning: usage of `FromIterator::from_iter`
   --> src/lib.rs:219:1
    |
219 | Vec::from_iter(2..5);
    | ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(2..5).collect::<Vec<_>>()`
    |
    = note: `#[warn(clippy::from_iter_instead_of_collect)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect

The Vec::from_iter call is much more succinct and readable than .collect::<Vec<_>>(), and rust-lang/rfcs#3114 is currently in FCP-disposition-merge to add std::iter::FromIterator to the Rust 2021 prelude. Having Clippy recommend against using it seems to go against the decision there that it is a trait to be used directly.

I'm really not sure if there's some subset of this lint that would feel ok, there's maybe some sort of "collect here would be more readable", but defining heuristics to detect when to enable that seems complicated.

Meta

  • cargo clippy -V: clippy 0.1.53 (42816d6 2021-04-24)
@Nemo157 Nemo157 added the C-bug Category: Clippy is not doing the correct thing label May 12, 2021
@djc
Copy link
Contributor

djc commented May 16, 2021

Previously: #6550. Let's say it's not a coincidence that I got involved in the prelude changes. Given that the RFC has been accepted, I do think it's time to make this allow by default.

@camsteffen
Copy link
Contributor

Congrats @djc on your success with appealing your case to the Supreme Court! 😉

I agree. This lint is founded on a style principle that is, well, going out of style.

@camsteffen
Copy link
Contributor

I think it would be better to fix the lint to only include cases where turbofish is not needed. Moving to pedantic doesn't really solve the issue here.

@camsteffen camsteffen reopened this Jun 19, 2021
@djc
Copy link
Contributor

djc commented Jun 21, 2021

What semantics do you suggest exactly? The lint should only trigger if the full destination type can be inferred?

@camsteffen
Copy link
Contributor

Right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants