impl rewrite_result for ControlFlow, Stmt, update rewrite_index

ding-young · ytmimi · commit 35b3889c77fd · 2024-09-01T10:15:00.000-04:00
diff --git a/src/comment.rs b/src/comment.rs
@@ -1703,12 +1703,11 @@ impl<'a> Iterator for CommentCodeSlices<'a> {
 }
 
 /// Checks is `new` didn't miss any comment from `span`, if it removed any, return previous text
-/// (if it fits in the width/offset, else return `None`), else return `new`
 pub(crate) fn recover_comment_removed(
     new: String,
     span: Span,
     context: &RewriteContext<'_>,
-) -> Option<String> {
+) -> String {
     let snippet = context.snippet(span);
     if snippet != new && changed_comment_content(snippet, &new) {
         // We missed some comments. Warn and keep the original text.
@@ -1722,9 +1721,9 @@ pub(crate) fn recover_comment_removed(
                 )],
             );
         }
-        Some(snippet.to_owned())
+        snippet.to_owned()
     } else {
-        Some(new)
+        new
     }
 }
 
diff --git a/src/expr.rs b/src/expr.rs
@@ -278,7 +278,7 @@ pub(crate) fn format_expr(
         )
         .ok(),
         ast::ExprKind::Index(ref expr, ref index, _) => {
-            rewrite_index(&**expr, &**index, context, shape)
+            rewrite_index(&**expr, &**index, context, shape).ok()
         }
         ast::ExprKind::Repeat(ref expr, ref repeats) => rewrite_pair(
             &**expr,
@@ -435,7 +435,7 @@ pub(crate) fn format_expr(
     };
 
     expr_rw
-        .and_then(|expr_str| recover_comment_removed(expr_str, expr.span, context))
+        .map(|expr_str| recover_comment_removed(expr_str, expr.span, context))
         .and_then(|expr_str| {
             let attrs = outer_attributes(&expr.attrs);
             let attrs_str = attrs.rewrite(context, shape)?;
@@ -672,6 +672,7 @@ pub(crate) fn rewrite_cond(
                 String::from("\n") + &shape.indent.block_only().to_string(context.config);
             control_flow
                 .rewrite_cond(context, shape, &alt_block_sep)
+                .ok()
                 .map(|rw| rw.0)
         }),
     }
@@ -896,20 +897,23 @@ impl<'a> ControlFlow<'a> {
         expr: &ast::Expr,
         shape: Shape,
         offset: usize,
-    ) -> Option<String> {
+    ) -> RewriteResult {
         debug!("rewrite_pat_expr {:?} {:?} {:?}", shape, self.pat, expr);
 
-        let cond_shape = shape.offset_left(offset)?;
+        let cond_shape = shape
+            .offset_left(offset)
+            .max_width_error(shape.width, expr.span)?;
         if let Some(pat) = self.pat {
             let matcher = if self.matcher.is_empty() {
                 self.matcher.to_owned()
             } else {
                 format!("{} ", self.matcher)
             };
             let pat_shape = cond_shape
-                .offset_left(matcher.len())?
-                .sub_width(self.connector.len())?;
-            let pat_string = pat.rewrite(context, pat_shape)?;
+                .offset_left(matcher.len())
+                .and_then(|s| s.sub_width(self.connector.len()))
+                .max_width_error(cond_shape.width, pat.span)?;
+            let pat_string = pat.rewrite_result(context, pat_shape)?;
             let comments_lo = context
                 .snippet_provider
                 .span_after(self.span.with_lo(pat.span.hi()), self.connector.trim());
@@ -923,14 +927,13 @@ impl<'a> ControlFlow<'a> {
                 RhsTactics::Default,
                 comments_span,
                 true,
-            )
-            .ok();
+            );
         }
 
-        let expr_rw = expr.rewrite(context, cond_shape);
+        let expr_rw = expr.rewrite_result(context, cond_shape);
         // The expression may (partially) fit on the current line.
         // We do not allow splitting between `if` and condition.
-        if self.keyword == "if" || expr_rw.is_some() {
+        if self.keyword == "if" || expr_rw.is_ok() {
             return expr_rw;
         }
 
@@ -939,7 +942,7 @@ impl<'a> ControlFlow<'a> {
             .block_indent(context.config.tab_spaces())
             .with_max_width(context.config);
         let nested_indent_str = nested_shape.indent.to_string_with_newline(context.config);
-        expr.rewrite(context, nested_shape)
+        expr.rewrite_result(context, nested_shape)
             .map(|expr_rw| format!("{}{}", nested_indent_str, expr_rw))
     }
 
@@ -948,7 +951,7 @@ impl<'a> ControlFlow<'a> {
         context: &RewriteContext<'_>,
         shape: Shape,
         alt_block_sep: &str,
-    ) -> Option<(String, usize)> {
+    ) -> Result<(String, usize), RewriteError> {
         // Do not take the rhs overhead from the upper expressions into account
         // when rewriting pattern.
         let new_width = context.budget(shape.used_width());
@@ -959,7 +962,9 @@ impl<'a> ControlFlow<'a> {
         let constr_shape = if self.nested_if {
             // We are part of an if-elseif-else chain. Our constraints are tightened.
             // 7 = "} else " .len()
-            fresh_shape.offset_left(7)?
+            fresh_shape
+                .offset_left(7)
+                .max_width_error(fresh_shape.width, self.span)?
         } else {
             fresh_shape
         };
@@ -995,7 +1000,7 @@ impl<'a> ControlFlow<'a> {
 
             if let Some(cond_str) = trial {
                 if cond_str.len() <= context.config.single_line_if_else_max_width() {
-                    return Some((cond_str, 0));
+                    return Ok((cond_str, 0));
                 }
             }
         }
@@ -1048,7 +1053,7 @@ impl<'a> ControlFlow<'a> {
             label_string.len() + self.keyword.len() + pat_expr_string.len() + 2
         };
 
-        Some((
+        Ok((
             format!(
                 "{}{}{}{}{}",
                 label_string,
@@ -1114,13 +1119,17 @@ pub(crate) fn rewrite_else_kw_with_comments(
 
 impl<'a> Rewrite for ControlFlow<'a> {
     fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
+        self.rewrite_result(context, shape).ok()
+    }
+
+    fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
         debug!("ControlFlow::rewrite {:?} {:?}", self, shape);
 
         let alt_block_sep = &shape.indent.to_string_with_newline(context.config);
         let (cond_str, used_width) = self.rewrite_cond(context, shape, alt_block_sep)?;
         // If `used_width` is 0, it indicates that whole control flow is written in a single line.
         if used_width == 0 {
-            return Some(cond_str);
+            return Ok(cond_str);
         }
 
         let block_width = shape.width.saturating_sub(used_width);
@@ -1138,8 +1147,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
         let block_str = {
             let old_val = context.is_if_else_block.replace(self.else_block.is_some());
             let result =
-                rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true)
-                    .ok();
+                rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true);
             context.is_if_else_block.replace(old_val);
             result?
         };
@@ -1165,7 +1173,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
                         true,
                         mk_sp(else_block.span.lo(), self.span.hi()),
                     )
-                    .rewrite(context, shape)
+                    .rewrite_result(context, shape)
                 }
                 _ => {
                     last_in_chain = true;
@@ -1176,6 +1184,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
                         ..shape
                     };
                     format_expr(else_block, ExprType::Statement, context, else_shape)
+                        .unknown_error()
                 }
             };
 
@@ -1190,7 +1199,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
             result.push_str(&rewrite?);
         }
 
-        Some(result)
+        Ok(result)
     }
 }
 
@@ -1567,8 +1576,8 @@ fn rewrite_index(
     index: &ast::Expr,
     context: &RewriteContext<'_>,
     shape: Shape,
-) -> Option<String> {
-    let expr_str = expr.rewrite(context, shape)?;
+) -> RewriteResult {
+    let expr_str = expr.rewrite_result(context, shape)?;
 
     let offset = last_line_width(&expr_str) + 1;
     let rhs_overhead = shape.rhs_overhead(context.config);
@@ -1583,37 +1592,45 @@ fn rewrite_index(
                 .and_then(|shape| shape.sub_width(1)),
             IndentStyle::Visual => shape.visual_indent(offset).sub_width(offset + 1),
         }
-    };
-    let orig_index_rw = index_shape.and_then(|s| index.rewrite(context, s));
+    }
+    .max_width_error(shape.width, index.span());
+    let orig_index_rw = index_shape.and_then(|s| index.rewrite_result(context, s));
 
     // Return if index fits in a single line.
     match orig_index_rw {
-        Some(ref index_str) if !index_str.contains('\n') => {
-            return Some(format!("{expr_str}[{index_str}]"));
+        Ok(ref index_str) if !index_str.contains('\n') => {
+            return Ok(format!("{expr_str}[{index_str}]"));
         }
         _ => (),
     }
 
     // Try putting index on the next line and see if it fits in a single line.
     let indent = shape.indent.block_indent(context.config);
-    let index_shape = Shape::indented(indent, context.config).offset_left(1)?;
-    let index_shape = index_shape.sub_width(1 + rhs_overhead)?;
-    let new_index_rw = index.rewrite(context, index_shape);
+    let index_shape = Shape::indented(indent, context.config)
+        .offset_left(1)
+        .max_width_error(shape.width, index.span())?;
+    let index_shape = index_shape
+        .sub_width(1 + rhs_overhead)
+        .max_width_error(index_shape.width, index.span())?;
+    let new_index_rw = index.rewrite_result(context, index_shape);
     match (orig_index_rw, new_index_rw) {
-        (_, Some(ref new_index_str)) if !new_index_str.contains('\n') => Some(format!(
+        (_, Ok(ref new_index_str)) if !new_index_str.contains('\n') => Ok(format!(
             "{}{}[{}]",
             expr_str,
             indent.to_string_with_newline(context.config),
             new_index_str,
         )),
-        (None, Some(ref new_index_str)) => Some(format!(
+        (Err(_), Ok(ref new_index_str)) => Ok(format!(
             "{}{}[{}]",
             expr_str,
             indent.to_string_with_newline(context.config),
             new_index_str,
         )),
-        (Some(ref index_str), _) => Some(format!("{expr_str}[{index_str}]")),
-        _ => None,
+        (Ok(ref index_str), _) => Ok(format!("{expr_str}[{index_str}]")),
+        // When both orig_index_rw and new_index_rw result in errors, we currently propagate the
+        // error from the second attempt since it is more generous with width constraints.
+        // This decision is somewhat arbitrary and is open to change.
+        (Err(_), Err(new_index_rw_err)) => Err(new_index_rw_err),
     }
 }
 
diff --git a/src/items.rs b/src/items.rs
@@ -2077,7 +2077,7 @@ fn rewrite_static(
             true,
         )
         .ok()
-        .and_then(|res| recover_comment_removed(res, static_parts.span, context))
+        .map(|res| recover_comment_removed(res, static_parts.span, context))
         .map(|s| if s.ends_with(';') { s } else { s + ";" })
     } else {
         Some(format!("{prefix}{ty_str};"))
diff --git a/src/overflow.rs b/src/overflow.rs
@@ -19,7 +19,7 @@ use crate::lists::{
 };
 use crate::macros::MacroArg;
 use crate::patterns::{can_be_overflowed_pat, TuplePatField};
-use crate::rewrite::{Rewrite, RewriteContext, RewriteErrorExt, RewriteResult};
+use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::Shape;
 use crate::source_map::SpanUtils;
 use crate::spanned::Spanned;
@@ -90,6 +90,10 @@ impl<'a> Rewrite for OverflowableItem<'a> {
     fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
         self.map(|item| item.rewrite(context, shape))
     }
+
+    fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
+        self.map(|item| item.rewrite_result(context, shape))
+    }
 }
 
 impl<'a> Spanned for OverflowableItem<'a> {
@@ -617,7 +621,7 @@ impl<'a> Context<'a> {
         tactic
     }
 
-    fn rewrite_items(&self) -> Option<(bool, String)> {
+    fn rewrite_items(&self) -> Result<(bool, String), RewriteError> {
         let span = self.items_span();
         debug!("items: {:?}", self.items);
 
@@ -661,7 +665,6 @@ impl<'a> Context<'a> {
             .ends_with_newline(ends_with_newline);
 
         write_list(&list_items, &fmt)
-            .ok()
             .map(|items_str| (tactic == DefinitiveListTactic::Horizontal, items_str))
     }
 
@@ -718,7 +721,7 @@ impl<'a> Context<'a> {
     }
 
     fn rewrite(&self, shape: Shape) -> RewriteResult {
-        let (extendable, items_str) = self.rewrite_items().unknown_error()?;
+        let (extendable, items_str) = self.rewrite_items()?;
 
         // If we are using visual indent style and failed to format, retry with block indent.
         if !self.context.use_block_indent()
diff --git a/src/stmt.rs b/src/stmt.rs
@@ -4,7 +4,7 @@ use rustc_span::Span;
 use crate::comment::recover_comment_removed;
 use crate::config::StyleEdition;
 use crate::expr::{format_expr, is_simple_block, ExprType};
-use crate::rewrite::{Rewrite, RewriteContext};
+use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::Shape;
 use crate::source_map::LineRangeUtils;
 use crate::spanned::Spanned;
@@ -90,6 +90,14 @@ impl<'a> Stmt<'a> {
 
 impl<'a> Rewrite for Stmt<'a> {
     fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
+        self.rewrite_result(context, shape).ok()
+    }
+
+    fn rewrite_result(
+        &self,
+        context: &RewriteContext<'_>,
+        shape: Shape,
+    ) -> crate::rewrite::RewriteResult {
         let expr_type =
             if context.config.style_edition() >= StyleEdition::Edition2024 && self.is_last_expr() {
                 ExprType::SubExpression
@@ -112,22 +120,28 @@ fn format_stmt(
     stmt: &ast::Stmt,
     expr_type: ExprType,
     is_last_expr: bool,
-) -> Option<String> {
-    skip_out_of_file_lines_range!(context, stmt.span());
+) -> RewriteResult {
+    skip_out_of_file_lines_range_err!(context, stmt.span());
 
     let result = match stmt.kind {
-        ast::StmtKind::Let(ref local) => local.rewrite(context, shape),
+        ast::StmtKind::Let(ref local) => local.rewrite_result(context, shape),
         ast::StmtKind::Expr(ref ex) | ast::StmtKind::Semi(ref ex) => {
             let suffix = if semicolon_for_stmt(context, stmt, is_last_expr) {
                 ";"
             } else {
                 ""
             };
 
-            let shape = shape.sub_width(suffix.len())?;
-            format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
+            let shape = shape
+                .sub_width(suffix.len())
+                .max_width_error(shape.width, ex.span())?;
+            format_expr(ex, expr_type, context, shape)
+                .map(|s| s + suffix)
+                .unknown_error()
+        }
+        ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => {
+            Err(RewriteError::Unknown)
         }
-        ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => None,
     };
-    result.and_then(|res| recover_comment_removed(res, stmt.span(), context))
+    result.map(|res| recover_comment_removed(res, stmt.span(), context))
 }
diff --git a/src/types.rs b/src/types.rs

Original file line number	Diff line number	Diff line change
`@@ -1703,12 +1703,11 @@ impl<'a> Iterator for CommentCodeSlices<'a> {`
`1703`	`1703`	`}`
`1704`	`1704`
`1705`	`1705`	/// Checks is `new` didn't miss any comment from `span`, if it removed any, return previous text
`1706`		-/// (if it fits in the width/offset, else return `None`), else return `new`
`1707`	`1706`	`pub(crate) fn recover_comment_removed(`
`1708`	`1707`	`new: String,`
`1709`	`1708`	`span: Span,`
`1710`	`1709`	`context: &RewriteContext<'_>,`
`1711`		`-) -> Option<String> {`
	`1710`	`+) -> String {`
`1712`	`1711`	`let snippet = context.snippet(span);`
`1713`	`1712`	`if snippet != new && changed_comment_content(snippet, &new) {`
`1714`	`1713`	`// We missed some comments. Warn and keep the original text.`
`@@ -1722,9 +1721,9 @@ pub(crate) fn recover_comment_removed(`
`1722`	`1721`	`)],`
`1723`	`1722`	`);`
`1724`	`1723`	`}`
`1725`		`- Some(snippet.to_owned())`
	`1724`	`+ snippet.to_owned()`
`1726`	`1725`	`} else {`
`1727`		`- Some(new)`
	`1726`	`+ new`
`1728`	`1727`	`}`
`1729`	`1728`	`}`
`1730`	`1729`
Original file line number	Diff line number	Diff line change
`@@ -2077,7 +2077,7 @@ fn rewrite_static(`
`2077`	`2077`	`true,`
`2078`	`2078`	`)`
`2079`	`2079`	`.ok()`
`2080`		`- .and_then(\|res\| recover_comment_removed(res, static_parts.span, context))`
	`2080`	`+ .map(\|res\| recover_comment_removed(res, static_parts.span, context))`
`2081`	`2081`	`.map(\|s\| if s.ends_with(';') { s } else { s + ";" })`
`2082`	`2082`	`} else {`
`2083`	`2083`	`Some(format!("{prefix}{ty_str};"))`