Check for try blocks in question_mark
more consistently
#12341
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 theimpl
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:
?
withintry {}
blocks desugars to abreak
to the block, rather than areturn
, 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 forlet-else-return
, but I chose to actually just move those checks out and instead have them inLintPass::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 intomanual_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