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

Split explicit_iter_loop lint into deref and std versions #11406

Closed
wants to merge 7 commits into from

Conversation

Benjscho
Copy link
Contributor

@Benjscho Benjscho commented Aug 25, 2023

Split [explicit_iter_loop] into [explicit_iter_loop_std] and [explicit_iter_loop_deref].

The purpose of this PR is to fix issue #11074 where the suggested code from explicit_iter_loop was too symbol dense for the deref versions of the lint. The proposed solution is to split the lint into two. The explicit_iter_loop_deref can then be allowed by those that use pedantic linting, prefer the standard iter loop, but dislike the added symbol clutter of the deref version.

For quick reference, deref versions:

  • *x
  • &*x
  • &mut *x

Std versions:

  • x
  • &x
  • &mut x

All tests passing locally.

fixes #11074

changelog: [explicit_iter_loop]: Split [explicit_iter_loop] into [explicit_iter_loop_std] and [explicit_iter_loop_deref].

Referencing this raised issue: rust-lang#11074

The lint adds too much visual noise when deref lints are enforced. This commit
splits the explicit_iter_loop into two versions, the first, `std`, where
non-deref types are linted, and the second `deref` where deref types are
linted.
Fix the explicit_iter_loop tests after splitting the lint into deref and std
versions. This completes the work of splitting the lint and leaving only the
standard (non-deref) aspect in general warn.
@rustbot
Copy link
Collaborator

rustbot commented Aug 25, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 25, 2023
Fix the dogfood test.

I also previously misunderstood `pedantic` designations. This adds both
`explicit_iter_loop_deref` and `explicit_iter_loop_std` as pedantic lints. The
split allows users to decide to ignore only the deref lint.
Fix failing doctests from changes, and add `explicit_iter_loop` ->
`explicit_iter_loop_std` to renamed lints list.
Fix failing rename test after renaming explicit_iter_loop.
@Alexendoo
Copy link
Member

A poll for how we want to handle this: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60explicit_iter_loop.60

@Benjscho Benjscho closed this Aug 28, 2023
@Benjscho
Copy link
Contributor Author

Closing after discussion on Zulip recommended disabling the lint for reborrows only via a configuration flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

explicit_iter_loop: x.iter_mut() vs &mut *x
4 participants