unnecessary_semicolon: do not lint if it may cause borrow errors (#14049)

Before edition 2024, some temporaries used in scrutinees of a `match`
used as the last expression of a block may outlive some referenced local
variables. Prevent those cases from happening by checking that alive
temporaries with significant drop do have a static lifetime.

The check is performed only for edition 2021 and earlier, and for the
last statement if it would become the last expression of the block.

changelog: [`unnecessary_semicolon`]: prevent borrow errors in editions
lower than 2024

r? @y21

Author: y21

clippy_lints/src/lib.rs

@@ -973,6 +973,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
     store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
     store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
-    store.register_late_pass(|_| Box::new(unnecessary_semicolon::UnnecessarySemicolon));
+    store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default());
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/returns.rs

@@ -1,10 +1,11 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::source::{SpanRangeExt, snippet_with_context};
 use clippy_utils::sugg::has_enclosing_paren;
-use clippy_utils::visitors::{Descend, for_each_expr, for_each_unconsumed_temporary};
+use clippy_utils::visitors::{Descend, for_each_expr};
 use clippy_utils::{
-    binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, path_res,
-    path_to_local_id, span_contains_cfg, span_find_starting_semi,
+    binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor,
+    leaks_droppable_temporary_with_limited_lifetime, path_res, path_to_local_id, span_contains_cfg,
+    span_find_starting_semi,
 };
 use core::ops::ControlFlow;
 use rustc_ast::MetaItemInner;
@@ -389,22 +390,8 @@ fn check_final_expr<'tcx>(
                 }
             };
 
-            if let Some(inner) = inner {
-                if for_each_unconsumed_temporary(cx, inner, |temporary_ty| {
-                    if temporary_ty.has_significant_drop(cx.tcx, cx.typing_env())
-                        && temporary_ty
-                            .walk()
-                            .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(re) if !re.is_static()))
-                    {
-                        ControlFlow::Break(())
-                    } else {
-                        ControlFlow::Continue(())
-                    }
-                })
-                .is_break()
-                {
-                    return;
-                }
+            if inner.is_some_and(|inner| leaks_droppable_temporary_with_limited_lifetime(cx, inner)) {
+                return;
             }
 
             if ret_span.from_expansion() || is_from_proc_macro(cx, expr) {

clippy_lints/src/unnecessary_semicolon.rs

@@ -1,8 +1,10 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::leaks_droppable_temporary_with_limited_lifetime;
 use rustc_errors::Applicability;
-use rustc_hir::{ExprKind, MatchSource, Stmt, StmtKind};
+use rustc_hir::{Block, ExprKind, HirId, MatchSource, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::declare_lint_pass;
+use rustc_session::impl_lint_pass;
+use rustc_span::edition::Edition::Edition2021;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -33,10 +35,50 @@ declare_clippy_lint! {
     "unnecessary semicolon after expression returning `()`"
 }
 
-declare_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);
+#[derive(Default)]
+pub struct UnnecessarySemicolon {
+    last_statements: Vec<HirId>,
+}
+
+impl_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);
+
+impl UnnecessarySemicolon {
+    /// Enter or leave a block, remembering the last statement of the block.
+    fn handle_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>, enter: bool) {
+        // Up to edition 2021, removing the semicolon of the last statement of a block
+        // may result in the scrutinee temporary values to live longer than the block
+        // variables. To avoid this problem, we do not lint the last statement of an
+        // expressionless block.
+        if cx.tcx.sess.edition() <= Edition2021
+            && block.expr.is_none()
+            && let Some(last_stmt) = block.stmts.last()
+        {
+            if enter {
+                self.last_statements.push(last_stmt.hir_id);
+            } else {
+                self.last_statements.pop();
+            }
+        }
+    }
+
+    /// Checks if `stmt` is the last statement in an expressionless block for edition ≤ 2021.
+    fn is_last_in_block(&self, stmt: &Stmt<'_>) -> bool {
+        self.last_statements
+            .last()
+            .is_some_and(|last_stmt_id| last_stmt_id == &stmt.hir_id)
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessarySemicolon {
+    fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
+        self.handle_block(cx, block, true);
+    }
 
-impl LateLintPass<'_> for UnnecessarySemicolon {
-    fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
+    fn check_block_post(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
+        self.handle_block(cx, block, false);
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) {
         // rustfmt already takes care of removing semicolons at the end
         // of loops.
         if let StmtKind::Semi(expr) = stmt.kind
@@ -48,6 +90,10 @@ impl LateLintPass<'_> for UnnecessarySemicolon {
             )
             && cx.typeck_results().expr_ty(expr) == cx.tcx.types.unit
         {
+            if self.is_last_in_block(stmt) && leaks_droppable_temporary_with_limited_lifetime(cx, expr) {
+                return;
+            }
+
             let semi_span = expr.span.shrink_to_hi().to(stmt.span.shrink_to_hi());
             span_lint_and_sugg(
                 cx,

clippy_utils/src/lib.rs

@@ -117,15 +117,15 @@ use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{
-    self as rustc_ty, Binder, BorrowKind, ClosureKind, EarlyBinder, FloatTy, GenericArgsRef, IntTy, Ty, TyCtxt,
-    TypeVisitableExt, UintTy, UpvarCapture,
+    self as rustc_ty, Binder, BorrowKind, ClosureKind, EarlyBinder, FloatTy, GenericArgKind, GenericArgsRef, IntTy, Ty,
+    TyCtxt, TypeVisitableExt, UintTy, UpvarCapture,
 };
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::{Ident, Symbol, kw};
 use rustc_span::{InnerSpan, Span, sym};
 use rustc_target::abi::Integer;
-use visitors::Visitable;
+use visitors::{Visitable, for_each_unconsumed_temporary};
 
 use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
 use crate::higher::Range;
@@ -3465,3 +3465,20 @@ pub fn is_receiver_of_method_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool
     }
     false
 }
+
+/// Returns true if `expr` creates any temporary whose type references a non-static lifetime and has
+/// a significant drop and does not consume it.
+pub fn leaks_droppable_temporary_with_limited_lifetime<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+    for_each_unconsumed_temporary(cx, expr, |temporary_ty| {
+        if temporary_ty.has_significant_drop(cx.tcx, cx.typing_env())
+            && temporary_ty
+                .walk()
+                .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(re) if !re.is_static()))
+        {
+            ControlFlow::Break(())
+        } else {
+            ControlFlow::Continue(())
+        }
+    })
+    .is_break()
+}

tests/ui/unnecessary_semicolon.edition2021.fixed

@@ -0,0 +1,57 @@
+//@revisions: edition2021 edition2024
+//@[edition2021] edition:2021
+//@[edition2024] edition:2024
+
+#![warn(clippy::unnecessary_semicolon)]
+#![feature(postfix_match)]
+#![allow(clippy::single_match)]
+
+fn no_lint(mut x: u32) -> Option<u32> {
+    Some(())?;
+
+    {
+        let y = 3;
+        dbg!(x + y)
+    };
+
+    {
+        let (mut a, mut b) = (10, 20);
+        (a, b) = (b + 1, a + 1);
+    }
+
+    Some(0)
+}
+
+fn main() {
+    let mut a = 3;
+    if a == 2 {
+        println!("This is weird");
+    }
+    //~^ ERROR: unnecessary semicolon
+
+    a.match {
+        3 => println!("three"),
+        _ => println!("not three"),
+    }
+    //~^ ERROR: unnecessary semicolon
+}
+
+// This is a problem in edition 2021 and below
+fn borrow_issue() {
+    let v = std::cell::RefCell::new(Some(vec![1]));
+    match &*v.borrow() {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    };
+}
+
+fn no_borrow_issue(a: u32, b: u32) {
+    match Some(a + b) {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    }
+}

tests/ui/unnecessary_semicolon.edition2021.stderr

@@ -0,0 +1,23 @@
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:29:6
+   |
+LL |     };
+   |      ^ help: remove
+   |
+   = note: `-D clippy::unnecessary-semicolon` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_semicolon)]`
+
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:35:6
+   |
+LL |     };
+   |      ^ help: remove
+
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:56:6
+   |
+LL |     };
+   |      ^ help: remove
+
+error: aborting due to 3 previous errors
+

tests/ui/unnecessary_semicolon.edition2024.fixed

@@ -0,0 +1,57 @@
+//@revisions: edition2021 edition2024
+//@[edition2021] edition:2021
+//@[edition2024] edition:2024
+
+#![warn(clippy::unnecessary_semicolon)]
+#![feature(postfix_match)]
+#![allow(clippy::single_match)]
+
+fn no_lint(mut x: u32) -> Option<u32> {
+    Some(())?;
+
+    {
+        let y = 3;
+        dbg!(x + y)
+    };
+
+    {
+        let (mut a, mut b) = (10, 20);
+        (a, b) = (b + 1, a + 1);
+    }
+
+    Some(0)
+}
+
+fn main() {
+    let mut a = 3;
+    if a == 2 {
+        println!("This is weird");
+    }
+    //~^ ERROR: unnecessary semicolon
+
+    a.match {
+        3 => println!("three"),
+        _ => println!("not three"),
+    }
+    //~^ ERROR: unnecessary semicolon
+}
+
+// This is a problem in edition 2021 and below
+fn borrow_issue() {
+    let v = std::cell::RefCell::new(Some(vec![1]));
+    match &*v.borrow() {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    }
+}
+
+fn no_borrow_issue(a: u32, b: u32) {
+    match Some(a + b) {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    }
+}

tests/ui/unnecessary_semicolon.edition2024.stderr

@@ -0,0 +1,29 @@
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:29:6
+   |
+LL |     };
+   |      ^ help: remove
+   |
+   = note: `-D clippy::unnecessary-semicolon` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_semicolon)]`
+
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:35:6
+   |
+LL |     };
+   |      ^ help: remove
+
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:47:6
+   |
+LL |     };
+   |      ^ help: remove
+
+error: unnecessary semicolon
+  --> tests/ui/unnecessary_semicolon.rs:56:6
+   |
+LL |     };
+   |      ^ help: remove
+
+error: aborting due to 4 previous errors
+

tests/ui/unnecessary_semicolon.rs

@@ -1,5 +1,10 @@
+//@revisions: edition2021 edition2024
+//@[edition2021] edition:2021
+//@[edition2024] edition:2024
+
 #![warn(clippy::unnecessary_semicolon)]
 #![feature(postfix_match)]
+#![allow(clippy::single_match)]
 
 fn no_lint(mut x: u32) -> Option<u32> {
     Some(())?;
@@ -30,3 +35,23 @@ fn main() {
     };
     //~^ ERROR: unnecessary semicolon
 }
+
+// This is a problem in edition 2021 and below
+fn borrow_issue() {
+    let v = std::cell::RefCell::new(Some(vec![1]));
+    match &*v.borrow() {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    };
+}
+
+fn no_borrow_issue(a: u32, b: u32) {
+    match Some(a + b) {
+        Some(v) => {
+            dbg!(v);
+        },
+        None => {},
+    };
+}