Account for previous double-counting of semicolon size within format_expr

Author: compiler-errors

src/config/mod.rs

@@ -673,7 +673,7 @@ force_multiline_blocks = false
 fn_params_layout = "Tall"
 brace_style = "SameLineWhere"
 control_brace_style = "AlwaysSameLine"
-trailing_semicolon = preserve
+trailing_semicolon = "Preserve"
 trailing_comma = "Vertical"
 match_block_trailing_comma = false
 blank_lines_upper_bound = 1

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

@@ -290,6 +290,31 @@ pub(crate) fn semicolon_for_expr(context: &RewriteContext<'_>, expr: &ast::Expr)
     }
 }
 
+/// 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,
+    }
+}
+
 #[inline]
 pub(crate) fn semicolon_for_stmt(
     context: &RewriteContext<'_>,

tests/target/expr.rs

@@ -536,14 +536,18 @@ fn issue3226() {
     {
         {
             {
-                return Err(ErrorKind::ManagementInterfaceError("Server exited unexpectedly").into())
+                return Err(
+                    ErrorKind::ManagementInterfaceError("Server exited unexpectedly").into(),
+                )
             }
         }
     }
     {
         {
             {
-                break Err(ErrorKind::ManagementInterfaceError("Server exited unexpectedlyy").into())
+                break Err(
+                    ErrorKind::ManagementInterfaceError("Server exited unexpectedlyy").into(),
+                )
             }
         }
     }