from_iter_instead_of_collect is too strict

[djc]

Lint name:

from_iter_instead_of_collect

clippy is suggesting I make this change:

-    let etlds = HashMap::from_iter(
-        data.etld_index
-            .iter()
-            .map(|(idx, etld)| (*etld, idx.clone())),
-    );
+    let etlds = data
+        .etld_index
+        .iter()
+        .map(|(idx, etld)| (*etld, idx.clone()))
+        .collect::<HashMap<_, _>>();

I don't think this is an improvement, because now I need to turbofish the return type including wildcard type arguments, which wasn't needed for the from_iter() call. If the return type could be inferred by the compiler, I'd agree, but I'm not sure how easy it is for clippy to determine that.

(This is pretty similar to #6533, but with a different argument.)

Meta

• cargo clippy -V: clippy 0.0.212 (e1884a8e 2020-12-29)
• rustc -Vv:

rustc 1.49.0 (e1884a8e3 2020-12-29)
binary: rustc
commit-hash: e1884a8e3c3e813aada8254edfa120e85bf5ffca
commit-date: 2020-12-29
host: x86_64-apple-darwin
release: 1.49.0

[camsteffen]

I believe collect is more idiomatic. Note that you may type annotate the variable instead of using turbofish: let etlds: HashMap<_, _> = ..

[glandium]

I hit something similar with test code like assert_eq!(Vec::from_iter(...), ...) where the type can't be inferred with collect in the assert.

[camsteffen]

From the Rust documentation, it is pretty clear that turbofish syntax is encouraged over using from_iter. The documentation for the lint also points to this.

FromIterator::from_iter() is rarely called explicitly, and is instead used through Iterator::collect() method. See Iterator::collect()'s documentation for more examples.

and

Because collect() is so general, it can cause problems with type inference. As such, collect() is one of the few times you'll see the syntax affectionately known as the 'turbofish': ::<>. This helps the inference algorithm understand specifically which collection you're trying to collect into.

[glandium]

From the Rust documentation, it is pretty clear that turbofish syntax is encouraged over using from_iter. The documentation for the lint also points to this.

FromIterator::from_iter() is rarely called explicitly, and is instead used through Iterator::collect() method. See Iterator::collect()'s documentation for more examples.

This part was written in November 2015, back in Rust 1.6.0. rust-lang/rust@9d663a3

and

Because collect() is so general, it can cause problems with type inference. As such, collect() is one of the few times you'll see the syntax affectionately known as the 'turbofish': ::<>. This helps the inference algorithm understand specifically which collection you're trying to collect into.

This part was written in October 2015, back in Rust 1.6.0 as well. rust-lang/rust@d91785a

It is fair to say Rust practices have evolved since then, and it might be worth reconsidering. @steveklabnik wdyt?

[djc]

I also wanted to say it's a bit of a leap to say that turbofish is "encouraged". The first snippet just says "is rarely called", factually noting that collect() is used more often, while I read the second snippet as "if you get into trouble with type inference, you'll need to turbofish", that is, it helps troubleshooting rather than recommending.

I did also run a poll over on users.r-l.o which does show a substantial preference for collecting. On the other hand, three of the poll respondents who volunteered additional data on their usage each highlighted that they sometimes prefer from_iter(). Each of these users are, in my estimation, more experienced than the average Rust user (and thus, perhaps, also more experienced than the average poll respondent) -- and each of their posts garnered several likes.

[steveklabnik]

A couple of intersecting things going on here, but in short, I am not sure I've ever seen from_iter in the wild. I think there's a couple of factors here, one big one is that it requires a use and collect does not. I would agree with @djc's characterization of the docs, but we mention collect and not from_iter because it is the default.

I would personally maybe consider annotating etlds rather than turbofish, maybe.