From bb60b2e659c378aa27165636a7e77c9f1dbd941f Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Thu, 23 Oct 2025 10:35:46 -0400 Subject: [PATCH 1/2] Add semicolons on the last statement of loop bodies --- src/comment.rs | 2 +- src/config/file_lines.rs | 2 +- src/modules.rs | 2 +- src/shape.rs | 4 ++-- src/stmt.rs | 2 +- tests/source/macro_rules.rs | 2 +- tests/target/macro_rules.rs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index 709031dda44..241934a7d3d 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -561,7 +561,7 @@ fn itemized_block_quote_start(line: &str, mut line_start: String, remove_indent: } for _ in 0..quote_level { - line_start.push_str("> ") + line_start.push_str("> "); } line_start } diff --git a/src/config/file_lines.rs b/src/config/file_lines.rs index 2f2a6c8d552..91f822db0fc 100644 --- a/src/config/file_lines.rs +++ b/src/config/file_lines.rs @@ -188,7 +188,7 @@ fn normalize_ranges(ranges: &mut HashMap>) { break; } } - result.push(next) + result.push(next); } *ranges = result; } diff --git a/src/modules.rs b/src/modules.rs index 4270f692910..a8b686855a9 100644 --- a/src/modules.rs +++ b/src/modules.rs @@ -569,7 +569,7 @@ impl<'ast, 'psess, 'c> ModResolver<'ast, 'psess> { Cow::Owned(items), Cow::Owned(attrs), ), - )) + )); } result } diff --git a/src/shape.rs b/src/shape.rs index c1c040f1f63..657e03b3237 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -91,10 +91,10 @@ impl Indent { indent.push('\n'); } for _ in 0..num_tabs { - indent.push('\t') + indent.push('\t'); } for _ in 0..num_spaces { - indent.push(' ') + indent.push(' '); } Cow::from(indent) } diff --git a/src/stmt.rs b/src/stmt.rs index 76194421c84..8839b61b353 100644 --- a/src/stmt.rs +++ b/src/stmt.rs @@ -62,7 +62,7 @@ impl<'a> Stmt<'a> { result.push(Stmt { inner: iter.next().unwrap(), is_last: iter.peek().is_none(), - }) + }); } result } diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index 5aaca0c83fa..766c135870c 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -184,7 +184,7 @@ macro_rules! binary { op, rhs, }, - } + }; } }; } diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index 97444aef404..d78c832247a 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -216,7 +216,7 @@ macro_rules! binary { op, rhs, }, - } + }; } }; } From b84bb8b22da58ed0c31fd2cdbf0895b5bcf7ac39 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Thu, 23 Oct 2025 08:37:28 -0400 Subject: [PATCH 2/2] Enforce that the last statement in a loop body has a semicolon --- src/expr.rs | 8 ++++++ src/rewrite.rs | 7 +++++ src/visitor.rs | 54 +++++++++++++++++++++++++++++++----- tests/source/loops-bodies.rs | 20 +++++++++++++ tests/target/loops-bodies.rs | 20 +++++++++++++ 5 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 tests/source/loops-bodies.rs create mode 100644 tests/target/loops-bodies.rs diff --git a/src/expr.rs b/src/expr.rs index 348cffc21ee..f043eb0449c 100644 --- a/src/expr.rs +++ b/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? }; diff --git a/src/rewrite.rs b/src/rewrite.rs index 7ca1ca522ff..dbe1cb44d6c 100644 --- a/src/rewrite.rs +++ b/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, + // 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, // When rewriting chain, veto going multi line except the last element pub(crate) force_one_line_chain: Cell, 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() + } } diff --git a/src/visitor.rs b/src/visitor.rs index d7b1178a4e0..79a231bfe11 100644 --- a/src/visitor.rs +++ b/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() + } + } + } } diff --git a/tests/source/loops-bodies.rs b/tests/source/loops-bodies.rs new file mode 100644 index 00000000000..f9115529bfe --- /dev/null +++ b/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!() + } +} diff --git a/tests/target/loops-bodies.rs b/tests/target/loops-bodies.rs new file mode 100644 index 00000000000..bf607303aee --- /dev/null +++ b/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!(); + } +}