Implement trailing_semicolon = "Preserve"

Configurations.md

@@ -2780,20 +2780,32 @@ See also: [`match_block_trailing_comma`](#match_block_trailing_comma).
 
 Add trailing semicolon after break, continue and return
 
-- **Default value**: `true`
-- **Possible values**: `true`, `false`
+- **Default value**: `preserve`
+- **Possible values**: `always`, `never`, `preserve`
 - **Stable**: No (tracking issue: [#3378](https://github.com/rust-lang/rustfmt/issues/3378))
 
-#### `true` (default):
+#### `always` (default):
 ```rust
 fn foo() -> usize {
     return 0;
 }
 ```
 
-#### `false`:
+#### `never`:
+```rust
+fn foo() -> usize {
+    return 0
+}
+```
+
+
+#### `preserve`:
 ```rust
 fn foo() -> usize {
+    return 0;
+}
+
+fn bar() -> usize {
     return 0
 }
 ```

src/config/mod.rs

@@ -135,7 +135,7 @@ create_config! {
     brace_style: BraceStyle, BraceStyle::SameLineWhere, false, "Brace style for items";
     control_brace_style: ControlBraceStyle, ControlBraceStyle::AlwaysSameLine, false,
         "Brace style for control flow constructs";
-    trailing_semicolon: bool, true, false,
+    trailing_semicolon: TrailingSemicolon, TrailingSemicolon::Preserve, false,
         "Add trailing semicolon after break, continue and return";
     trailing_comma: SeparatorTactic, SeparatorTactic::Vertical, false,
         "How to handle trailing commas for lists";
@@ -673,7 +673,7 @@ force_multiline_blocks = false
 fn_params_layout = "Tall"
 brace_style = "SameLineWhere"
 control_brace_style = "AlwaysSameLine"
-trailing_semicolon = true
+trailing_semicolon = "Preserve"
 trailing_comma = "Vertical"
 match_block_trailing_comma = false
 blank_lines_upper_bound = 1

src/config/options.rs

@@ -49,6 +49,13 @@ pub enum ControlBraceStyle {
     AlwaysNextLine,
 }
 
+#[config_type]
+pub enum TrailingSemicolon {
+    Always,
+    Never,
+    Preserve,
+}
+
 #[config_type]
 /// How to indent.
 pub enum IndentStyle {

src/expr.rs

@@ -32,7 +32,7 @@ use crate::types::{rewrite_path, PathContext};
 use crate::utils::{
     colon_spaces, contains_skip, count_newlines, filtered_str_fits, first_line_ends_with,
     inner_attributes, last_line_extendable, last_line_width, mk_sp, outer_attributes,
-    semicolon_for_expr, unicode_str_width, wrap_str,
+    semicolon_for_expr_extra_hacky_double_counted_spacing, unicode_str_width, wrap_str,
 };
 use crate::vertical::rewrite_with_alignment;
 use crate::visitor::FmtVisitor;
@@ -64,7 +64,9 @@ pub(crate) fn format_expr(
     if contains_skip(&*expr.attrs) {
         return Some(context.snippet(expr.span()).to_owned());
     }
-    let shape = if expr_type == ExprType::Statement && semicolon_for_expr(context, expr) {
+    let shape = if expr_type == ExprType::Statement
+        && semicolon_for_expr_extra_hacky_double_counted_spacing(context, expr)
+    {
         shape.sub_width(1)?
     } else {
         shape

src/stmt.rs

@@ -8,7 +8,7 @@ use crate::rewrite::{Rewrite, RewriteContext};
 use crate::shape::Shape;
 use crate::source_map::LineRangeUtils;
 use crate::spanned::Spanned;
-use crate::utils::semicolon_for_stmt;
+use crate::utils::{semicolon_for_expr, semicolon_for_stmt};
 
 pub(crate) struct Stmt<'a> {
     inner: &'a ast::Stmt,
@@ -116,7 +116,7 @@ fn format_stmt(
 
     let result = match stmt.kind {
         ast::StmtKind::Local(ref local) => local.rewrite(context, shape),
-        ast::StmtKind::Expr(ref ex) | ast::StmtKind::Semi(ref ex) => {
+        ast::StmtKind::Semi(ref ex) => {
             let suffix = if semicolon_for_stmt(context, stmt, is_last_expr) {
                 ";"
             } else {
@@ -126,6 +126,16 @@ fn format_stmt(
             let shape = shape.sub_width(suffix.len())?;
             format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
         }
+        ast::StmtKind::Expr(ref ex) => {
+            let suffix = if semicolon_for_expr(context, ex) {
+                ";"
+            } else {
+                ""
+            };
+
+            let shape = shape.sub_width(suffix.len())?;
+            format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
+        }
         ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => None,
     };
     result.and_then(|res| recover_comment_removed(res, stmt.span(), context))

src/utils.rs

@@ -10,7 +10,7 @@ use rustc_span::{sym, symbol, BytePos, LocalExpnId, Span, Symbol, SyntaxContext}
 use unicode_width::UnicodeWidthStr;
 
 use crate::comment::{filter_normal_code, CharClasses, FullCodeCharKind, LineClasses};
-use crate::config::{Config, Version};
+use crate::config::{Config, TrailingSemicolon, Version};
 use crate::rewrite::RewriteContext;
 use crate::shape::{Indent, Shape};
 
@@ -273,15 +273,43 @@ pub(crate) fn contains_skip(attrs: &[Attribute]) -> bool {
 #[inline]
 pub(crate) fn semicolon_for_expr(context: &RewriteContext<'_>, expr: &ast::Expr) -> bool {
     // Never try to insert semicolons on expressions when we're inside
-    // a macro definition - this can prevent the macro from compiling
+    // a macro definition - this  can prevent the macro from compiling
     // when used in expression position
     if context.is_macro_def {
         return false;
     }
 
     match expr.kind {
         ast::ExprKind::Ret(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Break(..) => {
-            context.config.trailing_semicolon()
+            match context.config.trailing_semicolon() {
+                TrailingSemicolon::Always => true,
+                TrailingSemicolon::Never | TrailingSemicolon::Preserve => false,
+            }
+        }
+        _ => false,
+    }
+}
+
+/// Previously, we used to have `trailing_semicolon = always` enabled, and due to
+/// a bug between `format_stmt` and `format_expr`, we used to subtract the size of
+/// `;` *TWICE* from the shape. This means that an expr that would fit onto a line
+/// of, e.g. 99 (limit 100) after subtracting one for the semicolon would still be
+/// wrapped.
+///
+/// This function reimplements the old heuristic of double counting the "phantom"
+/// semicolon that should have already been accounted for, to not break existing
+/// formatting with the `trailing_semicolon = preserve` behavior.
+#[inline]
+pub(crate) fn semicolon_for_expr_extra_hacky_double_counted_spacing(
+    context: &RewriteContext<'_>,
+    expr: &ast::Expr,
+) -> bool {
+    match expr.kind {
+        ast::ExprKind::Ret(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Break(..) => {
+            match context.config.trailing_semicolon() {
+                TrailingSemicolon::Always | TrailingSemicolon::Preserve => true,
+                TrailingSemicolon::Never => false,
+            }
         }
         _ => false,
     }
@@ -301,7 +329,14 @@ pub(crate) fn semicolon_for_stmt(
             ast::ExprKind::Break(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Ret(..) => {
                 // The only time we can skip the semi-colon is if the config option is set to false
                 // **and** this is the last expr (even though any following exprs are unreachable)
-                context.config.trailing_semicolon() || !is_last_expr
+                if !is_last_expr {
+                    true
+                } else {
+                    match context.config.trailing_semicolon() {
+                        TrailingSemicolon::Always | TrailingSemicolon::Preserve => true,
+                        TrailingSemicolon::Never => false,
+                    }
+                }
             }
             _ => true,
         },

tests/target/configs/single_line_let_else_max_width/100.rs

@@ -9,12 +9,12 @@ fn main() {
 
     let Some(c) = opt else {
         // a comment should always force the block to be multi-lined
-        return;
+        return
     };
 
     let Some(c) = opt else {
         /* a comment should always force the block to be multi-lined */
-        return;
+        return
     };
 
     let Some(d) = some_very_very_very_very_long_name else { return };
@@ -26,23 +26,23 @@ fn main() {
         range: _,
     }) = slice.as_ref()
     else {
-        return;
+        return
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true)? else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Some(doc_attr) = variant
         .attrs
         .iter()
         .find(|attr| attr.path().is_ident("doc"))
     else {
-        return Err(Error::new(variant.span(), r#"expected a doc comment"#));
+        return Err(Error::new(variant.span(), r#"expected a doc comment"#))
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true) else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Stmt::Expr(
@@ -55,6 +55,6 @@ fn main() {
         return Err(Error::new(
             last_stmt.span(),
             "expected last expression to be `Some(match (..) { .. })`",
-        ));
+        ))
     };
 }

tests/target/configs/single_line_let_else_max_width/50.rs

@@ -9,16 +9,16 @@ fn main() {
 
     let Some(c) = opt else {
         // a comment should always force the block to be multi-lined
-        return;
+        return
     };
 
     let Some(c) = opt else {
         /* a comment should always force the block to be multi-lined */
-        return;
+        return
     };
 
     let Some(d) = some_very_very_very_very_long_name else {
-        return;
+        return
     };
 
     let Expr::Slice(ast::ExprSlice {
@@ -28,23 +28,23 @@ fn main() {
         range: _,
     }) = slice.as_ref()
     else {
-        return;
+        return
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true)? else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Some(doc_attr) = variant
         .attrs
         .iter()
         .find(|attr| attr.path().is_ident("doc"))
     else {
-        return Err(Error::new(variant.span(), r#"expected a doc comment"#));
+        return Err(Error::new(variant.span(), r#"expected a doc comment"#))
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true) else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Stmt::Expr(
@@ -57,6 +57,6 @@ fn main() {
         return Err(Error::new(
             last_stmt.span(),
             "expected last expression to be `Some(match (..) { .. })`",
-        ));
+        ))
     };
 }

tests/target/configs/single_line_let_else_max_width/zero.rs

@@ -4,25 +4,25 @@ fn main() {
     let Some(a) = opt else {};
 
     let Some(b) = opt else {
-        return;
+        return
     };
 
     let Some(c) = opt else {
-        return;
+        return
     };
 
     let Some(c) = opt else {
         // a comment should always force the block to be multi-lined
-        return;
+        return
     };
 
     let Some(c) = opt else {
         /* a comment should always force the block to be multi-lined */
-        return;
+        return
     };
 
     let Some(d) = some_very_very_very_very_long_name else {
-        return;
+        return
     };
 
     let Expr::Slice(ast::ExprSlice {
@@ -32,23 +32,23 @@ fn main() {
         range: _,
     }) = slice.as_ref()
     else {
-        return;
+        return
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true)? else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Some(doc_attr) = variant
         .attrs
         .iter()
         .find(|attr| attr.path().is_ident("doc"))
     else {
-        return Err(Error::new(variant.span(), r#"expected a doc comment"#));
+        return Err(Error::new(variant.span(), r#"expected a doc comment"#))
     };
 
     let Some((base_place, current)) = self.lower_expr_as_place(current, *base, true) else {
-        return Ok(None);
+        return Ok(None)
     };
 
     let Stmt::Expr(
@@ -61,6 +61,6 @@ fn main() {
         return Err(Error::new(
             last_stmt.span(),
             "expected last expression to be `Some(match (..) { .. })`",
-        ));
+        ))
     };
 }

tests/target/configs/trailing_semicolon/true.rs → tests/target/configs/trailing_semicolon/always.rs

@@ -1,4 +1,4 @@
-// rustfmt-trailing_semicolon: true
+// rustfmt-trailing_semicolon: always
 
 #![feature(loop_break_value)]
 

tests/target/configs/trailing_semicolon/false.rs → tests/target/configs/trailing_semicolon/never.rs

@@ -1,4 +1,4 @@
-// rustfmt-trailing_semicolon: false
+// rustfmt-trailing_semicolon: never
 
 #![feature(loop_break_value)]