Add block_scrutinee lint

[shivendra02467]

This PR introduces a new late pass lint to catch scrutinees unnecessarily wrapped in blocks on older editions, preventing unintended behavior regarding temporary lifetimes.

Fixes #16827

changelog: new lint: [block_scrutinee] to warn on scrutinees wrapped in blocks in older editions

[rustbot]

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

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

[github-actions]

Lintcheck changes for 89131a5

Lint	Added	Removed	Changed
clippy::block_scrutinee	140	0	0

This comment will be updated if you push new changes

[rustbot]

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.

[llogiq]

What is the behavior in the current edition? Would it make sense to check whether the block survives a lifetime check if the edition was updated?

[shivendra02467]

Hi @llogiq, thanks for taking a look!
To answer your questions:

1. Behavior in the 2024 edition:As of the 2024 edition, temporaries created in the tail expression of a block are dropped "before" the body executes (at the end of the block). Prior to 2024, they were not dropped until the end of the entire match/if let statement. This can cause a real bug like someone tried to fix a deadlock by doing if let Some(x) = { lock().foo() } { ... }. They assumed the lock would drop before the body. That works in 2024, but silently deadlocks in 2021.

2. Regarding a lifetime check:
   @Darksonn explicitly addressed this in the original issue (Warn on old editions when scrutinee is wrapped in a block #16827) and determined that an advanced lifetime check isn't necessary because removing the block (when it only contains a tail expression) "never" changes behavior prior to the 2024 edition:
   If the user meant to drop the temporaries (like the deadlock fix attempt), the block doesn't actually work in the 2021 edition anyway. The code is broken and they must rewrite it (eg. extracting the value to a separate let statement).
   If the user didnt mean to drop temporaries, the block is just unnecessary visual noise, and removing it makes the code less confusing without changing the drop semantics.

because of this, we should warn unconditionally on older editions to catch these false assumptions without needing deeper type/lifetime checking. Let me know if you think we should still try to narrow the lint down!

[Darksonn]

Should clarify that it only triggers on 2021 edition.

View changes since the review

[shivendra02467]

done!

[Darksonn]

Suggested change

	/// ### Why is this bad?
	/// Prior to the 2024 edition, wrapping the scrutinee in a block did not drop
	/// temporaries before the body executes.
	/// ### Why is this bad?
	/// It is unusual to write `{ expr }` when you could just have written
	/// `expr`, and it is unlikely that anyone would write that for any reason
	/// other than wanting temporaries in `expr` to be dropped before executing
	/// the body of the `match`/`if let`/`while` statement. However, prior to
	/// the 2024 edition, wrapping the scrutinee in a block did not drop
	/// temporaries before the body executes.

View changes since the review

[shivendra02467]

updated!

[Darksonn]

Suggested change

	"scrutinee is wrapped in a block which will not drop temporaries until the end of the statement in this edition",
	"scrutinee is wrapped in a block which will not drop temporaries until the end of the statement on this edition",

View changes since the review

[shivendra02467]

changed this error message to the new improved one

[Darksonn]

I think this error message would look better if changed along these lines.

error: temporary values in this block-wrapped scrutinee will not be dropped until the end of the `if let` statement
  --> tests/ui/block_scrutinee.rs:10:22
   |
LL |     if let Some(x) = { my_function() } {
   |                      ^^^^^^^^^^^^^^^^^
   |
   = note: this behavior is specific to Rust editions prior to 2024
   = note: in Rust 2024, temporaries in a block scrutinee drop immediately before the match arm or block body
   = note: `-D clippy::block-scrutinee` implied by `-D warnings`
help: remove the block to yield the same behavior but with cleaner code
   |
LL |     if let Some(x) = my_function() {
   |                      ~~~~~~~~~~~~~
help: if you intended to drop temporaries early, move the expression to a separate local binding
   |
LL |     if let Some(x) = { let res = my_function(); res } {
   |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View changes since the review

[shivendra02467]

Updated! Thank you

[Darksonn]

Please include a test that verifies that this does not trigger on the 2024 edition.

[Darksonn]

What is the behavior in the current edition?

The behavior of { expr } in the current edition is what you would expect. Temporaries in the expression are dropped right away when the block returns, and not after the end of the parent expression's scope.

Would it make sense to check whether the block survives a lifetime check if the edition was updated?

I don't think so. Nobody is going to write { expr } when they could have written expr without a good reason, and on older editions, the behavior is exactly identical, so there is no good reason to do it on older editions. Anyone writing { expr } did so with the intent of getting the 2024 edition behavior. If they don't get that behavior, we should tell them.

[shivendra02467]

@Darksonn Thanks for suggesting the improvements, I will update the PR to include them.

[llogiq]

Ok. I think the lint should suggest updating the edition in case the user actually wants the early drop – or moving the block into its own let stmt.