modfiy ListItem to hold RewriteResult as the item field

Author: ding-young

src/attr.rs

@@ -147,7 +147,7 @@ fn format_derive(
         .tactic(tactic)
         .trailing_separator(trailing_separator)
         .ends_with_newline(false);
-    let item_str = write_list(&all_items, &fmt)?;
+    let item_str = write_list(&all_items, &fmt).ok()?;
 
     debug!("item_str: '{}'", item_str);
 

src/closures.rs

@@ -336,7 +336,7 @@ fn rewrite_closure_fn_decl(
     let fmt = ListFormatting::new(param_shape, context.config)
         .tactic(tactic)
         .preserve_newline(true);
-    let list_str = write_list(&item_vec, &fmt).unknown_error()?;
+    let list_str = write_list(&item_vec, &fmt)?;
     let mut prefix = format!("{binder}{const_}{immovable}{coro}{mover}|{list_str}|");
 
     if !ret_str.is_empty() {

src/expr.rs

@@ -1677,7 +1677,6 @@ fn rewrite_struct_lit<'a>(
                     v_shape.sub_width(1).max_width_error(v_shape.width, span)?,
                     0,
                 )
-                .unknown_error()
             }
             StructLitField::Base(expr) => {
                 // 2 = ..
@@ -1719,7 +1718,7 @@ fn rewrite_struct_lit<'a>(
             force_no_trailing_comma || has_base_or_rest || !context.use_block_indent(),
         );
 
-        write_list(&item_vec, &fmt).unknown_error()?
+        write_list(&item_vec, &fmt)?
     };
 
     let fields_str =
@@ -1868,7 +1867,7 @@ fn rewrite_tuple_in_visual_indent_style<'a, T: 'a + IntoOverflowableItem<'a>>(
     let fmt = ListFormatting::new(nested_shape, context.config)
         .tactic(tactic)
         .ends_with_newline(false);
-    let list_str = write_list(&item_vec, &fmt)?;
+    let list_str = write_list(&item_vec, &fmt).ok()?;
 
     Some(format!("({list_str})"))
 }

src/imports.rs

@@ -993,7 +993,7 @@ fn rewrite_nested_use_tree(
     };
     for use_tree in use_tree_list {
         if let Some(mut list_item) = use_tree.list_item.clone() {
-            list_item.item = use_tree.rewrite(context, nested_shape);
+            list_item.item = use_tree.rewrite_result(context, nested_shape);
             list_items.push(list_item);
         } else {
             list_items.push(ListItem::from_str(use_tree.rewrite(context, nested_shape)?));
@@ -1032,7 +1032,7 @@ fn rewrite_nested_use_tree(
         .preserve_newline(true)
         .nested(has_nested_list);
 
-    let list_str = write_list(&list_items, &fmt)?;
+    let list_str = write_list(&list_items, &fmt).ok()?;
 
     let result = if (list_str.contains('\n')
         || list_str.len() > remaining_width

src/items.rs

@@ -653,7 +653,7 @@ impl<'a> FmtVisitor<'a> {
             .trailing_separator(self.config.trailing_comma())
             .preserve_newline(true);
 
-        let list = write_list(&items, &fmt)?;
+        let list = write_list(&items, &fmt).ok()?;
         result.push_str(&list);
         result.push_str(&original_offset.to_string_with_newline(self.config));
         result.push('}');
@@ -2819,7 +2819,7 @@ fn rewrite_params(
         .trailing_separator(trailing_separator)
         .ends_with_newline(tactic.ends_with_newline(context.config.indent_style()))
         .preserve_newline(true);
-    write_list(&param_items, &fmt)
+    write_list(&param_items, &fmt).ok()
 }
 
 fn compute_budgets_for_params(
@@ -3076,7 +3076,7 @@ fn rewrite_bounds_on_where_clause(
         .tactic(shape_tactic)
         .trailing_separator(comma_tactic)
         .preserve_newline(preserve_newline);
-    write_list(&items.collect::<Vec<_>>(), &fmt)
+    write_list(&items.collect::<Vec<_>>(), &fmt).ok()
 }
 
 fn rewrite_where_clause(
@@ -3152,7 +3152,7 @@ fn rewrite_where_clause(
         .trailing_separator(comma_tactic)
         .ends_with_newline(tactic.ends_with_newline(context.config.indent_style()))
         .preserve_newline(true);
-    let preds_str = write_list(&item_vec, &fmt)?;
+    let preds_str = write_list(&item_vec, &fmt).ok()?;
 
     let end_length = if terminator == "{" {
         // If the brace is on the next line we don't need to count it otherwise it needs two

src/lists.rs

@@ -8,7 +8,7 @@ use rustc_span::BytePos;
 use crate::comment::{find_comment_end, rewrite_comment, FindUncommented};
 use crate::config::lists::*;
 use crate::config::{Config, IndentStyle};
-use crate::rewrite::{RewriteContext, RewriteResult};
+use crate::rewrite::{RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::{Indent, Shape};
 use crate::utils::{
     count_newlines, first_line_width, last_line_width, mk_sp, starts_with_newline,
@@ -125,18 +125,18 @@ pub(crate) struct ListItem {
     pub(crate) pre_comment_style: ListItemCommentStyle,
     // Item should include attributes and doc comments. None indicates a failed
     // rewrite.
-    pub(crate) item: Option<String>,
+    pub(crate) item: RewriteResult,
     pub(crate) post_comment: Option<String>,
     // Whether there is extra whitespace before this item.
     pub(crate) new_lines: bool,
 }
 
 impl ListItem {
-    pub(crate) fn empty() -> ListItem {
+    pub(crate) fn from_item(item: RewriteResult) -> ListItem {
         ListItem {
             pre_comment: None,
             pre_comment_style: ListItemCommentStyle::None,
-            item: None,
+            item: item,
             post_comment: None,
             new_lines: false,
         }
@@ -185,7 +185,7 @@ impl ListItem {
         ListItem {
             pre_comment: None,
             pre_comment_style: ListItemCommentStyle::None,
-            item: Some(s.into()),
+            item: Ok(s.into()),
             post_comment: None,
             new_lines: false,
         }
@@ -197,7 +197,11 @@ impl ListItem {
             !matches!(*s, Some(ref s) if !s.is_empty())
         }
 
-        !(empty(&self.pre_comment) && empty(&self.item) && empty(&self.post_comment))
+        fn empty_result(s: &RewriteResult) -> bool {
+            !matches!(*s, Ok(ref s) if !s.is_empty())
+        }
+
+        !(empty(&self.pre_comment) && empty_result(&self.item) && empty(&self.post_comment))
     }
 }
 
@@ -257,7 +261,7 @@ where
 }
 
 // Format a list of commented items into a string.
-pub(crate) fn write_list<I, T>(items: I, formatting: &ListFormatting<'_>) -> Option<String>
+pub(crate) fn write_list<I, T>(items: I, formatting: &ListFormatting<'_>) -> RewriteResult
 where
     I: IntoIterator<Item = T> + Clone,
     T: AsRef<ListItem>,
@@ -281,8 +285,7 @@ where
     let indent_str = &formatting.shape.indent.to_string(formatting.config);
     while let Some((i, item)) = iter.next() {
         let item = item.as_ref();
-        // TODO here Is it possible to 실제로 list item이 없으면..
-        let inner_item = item.item.as_ref()?;
+        let inner_item = item.item.as_ref().or_else(|err| Err(err.clone()))?;
         let first = i == 0;
         let last = iter.peek().is_none();
         let mut separate = match sep_place {
@@ -363,8 +366,8 @@ where
             // Block style in non-vertical mode.
             let block_mode = tactic == DefinitiveListTactic::Horizontal;
             // Width restriction is only relevant in vertical mode.
-            let comment =
-                rewrite_comment(comment, block_mode, formatting.shape, formatting.config)?;
+            let comment = rewrite_comment(comment, block_mode, formatting.shape, formatting.config)
+                .unknown_error()?;
             result.push_str(&comment);
 
             if !inner_item.is_empty() {
@@ -410,7 +413,8 @@ where
                 true,
                 Shape::legacy(formatting.shape.width, Indent::empty()),
                 formatting.config,
-            )?;
+            )
+            .unknown_error()?;
 
             result.push(' ');
             result.push_str(&formatted_comment);
@@ -461,7 +465,8 @@ where
                 )
             };
 
-            let mut formatted_comment = rewrite_post_comment(&mut item_max_width)?;
+            let mut formatted_comment =
+                rewrite_post_comment(&mut item_max_width).unknown_error()?;
 
             if !starts_with_newline(comment) {
                 if formatting.align_comments {
@@ -474,7 +479,8 @@ where
                         > formatting.config.max_width()
                     {
                         item_max_width = None;
-                        formatted_comment = rewrite_post_comment(&mut item_max_width)?;
+                        formatted_comment =
+                            rewrite_post_comment(&mut item_max_width).unknown_error()?;
                         comment_alignment =
                             post_comment_alignment(item_max_width, unicode_str_width(inner_item));
                     }
@@ -517,7 +523,7 @@ where
         prev_item_is_nested_import = inner_item.contains("::");
     }
 
-    Some(result)
+    Ok(result)
 }
 
 fn max_width_of_item_with_post_comment<I, T>(
@@ -776,10 +782,11 @@ where
             ListItem {
                 pre_comment,
                 pre_comment_style,
+                // leave_last is set to true only for rewrite_items
                 item: if self.inner.peek().is_none() && self.leave_last {
-                    None
+                    Err(RewriteError::SkipFormatting)
                 } else {
-                    (self.get_item_string)(&item).ok()
+                    (self.get_item_string)(&item)
                 },
                 post_comment,
                 new_lines,

src/macros.rs

@@ -477,8 +477,8 @@ pub(crate) fn rewrite_macro_def(
     }
 
     match write_list(&branch_items, &fmt) {
-        Some(ref s) => result += s,
-        None => return snippet,
+        Ok(ref s) => result += s,
+        Err(_) => return snippet,
     }
 
     if multi_branch_style {

src/matches.rs

@@ -236,7 +236,7 @@ fn rewrite_match_arms(
         .separator("")
         .preserve_newline(true);
 
-    write_list(&arms_vec, &fmt).unknown_error()
+    write_list(&arms_vec, &fmt)
 }
 
 fn rewrite_match_arm(

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};
+use crate::rewrite::{Rewrite, RewriteContext, RewriteErrorExt};
 use crate::shape::Shape;
 use crate::source_map::SpanUtils;
 use crate::spanned::Spanned;
@@ -456,7 +456,7 @@ impl<'a> Context<'a> {
 
         if let Some(rewrite) = rewrite {
             // splitn(2, *).next().unwrap() is always safe.
-            let rewrite_first_line = Some(rewrite.splitn(2, '\n').next().unwrap().to_owned());
+            let rewrite_first_line = Ok(rewrite.splitn(2, '\n').next().unwrap().to_owned());
             last_list_item.item = rewrite_first_line;
             Some(rewrite)
         } else {
@@ -544,22 +544,23 @@ impl<'a> Context<'a> {
                         .and_then(|last_item| last_item.rewrite(self.context, self.nested_shape));
                     let no_newline = rw.as_ref().map_or(false, |s| !s.contains('\n'));
                     if no_newline {
-                        list_items[self.items.len() - 1].item = rw;
+                        list_items[self.items.len() - 1].item = rw.unknown_error();
                     } else {
-                        list_items[self.items.len() - 1].item = Some(overflowed.to_owned());
+                        list_items[self.items.len() - 1].item = Ok(overflowed.to_owned());
                     }
                 } else {
-                    list_items[self.items.len() - 1].item = Some(overflowed.to_owned());
+                    list_items[self.items.len() - 1].item = Ok(overflowed.to_owned());
                 }
             }
             (true, DefinitiveListTactic::Horizontal, placeholder @ Some(..)) => {
-                list_items[self.items.len() - 1].item = placeholder;
+                list_items[self.items.len() - 1].item = placeholder.unknown_error();
             }
             _ if !self.items.is_empty() => {
                 list_items[self.items.len() - 1].item = self
                     .items
                     .last()
-                    .and_then(|last_item| last_item.rewrite(self.context, self.nested_shape));
+                    .and_then(|last_item| last_item.rewrite(self.context, self.nested_shape))
+                    .unknown_error();
 
                 // Use horizontal layout for a function with a single argument as long as
                 // everything fits in a single line.
@@ -656,6 +657,7 @@ impl<'a> Context<'a> {
             .ends_with_newline(ends_with_newline);
 
         write_list(&list_items, &fmt)
+            .ok()
             .map(|items_str| (tactic == DefinitiveListTactic::Horizontal, items_str))
     }
 

src/patterns.rs

@@ -101,7 +101,7 @@ impl Rewrite for Pat {
                     .separator(" |")
                     .separator_place(context.config.binop_separator())
                     .ends_with_newline(false);
-                write_list(&items, &fmt)
+                write_list(&items, &fmt).ok()
             }
             PatKind::Box(ref pat) => rewrite_unary_prefix(context, "box ", &**pat, shape),
             PatKind::Ident(BindingMode(by_ref, mutability), ident, ref sub_pat) => {
@@ -354,7 +354,7 @@ fn rewrite_struct_pat(
     let nested_shape = shape_for_tactic(tactic, h_shape, v_shape);
     let fmt = struct_lit_formatting(nested_shape, tactic, context, false);
 
-    let mut fields_str = write_list(&item_vec, &fmt).unknown_error()?;
+    let mut fields_str = write_list(&item_vec, &fmt)?;
     let one_line_width = h_shape.map_or(0, |shape| shape.width);
 
     let has_trailing_comma = fmt.needs_trailing_separator();
@@ -561,7 +561,7 @@ fn count_wildcard_suffix_len(
     for item in items
         .iter()
         .rev()
-        .take_while(|i| matches!(i.item, Some(ref internal_string) if internal_string == "_"))
+        .take_while(|i| matches!(i.item, Ok(ref internal_string) if internal_string == "_"))
     {
         suffix_len += 1;
 

src/reorder.rs

@@ -59,7 +59,7 @@ fn wrap_reorderable_items(
     let fmt = ListFormatting::new(shape, context.config)
         .separator("")
         .align_comments(false);
-    write_list(list_items, &fmt)
+    write_list(list_items, &fmt).ok()
 }
 
 fn rewrite_reorderable_item(
@@ -131,9 +131,18 @@ fn rewrite_reorderable_or_regroupable_items(
                 .map(|use_group| {
                     let item_vec: Vec<_> = use_group
                         .into_iter()
-                        .map(|use_tree| ListItem {
-                            item: use_tree.rewrite_top_level(context, nested_shape),
-                            ..use_tree.list_item.unwrap_or_else(ListItem::empty)
+                        .map(|use_tree| {
+                            let item = use_tree
+                                .rewrite_top_level(context, nested_shape)
+                                .unknown_error();
+                            if let Some(list_item) = use_tree.list_item {
+                                ListItem {
+                                    item: item,
+                                    ..list_item
+                                }
+                            } else {
+                                ListItem::from_item(item)
+                            }
                         })
                         .collect();
                     wrap_reorderable_items(context, &item_vec, nested_shape)

src/rewrite.rs

@@ -30,7 +30,7 @@ impl<T: Rewrite> Rewrite for ptr::P<T> {
     }
 }
 
-#[derive(Error, Debug)]
+#[derive(Clone, Error, Debug)]
 pub(crate) enum RewriteError {
     #[error("Formatting was skipped due to skip attribute or out of file range.")]
     SkipFormatting,

src/types.rs

@@ -407,7 +407,7 @@ where
             .trailing_separator(trailing_separator)
             .ends_with_newline(tactic.ends_with_newline(context.config.indent_style()))
             .preserve_newline(true);
-        (write_list(&item_vec, &fmt).unknown_error()?, tactic)
+        (write_list(&item_vec, &fmt)?, tactic)
     };
 
     let args = if tactic == DefinitiveListTactic::Horizontal