Skip to content

Commit

Permalink
refactor rewrite_block
Browse files Browse the repository at this point in the history
  • Loading branch information
ding-young authored and ytmimi committed Jul 15, 2024
1 parent add0459 commit bb56224
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 42 deletions.
3 changes: 2 additions & 1 deletion src/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ fn rewrite_closure_with_block(
None,
shape,
false,
)?;
)
.ok()?;
Some(format!("{prefix} {block}"))
}

Expand Down
81 changes: 44 additions & 37 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::macros::{rewrite_macro, MacroPosition};
use crate::matches::rewrite_match;
use crate::overflow::{self, IntoOverflowableItem, OverflowableItem};
use crate::pairs::{rewrite_all_pairs, rewrite_pair, PairParts};
use crate::rewrite::{Rewrite, RewriteContext};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
use crate::shape::{Indent, Shape};
use crate::source_map::{LineRangeUtils, SpanUtils};
use crate::spanned::Spanned;
Expand Down Expand Up @@ -145,7 +145,7 @@ pub(crate) fn format_expr(
// not the `ast::Block` node we're about to rewrite. To prevent dropping inner
// attributes call `rewrite_block` directly.
// See https://github.com/rust-lang/rustfmt/issues/6158
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape)?
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok()?
}
_ => anon_const.rewrite(context, shape)?,
};
Expand All @@ -155,7 +155,7 @@ pub(crate) fn format_expr(
match expr_type {
ExprType::Statement => {
if is_unsafe_block(block) {
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape)
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok()
} else if let rw @ Some(_) =
rewrite_empty_block(context, block, Some(&expr.attrs), opt_label, "", shape)
{
Expand All @@ -173,10 +173,11 @@ pub(crate) fn format_expr(
shape,
true,
)
.ok()
}
}
ExprType::SubExpression => {
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape)
rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok()
}
}
}
Expand Down Expand Up @@ -352,10 +353,10 @@ pub(crate) fn format_expr(
// https://github.com/rust-dev-tools/fmt-rfcs/issues/152
ast::ExprKind::InlineAsm(..) => Some(context.snippet(expr.span).to_owned()),
ast::ExprKind::TryBlock(ref block) => {
if let rw @ Some(_) =
if let rw @ Ok(_) =
rewrite_single_line_block(context, "try ", block, Some(&expr.attrs), None, shape)
{
rw
rw.ok()
} else {
// 9 = `try `
let budget = shape.width.saturating_sub(9);
Expand All @@ -368,7 +369,8 @@ pub(crate) fn format_expr(
None,
context,
Shape::legacy(budget, shape.indent)
)?
)
.ok()?
))
}
}
Expand All @@ -378,15 +380,15 @@ pub(crate) fn format_expr(
} else {
""
};
if let rw @ Some(_) = rewrite_single_line_block(
if let rw @ Ok(_) = rewrite_single_line_block(
context,
format!("{kind} {mover}").as_str(),
block,
Some(&expr.attrs),
None,
shape,
) {
rw
rw.ok()
} else {
// 6 = `async `
let budget = shape.width.saturating_sub(6);
Expand All @@ -398,7 +400,8 @@ pub(crate) fn format_expr(
None,
context,
Shape::legacy(budget, shape.indent)
)?
)
.ok()?
))
}
}
Expand Down Expand Up @@ -522,17 +525,19 @@ fn rewrite_single_line_block(
attrs: Option<&[ast::Attribute]>,
label: Option<ast::Label>,
shape: Shape,
) -> Option<String> {
) -> RewriteResult {
if let Some(block_expr) = stmt::Stmt::from_simple_block(context, block, attrs) {
let expr_shape = shape.offset_left(last_line_width(prefix))?;
let expr_str = block_expr.rewrite(context, expr_shape)?;
let expr_shape = shape
.offset_left(last_line_width(prefix))
.max_width_error(shape.width, block_expr.span())?;
let expr_str = block_expr.rewrite_result(context, expr_shape)?;
let label_str = rewrite_label(label);
let result = format!("{prefix}{label_str}{{ {expr_str} }}");
if result.len() <= shape.width && !result.contains('\n') {
return Some(result);
return Ok(result);
}
}
None
Err(RewriteError::Unknown)
}

pub(crate) fn rewrite_block_with_visitor(
Expand All @@ -543,9 +548,9 @@ pub(crate) fn rewrite_block_with_visitor(
label: Option<ast::Label>,
shape: Shape,
has_braces: bool,
) -> Option<String> {
if let rw @ Some(_) = rewrite_empty_block(context, block, attrs, label, prefix, shape) {
return rw;
) -> RewriteResult {
if let Some(rw_str) = rewrite_empty_block(context, block, attrs, label, prefix, shape) {
return Ok(rw_str);
}

let mut visitor = FmtVisitor::from_context(context);
Expand All @@ -554,7 +559,7 @@ pub(crate) fn rewrite_block_with_visitor(
match (block.rules, label) {
(ast::BlockCheckMode::Unsafe(..), _) | (ast::BlockCheckMode::Default, Some(_)) => {
let snippet = context.snippet(block.span);
let open_pos = snippet.find_uncommented("{")?;
let open_pos = snippet.find_uncommented("{").unknown_error()?;
visitor.last_pos = block.span.lo() + BytePos(open_pos as u32)
}
(ast::BlockCheckMode::Default, None) => visitor.last_pos = block.span.lo(),
Expand All @@ -568,11 +573,15 @@ pub(crate) fn rewrite_block_with_visitor(
.skipped_range
.borrow_mut()
.append(&mut visitor_context.skipped_range.borrow_mut());
Some(format!("{}{}{}", prefix, label_str, visitor.buffer))
Ok(format!("{}{}{}", prefix, label_str, visitor.buffer))
}

impl Rewrite for ast::Block {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
rewrite_block(self, None, None, context, shape)
}
}
Expand All @@ -583,7 +592,7 @@ fn rewrite_block(
label: Option<ast::Label>,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
) -> RewriteResult {
rewrite_block_inner(block, attrs, label, true, context, shape)
}

Expand All @@ -594,27 +603,24 @@ fn rewrite_block_inner(
allow_single_line: bool,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
let prefix = block_prefix(context, block, shape)?;
) -> RewriteResult {
let prefix = block_prefix(context, block, shape).unknown_error()?;

// shape.width is used only for the single line case: either the empty block `{}`,
// or an unsafe expression `unsafe { e }`.
if let rw @ Some(_) = rewrite_empty_block(context, block, attrs, label, &prefix, shape) {
return rw;
if let Some(rw_str) = rewrite_empty_block(context, block, attrs, label, &prefix, shape) {
return Ok(rw_str);
}

let result = rewrite_block_with_visitor(context, &prefix, block, attrs, label, shape, true);
if let Some(ref result_str) = result {
if allow_single_line && result_str.lines().count() <= 3 {
if let rw @ Some(_) =
rewrite_single_line_block(context, &prefix, block, attrs, label, shape)
{
return rw;
}
let result_str =
rewrite_block_with_visitor(context, &prefix, block, attrs, label, shape, true)?;
if allow_single_line && result_str.lines().count() <= 3 {
if let rw @ Ok(_) = rewrite_single_line_block(context, &prefix, block, attrs, label, shape)
{
return rw;
}
}

result
Ok(result_str)
}

/// Rewrite the divergent block of a `let-else` statement.
Expand All @@ -623,7 +629,7 @@ pub(crate) fn rewrite_let_else_block(
allow_single_line: bool,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
) -> RewriteResult {
rewrite_block_inner(block, None, None, allow_single_line, context, shape)
}

Expand Down Expand Up @@ -1112,7 +1118,8 @@ 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);
rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true)
.ok();
context.is_if_else_block.replace(old_val);
result?
};
Expand Down
6 changes: 2 additions & 4 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ impl Rewrite for ast::Local {
&& allow_single_line_let_else_block(assign_str_with_else_kw, block);

let mut rw_else_block =
rewrite_let_else_block(block, allow_single_line, context, shape)
.unknown_error()?;
rewrite_let_else_block(block, allow_single_line, context, shape)?;

let single_line_else = !rw_else_block.contains('\n');
// +1 for the trailing `;`
Expand All @@ -191,8 +190,7 @@ impl Rewrite for ast::Local {
if allow_single_line && single_line_else && else_block_exceeds_width {
// writing this on one line would exceed the available width
// so rewrite the else block over multiple lines.
rw_else_block =
rewrite_let_else_block(block, false, context, shape).unknown_error()?;
rw_else_block = rewrite_let_else_block(block, false, context, shape)?;
}

result.push_str(&rw_else_block);
Expand Down

0 comments on commit bb56224

Please sign in to comment.