Skip to content

modify ListItem to hold RewriteResult as the item field #6244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 26, 2024
Merged
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
4 changes: 2 additions & 2 deletions src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn format_derive(
",",
|span| span.lo(),
|span| span.hi(),
|span| Some(context.snippet(*span).to_owned()),
|span| Ok(context.snippet(*span).to_owned()),
// We update derive attribute spans to start after the opening '('
// This helps us focus parsing to just what's inside #[derive(...)]
context.snippet_provider.span_after(attr.span, "("),
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ fn rewrite_closure_fn_decl(
",",
|param| span_lo_for_param(param),
|param| span_hi_for_param(context, param),
|param| param.rewrite(context, param_shape),
|param| param.rewrite_result(context, param_shape),
context.snippet_provider.span_after(span, "|"),
body.span.lo(),
false,
Expand All @@ -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() {
Expand Down
25 changes: 17 additions & 8 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1671,15 +1671,24 @@ fn rewrite_struct_lit<'a>(
let rewrite = |item: &StructLitField<'_>| match *item {
StructLitField::Regular(field) => {
// The 1 taken from the v_budget is for the comma.
let v_shape = v_shape.sub_width(1)?;
rewrite_field(context, field, v_shape, 0).ok()
rewrite_field(
context,
field,
v_shape.sub_width(1).max_width_error(v_shape.width, span)?,
0,
)
}
StructLitField::Base(expr) => {
// 2 = ..
let v_shape = v_shape.sub_width(2)?;
expr.rewrite(context, v_shape).map(|s| format!("..{}", s))
expr.rewrite_result(
context,
v_shape
.offset_left(2)
.max_width_error(v_shape.width, span)?,
)
.map(|s| format!("..{}", s))
}
StructLitField::Rest(_) => Some("..".to_owned()),
StructLitField::Rest(_) => Ok("..".to_owned()),
};

let items = itemize_list(
Expand Down Expand Up @@ -1709,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 =
Expand Down Expand Up @@ -1843,7 +1852,7 @@ fn rewrite_tuple_in_visual_indent_style<'a, T: 'a + IntoOverflowableItem<'a>>(
",",
|item| item.span().lo(),
|item| item.span().hi(),
|item| item.rewrite(context, nested_shape),
|item| item.rewrite_result(context, nested_shape),
list_lo,
span.hi() - BytePos(1),
false,
Expand All @@ -1858,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})"))
}
Expand Down
6 changes: 3 additions & 3 deletions src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ impl UseTree {
",",
|tree| tree.span.lo(),
|tree| tree.span.hi(),
|_| Some("".to_owned()), // We only need comments for now.
|_| Ok("".to_owned()), // We only need comments for now.
context.snippet_provider.span_after(a.span, "{"),
a.span.hi(),
false,
Expand Down Expand Up @@ -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)?));
Expand Down Expand Up @@ -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
Expand Down
21 changes: 12 additions & 9 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,10 @@ impl<'a> FmtVisitor<'a> {
}
},
|f| f.span.hi(),
|f| self.format_variant(f, one_line_width, pad_discrim_ident_to),
|f| {
self.format_variant(f, one_line_width, pad_discrim_ident_to)
.unknown_error()
},
body_lo,
body_hi,
false,
Expand All @@ -650,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('}');
Expand Down Expand Up @@ -2777,8 +2780,8 @@ fn rewrite_params(
|param| param.ty.span.hi(),
|param| {
param
.rewrite(context, Shape::legacy(multi_line_budget, param_indent))
.or_else(|| Some(context.snippet(param.span()).to_owned()))
.rewrite_result(context, Shape::legacy(multi_line_budget, param_indent))
.or_else(|_| Ok(context.snippet(param.span()).to_owned()))
},
span.lo(),
span.hi(),
Expand Down Expand Up @@ -2816,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(
Expand Down Expand Up @@ -3048,7 +3051,7 @@ fn rewrite_bounds_on_where_clause(
",",
|pred| pred.span().lo(),
|pred| pred.span().hi(),
|pred| pred.rewrite(context, shape),
|pred| pred.rewrite_result(context, shape),
span_start,
span_end,
false,
Expand All @@ -3073,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(
Expand Down Expand Up @@ -3129,7 +3132,7 @@ fn rewrite_where_clause(
",",
|pred| pred.span().lo(),
|pred| pred.span().hi(),
|pred| pred.rewrite(context, Shape::legacy(budget, offset)),
|pred| pred.rewrite_result(context, Shape::legacy(budget, offset)),
span_start,
span_end,
false,
Expand All @@ -3149,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
Expand Down
42 changes: 25 additions & 17 deletions src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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,
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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>,
Expand All @@ -281,7 +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();
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 {
Expand Down Expand Up @@ -362,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() {
Expand Down Expand Up @@ -409,7 +413,8 @@ where
true,
Shape::legacy(formatting.shape.width, Indent::empty()),
formatting.config,
)?;
)
.unknown_error()?;

result.push(' ');
result.push_str(&formatted_comment);
Expand Down Expand Up @@ -460,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 {
Expand All @@ -473,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));
}
Expand Down Expand Up @@ -516,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>(
Expand Down Expand Up @@ -741,7 +748,7 @@ where
I: Iterator<Item = T>,
F1: Fn(&T) -> BytePos,
F2: Fn(&T) -> BytePos,
F3: Fn(&T) -> Option<String>,
F3: Fn(&T) -> RewriteResult,
{
type Item = ListItem;

Expand Down Expand Up @@ -775,8 +782,9 @@ 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)
},
Expand Down Expand Up @@ -805,7 +813,7 @@ where
I: Iterator<Item = T>,
F1: Fn(&T) -> BytePos,
F2: Fn(&T) -> BytePos,
F3: Fn(&T) -> Option<String>,
F3: Fn(&T) -> RewriteResult,
{
ListItems {
snippet_provider,
Expand Down
12 changes: 6 additions & 6 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::lists::{itemize_list, write_list, ListFormatting};
use crate::overflow;
use crate::parse::macros::lazy_static::parse_lazy_static;
use crate::parse::macros::{parse_expr, parse_macro_args, ParsedMacroArgs};
use crate::rewrite::{Rewrite, RewriteContext};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError};
use crate::shape::{Indent, Shape};
use crate::source_map::SpanUtils;
use crate::spanned::Spanned;
Expand Down Expand Up @@ -452,13 +452,13 @@ pub(crate) fn rewrite_macro_def(
|branch| branch.span.lo(),
|branch| branch.span.hi(),
|branch| match branch.rewrite(context, arm_shape, multi_branch_style) {
Some(v) => Some(v),
Some(v) => Ok(v),
// if the rewrite returned None because a macro could not be rewritten, then return the
// original body
None if context.macro_rewrite_failure.get() => {
Some(context.snippet(branch.body).trim().to_string())
Ok(context.snippet(branch.body).trim().to_string())
}
None => None,
None => Err(RewriteError::Unknown),
},
context.snippet_provider.span_after(span, "{"),
span.hi(),
Expand All @@ -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 {
Expand Down
Loading
Loading