Suppress iter_on_empty_collections if the iterator's concrete type is relied upon
#12823
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Alexendoo (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
the
S-waiting-on-review
|
|
clippy_lints/src/methods/iter_on_single_or_empty_collections.rs
Outdated
Show resolved
Hide resolved
clippy_lints/src/methods/iter_on_single_or_empty_collections.rs
Outdated
Show resolved
Hide resolved
|
r? @y21 |
There was a problem hiding this comment.
There are still lots of similar-ish false positives with the type unification checks in this lint (for example, this won't suppress the lint for mem::replace(&mut /* Iter<()> */, [].iter()), or /* Option<Iter<()>> */.unwrap_or([].iter())).
But they seem orthogonal to this change (and have existed before) so I don't really consider them a blocker for this PR. Small improvements are always great.
clippy_lints/src/methods/iter_on_single_or_empty_collections.rs
Outdated
Show resolved
Hide resolved
| let arg_id_in_args = args.into_iter().position(|e| e.hir_id == arg_id).unwrap(); | ||
| let arg_ty_in_args = fn_sig.input(arg_id_in_args); | ||
|
|
||
| cx.tcx.predicates_of(fn_id).predicates.iter().any(|(clause, _)| { |
There was a problem hiding this comment.
Maybe an interesting followup (in addition to looking through predicates, doesn't need to be done in this PR) would be to run a type visitor through each parameter type and check if the argument type is in there. I.e. when we have fn<T>(&mut T, T) and the argument maps to T, figure out that it unifies with the &mut T parameter. That would catch both of the false positives I mentioned before (mem::replace & unwrap_or).
There was a problem hiding this comment.
Added that as well & added a use of std::mem::replace to the tests, seems to be fine, not linted
clippy_lints/src/methods/iter_on_single_or_empty_collections.rs
Outdated
Show resolved
Hide resolved
|
Sorry for the delay, forgot about this one. Nice to see even those two false positives fixed. Thanks! @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: fixed #12807