about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-01-29 19:02:18 +0100
committerSamuel Tardieu <sam@rfc1149.net>2025-01-29 23:01:08 +0100
commit78d6b2ea4ea69308d47e96ec1af3c54d51172c70 (patch)
tree231f6e190b41f990b9a7d8857272af0bf698b457
parente02c8857e83e9113c29e8bd2b429f62dfaba04a7 (diff)
downloadrust-78d6b2ea4ea69308d47e96ec1af3c54d51172c70.tar.gz
rust-78d6b2ea4ea69308d47e96ec1af3c54d51172c70.zip
Do not remove semicolon if it changes the block type
Removing the semicolon of the last statement of an expressionless block
may change the block type even if the statement's type is `()`. If the
block type is `!` because of a systematic early return, typing it as
`()` may make it incompatible with the expected type for the block (to
which `!` is cast).
-rw-r--r--clippy_lints/src/unnecessary_semicolon.rs38
-rw-r--r--tests/ui/unnecessary_semicolon.edition2021.fixed6
-rw-r--r--tests/ui/unnecessary_semicolon.edition2024.fixed6
-rw-r--r--tests/ui/unnecessary_semicolon.rs6
4 files changed, 43 insertions, 13 deletions
diff --git a/clippy_lints/src/unnecessary_semicolon.rs b/clippy_lints/src/unnecessary_semicolon.rs
index efbc536dcb4..e5267620c4f 100644
--- a/clippy_lints/src/unnecessary_semicolon.rs
+++ b/clippy_lints/src/unnecessary_semicolon.rs
@@ -37,7 +37,7 @@ declare_clippy_lint! {
 
 #[derive(Default)]
 pub struct UnnecessarySemicolon {
-    last_statements: Vec<HirId>,
+    last_statements: Vec<(HirId, bool)>,
 }
 
 impl_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);
@@ -45,27 +45,25 @@ 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()
+        // The last statement of an expressionless block deserves a special treatment.
+        if block.expr.is_none()
             && let Some(last_stmt) = block.stmts.last()
         {
             if enter {
-                self.last_statements.push(last_stmt.hir_id);
+                let block_ty = cx.typeck_results().node_type(block.hir_id);
+                self.last_statements.push((last_stmt.hir_id, block_ty.is_unit()));
             } 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 {
+    /// Checks if `stmt` is the last statement in an expressionless block. In this case,
+    /// return `Some` with a boolean which is `true` if the block type is `()`.
+    fn is_last_in_block(&self, stmt: &Stmt<'_>) -> Option<bool> {
         self.last_statements
             .last()
-            .is_some_and(|last_stmt_id| last_stmt_id == &stmt.hir_id)
+            .and_then(|&(stmt_id, is_unit)| (stmt_id == stmt.hir_id).then_some(is_unit))
     }
 }
 
@@ -90,8 +88,22 @@ impl<'tcx> LateLintPass<'tcx> 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;
+            if let Some(block_is_unit) = self.is_last_in_block(stmt) {
+                if cx.tcx.sess.edition() <= Edition2021 && leaks_droppable_temporary_with_limited_lifetime(cx, expr) {
+                    // The expression contains temporaries with limited lifetimes in edition lower than 2024. Those may
+                    // survive until after the end of the current scope instead of until the end of the statement, so do
+                    // not lint this situation.
+                    return;
+                }
+
+                if !block_is_unit {
+                    // Although the expression returns `()`, the block doesn't. This may happen if the expression
+                    // returns early in all code paths, such as a `return value` in the condition of an `if` statement,
+                    // in which case the block type would be `!`. Do not lint in this case, as the statement would
+                    // become the block expression; the block type would become `()` and this may not type correctly
+                    // if the expected type for the block is not `()`.
+                    return;
+                }
             }
 
             let semi_span = expr.span.shrink_to_hi().to(stmt.span.shrink_to_hi());
diff --git a/tests/ui/unnecessary_semicolon.edition2021.fixed b/tests/ui/unnecessary_semicolon.edition2021.fixed
index 7a3b79553de..343c88b9815 100644
--- a/tests/ui/unnecessary_semicolon.edition2021.fixed
+++ b/tests/ui/unnecessary_semicolon.edition2021.fixed
@@ -55,3 +55,9 @@ fn no_borrow_issue(a: u32, b: u32) {
         None => {},
     }
 }
+
+fn issue14100() -> bool {
+    // Removing the `;` would make the block type be `()` instead of `!`, and this could no longer be
+    // cast into the `bool` function return type.
+    if return true {};
+}
diff --git a/tests/ui/unnecessary_semicolon.edition2024.fixed b/tests/ui/unnecessary_semicolon.edition2024.fixed
index d186d5e7ebc..1cba5760eb0 100644
--- a/tests/ui/unnecessary_semicolon.edition2024.fixed
+++ b/tests/ui/unnecessary_semicolon.edition2024.fixed
@@ -55,3 +55,9 @@ fn no_borrow_issue(a: u32, b: u32) {
         None => {},
     }
 }
+
+fn issue14100() -> bool {
+    // Removing the `;` would make the block type be `()` instead of `!`, and this could no longer be
+    // cast into the `bool` function return type.
+    if return true {};
+}
diff --git a/tests/ui/unnecessary_semicolon.rs b/tests/ui/unnecessary_semicolon.rs
index 3028c5b27b3..6abbbd79aaf 100644
--- a/tests/ui/unnecessary_semicolon.rs
+++ b/tests/ui/unnecessary_semicolon.rs
@@ -55,3 +55,9 @@ fn no_borrow_issue(a: u32, b: u32) {
         None => {},
     };
 }
+
+fn issue14100() -> bool {
+    // Removing the `;` would make the block type be `()` instead of `!`, and this could no longer be
+    // cast into the `bool` function return type.
+    if return true {};
+}