Skip to content

Commit fa9cfe7

Browse files
authored
Add block_scrutinee lint (#16855)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16855)* 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
2 parents c3ec34d + 3ff2380 commit fa9cfe7

9 files changed

Lines changed: 417 additions & 0 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6763,6 +6763,7 @@ Released 2018-09-13
67636763
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
67646764
[`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr
67656765
[`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt
6766+
[`block_scrutinee`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_scrutinee
67666767
[`blocks_in_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
67676768
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
67686769
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::{indent_of, snippet};
3+
use rustc_errors::Applicability;
4+
use rustc_hir::{BlockCheckMode, Expr, ExprKind, LoopSource};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::declare_lint_pass;
7+
use rustc_span::edition::Edition;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Warns when a match, if let, or while let scrutinee is wrapped in a block.
12+
/// This lint only triggers on the 2021 edition and older.
13+
///
14+
/// ### Why is this bad?
15+
/// It is unusual to write `{ expr }` when you could just have written
16+
/// `expr`, and it is unlikely that anyone would write that for any reason
17+
/// other than wanting temporaries in `expr` to be dropped before executing
18+
/// the body of the `match`/`if let`/`while` statement. However, prior to
19+
/// the 2024 edition, wrapping the scrutinee in a block did not drop
20+
/// temporaries before the body executes.
21+
///
22+
/// ### Example
23+
/// ```rust,ignore
24+
/// if let Some(x) = { my_function() } { .. }
25+
/// ```
26+
#[clippy::version = "1.98.0"]
27+
pub BLOCK_SCRUTINEE,
28+
suspicious,
29+
"warns when the scrutinee is wrapped in a block in older editions"
30+
}
31+
32+
declare_lint_pass!(BlockScrutinee => [BLOCK_SCRUTINEE]);
33+
34+
impl<'tcx> LateLintPass<'tcx> for BlockScrutinee {
35+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
36+
if cx.tcx.sess.edition() >= Edition::Edition2024 {
37+
return;
38+
}
39+
40+
let (scrutinee, keyword_fallback) = match expr.kind {
41+
ExprKind::Match(scrutinee, _, _) => (scrutinee, "`match`"),
42+
ExprKind::Let(let_expr) => (let_expr.init, "`if let` / `while let`"),
43+
_ => return,
44+
};
45+
46+
if scrutinee.span.from_expansion() || expr.span.from_expansion() {
47+
return;
48+
}
49+
50+
if let ExprKind::Block(block, _) = scrutinee.kind
51+
&& matches!(block.rules, BlockCheckMode::DefaultBlock)
52+
&& block.stmts.is_empty()
53+
&& let Some(inner_expr) = block.expr
54+
{
55+
let inner_snippet = snippet(cx, inner_expr.span, "..");
56+
57+
let main_msg = "this scrutinee is wrapped in a block";
58+
59+
span_lint_and_then(cx, BLOCK_SCRUTINEE, scrutinee.span, main_msg, |diag| {
60+
let mut keyword = keyword_fallback;
61+
let mut outer_span = expr.span;
62+
63+
if let ExprKind::Let(_) = expr.kind {
64+
keyword = "`if let`";
65+
for (_, node) in cx.tcx.hir_parent_iter(expr.hir_id) {
66+
if let rustc_hir::Node::Expr(e) = node {
67+
if let ExprKind::If(..) = e.kind {
68+
if keyword == "`if let`" {
69+
outer_span = e.span;
70+
}
71+
} else if let ExprKind::Loop(_, _, LoopSource::While, _) = e.kind {
72+
keyword = "`while let`";
73+
outer_span = e.span;
74+
break;
75+
}
76+
} else if matches!(node, rustc_hir::Node::Item(..) | rustc_hir::Node::ImplItem(..)) {
77+
break;
78+
}
79+
}
80+
}
81+
82+
diag.note(format!(
83+
"temporary values in this block-wrapped scrutinee will be dropped after the body of the {keyword} statement"
84+
));
85+
86+
diag.note("starting with the 2024 edition, temporaries within a block's final expression are dropped immediately at the end of the block");
87+
88+
let suggestion_msg = format!("to drop temporaries after the surrounding {keyword}, remove the block");
89+
90+
diag.span_suggestion(
91+
scrutinee.span,
92+
suggestion_msg,
93+
inner_snippet.to_string(),
94+
Applicability::MaybeIncorrect,
95+
);
96+
97+
let indent = indent_of(cx, outer_span).unwrap_or(0);
98+
let pad = " ".repeat(indent);
99+
100+
diag.multipart_suggestion(
101+
"to drop temporaries early, move them to a separate local binding (or update your `Cargo.toml` to the 2024 edition)",
102+
vec![
103+
(outer_span.shrink_to_lo(), format!("let res = {inner_snippet};\n{pad}")),
104+
(scrutinee.span, "res".to_string()),
105+
],
106+
Applicability::MaybeIncorrect,
107+
);
108+
});
109+
}
110+
}
111+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
3535
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
3636
crate::bit_width::MANUAL_BIT_WIDTH_INFO,
3737
crate::bit_width::MISMATCHED_BIT_WIDTH_TYPE_INFO,
38+
crate::block_scrutinee::BLOCK_SCRUTINEE_INFO,
3839
crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO,
3940
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
4041
crate::bool_comparison::BOOL_COMPARISON_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ mod async_yields_async;
7676
mod attrs;
7777
mod await_holding_invalid;
7878
mod bit_width;
79+
mod block_scrutinee;
7980
mod blocks_in_conditions;
8081
mod bool_assert_comparison;
8182
mod bool_comparison;
@@ -866,6 +867,7 @@ rustc_lint::late_lint_methods!(
866867
RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
867868
RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
868869
RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
870+
BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee,
869871
// add late passes here, used by `cargo dev new_lint`
870872
]]
871873
);

tests/ui/block_scrutinee.1.fixed

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//@ edition: 2021
2+
#![warn(clippy::block_scrutinee)]
3+
#![allow(clippy::blocks_in_conditions)]
4+
#![allow(clippy::let_and_return)]
5+
6+
fn my_function() -> Option<i32> {
7+
Some(1)
8+
}
9+
10+
fn main() {
11+
if let Some(x) = my_function() {
12+
//~^ ERROR: this scrutinee is wrapped in a block
13+
let _ = x;
14+
}
15+
16+
match my_function() {
17+
//~^ ERROR: this scrutinee is wrapped in a block
18+
Some(1) => println!("one"),
19+
Some(_) => println!("other"),
20+
None => println!("none"),
21+
}
22+
23+
let mut v = vec![1, 2, 3];
24+
while let Some(x) = v.pop() {
25+
//~^ ERROR: this scrutinee is wrapped in a block
26+
let _ = x;
27+
}
28+
29+
if let Some(x) = my_function() {
30+
let _ = x;
31+
}
32+
33+
if let Some(x) = {
34+
let _y = 2;
35+
my_function()
36+
} {
37+
let _ = x;
38+
}
39+
40+
//~v ERROR: this scrutinee is wrapped in a block
41+
if let Some(x) = v.pop() {
42+
let _ = x;
43+
}
44+
45+
macro_rules! get_val {
46+
() => {{ my_function() }};
47+
}
48+
if let Some(x) = get_val!() {
49+
let _ = x;
50+
}
51+
52+
// Test that `unsafe` blocks are ignored
53+
unsafe fn my_unsafe_fn() -> Option<i32> {
54+
Some(1)
55+
}
56+
57+
if let Some(x) = unsafe { my_unsafe_fn() } {
58+
let _ = x;
59+
}
60+
}

tests/ui/block_scrutinee.2.fixed

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//@ edition: 2021
2+
#![warn(clippy::block_scrutinee)]
3+
#![allow(clippy::blocks_in_conditions)]
4+
#![allow(clippy::let_and_return)]
5+
6+
fn my_function() -> Option<i32> {
7+
Some(1)
8+
}
9+
10+
fn main() {
11+
let res = my_function();
12+
if let Some(x) = res {
13+
//~^ ERROR: this scrutinee is wrapped in a block
14+
let _ = x;
15+
}
16+
17+
let res = my_function();
18+
match res {
19+
//~^ ERROR: this scrutinee is wrapped in a block
20+
Some(1) => println!("one"),
21+
Some(_) => println!("other"),
22+
None => println!("none"),
23+
}
24+
25+
let mut v = vec![1, 2, 3];
26+
let res = v.pop();
27+
while let Some(x) = res {
28+
//~^ ERROR: this scrutinee is wrapped in a block
29+
let _ = x;
30+
}
31+
32+
if let Some(x) = my_function() {
33+
let _ = x;
34+
}
35+
36+
if let Some(x) = {
37+
let _y = 2;
38+
my_function()
39+
} {
40+
let _ = x;
41+
}
42+
43+
//~v ERROR: this scrutinee is wrapped in a block
44+
let res = v.pop();
45+
if let Some(x) = res {
46+
let _ = x;
47+
}
48+
49+
macro_rules! get_val {
50+
() => {{ my_function() }};
51+
}
52+
if let Some(x) = get_val!() {
53+
let _ = x;
54+
}
55+
56+
// Test that `unsafe` blocks are ignored
57+
unsafe fn my_unsafe_fn() -> Option<i32> {
58+
Some(1)
59+
}
60+
61+
if let Some(x) = unsafe { my_unsafe_fn() } {
62+
let _ = x;
63+
}
64+
}

tests/ui/block_scrutinee.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//@ edition: 2021
2+
#![warn(clippy::block_scrutinee)]
3+
#![allow(clippy::blocks_in_conditions)]
4+
#![allow(clippy::let_and_return)]
5+
6+
fn my_function() -> Option<i32> {
7+
Some(1)
8+
}
9+
10+
fn main() {
11+
if let Some(x) = { my_function() } {
12+
//~^ ERROR: this scrutinee is wrapped in a block
13+
let _ = x;
14+
}
15+
16+
match { my_function() } {
17+
//~^ ERROR: this scrutinee is wrapped in a block
18+
Some(1) => println!("one"),
19+
Some(_) => println!("other"),
20+
None => println!("none"),
21+
}
22+
23+
let mut v = vec![1, 2, 3];
24+
while let Some(x) = { v.pop() } {
25+
//~^ ERROR: this scrutinee is wrapped in a block
26+
let _ = x;
27+
}
28+
29+
if let Some(x) = my_function() {
30+
let _ = x;
31+
}
32+
33+
if let Some(x) = {
34+
let _y = 2;
35+
my_function()
36+
} {
37+
let _ = x;
38+
}
39+
40+
//~v ERROR: this scrutinee is wrapped in a block
41+
if let Some(x) = {
42+
// We are popping a value
43+
v.pop()
44+
} {
45+
let _ = x;
46+
}
47+
48+
macro_rules! get_val {
49+
() => {{ my_function() }};
50+
}
51+
if let Some(x) = get_val!() {
52+
let _ = x;
53+
}
54+
55+
// Test that `unsafe` blocks are ignored
56+
unsafe fn my_unsafe_fn() -> Option<i32> {
57+
Some(1)
58+
}
59+
60+
if let Some(x) = unsafe { my_unsafe_fn() } {
61+
let _ = x;
62+
}
63+
}

0 commit comments

Comments
 (0)