Skip to content

Commit

Permalink
impl rewrite_result for ControlFlow, Stmt, update rewrite_index
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 1930514 commit 833d4d1
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 60 deletions.
7 changes: 3 additions & 4 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1703,12 +1703,11 @@ impl<'a> Iterator for CommentCodeSlices<'a> {
}

/// Checks is `new` didn't miss any comment from `span`, if it removed any, return previous text
/// (if it fits in the width/offset, else return `None`), else return `new`
pub(crate) fn recover_comment_removed(
new: String,
span: Span,
context: &RewriteContext<'_>,
) -> Option<String> {
) -> String {
let snippet = context.snippet(span);
if snippet != new && changed_comment_content(snippet, &new) {
// We missed some comments. Warn and keep the original text.
Expand All @@ -1722,9 +1721,9 @@ pub(crate) fn recover_comment_removed(
)],
);
}
Some(snippet.to_owned())
snippet.to_owned()
} else {
Some(new)
new
}
}

Expand Down
85 changes: 51 additions & 34 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ pub(crate) fn format_expr(
)
.ok(),
ast::ExprKind::Index(ref expr, ref index, _) => {
rewrite_index(&**expr, &**index, context, shape)
rewrite_index(&**expr, &**index, context, shape).ok()
}
ast::ExprKind::Repeat(ref expr, ref repeats) => rewrite_pair(
&**expr,
Expand Down Expand Up @@ -435,7 +435,7 @@ pub(crate) fn format_expr(
};

expr_rw
.and_then(|expr_str| recover_comment_removed(expr_str, expr.span, context))
.map(|expr_str| recover_comment_removed(expr_str, expr.span, context))
.and_then(|expr_str| {
let attrs = outer_attributes(&expr.attrs);
let attrs_str = attrs.rewrite(context, shape)?;
Expand Down Expand Up @@ -672,6 +672,7 @@ pub(crate) fn rewrite_cond(
String::from("\n") + &shape.indent.block_only().to_string(context.config);
control_flow
.rewrite_cond(context, shape, &alt_block_sep)
.ok()
.map(|rw| rw.0)
}),
}
Expand Down Expand Up @@ -896,20 +897,23 @@ impl<'a> ControlFlow<'a> {
expr: &ast::Expr,
shape: Shape,
offset: usize,
) -> Option<String> {
) -> RewriteResult {
debug!("rewrite_pat_expr {:?} {:?} {:?}", shape, self.pat, expr);

let cond_shape = shape.offset_left(offset)?;
let cond_shape = shape
.offset_left(offset)
.max_width_error(shape.width, expr.span)?;
if let Some(pat) = self.pat {
let matcher = if self.matcher.is_empty() {
self.matcher.to_owned()
} else {
format!("{} ", self.matcher)
};
let pat_shape = cond_shape
.offset_left(matcher.len())?
.sub_width(self.connector.len())?;
let pat_string = pat.rewrite(context, pat_shape)?;
.offset_left(matcher.len())
.and_then(|s| s.sub_width(self.connector.len()))
.max_width_error(cond_shape.width, pat.span)?;
let pat_string = pat.rewrite_result(context, pat_shape)?;
let comments_lo = context
.snippet_provider
.span_after(self.span.with_lo(pat.span.hi()), self.connector.trim());
Expand All @@ -923,14 +927,13 @@ impl<'a> ControlFlow<'a> {
RhsTactics::Default,
comments_span,
true,
)
.ok();
);
}

let expr_rw = expr.rewrite(context, cond_shape);
let expr_rw = expr.rewrite_result(context, cond_shape);
// The expression may (partially) fit on the current line.
// We do not allow splitting between `if` and condition.
if self.keyword == "if" || expr_rw.is_some() {
if self.keyword == "if" || expr_rw.is_ok() {
return expr_rw;
}

Expand All @@ -939,7 +942,7 @@ impl<'a> ControlFlow<'a> {
.block_indent(context.config.tab_spaces())
.with_max_width(context.config);
let nested_indent_str = nested_shape.indent.to_string_with_newline(context.config);
expr.rewrite(context, nested_shape)
expr.rewrite_result(context, nested_shape)
.map(|expr_rw| format!("{}{}", nested_indent_str, expr_rw))
}

Expand All @@ -948,7 +951,7 @@ impl<'a> ControlFlow<'a> {
context: &RewriteContext<'_>,
shape: Shape,
alt_block_sep: &str,
) -> Option<(String, usize)> {
) -> Result<(String, usize), RewriteError> {
// Do not take the rhs overhead from the upper expressions into account
// when rewriting pattern.
let new_width = context.budget(shape.used_width());
Expand All @@ -959,7 +962,9 @@ impl<'a> ControlFlow<'a> {
let constr_shape = if self.nested_if {
// We are part of an if-elseif-else chain. Our constraints are tightened.
// 7 = "} else " .len()
fresh_shape.offset_left(7)?
fresh_shape
.offset_left(7)
.max_width_error(fresh_shape.width, self.span)?
} else {
fresh_shape
};
Expand Down Expand Up @@ -995,7 +1000,7 @@ impl<'a> ControlFlow<'a> {

if let Some(cond_str) = trial {
if cond_str.len() <= context.config.single_line_if_else_max_width() {
return Some((cond_str, 0));
return Ok((cond_str, 0));
}
}
}
Expand Down Expand Up @@ -1048,7 +1053,7 @@ impl<'a> ControlFlow<'a> {
label_string.len() + self.keyword.len() + pat_expr_string.len() + 2
};

Some((
Ok((
format!(
"{}{}{}{}{}",
label_string,
Expand Down Expand Up @@ -1114,13 +1119,17 @@ pub(crate) fn rewrite_else_kw_with_comments(

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

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
debug!("ControlFlow::rewrite {:?} {:?}", self, shape);

let alt_block_sep = &shape.indent.to_string_with_newline(context.config);
let (cond_str, used_width) = self.rewrite_cond(context, shape, alt_block_sep)?;
// If `used_width` is 0, it indicates that whole control flow is written in a single line.
if used_width == 0 {
return Some(cond_str);
return Ok(cond_str);
}

let block_width = shape.width.saturating_sub(used_width);
Expand All @@ -1138,8 +1147,7 @@ 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)
.ok();
rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true);
context.is_if_else_block.replace(old_val);
result?
};
Expand All @@ -1165,7 +1173,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
true,
mk_sp(else_block.span.lo(), self.span.hi()),
)
.rewrite(context, shape)
.rewrite_result(context, shape)
}
_ => {
last_in_chain = true;
Expand All @@ -1176,6 +1184,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
..shape
};
format_expr(else_block, ExprType::Statement, context, else_shape)
.unknown_error()
}
};

Expand All @@ -1190,7 +1199,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
result.push_str(&rewrite?);
}

Some(result)
Ok(result)
}
}

Expand Down Expand Up @@ -1567,8 +1576,8 @@ fn rewrite_index(
index: &ast::Expr,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
let expr_str = expr.rewrite(context, shape)?;
) -> RewriteResult {
let expr_str = expr.rewrite_result(context, shape)?;

let offset = last_line_width(&expr_str) + 1;
let rhs_overhead = shape.rhs_overhead(context.config);
Expand All @@ -1583,37 +1592,45 @@ fn rewrite_index(
.and_then(|shape| shape.sub_width(1)),
IndentStyle::Visual => shape.visual_indent(offset).sub_width(offset + 1),
}
};
let orig_index_rw = index_shape.and_then(|s| index.rewrite(context, s));
}
.max_width_error(shape.width, index.span());
let orig_index_rw = index_shape.and_then(|s| index.rewrite_result(context, s));

// Return if index fits in a single line.
match orig_index_rw {
Some(ref index_str) if !index_str.contains('\n') => {
return Some(format!("{expr_str}[{index_str}]"));
Ok(ref index_str) if !index_str.contains('\n') => {
return Ok(format!("{expr_str}[{index_str}]"));
}
_ => (),
}

// Try putting index on the next line and see if it fits in a single line.
let indent = shape.indent.block_indent(context.config);
let index_shape = Shape::indented(indent, context.config).offset_left(1)?;
let index_shape = index_shape.sub_width(1 + rhs_overhead)?;
let new_index_rw = index.rewrite(context, index_shape);
let index_shape = Shape::indented(indent, context.config)
.offset_left(1)
.max_width_error(shape.width, index.span())?;
let index_shape = index_shape
.sub_width(1 + rhs_overhead)
.max_width_error(index_shape.width, index.span())?;
let new_index_rw = index.rewrite_result(context, index_shape);
match (orig_index_rw, new_index_rw) {
(_, Some(ref new_index_str)) if !new_index_str.contains('\n') => Some(format!(
(_, Ok(ref new_index_str)) if !new_index_str.contains('\n') => Ok(format!(
"{}{}[{}]",
expr_str,
indent.to_string_with_newline(context.config),
new_index_str,
)),
(None, Some(ref new_index_str)) => Some(format!(
(Err(_), Ok(ref new_index_str)) => Ok(format!(
"{}{}[{}]",
expr_str,
indent.to_string_with_newline(context.config),
new_index_str,
)),
(Some(ref index_str), _) => Some(format!("{expr_str}[{index_str}]")),
_ => None,
(Ok(ref index_str), _) => Ok(format!("{expr_str}[{index_str}]")),
// When both orig_index_rw and new_index_rw 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_index_rw_err)) => Err(new_index_rw_err),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2077,7 +2077,7 @@ fn rewrite_static(
true,
)
.ok()
.and_then(|res| recover_comment_removed(res, static_parts.span, context))
.map(|res| recover_comment_removed(res, static_parts.span, context))
.map(|s| if s.ends_with(';') { s } else { s + ";" })
} else {
Some(format!("{prefix}{ty_str};"))
Expand Down
11 changes: 7 additions & 4 deletions src/overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::lists::{
};
use crate::macros::MacroArg;
use crate::patterns::{can_be_overflowed_pat, TuplePatField};
use crate::rewrite::{Rewrite, RewriteContext, RewriteErrorExt, RewriteResult};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
use crate::shape::Shape;
use crate::source_map::SpanUtils;
use crate::spanned::Spanned;
Expand Down Expand Up @@ -90,6 +90,10 @@ impl<'a> Rewrite for OverflowableItem<'a> {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
self.map(|item| item.rewrite(context, shape))
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
self.map(|item| item.rewrite_result(context, shape))
}
}

impl<'a> Spanned for OverflowableItem<'a> {
Expand Down Expand Up @@ -617,7 +621,7 @@ impl<'a> Context<'a> {
tactic
}

fn rewrite_items(&self) -> Option<(bool, String)> {
fn rewrite_items(&self) -> Result<(bool, String), RewriteError> {
let span = self.items_span();
debug!("items: {:?}", self.items);

Expand Down Expand Up @@ -661,7 +665,6 @@ impl<'a> Context<'a> {
.ends_with_newline(ends_with_newline);

write_list(&list_items, &fmt)
.ok()
.map(|items_str| (tactic == DefinitiveListTactic::Horizontal, items_str))
}

Expand Down Expand Up @@ -718,7 +721,7 @@ impl<'a> Context<'a> {
}

fn rewrite(&self, shape: Shape) -> RewriteResult {
let (extendable, items_str) = self.rewrite_items().unknown_error()?;
let (extendable, items_str) = self.rewrite_items()?;

// If we are using visual indent style and failed to format, retry with block indent.
if !self.context.use_block_indent()
Expand Down
30 changes: 22 additions & 8 deletions src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_span::Span;
use crate::comment::recover_comment_removed;
use crate::config::StyleEdition;
use crate::expr::{format_expr, is_simple_block, ExprType};
use crate::rewrite::{Rewrite, RewriteContext};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
use crate::shape::Shape;
use crate::source_map::LineRangeUtils;
use crate::spanned::Spanned;
Expand Down Expand Up @@ -90,6 +90,14 @@ impl<'a> Stmt<'a> {

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

fn rewrite_result(
&self,
context: &RewriteContext<'_>,
shape: Shape,
) -> crate::rewrite::RewriteResult {
let expr_type =
if context.config.style_edition() >= StyleEdition::Edition2024 && self.is_last_expr() {
ExprType::SubExpression
Expand All @@ -112,22 +120,28 @@ fn format_stmt(
stmt: &ast::Stmt,
expr_type: ExprType,
is_last_expr: bool,
) -> Option<String> {
skip_out_of_file_lines_range!(context, stmt.span());
) -> RewriteResult {
skip_out_of_file_lines_range_err!(context, stmt.span());

let result = match stmt.kind {
ast::StmtKind::Let(ref local) => local.rewrite(context, shape),
ast::StmtKind::Let(ref local) => local.rewrite_result(context, shape),
ast::StmtKind::Expr(ref ex) | ast::StmtKind::Semi(ref ex) => {
let suffix = if semicolon_for_stmt(context, stmt, is_last_expr) {
";"
} else {
""
};

let shape = shape.sub_width(suffix.len())?;
format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
let shape = shape
.sub_width(suffix.len())
.max_width_error(shape.width, ex.span())?;
format_expr(ex, expr_type, context, shape)
.map(|s| s + suffix)
.unknown_error()
}
ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => {
Err(RewriteError::Unknown)
}
ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => None,
};
result.and_then(|res| recover_comment_removed(res, stmt.span(), context))
result.map(|res| recover_comment_removed(res, stmt.span(), context))
}
Loading

0 comments on commit 833d4d1

Please sign in to comment.