Skip to content
Open
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
2 changes: 1 addition & 1 deletion src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/config/file_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn normalize_ranges(ranges: &mut HashMap<FileName, Vec<Range>>) {
break;
}
}
result.push(next)
result.push(next);
}
*ranges = result;
}
Expand Down
8 changes: 8 additions & 0 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -784,6 +786,7 @@ impl<'a> ControlFlow<'a> {
connector: " =",
allow_single_line,
nested_if,
is_loop: false,
span,
}
}
Expand All @@ -800,6 +803,7 @@ impl<'a> ControlFlow<'a> {
connector: "",
allow_single_line: false,
nested_if: false,
is_loop: true,
span,
}
}
Expand All @@ -823,6 +827,7 @@ impl<'a> ControlFlow<'a> {
connector: " =",
allow_single_line: false,
nested_if: false,
is_loop: true,
span,
}
}
Expand All @@ -849,6 +854,7 @@ impl<'a> ControlFlow<'a> {
connector: " in",
allow_single_line: false,
nested_if: false,
is_loop: true,
span,
}
}
Expand Down Expand Up @@ -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?
};
Expand Down
2 changes: 1 addition & 1 deletion src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ impl<'ast, 'psess, 'c> ModResolver<'ast, 'psess> {
Cow::Owned(items),
Cow::Owned(attrs),
),
))
));
}
result
}
Expand Down
7 changes: 7 additions & 0 deletions src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
}
4 changes: 2 additions & 2 deletions src/shape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl<'a> Stmt<'a> {
result.push(Stmt {
inner: iter.next().unwrap(),
is_last: iter.peek().is_none(),
})
});
}
result
}
Expand Down
54 changes: 47 additions & 7 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(";");
}
}

Expand Down Expand Up @@ -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![])),
Expand Down Expand Up @@ -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),
Expand All @@ -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,
Comment on lines +1047 to +1055
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about macros that use curly braces?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative here might be to use span_to_snippet and check if the returned string ends in }.

Copy link
Member Author

@shepmaster shepmaster Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about macros that use curly braces?

This appears to work with the current code as I'd want it to, but I couldn't tell you why...

Before

    loop {
        dummy! {}
    }
    loop {
        dummy!()
    }
    loop {
        dummy![]
    }

After

    loop {
        dummy! {}
    }
    loop {
        dummy!();
    }
    loop {
        dummy![];
    }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to work as I'd expect, but I couldn't tell you why...

Probably maybe that the block macro is a statement..?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be to use span_to_snippet

I changed it to this

            _ => {
                let is_curly = self
                    .opt_snippet(expr.span())
                    .is_some_and(|s| s.ends_with("}"));

                if is_curly {
                    return false;
                }

                // Checking the edition as before 2024 the lack of a

And the tests still passed, so I think it's up to you to decide which is a more reasonable path.


_ => {
// 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()
}
}
}
}
20 changes: 20 additions & 0 deletions tests/source/loops-bodies.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add two more test cases:

  • One for rustfmt-edition: 2024 and rustfmt-style_edition: 2024
  • One for rustfmt-edition: 2021 and rustfmt-style_edition: 2027

In both case I wouldn't expect us to add a ;

Original file line number Diff line number Diff line change
@@ -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!()
}
}
2 changes: 1 addition & 1 deletion tests/source/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ macro_rules! binary {
op,
rhs,
},
}
};
Copy link
Contributor

@ytmimi ytmimi Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this a necessary change? If not I'd prefer we revert it since we try not to change existing test cases.

}
};
}
Expand Down
20 changes: 20 additions & 0 deletions tests/target/loops-bodies.rs
Original file line number Diff line number Diff line change
@@ -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!();
}
}
2 changes: 1 addition & 1 deletion tests/target/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ macro_rules! binary {
op,
rhs,
},
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this change?

}
};
}
Expand Down
Loading