From 00fef2d51d417b62c2154a05237a16bed37244f5 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 12 Feb 2023 14:50:44 -0500 Subject: [PATCH] Implement wrapping rules to force `else` on a newline in `let-else` --- src/expr.rs | 4 ++++ src/items.rs | 47 +++++++++++++++++++++++++++++++++++++++- tests/source/let_else.rs | 4 ++++ tests/target/let_else.rs | 25 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/expr.rs b/src/expr.rs index a4e60659a0ed..715be57ada84 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1027,12 +1027,14 @@ 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, @@ -1048,6 +1050,7 @@ pub(crate) fn rewrite_else_kw_with_comments( 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() } @@ -1132,6 +1135,7 @@ impl<'a> Rewrite for ControlFlow<'a> { }; let else_kw = rewrite_else_kw_with_comments( + false, last_in_chain, context, self.block.span.between(else_block.span), diff --git a/src/items.rs b/src/items.rs index 4693e57b8a16..64d2ace2bee6 100644 --- a/src/items.rs +++ b/src/items.rs @@ -126,10 +126,14 @@ impl Rewrite for ast::Local { )?; 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, - init.span.between(block.span), + else_kw_span, shape, ); result.push_str(&else_kw); @@ -148,6 +152,47 @@ 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 so the "else {" should be placed on the same line + return true; + } + + // 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)) +} + // 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 26e0ebf07364..932a64a050ff 100644 --- a/tests/source/let_else.rs +++ b/tests/source/let_else.rs @@ -7,4 +7,8 @@ fn main() { // nope return; }; + + 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() }; } diff --git a/tests/target/let_else.rs b/tests/target/let_else.rs index a910f5389a3a..fafbe7932ead 100644 --- a/tests/target/let_else.rs +++ b/tests/target/let_else.rs @@ -9,4 +9,29 @@ fn main() { // nope return; }; + + 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() + }; }