-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Added lints into_iter_on_ref and into_iter_on_array.
#3344
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
Conversation
5f5ca79 to
281ff1d
Compare
clippy_lints/src/methods/mod.rs
Outdated
| /// | ||
| /// **Why is this bad?** Arrays and `PathBuf` do not yet have an `into_iter` method which move out | ||
| /// their content into an iterator. Calling `into_iter` instead just forwards to `iter` or | ||
| /// `iter_mut` due to auto-referencing, of which only yield references. Furthermore, when the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"of which only yield"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oli-obk sorry I'm not sure what do you mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence doesn't parse for me. I'm not sure what you're trying to say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC iter_mut doesn't directly apply, because it will always try and succeed with the immutable reference first, matching only iter.
Perhaps something like: "Auto-referencing resolves the into_iter call onto a container reference instead, like <&[T; N] as IntoIterator>::into_iter, which just iterates over item references like calling iter would."
oli-obk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! After reading the new text I understand what the old text was trying to say ;)
|
Sorry for not getting around to merge this faster, could you rebase this for the README.md change? @kennytm |
a67edf9 to
d824df1
Compare
|
@phansch Rebased, thanks! |
d824df1 to
a44f41c
Compare
a44f41c to
5563bd6
Compare
|
bors r+ before another rebase is needed 😅 thanks! |
3344: Added lints `into_iter_on_ref` and `into_iter_on_array`. r=phansch a=kennytm Fixes #1565. `into_iter_on_array` is a correctness lint against `into_iter()` on `[T; n]` and `PathBuf` (please provide a concise noun that covers both types 🙃). `into_iter_on_ref` is a style lint against `into_iter()` on `&C` where `C` is `[T]`, `Vec<T>`, `BTreeSet<T>` etc. Both suggests replacing the `into_iter()` with `iter()` or `iter_mut()`. `into_iter_on_array` is a correctness lint since it is very likely the standard library would provide an `into_iter()` method for `[T; n]` (rust-lang/rust#25725) and existing type inference of `[a, b, c].into_iter()` will be broken. `PathBuf` is also placed under this lint since `PathBuf::into_iter` currently doesn't exist and it makes some sense to implement `IntoIterator` on it yielding `OsString`s. Co-authored-by: kennytm <[email protected]>
Fixes #1565.
into_iter_on_arrayis a correctness lint againstinto_iter()on[T; n]andPathBuf(please provide a concise noun that covers both types 🙃).into_iter_on_refis a style lint againstinto_iter()on&CwhereCis[T],Vec<T>,BTreeSet<T>etc.Both suggests replacing the
into_iter()withiter()oriter_mut().into_iter_on_arrayis a correctness lint since it is very likely the standard library would provide aninto_iter()method for[T; n](rust-lang/rust#25725) and existing type inference of[a, b, c].into_iter()will be broken.PathBufis also placed under this lint sincePathBuf::into_itercurrently doesn't exist and it makes some sense to implementIntoIteratoron it yieldingOsStrings.