Skip to content

manual_pop_if: lint more cases, even if we do not provide a suggestion#16683

Open
pbor wants to merge 1 commit intorust-lang:masterfrom
pbor:manual_pop_if_no_suggestions
Open

manual_pop_if: lint more cases, even if we do not provide a suggestion#16683
pbor wants to merge 1 commit intorust-lang:masterfrom
pbor:manual_pop_if_no_suggestions

Conversation

@pbor
Copy link
Contributor

@pbor pbor commented Mar 7, 2026

Extend the lint to detect the case where the popped value is used, but in such cases just emit the lint with no suggestion.

changelog: none

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

rustbot commented Mar 7, 2026

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: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@pbor
Copy link
Contributor Author

pbor commented Mar 7, 2026

r? @ada4a

This is a follow up to #16582

I did not include the changelog since the new lint was just added and that will be in the changelog

@rustbot rustbot assigned ada4a and unassigned Jarcho Mar 7, 2026
Copy link
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.

Some tentative review

View changes since this review

@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 Mar 7, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2026

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

@ada4a
Copy link
Contributor

ada4a commented Mar 7, 2026

I did not include the changelog since the new lint was just added and that will be in the changelog

In that case, changelog: none is still required^^

Extend the lint to detect the case where the popped value is used, but
in such cases just emit the lint with no suggestion.

changelog: none
@pbor pbor force-pushed the manual_pop_if_no_suggestions branch from cbb440a to 06f5f41 Compare March 9, 2026 13:13
@rustbot
Copy link
Collaborator

rustbot commented Mar 9, 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.

@pbor
Copy link
Contributor Author

pbor commented Mar 9, 2026

Thanks for the review and suggestions, PR updated.

@pbor
Copy link
Contributor Author

pbor commented Mar 9, 2026

@rustbot ready

@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 Mar 9, 2026
@pbor pbor requested a review from ada4a March 9, 2026 15:11
@ada4a
Copy link
Contributor

ada4a commented Mar 9, 2026

Hm, it didn't come out as nice as I'd imagined – it's a bit too verbose. Let me play around with it locally and get back to you when I find something nicer

@ada4a
Copy link
Contributor

ada4a commented Mar 9, 2026

Hm, nevermind, I think the original version was the best after all. Could you please revert to that?^^

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.

4 participants