From 193051438d7083f465f68c5fdca86c5e02b00a8f Mon Sep 17 00:00:00 2001 From: ding-young Date: Wed, 28 Aug 2024 13:24:17 +0900 Subject: [PATCH] update rewrite_assignment to return RewriteResult --- src/expr.rs | 70 +++++++++++++++++++++++++++++---------------------- src/items.rs | 18 +++++++------ src/macros.rs | 21 +++++++--------- src/types.rs | 5 ++-- 4 files changed, 63 insertions(+), 51 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 02372e7be13..35b7bada92d 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -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 { @@ -2050,15 +2050,21 @@ fn rewrite_assignment( rhs: &ast::Expr, op: Option<&ast::BinOp>, shape: Shape, -) -> Option { +) -> 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, @@ -2089,7 +2095,7 @@ pub(crate) fn rewrite_assign_rhs, R: Rewrite>( ex: &R, rhs_kind: &RhsAssignKind<'_>, shape: Shape, -) -> Option { +) -> RewriteResult { rewrite_assign_rhs_with(context, lhs, ex, shape, rhs_kind, RhsTactics::Default) } @@ -2100,7 +2106,7 @@ pub(crate) fn rewrite_assign_rhs_expr( shape: Shape, rhs_kind: &RhsAssignKind<'_>, rhs_tactics: RhsTactics, -) -> Option { +) -> RewriteResult { let last_line_width = last_line_width(lhs).saturating_sub(if lhs.contains('\n') { shape.indent.width() } else { @@ -2122,7 +2128,7 @@ pub(crate) fn rewrite_assign_rhs_expr( context, ex, orig_shape, - ex.rewrite(context, orig_shape), + ex.rewrite_result(context, orig_shape), rhs_kind, rhs_tactics, has_rhs_comment, @@ -2136,10 +2142,10 @@ pub(crate) fn rewrite_assign_rhs_with, R: Rewrite>( shape: Shape, rhs_kind: &RhsAssignKind<'_>, rhs_tactics: RhsTactics, -) -> Option { +) -> 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, R: Rewrite + Spanned>( @@ -2161,8 +2167,7 @@ pub(crate) fn rewrite_assign_rhs_with_comments, 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) @@ -2175,23 +2180,25 @@ fn choose_rhs( context: &RewriteContext<'_>, expr: &R, shape: Shape, - orig_rhs: Option, + orig_rhs: RewriteResult, _rhs_kind: &RhsAssignKind<'_>, rhs_tactics: RhsTactics, has_rhs_comment: bool, -) -> Option { +) -> 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) @@ -2199,24 +2206,27 @@ fn choose_rhs( 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}")), } } } diff --git a/src/items.rs b/src/items.rs index 3894ee2cdf8..4ea9751e816 100644 --- a/src/items.rs +++ b/src/items.rs @@ -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); @@ -711,7 +710,8 @@ impl<'a> FmtVisitor<'a> { shape, &RhsAssignKind::Expr(&ex.kind, ex.span), RhsTactics::AllowOverflow, - )? + ) + .ok()? } else { variant_body }; @@ -1206,7 +1206,8 @@ pub(crate) fn format_trait( shape, &RhsAssignKind::Bounds, RhsTactics::ForceNextLineWithoutIndent, - )?; + ) + .ok()?; } // Rewrite where-clause. @@ -1396,6 +1397,7 @@ pub(crate) fn format_trait_alias( shape.sub_width(1)?, ) .map(|s| s + ";") + .ok() } fn format_unit_struct( @@ -1835,7 +1837,9 @@ fn rewrite_ty( // 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};")) } @@ -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() @@ -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); diff --git a/src/macros.rs b/src/macros.rs index 51ded869229..5e537002324 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -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)); diff --git a/src/types.rs b/src/types.rs index 76eb0ea0529..d1cd291d51f 100644 --- a/src/types.rs +++ b/src/types.rs @@ -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, @@ -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()? } };