Skip to content

Commit

Permalink
update rewrite_assignment to return RewriteResult
Browse files Browse the repository at this point in the history
  • Loading branch information
ding-young authored and ytmimi committed Sep 1, 2024
1 parent 46cb7d3 commit 1930514
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 51 deletions.
70 changes: 40 additions & 30 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,10 @@ pub(crate) fn format_expr(
rewrite_path(context, PathContext::Expr, qself, path, shape).ok()
}
ast::ExprKind::Assign(ref lhs, ref rhs, _) => {
rewrite_assignment(context, lhs, rhs, None, shape)
rewrite_assignment(context, lhs, rhs, None, shape).ok()
}
ast::ExprKind::AssignOp(ref op, ref lhs, ref rhs) => {
rewrite_assignment(context, lhs, rhs, Some(op), shape)
rewrite_assignment(context, lhs, rhs, Some(op), shape).ok()
}
ast::ExprKind::Continue(ref opt_label) => {
let id_str = match *opt_label {
Expand Down Expand Up @@ -2050,15 +2050,21 @@ fn rewrite_assignment(
rhs: &ast::Expr,
op: Option<&ast::BinOp>,
shape: Shape,
) -> Option<String> {
) -> RewriteResult {
let operator_str = match op {
Some(op) => context.snippet(op.span),
None => "=",
};

// 1 = space between lhs and operator.
let lhs_shape = shape.sub_width(operator_str.len() + 1)?;
let lhs_str = format!("{} {}", lhs.rewrite(context, lhs_shape)?, operator_str);
let lhs_shape = shape
.sub_width(operator_str.len() + 1)
.max_width_error(shape.width, lhs.span())?;
let lhs_str = format!(
"{} {}",
lhs.rewrite_result(context, lhs_shape)?,
operator_str
);

rewrite_assign_rhs(
context,
Expand Down Expand Up @@ -2089,7 +2095,7 @@ pub(crate) fn rewrite_assign_rhs<S: Into<String>, R: Rewrite>(
ex: &R,
rhs_kind: &RhsAssignKind<'_>,
shape: Shape,
) -> Option<String> {
) -> RewriteResult {
rewrite_assign_rhs_with(context, lhs, ex, shape, rhs_kind, RhsTactics::Default)
}

Expand All @@ -2100,7 +2106,7 @@ pub(crate) fn rewrite_assign_rhs_expr<R: Rewrite>(
shape: Shape,
rhs_kind: &RhsAssignKind<'_>,
rhs_tactics: RhsTactics,
) -> Option<String> {
) -> RewriteResult {
let last_line_width = last_line_width(lhs).saturating_sub(if lhs.contains('\n') {
shape.indent.width()
} else {
Expand All @@ -2122,7 +2128,7 @@ pub(crate) fn rewrite_assign_rhs_expr<R: Rewrite>(
context,
ex,
orig_shape,
ex.rewrite(context, orig_shape),
ex.rewrite_result(context, orig_shape),
rhs_kind,
rhs_tactics,
has_rhs_comment,
Expand All @@ -2136,10 +2142,10 @@ pub(crate) fn rewrite_assign_rhs_with<S: Into<String>, R: Rewrite>(
shape: Shape,
rhs_kind: &RhsAssignKind<'_>,
rhs_tactics: RhsTactics,
) -> Option<String> {
) -> RewriteResult {
let lhs = lhs.into();
let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_kind, rhs_tactics)?;
Some(lhs + &rhs)
Ok(lhs + &rhs)
}

pub(crate) fn rewrite_assign_rhs_with_comments<S: Into<String>, R: Rewrite + Spanned>(
Expand All @@ -2161,8 +2167,7 @@ pub(crate) fn rewrite_assign_rhs_with_comments<S: Into<String>, R: Rewrite + Spa
} else {
shape
};
let rhs =
rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_kind, rhs_tactics).unknown_error()?;
let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_kind, rhs_tactics)?;
if contains_comment {
let rhs = rhs.trim_start();
combine_strs_with_missing_comments(context, &lhs, rhs, between_span, shape, allow_extend)
Expand All @@ -2175,48 +2180,53 @@ fn choose_rhs<R: Rewrite>(
context: &RewriteContext<'_>,
expr: &R,
shape: Shape,
orig_rhs: Option<String>,
orig_rhs: RewriteResult,
_rhs_kind: &RhsAssignKind<'_>,
rhs_tactics: RhsTactics,
has_rhs_comment: bool,
) -> Option<String> {
) -> RewriteResult {
match orig_rhs {
Some(ref new_str) if new_str.is_empty() => Some(String::new()),
Some(ref new_str)
if !new_str.contains('\n') && unicode_str_width(new_str) <= shape.width =>
{
Some(format!(" {new_str}"))
Ok(ref new_str) if new_str.is_empty() => Ok(String::new()),
Ok(ref new_str) if !new_str.contains('\n') && unicode_str_width(new_str) <= shape.width => {
Ok(format!(" {new_str}"))
}
_ => {
// Expression did not fit on the same line as the identifier.
// Try splitting the line and see if that works better.
let new_shape = shape_from_rhs_tactic(context, shape, rhs_tactics)?;
let new_rhs = expr.rewrite(context, new_shape);
let new_shape = shape_from_rhs_tactic(context, shape, rhs_tactics)
// TODO(ding-young) Ideally, we can replace unknown_error() with max_width_error(),
// but this requires either implementing the Spanned trait for ast::GenericBounds
// or grabbing the span from the call site.
.unknown_error()?;
let new_rhs = expr.rewrite_result(context, new_shape);
let new_indent_str = &shape
.indent
.block_indent(context.config)
.to_string_with_newline(context.config);
let before_space_str = if has_rhs_comment { "" } else { " " };

match (orig_rhs, new_rhs) {
(Some(ref orig_rhs), Some(ref new_rhs))
(Ok(ref orig_rhs), Ok(ref new_rhs))
if !filtered_str_fits(&new_rhs, context.config.max_width(), new_shape) =>
{
Some(format!("{before_space_str}{orig_rhs}"))
Ok(format!("{before_space_str}{orig_rhs}"))
}
(Some(ref orig_rhs), Some(ref new_rhs))
(Ok(ref orig_rhs), Ok(ref new_rhs))
if prefer_next_line(orig_rhs, new_rhs, rhs_tactics) =>
{
Some(format!("{new_indent_str}{new_rhs}"))
Ok(format!("{new_indent_str}{new_rhs}"))
}
(None, Some(ref new_rhs)) => Some(format!("{new_indent_str}{new_rhs}")),
(None, None) if rhs_tactics == RhsTactics::AllowOverflow => {
(Err(_), Ok(ref new_rhs)) => Ok(format!("{new_indent_str}{new_rhs}")),
(Err(_), Err(_)) if rhs_tactics == RhsTactics::AllowOverflow => {
let shape = shape.infinite_width();
expr.rewrite(context, shape)
expr.rewrite_result(context, shape)
.map(|s| format!("{}{}", before_space_str, s))
}
(None, None) => None,
(Some(orig_rhs), _) => Some(format!("{before_space_str}{orig_rhs}")),
// When both orig_rhs and new_rhs 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_rhs_err)) => Err(new_rhs_err),
(Ok(orig_rhs), _) => Ok(format!("{before_space_str}{orig_rhs}")),
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ impl Rewrite for ast::Local {
init,
&RhsAssignKind::Expr(&init.kind, init.span),
nested_shape,
)
.max_width_error(shape.width, self.span())?;
)?;

if let Some(block) = else_block {
let else_kw_span = init.span.between(block.span);
Expand Down Expand Up @@ -711,7 +710,8 @@ impl<'a> FmtVisitor<'a> {
shape,
&RhsAssignKind::Expr(&ex.kind, ex.span),
RhsTactics::AllowOverflow,
)?
)
.ok()?
} else {
variant_body
};
Expand Down Expand Up @@ -1206,7 +1206,8 @@ pub(crate) fn format_trait(
shape,
&RhsAssignKind::Bounds,
RhsTactics::ForceNextLineWithoutIndent,
)?;
)
.ok()?;
}

// Rewrite where-clause.
Expand Down Expand Up @@ -1396,6 +1397,7 @@ pub(crate) fn format_trait_alias(
shape.sub_width(1)?,
)
.map(|s| s + ";")
.ok()
}

fn format_unit_struct(
Expand Down Expand Up @@ -1835,7 +1837,9 @@ fn rewrite_ty<R: Rewrite>(

// 1 = `;`
let shape = Shape::indented(indent, context.config).sub_width(1)?;
rewrite_assign_rhs(context, lhs, &*ty, &RhsAssignKind::Ty, shape).map(|s| s + ";")
rewrite_assign_rhs(context, lhs, &*ty, &RhsAssignKind::Ty, shape)
.map(|s| s + ";")
.ok()
} else {
Some(format!("{result};"))
}
Expand Down Expand Up @@ -1931,8 +1935,7 @@ pub(crate) fn rewrite_struct_field(

let is_prefix_empty = prefix.is_empty();
// We must use multiline. We are going to put attributes and a field on different lines.
let field_str = rewrite_assign_rhs(context, prefix, &*field.ty, &RhsAssignKind::Ty, shape)
.unknown_error()?;
let field_str = rewrite_assign_rhs(context, prefix, &*field.ty, &RhsAssignKind::Ty, shape)?;
// Remove a leading white-space from `rewrite_assign_rhs()` when rewriting a tuple struct.
let field_str = if is_prefix_empty {
field_str.trim_start()
Expand Down Expand Up @@ -3421,6 +3424,7 @@ impl Rewrite for ast::ForeignItem {
shape.sub_width(1)?,
)
.map(|s| s + ";")
.ok()
}
ast::ForeignItemKind::TyAlias(ref ty_alias) => {
let (kind, span) = (&ItemVisitorKind::ForeignItem(self), self.span);
Expand Down
21 changes: 9 additions & 12 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,18 +1437,15 @@ fn format_lazy_static(
id,
ty.rewrite_result(context, nested_shape)?
));
result.push_str(
&rewrite_assign_rhs(
context,
stmt,
&*expr,
&RhsAssignKind::Expr(&expr.kind, expr.span),
nested_shape
.sub_width(1)
.max_width_error(nested_shape.width, expr.span)?,
)
.unknown_error()?,
);
result.push_str(&rewrite_assign_rhs(
context,
stmt,
&*expr,
&RhsAssignKind::Expr(&expr.kind, expr.span),
nested_shape
.sub_width(1)
.max_width_error(nested_shape.width, expr.span)?,
)?);
result.push(';');
if i != last {
result.push_str(&nested_shape.indent.to_string_with_newline(context.config));
Expand Down
5 changes: 3 additions & 2 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ impl Rewrite for ast::WherePredicate {
format!("{type_str}{colon}")
};

rewrite_assign_rhs(context, lhs, bounds, &RhsAssignKind::Bounds, shape)?
rewrite_assign_rhs(context, lhs, bounds, &RhsAssignKind::Bounds, shape).ok()?
}
ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate {
ref lifetime,
Expand All @@ -488,7 +488,8 @@ impl Rewrite for ast::WherePredicate {
..
}) => {
let lhs_ty_str = lhs_ty.rewrite(context, shape).map(|lhs| lhs + " =")?;
rewrite_assign_rhs(context, lhs_ty_str, &**rhs_ty, &RhsAssignKind::Ty, shape)?
rewrite_assign_rhs(context, lhs_ty_str, &**rhs_ty, &RhsAssignKind::Ty, shape)
.ok()?
}
};

Expand Down

0 comments on commit 1930514

Please sign in to comment.