rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Check for try blocks in `question_mark` more consistently

Open y21 opened this issue 1 year ago • 2 comments

Fixes #12337

I split this PR up into two commits since this moves a method out of an impl, which makes for a pretty bad diff (the &self parameter is now unused, and there isn't a reason for that function to be part of the impl now).

The first commit is the actual relevant change and the 2nd commit just moves stuff (github's "hide whitespace" makes the diff easier to look at)


Now for the actual issue:

? within try {} blocks desugars to a break to the block, rather than a return, so that changes behavior in those cases.

The lint has multiple patterns to look for and in some of them it already does correctly check whether we're in a try block, but this isn't done for all of its patterns.

We could add another self.inside_try_block() check to the function that looks for let-else-return, but I chose to actually just move those checks out and instead have them in LintPass::check_{stmt,expr}. This has the advantage that we can't (easily) accidentally forget to add that check in new patterns that might be added in the future.

(There's also a bit of a subtle interaction between two lints, where question_mark's LintPass calls into manual_let_else, so I added a check to make sure we don't avoid linting for something that doesn't have anything to do with ?)

changelog: [question_mark]: avoid linting on try blocks in more cases

y21 avatar Feb 23 '24 18:02 y21

r? @dswij

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

rustbot avatar Feb 23 '24 18:02 rustbot

:umbrella: The latest upstream changes (presumably #12348) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Feb 25 '24 18:02 bors

(Rebased.)

y21 avatar Mar 04 '24 13:03 y21

Thank you! @bors r+

dswij avatar Mar 04 '24 14:03 dswij

:pushpin: Commit 1430623e0470fa90bf6415f18225ecac88e0cba8 has been approved by dswij

It is now in the queue for this repository.

bors avatar Mar 04 '24 14:03 bors

:hourglass: Testing commit 1430623e0470fa90bf6415f18225ecac88e0cba8 with merge e970fa52e799cdeeee3fe022d390ca321f1767ef...

bors avatar Mar 04 '24 14:03 bors

:sunny: Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test Approved by: dswij Pushing e970fa52e799cdeeee3fe022d390ca321f1767ef to master...

bors avatar Mar 04 '24 14:03 bors