Enforce that the last statement in a loop body has a semicolon

Author: shepmaster

src/expr.rs

@@ -596,6 +596,7 @@ pub(crate) fn rewrite_block_with_visitor(
     let mut visitor = FmtVisitor::from_context(context);
     visitor.block_indent = shape.indent;
     visitor.is_if_else_block = context.is_if_else_block();
+    visitor.is_loop_block = context.is_loop_block();
     match (block.rules, label) {
         (ast::BlockCheckMode::Unsafe(..), _) | (ast::BlockCheckMode::Default, Some(_)) => {
             let snippet = context.snippet(block.span);
@@ -713,6 +714,7 @@ struct ControlFlow<'a> {
     allow_single_line: bool,
     // HACK: `true` if this is an `if` expression in an `else if`.
     nested_if: bool,
+    is_loop: bool,
     span: Span,
 }
 
@@ -784,6 +786,7 @@ impl<'a> ControlFlow<'a> {
             connector: " =",
             allow_single_line,
             nested_if,
+            is_loop: false,
             span,
         }
     }
@@ -800,6 +803,7 @@ impl<'a> ControlFlow<'a> {
             connector: "",
             allow_single_line: false,
             nested_if: false,
+            is_loop: true,
             span,
         }
     }
@@ -823,6 +827,7 @@ impl<'a> ControlFlow<'a> {
             connector: " =",
             allow_single_line: false,
             nested_if: false,
+            is_loop: true,
             span,
         }
     }
@@ -849,6 +854,7 @@ impl<'a> ControlFlow<'a> {
             connector: " in",
             allow_single_line: false,
             nested_if: false,
+            is_loop: true,
             span,
         }
     }
@@ -1162,8 +1168,10 @@ impl<'a> Rewrite for ControlFlow<'a> {
         };
         let block_str = {
             let old_val = context.is_if_else_block.replace(self.else_block.is_some());
+            let old_is_loop = context.is_loop_block.replace(self.is_loop);
             let result =
                 rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true);
+            context.is_loop_block.replace(old_is_loop);
             context.is_if_else_block.replace(old_val);
             result?
         };

src/rewrite.rs

@@ -114,6 +114,9 @@ pub(crate) struct RewriteContext<'a> {
     // When `is_if_else_block` is true, unindent the comment on top
     // of the `else` or `else if`.
     pub(crate) is_if_else_block: Cell<bool>,
+    // When `is_loop_block` is true, we can more aggressively end the
+    // last statement of the block with a semicolon.
+    pub(crate) is_loop_block: Cell<bool>,
     // When rewriting chain, veto going multi line except the last element
     pub(crate) force_one_line_chain: Cell<bool>,
     pub(crate) snippet_provider: &'a SnippetProvider,
@@ -175,4 +178,8 @@ impl<'a> RewriteContext<'a> {
     pub(crate) fn is_if_else_block(&self) -> bool {
         self.is_if_else_block.get()
     }
+
+    pub(crate) fn is_loop_block(&self) -> bool {
+        self.is_loop_block.get()
+    }
 }

src/visitor.rs

@@ -6,7 +6,6 @@ use rustc_ast::{ast, token::Delimiter, visit};
 use rustc_span::{BytePos, Ident, Pos, Span, symbol};
 use tracing::debug;
 
-use crate::attr::*;
 use crate::comment::{CodeCharKind, CommentCodeSlices, contains_comment, rewrite_comment};
 use crate::config::{BraceStyle, Config, MacroSelector, StyleEdition};
 use crate::coverage::transform_missing_snippet;
@@ -25,8 +24,9 @@ use crate::spanned::Spanned;
 use crate::stmt::Stmt;
 use crate::utils::{
     self, contains_skip, count_newlines, depr_skip_annotation, format_safety, inner_attributes,
-    last_line_width, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, starts_with_newline, stmt_expr,
+    last_line_width, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, starts_with_newline,
 };
+use crate::{Edition, attr::*};
 use crate::{ErrorKind, FormatReport, FormattingError};
 
 /// Creates a string slice corresponding to the specified span.
@@ -78,6 +78,7 @@ pub(crate) struct FmtVisitor<'a> {
     pub(crate) block_indent: Indent,
     pub(crate) config: &'a Config,
     pub(crate) is_if_else_block: bool,
+    pub(crate) is_loop_block: bool,
     pub(crate) snippet_provider: &'a SnippetProvider,
     pub(crate) line_number: usize,
     /// List of 1-based line ranges which were annotated with skip
@@ -231,11 +232,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
         self.walk_block_stmts(b);
 
-        if !b.stmts.is_empty() {
-            if let Some(expr) = stmt_expr(&b.stmts[b.stmts.len() - 1]) {
-                if utils::semicolon_for_expr(&self.get_context(), expr) {
-                    self.push_str(";");
-                }
+        if let Some(stmt) = b.stmts.last() {
+            if self.add_semi_on_last_block_stmt(stmt) {
+                self.push_str(";");
             }
         }
 
@@ -802,6 +801,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             block_indent: Indent::empty(),
             config,
             is_if_else_block: false,
+            is_loop_block: false,
             snippet_provider,
             line_number: 0,
             skipped_range: Rc::new(RefCell::new(vec![])),
@@ -1019,6 +1019,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             inside_macro: Rc::new(Cell::new(false)),
             use_block: Cell::new(false),
             is_if_else_block: Cell::new(false),
+            is_loop_block: Cell::new(false),
             force_one_line_chain: Cell::new(false),
             snippet_provider: self.snippet_provider,
             macro_rewrite_failure: Cell::new(false),
@@ -1028,4 +1029,43 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             skipped_range: self.skipped_range.clone(),
         }
     }
+
+    fn add_semi_on_last_block_stmt(&self, stmt: &ast::Stmt) -> bool {
+        let ast::StmtKind::Expr(expr) = &stmt.kind else {
+            return false;
+        };
+
+        if self.is_macro_def {
+            return false;
+        }
+
+        match expr.kind {
+            ast::ExprKind::Ret(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Break(..) => {
+                self.config.trailing_semicolon()
+            }
+
+            // TODO[reviewer-help]: This is roughly "does it end in a
+            // curly". There might be a helper for this, or cases I'm
+            // missing.
+            ast::ExprKind::Loop(..)
+            | ast::ExprKind::While(..)
+            | ast::ExprKind::ForLoop { .. }
+            | ast::ExprKind::Let(..)
+            | ast::ExprKind::If(..)
+            | ast::ExprKind::Match(..) => false,
+
+            _ => {
+                // Checking the edition as before 2024 the lack of a
+                // semicolon could impact temporary lifetimes[1].
+                //
+                // 1: https://rust-lang.github.io/rfcs/
+                //      3606-temporary-lifetimes-in-tail-expressions.html
+                let allowed_to_add_semi = self.is_loop_block
+                    && self.config.edition() >= Edition::Edition2024
+                    && self.config.style_edition() >= StyleEdition::Edition2027;
+
+                allowed_to_add_semi && self.config.trailing_semicolon()
+            }
+        }
+    }
 }

tests/source/loops-bodies.rs

@@ -0,0 +1,20 @@
+// rustfmt-edition: 2024
+// rustfmt-style_edition: 2027
+
+fn main() {
+    for x in [1] {
+        println!()
+    }
+
+    while false {
+        println!()
+    }
+
+    while let Some('x') = None {
+        println!()
+    }
+
+    loop {
+        println!()
+    }
+}

tests/target/loops-bodies.rs

@@ -0,0 +1,20 @@
+// rustfmt-edition: 2024
+// rustfmt-style_edition: 2027
+
+fn main() {
+    for x in [1] {
+        println!();
+    }
+
+    while false {
+        println!();
+    }
+
+    while let Some('x') = None {
+        println!();
+    }
+
+    loop {
+        println!();
+    }
+}