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

Implement let-else formatting #5690

Merged
merged 8 commits into from
Jun 20, 2023
107 changes: 72 additions & 35 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,17 @@ fn rewrite_block(
label: Option<ast::Label>,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
rewrite_block_inner(block, attrs, label, true, context, shape)
}

fn rewrite_block_inner(
block: &ast::Block,
attrs: Option<&[ast::Attribute]>,
label: Option<ast::Label>,
allow_single_line: bool,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
let prefix = block_prefix(context, block, shape)?;

Expand All @@ -586,7 +597,7 @@ fn rewrite_block(

let result = rewrite_block_with_visitor(context, &prefix, block, attrs, label, shape, true);
if let Some(ref result_str) = result {
if result_str.lines().count() <= 3 {
if allow_single_line && result_str.lines().count() <= 3 {
if let rw @ Some(_) =
rewrite_single_line_block(context, &prefix, block, attrs, label, shape)
{
Expand All @@ -598,6 +609,16 @@ fn rewrite_block(
result
}

/// Rewrite the divergent block of a `let-else` statement.
pub(crate) fn rewrite_let_else_block(
block: &ast::Block,
allow_single_line: bool,
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
rewrite_block_inner(block, None, None, allow_single_line, context, shape)
}

// Rewrite condition if the given expression has one.
pub(crate) fn rewrite_cond(
context: &RewriteContext<'_>,
Expand Down Expand Up @@ -1004,6 +1025,49 @@ impl<'a> ControlFlow<'a> {
}
}

/// Rewrite the `else` keyword with surrounding comments.
///
/// force_newline_else: whether or not to rewrite the `else` keyword on a newline.
/// is_last: true if this is an `else` and `false` if this is an `else if` block.
/// context: rewrite context
/// span: Span between the end of the last expression and the start of the else block,
/// which contains the `else` keyword
/// shape: Shape
pub(crate) fn rewrite_else_kw_with_comments(
force_newline_else: bool,
is_last: bool,
context: &RewriteContext<'_>,
span: Span,
shape: Shape,
) -> String {
let else_kw_lo = context.snippet_provider.span_before(span, "else");
let before_else_kw = mk_sp(span.lo(), else_kw_lo);
let before_else_kw_comment = extract_comment(before_else_kw, context, shape);

let else_kw_hi = context.snippet_provider.span_after(span, "else");
let after_else_kw = mk_sp(else_kw_hi, span.hi());
let after_else_kw_comment = extract_comment(after_else_kw, context, shape);

let newline_sep = &shape.indent.to_string_with_newline(context.config);
let before_sep = match context.config.control_brace_style() {
_ if force_newline_else => newline_sep.as_ref(),
ControlBraceStyle::AlwaysNextLine | ControlBraceStyle::ClosingNextLine => {
newline_sep.as_ref()
}
ControlBraceStyle::AlwaysSameLine => " ",
};
let after_sep = match context.config.control_brace_style() {
ControlBraceStyle::AlwaysNextLine if is_last => newline_sep.as_ref(),
_ => " ",
};

format!(
"{}else{}",
before_else_kw_comment.as_ref().map_or(before_sep, |s| &**s),
after_else_kw_comment.as_ref().map_or(after_sep, |s| &**s),
)
}

impl<'a> Rewrite for ControlFlow<'a> {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
debug!("ControlFlow::rewrite {:?} {:?}", self, shape);
Expand Down Expand Up @@ -1070,41 +1134,14 @@ impl<'a> Rewrite for ControlFlow<'a> {
}
};

let between_kwd_else_block = mk_sp(
self.block.span.hi(),
context
.snippet_provider
.span_before(mk_sp(self.block.span.hi(), else_block.span.lo()), "else"),
);
let between_kwd_else_block_comment =
extract_comment(between_kwd_else_block, context, shape);

let after_else = mk_sp(
context
.snippet_provider
.span_after(mk_sp(self.block.span.hi(), else_block.span.lo()), "else"),
else_block.span.lo(),
let else_kw = rewrite_else_kw_with_comments(
false,
last_in_chain,
context,
self.block.span.between(else_block.span),
shape,
);
let after_else_comment = extract_comment(after_else, context, shape);

let between_sep = match context.config.control_brace_style() {
ControlBraceStyle::AlwaysNextLine | ControlBraceStyle::ClosingNextLine => {
&*alt_block_sep
}
ControlBraceStyle::AlwaysSameLine => " ",
};
let after_sep = match context.config.control_brace_style() {
ControlBraceStyle::AlwaysNextLine if last_in_chain => &*alt_block_sep,
_ => " ",
};

result.push_str(&format!(
"{}else{}",
between_kwd_else_block_comment
.as_ref()
.map_or(between_sep, |s| &**s),
after_else_comment.as_ref().map_or(after_sep, |s| &**s),
));
result.push_str(&else_kw);
result.push_str(&rewrite?);
}

Expand Down
94 changes: 89 additions & 5 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use crate::config::lists::*;
use crate::config::{BraceStyle, Config, IndentStyle, Version};
use crate::expr::{
is_empty_block, is_simple_block_stmt, rewrite_assign_rhs, rewrite_assign_rhs_with,
rewrite_assign_rhs_with_comments, RhsAssignKind, RhsTactics,
rewrite_assign_rhs_with_comments, rewrite_else_kw_with_comments, rewrite_let_else_block,
RhsAssignKind, RhsTactics,
};
use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator};
use crate::macros::{rewrite_macro, MacroPosition};
Expand All @@ -44,7 +45,7 @@ fn type_annotation_separator(config: &Config) -> &str {
}

// Statements of the form
// let pat: ty = init;
// let pat: ty = init; or let pat: ty = init else { .. };
impl Rewrite for ast::Local {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
debug!(
Expand All @@ -54,7 +55,7 @@ impl Rewrite for ast::Local {

skip_out_of_file_lines_range!(context, self.span);

if contains_skip(&self.attrs) || matches!(self.kind, ast::LocalKind::InitElse(..)) {
if contains_skip(&self.attrs) {
return None;
}

Expand Down Expand Up @@ -112,7 +113,7 @@ impl Rewrite for ast::Local {

result.push_str(&infix);

if let Some((init, _els)) = self.kind.init_else_opt() {
if let Some((init, else_block)) = self.kind.init_else_opt() {
// 1 = trailing semicolon;
let nested_shape = shape.sub_width(1)?;

Expand All @@ -123,14 +124,97 @@ impl Rewrite for ast::Local {
&RhsAssignKind::Expr(&init.kind, init.span),
nested_shape,
)?;
// todo else

if let Some(block) = else_block {
let else_kw_span = init.span.between(block.span);
let force_newline_else =
!same_line_else_kw_and_brace(&result, context, else_kw_span, nested_shape);
let else_kw = rewrite_else_kw_with_comments(
force_newline_else,
true,
context,
else_kw_span,
shape,
);
result.push_str(&else_kw);

let allow_single_line = allow_single_line_let_else_block(&result, block);

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

if allow_single_line && !rw_else_block.contains('\n') {
let available_space = shape.width.saturating_sub(result.len());
if available_space <= rw_else_block.len() {
// writing this on one line would exceed the available width
rw_else_block = rewrite_let_else_block(block, false, context, shape)?;
}
}

result.push_str(&rw_else_block);
};
}

result.push(';');
Some(result)
}
}

/// When the initializer expression is multi-lined, then the else keyword and opening brace of the
/// block ( i.e. "else {") should be put on the same line as the end of the initializer expression
/// if all the following are true:
///
/// 1. The initializer expression ends with one or more closing parentheses, square brackets,
/// or braces
/// 2. There is nothing else on that line
/// 3. That line is not indented beyond the indent on the first line of the let keyword
fn same_line_else_kw_and_brace(
init_str: &str,
context: &RewriteContext<'_>,
else_kw_span: Span,
init_shape: Shape,
) -> bool {
if !init_str.contains('\n') {
// initializer expression is single lined. The "else {" can only be placed on the same line
// as the initializer expression if there is enough room for it.
// 7 = ` else {`
return init_shape.width.saturating_sub(init_str.len()) >= 7;
}

// 1. The initializer expression ends with one or more `)`, `]`, `}`.
if !init_str.ends_with([')', ']', '}']) {
return false;
}

// 2. There is nothing else on that line
// For example, there are no comments
let else_kw_snippet = context.snippet(else_kw_span).trim();
if else_kw_snippet != "else" {
return false;
}

// 3. The last line of the initializer expression is not indented beyond the `let` keyword
let indent = init_shape.indent.to_string(context.config);
init_str
.lines()
.last()
.expect("initializer expression is multi-lined")
.strip_prefix(indent.as_ref())
.map_or(false, |l| !l.starts_with(char::is_whitespace))
}

fn allow_single_line_let_else_block(result: &str, block: &ast::Block) -> bool {
if result.contains('\n') {
return false;
}

if block.stmts.len() <= 1 {
return true;
}

false
}

// FIXME convert to using rewrite style rather than visitor
// FIXME format modules in this style
#[allow(dead_code)]
Expand Down
Loading