Skip to content

filter_next: big clean-up#17233

Merged
Jarcho merged 6 commits into
rust-lang:masterfrom
ada4a:cleanup-filter_next
Jun 14, 2026
Merged

filter_next: big clean-up#17233
Jarcho merged 6 commits into
rust-lang:masterfrom
ada4a:cleanup-filter_next

Conversation

@ada4a

@ada4a ada4a commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

changelog: none

Among other things, this duplicates the repro for issue10029 for `.next_back()`
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 13, 2026
@rustbot

rustbot commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 8 candidates
  • 8 candidates expanded to 8 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown

Lintcheck changes for bef6f41

Lint Added Removed Changed
clippy::filter_next 0 0 2

This comment will be updated if you push new changes

ada4a added 2 commits June 14, 2026 01:30
..and inline `msg`, to make next commit's diff a bit nicer
@ada4a ada4a force-pushed the cleanup-filter_next branch 2 times, most recently from ee02d36 to 0bdf023 Compare June 13, 2026 23:36

@Jarcho Jarcho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title is both accurate and incredibly misleading. Made it sound like I would have to set aside a block of time to read it.

View changes since this review

Comment thread clippy_lints/src/methods/filter_next.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 14, 2026
@rustbot

rustbot commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@Jarcho

Jarcho commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Removed the changelog message. This isn't a big enough change to have an entry there.

@ada4a

ada4a commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

The PR title is both accurate and incredibly misleading. Made it sound like I would have to set aside a block of time to read it.

Apologies, I did in fact somewhat struggle with titling this 😅 For me, a clean-up, even a big one, is always a series of straightforward and (hopefully) uncontroversial changes. For deeper architecture changes I normally use "overhaul" instead

@ada4a

ada4a commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Removed the changelog message. This isn't a big enough change to have an entry there.

Yeah that's fair, I've used this same changelog message in several recent PRs, I'd go clean them up as well then.

ada4a added 2 commits June 14, 2026 13:33
1. In the singe-line case, we switch to verbose suggestions, otherwise
   the suggestion line gets too long due to the longer message.
2. In the multi-line case, we use span-less help, otherwise the whole
   multiline span gets highlighted the second time.
3. Also make the suggestion message more concise.
@ada4a ada4a force-pushed the cleanup-filter_next branch from 0bdf023 to 537443b Compare June 14, 2026 11:34
@ada4a

ada4a commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Btw, the "only emit a suggestion if the snippet is single-line" thing is only done due to limitations in Clippy (or rustc I guess) which are no longer there, right? Should I go on and remove that as well? (in this or separate PR, as you prefer)

@Jarcho

Jarcho commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

I don't know if that was ever fixed. It was due to a diagnostic rendering issue in rustc.

This used to be a workaround for a rustc diagnostics issue which has
since been resolved
@ada4a

ada4a commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Well, I just tested it, and it does seem to work -- see the latest commit. Let me know if I should keep it in

@Jarcho Jarcho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

The verbose suggestion style may have also fixed it, but that's generally the better display anyway.

View changes since this review

@ada4a

ada4a commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

The verbose suggestion style may have also fixed it

Fwiw I think rustc switches to it automatically when the suggestion spans multiple lines. Though it could be that that's actually a part of their fix of this issue

Merged via the queue into rust-lang:master with commit 6032abe Jun 14, 2026
11 checks passed
@ada4a ada4a deleted the cleanup-filter_next branch June 14, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants