Skip to content
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

impl rewrite_result for ControlFlow, Stmt, update rewrite_index #6291

Merged
merged 1 commit into from
Sep 1, 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
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`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before updating recover_comment_removed, it always returned Some(String). The previous comment about this function is no longer accurate, as it was written when the function used to call wrap_str a long time ago.

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
Loading