diff --git a/src/expr.rs b/src/expr.rs index e179b4646ef..715be57ada8 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -575,6 +575,17 @@ fn rewrite_block( label: Option, context: &RewriteContext<'_>, shape: Shape, +) -> Option { + rewrite_block_inner(block, attrs, label, true, context, shape) +} + +fn rewrite_block_inner( + block: &ast::Block, + attrs: Option<&[ast::Attribute]>, + label: Option, + allow_single_line: bool, + context: &RewriteContext<'_>, + shape: Shape, ) -> Option { let prefix = block_prefix(context, block, shape)?; @@ -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) { @@ -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 { + 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<'_>, @@ -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 { debug!("ControlFlow::rewrite {:?} {:?}", self, shape); @@ -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?); } diff --git a/src/items.rs b/src/items.rs index 3ecdb5b4c60..97a76f6bb32 100644 --- a/src/items.rs +++ b/src/items.rs @@ -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}; @@ -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 { debug!( @@ -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; } @@ -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)?; @@ -123,7 +124,35 @@ 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(';'); @@ -131,6 +160,61 @@ impl Rewrite for ast::Local { } } +/// 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)] diff --git a/tests/source/let_else.rs b/tests/source/let_else.rs index a6e816fb524..9c02117c6e8 100644 --- a/tests/source/let_else.rs +++ b/tests/source/let_else.rs @@ -1,3 +1,151 @@ fn main() { - let Some(1) = Some(1) else { return }; + // Although this won't compile it still parses so make sure we can format empty else blocks + let Some(x) = opt else {}; + + // let-else may be formatted on a single line if they are "short" + // and only contain a single expression + let Some(x) = opt else { return }; + + let Some(x) = opt else { + return + }; + + let Some(x) = opt else { return; }; + + let Some(x) = opt else { + // nope + return; + }; + + let Some(x) = opt else { let y = 1; return y }; + + let Some(x) = y.foo("abc", fairly_long_identifier, "def", "123456", "string", "cheese") else { bar() }; + + let Some(x) = abcdef().foo("abc", some_really_really_really_long_ident, "ident", "123456").bar().baz().qux("fffffffffffffffff") else { foo_bar() }; +} + +fn with_comments_around_else_keyword() { + let Some(x) = opt /* pre else keyword block-comment */ else { return }; + + let Some(x) = opt else /* post else keyword block-comment */ { return }; + + let Some(x) = opt /* pre else keyword block-comment */ else /* post else keyword block-comment */ { return }; + + let Some(x) = opt // pre else keyword line-comment + else { return }; + + let Some(x) = opt else + // post else keyword line-comment + { return }; + + let Some(x) = opt // pre else keyword line-comment + else + // post else keyword line-comment + { return }; + +} + +fn unbreakable_initializer_expr_pre_formatting_let_else_length_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The formatting is left unchanged! + let Some(x) = some_really_really_really_really_really_really_really_long_name_A else { return }; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_long_name___B else {return}; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_long_name_____C else {some_divergent_function()}; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 101 (> max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_long_name__D else { return }; +} + +fn unbreakable_initializer_expr_pre_formatting_length_up_to_opening_brace_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init else {` is 99 (< max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_really_long_name___E else {return}; + + // Pre Formatting: + // The length of `(indent)let pat = init else {` is 101 (> max_width) + // Post Formatting: + // The else keyword and opening brace cannot fit on the same line as the initializer expr. + // They are formatted on the next line. + let Some(x) = some_really_really_really_really_really_really_really_really_long_name_____F else {return}; +} + +fn unbreakable_initializer_expr_pre_formatting_length_through_initializer_expr_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init` is 99 (< max_width) + // Post Formatting: + // The else keyword and opening brace cannot fit on the same line as the initializer expr. + // They are formatted on the next line. + let Some(x) = some_really_really_really_really_really_really_really_really_really_long_name___G else {return}; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 100 (max_width) + // Post Formatting: + // Break after the `=` and put the initializer expr on it's own line. + // Because the initializer expr is multi-lined the else is placed on it's own line. + let Some(x) = some_really_really_really_really_really_really_really_really_really_long_name____H else {return}; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 109 (> max_width) + // Post Formatting: + // Break after the `=` and put the initializer expr on it's own line. + // Because the initializer expr is multi-lined the else is placed on it's own line. + // The initializer expr has a length of 91, which when indented on the next line + // The `(indent)init` line has a lengh of 99. This is the max length that the `init` can be + // before we start running into max_width issues. I suspect this is becuase the shape is + // accounting for the `;` at the end of the `let-else` statement. + let Some(x) = some_really_really_really_really_really_really_really_really_really_really_long_name______I else {return}; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 110 (> max_width) + // Post Formatting: + // Max length issues prevent us from formatting. + // The initializer expr has a length of 92, which if it would be indented on the next line + // the `(indent)init` line has a lengh of 100 which == max_width of 100. + // One might expect formatting to succeed, but I suspect the reason we hit max_width issues is + // because the Shape is accounting for the `;` at the end of the `let-else` statement. + let Some(x) = some_really_really_really_really_really_really_really_really_really_really_really_long_nameJ else {return}; +} + +fn long_patterns() { + let Foo {x: Bar(..), y: FooBar(..), z: Baz(..)} = opt else { + return; + }; + + // with version=One we don't wrap long array patterns + let [aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbb, cccccccccccccccccc, dddddddddddddddddd] = opt else { + return; + }; + + let ("aaaaaaaaaaaaaaaaaaa" | "bbbbbbbbbbbbbbbbb" | "cccccccccccccccccccccccc" | "dddddddddddddddd" | "eeeeeeeeeeeeeeee") = opt else { + return; + }; + + let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) = opt else { + return; + }; } diff --git a/tests/target/let_else.rs b/tests/target/let_else.rs index a6e816fb524..88115d129aa 100644 --- a/tests/target/let_else.rs +++ b/tests/target/let_else.rs @@ -1,3 +1,230 @@ fn main() { - let Some(1) = Some(1) else { return }; + // Although this won't compile it still parses so make sure we can format empty else blocks + let Some(x) = opt else {}; + + // let-else may be formatted on a single line if they are "short" + // and only contain a single expression + let Some(x) = opt else { return }; + + let Some(x) = opt else { return }; + + let Some(x) = opt else { + return; + }; + + let Some(x) = opt else { + // nope + return; + }; + + let Some(x) = opt else { + let y = 1; + return y; + }; + + let Some(x) = y.foo( + "abc", + fairly_long_identifier, + "def", + "123456", + "string", + "cheese", + ) else { + bar() + }; + + let Some(x) = abcdef() + .foo( + "abc", + some_really_really_really_long_ident, + "ident", + "123456", + ) + .bar() + .baz() + .qux("fffffffffffffffff") + else { + foo_bar() + }; +} + +fn with_comments_around_else_keyword() { + let Some(x) = opt + /* pre else keyword block-comment */ + else { + return; + }; + + let Some(x) = opt else + /* post else keyword block-comment */ + { + return; + }; + + let Some(x) = opt + /* pre else keyword block-comment */ + else + /* post else keyword block-comment */ + { + return; + }; + + let Some(x) = opt + // pre else keyword line-comment + else { + return; + }; + + let Some(x) = opt else + // post else keyword line-comment + { + return; + }; + + let Some(x) = opt + // pre else keyword line-comment + else + // post else keyword line-comment + { + return; + }; +} + +fn unbreakable_initializer_expr_pre_formatting_let_else_length_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The formatting is left unchanged! + let Some(x) = some_really_really_really_really_really_really_really_long_name_A else { return }; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_long_name___B else { + return; + }; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 100 (max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_long_name_____C else { + some_divergent_function() + }; + + // Pre Formatting: + // The length of `(indent)let pat = init else block;` is 101 (> max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_long_name__D else { + return; + }; +} + +fn unbreakable_initializer_expr_pre_formatting_length_up_to_opening_brace_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init else {` is 99 (< max_width) + // Post Formatting: + // The else keyword and opening brace remain on the same line as the initializer expr, + // and the else block is formatted over multiple lines because we can't fit the + // else block on the same line as the initializer expr. + let Some(x) = some_really_really_really_really_really_really_really_really_long_name___E else { + return; + }; + + // Pre Formatting: + // The length of `(indent)let pat = init else {` is 101 (> max_width) + // Post Formatting: + // The else keyword and opening brace cannot fit on the same line as the initializer expr. + // They are formatted on the next line. + let Some(x) = some_really_really_really_really_really_really_really_really_long_name_____F + else { + return; + }; +} + +fn unbreakable_initializer_expr_pre_formatting_length_through_initializer_expr_near_max_width() { + // Pre Formatting: + // The length of `(indent)let pat = init` is 99 (< max_width) + // Post Formatting: + // The else keyword and opening brace cannot fit on the same line as the initializer expr. + // They are formatted on the next line. + let Some(x) = some_really_really_really_really_really_really_really_really_really_long_name___G + else { + return; + }; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 100 (max_width) + // Post Formatting: + // Break after the `=` and put the initializer expr on it's own line. + // Because the initializer expr is multi-lined the else is placed on it's own line. + let Some(x) = + some_really_really_really_really_really_really_really_really_really_long_name____H + else { + return; + }; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 109 (> max_width) + // Post Formatting: + // Break after the `=` and put the initializer expr on it's own line. + // Because the initializer expr is multi-lined the else is placed on it's own line. + // The initializer expr has a length of 91, which when indented on the next line + // The `(indent)init` line has a lengh of 99. This is the max length that the `init` can be + // before we start running into max_width issues. I suspect this is becuase the shape is + // accounting for the `;` at the end of the `let-else` statement. + let Some(x) = + some_really_really_really_really_really_really_really_really_really_really_long_name______I + else { + return; + }; + + // Pre Formatting: + // The length of `(indent)let pat = init` is 110 (> max_width) + // Post Formatting: + // Max length issues prevent us from formatting. + // The initializer expr has a length of 92, which if it would be indented on the next line + // the `(indent)init` line has a lengh of 100 which == max_width of 100. + // One might expect formatting to succeed, but I suspect the reason we hit max_width issues is + // because the Shape is accounting for the `;` at the end of the `let-else` statement. + let Some(x) = some_really_really_really_really_really_really_really_really_really_really_really_long_nameJ else {return}; +} + +fn long_patterns() { + let Foo { + x: Bar(..), + y: FooBar(..), + z: Baz(..), + } = opt + else { + return; + }; + + // with version=One we don't wrap long array patterns + let [aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbb, cccccccccccccccccc, dddddddddddddddddd] = opt else { + return; + }; + + let ("aaaaaaaaaaaaaaaaaaa" + | "bbbbbbbbbbbbbbbbb" + | "cccccccccccccccccccccccc" + | "dddddddddddddddd" + | "eeeeeeeeeeeeeeee") = opt + else { + return; + }; + + let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) = + opt + else { + return; + }; }