Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6763,6 +6763,7 @@ Released 2018-09-13
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
[`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr
[`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt
[`block_scrutinee`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_scrutinee
[`blocks_in_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
Expand Down
111 changes: 111 additions & 0 deletions clippy_lints/src/block_scrutinee.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{indent_of, snippet};
use rustc_errors::Applicability;
use rustc_hir::{BlockCheckMode, Expr, ExprKind, LoopSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::edition::Edition;

declare_clippy_lint! {
/// ### What it does
/// Warns when a match, if let, or while let scrutinee is wrapped in a block.
Comment thread
llogiq marked this conversation as resolved.
/// This lint only triggers on the 2021 edition and older.
///
/// ### 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.
Comment thread
llogiq marked this conversation as resolved.
///
/// ### Example
/// ```rust,ignore
/// if let Some(x) = { my_function() } { .. }
/// ```
#[clippy::version = "1.98.0"]
pub BLOCK_SCRUTINEE,
suspicious,
"warns when the scrutinee is wrapped in a block in older editions"
}

declare_lint_pass!(BlockScrutinee => [BLOCK_SCRUTINEE]);

impl<'tcx> LateLintPass<'tcx> for BlockScrutinee {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if cx.tcx.sess.edition() >= Edition::Edition2024 {
return;
}

let (scrutinee, keyword_fallback) = match expr.kind {
ExprKind::Match(scrutinee, _, _) => (scrutinee, "`match`"),
ExprKind::Let(let_expr) => (let_expr.init, "`if let` / `while let`"),
_ => return,
};

if scrutinee.span.from_expansion() || expr.span.from_expansion() {
return;
}

if let ExprKind::Block(block, _) = scrutinee.kind
&& matches!(block.rules, BlockCheckMode::DefaultBlock)
&& block.stmts.is_empty()
&& let Some(inner_expr) = block.expr
{
let inner_snippet = snippet(cx, inner_expr.span, "..");

let main_msg = "this scrutinee is wrapped in a block";

span_lint_and_then(cx, BLOCK_SCRUTINEE, scrutinee.span, main_msg, |diag| {
let mut keyword = keyword_fallback;
let mut outer_span = expr.span;

if let ExprKind::Let(_) = expr.kind {
keyword = "`if let`";
for (_, node) in cx.tcx.hir_parent_iter(expr.hir_id) {
if let rustc_hir::Node::Expr(e) = node {
if let ExprKind::If(..) = e.kind {
if keyword == "`if let`" {
outer_span = e.span;
}
} else if let ExprKind::Loop(_, _, LoopSource::While, _) = e.kind {
keyword = "`while let`";
outer_span = e.span;
break;
}
} else if matches!(node, rustc_hir::Node::Item(..) | rustc_hir::Node::ImplItem(..)) {
break;
}
}
}

diag.note(format!(
"temporary values in this block-wrapped scrutinee will be dropped after the body of the {keyword} statement"
));

diag.note("starting with the 2024 edition, temporaries within a block's final expression are dropped immediately at the end of the block");

let suggestion_msg = format!("to drop temporaries after the surrounding {keyword}, remove the block");

diag.span_suggestion(
scrutinee.span,
suggestion_msg,
inner_snippet.to_string(),
Applicability::MaybeIncorrect,
);

let indent = indent_of(cx, outer_span).unwrap_or(0);
let pad = " ".repeat(indent);

diag.multipart_suggestion(
"to drop temporaries early, move them to a separate local binding (or update your `Cargo.toml` to the 2024 edition)",
vec![
(outer_span.shrink_to_lo(), format!("let res = {inner_snippet};\n{pad}")),
(scrutinee.span, "res".to_string()),
],
Applicability::MaybeIncorrect,
);
});
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
crate::bit_width::MANUAL_BIT_WIDTH_INFO,
crate::bit_width::MISMATCHED_BIT_WIDTH_TYPE_INFO,
crate::block_scrutinee::BLOCK_SCRUTINEE_INFO,
crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO,
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
crate::bool_comparison::BOOL_COMPARISON_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ mod async_yields_async;
mod attrs;
mod await_holding_invalid;
mod bit_width;
mod block_scrutinee;
mod blocks_in_conditions;
mod bool_assert_comparison;
mod bool_comparison;
Expand Down Expand Up @@ -866,6 +867,7 @@ rustc_lint::late_lint_methods!(
RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee,
// add late passes here, used by `cargo dev new_lint`
]]
);
60 changes: 60 additions & 0 deletions tests/ui/block_scrutinee.1.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//@ edition: 2021
#![warn(clippy::block_scrutinee)]
#![allow(clippy::blocks_in_conditions)]
#![allow(clippy::let_and_return)]

fn my_function() -> Option<i32> {
Some(1)
}

fn main() {
if let Some(x) = my_function() {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

match my_function() {
//~^ ERROR: this scrutinee is wrapped in a block
Some(1) => println!("one"),
Some(_) => println!("other"),
None => println!("none"),
}

let mut v = vec![1, 2, 3];
while let Some(x) = v.pop() {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

if let Some(x) = my_function() {
let _ = x;
}

if let Some(x) = {
let _y = 2;
my_function()
} {
let _ = x;
}

//~v ERROR: this scrutinee is wrapped in a block
if let Some(x) = v.pop() {
let _ = x;
}

macro_rules! get_val {
() => {{ my_function() }};
}
if let Some(x) = get_val!() {
let _ = x;
}

// Test that `unsafe` blocks are ignored
unsafe fn my_unsafe_fn() -> Option<i32> {
Some(1)
}

if let Some(x) = unsafe { my_unsafe_fn() } {
let _ = x;
}
}
64 changes: 64 additions & 0 deletions tests/ui/block_scrutinee.2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//@ edition: 2021
#![warn(clippy::block_scrutinee)]
#![allow(clippy::blocks_in_conditions)]
#![allow(clippy::let_and_return)]

fn my_function() -> Option<i32> {
Some(1)
}

fn main() {
let res = my_function();
if let Some(x) = res {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

let res = my_function();
match res {
//~^ ERROR: this scrutinee is wrapped in a block
Some(1) => println!("one"),
Some(_) => println!("other"),
None => println!("none"),
}

let mut v = vec![1, 2, 3];
let res = v.pop();
while let Some(x) = res {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

if let Some(x) = my_function() {
let _ = x;
}

if let Some(x) = {
let _y = 2;
my_function()
} {
let _ = x;
}

//~v ERROR: this scrutinee is wrapped in a block
let res = v.pop();
if let Some(x) = res {
let _ = x;
}

macro_rules! get_val {
() => {{ my_function() }};
}
if let Some(x) = get_val!() {
let _ = x;
}

// Test that `unsafe` blocks are ignored
unsafe fn my_unsafe_fn() -> Option<i32> {
Some(1)
}

if let Some(x) = unsafe { my_unsafe_fn() } {
let _ = x;
}
}
63 changes: 63 additions & 0 deletions tests/ui/block_scrutinee.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//@ edition: 2021
#![warn(clippy::block_scrutinee)]
#![allow(clippy::blocks_in_conditions)]
#![allow(clippy::let_and_return)]

fn my_function() -> Option<i32> {
Some(1)
}

fn main() {
if let Some(x) = { my_function() } {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

match { my_function() } {
//~^ ERROR: this scrutinee is wrapped in a block
Some(1) => println!("one"),
Some(_) => println!("other"),
None => println!("none"),
}

let mut v = vec![1, 2, 3];
while let Some(x) = { v.pop() } {
//~^ ERROR: this scrutinee is wrapped in a block
let _ = x;
}

if let Some(x) = my_function() {
let _ = x;
}

if let Some(x) = {
let _y = 2;
my_function()
} {
let _ = x;
}

//~v ERROR: this scrutinee is wrapped in a block
if let Some(x) = {
// We are popping a value
v.pop()
} {
let _ = x;
}

macro_rules! get_val {
() => {{ my_function() }};
}
if let Some(x) = get_val!() {
let _ = x;
}

// Test that `unsafe` blocks are ignored
unsafe fn my_unsafe_fn() -> Option<i32> {
Some(1)
}

if let Some(x) = unsafe { my_unsafe_fn() } {
let _ = x;
}
}
Comment thread
llogiq marked this conversation as resolved.
Loading