Skip to content

Extend manual_filter to cover and_then#16456

Merged
ada4a merged 1 commit intorust-lang:masterfrom
profetia:issue14440
Apr 11, 2026
Merged

Extend manual_filter to cover and_then#16456
ada4a merged 1 commit intorust-lang:masterfrom
profetia:issue14440

Conversation

@profetia
Copy link
Copy Markdown
Member

@profetia profetia commented Jan 25, 2026

View all comments

Closes #14440

Implemented as enhancement to manual_filter

changelog: [manual_filter] enhance to cover and_then

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 25, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jan 25, 2026

r? @llogiq

rustbot has assigned @llogiq.
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

@profetia profetia force-pushed the issue14440 branch 2 times, most recently from b147757 to b12c8eb Compare January 25, 2026 01:24
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 25, 2026

Lintcheck changes for 3351369

Lint Added Removed Changed
clippy::manual_filter 3 0 0

This comment will be updated if you push new changes

Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Comment thread clippy_lints/src/matches/mod.rs Outdated
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
@profetia profetia force-pushed the issue14440 branch 3 times, most recently from 837a8aa to 4c0d9c2 Compare January 31, 2026 17:16
@profetia
Copy link
Copy Markdown
Member Author

profetia commented Feb 5, 2026

r? clippy

@rustbot rustbot assigned Centri3 and unassigned llogiq Feb 5, 2026
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Comment thread clippy_lints/src/matches/manual_filter.rs
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
@rustbot

This comment has been minimized.

@profetia
Copy link
Copy Markdown
Member Author

r? clippy

@rustbot rustbot assigned ada4a and unassigned Centri3 Mar 10, 2026
@ada4a
Copy link
Copy Markdown
Contributor

ada4a commented Mar 14, 2026

@Jarcho since you are intending to eventually split the lints crate, these changes would probably require eventually splitting this lint into two as well: one for the matches pass, and one for the methods pass. But imo that would be awkward -- not least because we'll need to invent a new name for what's essentially the same lint. So I was thinking about some alternative approaches.

At first, I thought we could:

  1. move the declare_clippy_lint! to clippy_lints/src/lib.rs
  2. import the lint struct from both methods/mod.rs and matches/mod.rs, and add it to both passes
  3. have matches/manual_filter.rs for the part of the lint that deals with matches (the current code), and methods/manual_filter.rs for the part that deals with methods (the new code in this PR), and call their corresponding functions from inside the passes.

One problem with that is this helper function get_cond_expr which is shared between the two parts of the implementation, pushing us toward having the whole lint in one file. Therefore, a second idea would be to:

  1. have a new crate like clippy_multipass_lints for crates like this one, and add it as a dependency for each pass crate as needed
  2. define the whole lint (including the declaration) in this new crate, in one file
  3. import the declaration and the functions to both passes' crates, and use them there

I think this would retain the benefits of crate splitting (lower compile times), since this multipass crate, while being a dependence of many pass crates, should be pretty small, and so should compile quickly and let the parallelism kick in directly afterwards.

What do you think?

@profetia
Copy link
Copy Markdown
Member Author

@ada4a Is there any update needed for this PR? Or maybe we can have it merged.

Copy link
Copy Markdown
Contributor

@ada4a ada4a left a comment

Choose a reason for hiding this comment

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

One more question, after that let's merge this -- it should be easy enough to adjust later if desired

View changes since this review

Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Comment thread clippy_lints/src/matches/manual_filter.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 Apr 4, 2026
Comment thread clippy_lints/src/matches/manual_filter.rs
@rustbot

This comment has been minimized.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 4, 2026

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 4, 2026
Comment thread clippy_lints/src/matches/manual_filter.rs Outdated
Copy link
Copy Markdown
Contributor

@ada4a ada4a left a comment

Choose a reason for hiding this comment

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

@ada4a ada4a added this pull request to the merge queue Apr 11, 2026
Merged via the queue into rust-lang:master with commit d588ba3 Apr 11, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new lint: option manual filter Option::and_then(|x| { if ... then {None} else {Some(x) })

5 participants