Fix checking if newline is needed before else in let-else statement

Fixes 5901

Take leading attributes and comments into consideration when determining if we need to wrap the `else` keyword onto the next line.
This commit is contained in:
Linda_pp 2023-09-09 10:05:07 +09:00 committed by GitHub
parent b6367235eb
commit 262feb3337
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 196 additions and 3 deletions

View File

@ -75,6 +75,7 @@ fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String>
false,
)?
};
let let_kw_offset = result.len() - "let ".len();
// 4 = "let ".len()
let pat_shape = shape.offset_left(4)?;
@ -127,8 +128,15 @@ fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String>
if let Some(block) = else_block {
let else_kw_span = init.span.between(block.span);
// Strip attributes and comments to check if newline is needed before the else
// keyword from the initializer part. (#5901)
let init_str = if context.config.version() == Version::Two {
&result[let_kw_offset..]
} else {
result.as_str()
};
let force_newline_else = pat_str.contains('\n')
|| !same_line_else_kw_and_brace(&result, context, else_kw_span, nested_shape);
|| !same_line_else_kw_and_brace(init_str, context, else_kw_span, nested_shape);
let else_kw = rewrite_else_kw_with_comments(
force_newline_else,
true,
@ -146,11 +154,16 @@ fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String>
std::cmp::min(shape.width, context.config.single_line_let_else_max_width());
// If available_space hits zero we know for sure this will be a multi-lined block
let available_space = max_width.saturating_sub(result.len());
let assign_str_with_else_kw = if context.config.version() == Version::Two {
&result[let_kw_offset..]
} else {
result.as_str()
};
let available_space = max_width.saturating_sub(assign_str_with_else_kw.len());
let allow_single_line = !force_newline_else
&& available_space > 0
&& allow_single_line_let_else_block(&result, block);
&& allow_single_line_let_else_block(assign_str_with_else_kw, block);
let mut rw_else_block =
rewrite_let_else_block(block, allow_single_line, context, shape)?;

View File

@ -160,3 +160,23 @@ fn with_trailing_try_operator() {
// Maybe this is a workaround?
let Ok(Some(next_bucket)) = ranking_rules[cur_ranking_rule_index].next_bucket(ctx, logger, &ranking_rule_universes[cur_ranking_rule_index]) else { return };
}
fn issue5901() {
#[cfg(target_os = "linux")]
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
// Some comments between attributes and let-else statement
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = foo else { todo!() };
// The else block will be multi-lined because attributes and comments before `let`
// are included when calculating max width
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
// Some comments between attributes and let-else statement
let Some(x) = foo() else { todo!() };
}

View File

@ -0,0 +1,56 @@
// rustfmt-version: Two
// rustfmt-single_line_let_else_max_width: 100
fn issue5901() {
#[cfg(target_os = "linux")]
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
// Some comments between attributes and let-else statement
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = foo else { todo!() };
// The else block is multi-lined
#[cfg(target_os = "linux")]
let Some(x) = foo else { return; };
// The else block will be single-lined because attributes and comments before `let`
// are no longer included when calculating max width
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
// Some comments between attributes and let-else statement
let Some(x) = foo else { todo!() };
// Some more test cases for v2 formatting with attributes
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = opt
// pre else keyword line-comment
else { return; };
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = opt else
// post else keyword line-comment
{ return; };
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Foo {x: Bar(..), y: FooBar(..), z: Baz(..)} = opt else {
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) = opt else {
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = very_very_very_very_very_very_very_very_very_very_very_very_long_expression_in_assign_rhs() else { return; };
}

View File

@ -252,3 +252,34 @@ fn with_trailing_try_operator() {
return;
};
}
fn issue5901() {
#[cfg(target_os = "linux")]
let Some(x) = foo
else {
todo!()
};
#[cfg(target_os = "linux")]
// Some comments between attributes and let-else statement
let Some(x) = foo
else {
todo!()
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = foo
else {
todo!()
};
// The else block will be multi-lined because attributes and comments before `let`
// are included when calculating max width
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
// Some comments between attributes and let-else statement
let Some(x) = foo() else {
todo!()
};
}

View File

@ -0,0 +1,73 @@
// rustfmt-version: Two
// rustfmt-single_line_let_else_max_width: 100
fn issue5901() {
#[cfg(target_os = "linux")]
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
// Some comments between attributes and let-else statement
let Some(x) = foo else { todo!() };
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = foo else { todo!() };
// The else block is multi-lined
#[cfg(target_os = "linux")]
let Some(x) = foo else {
return;
};
// The else block will be single-lined because attributes and comments before `let`
// are no longer included when calculating max width
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
// Some comments between attributes and let-else statement
let Some(x) = foo else { todo!() };
// Some more test cases for v2 formatting with attributes
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = opt
// pre else keyword line-comment
else {
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) = opt else
// post else keyword line-comment
{
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Foo {
x: Bar(..),
y: FooBar(..),
z: Baz(..),
} = opt
else {
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) =
opt
else {
return;
};
#[cfg(target_os = "linux")]
#[cfg(target_arch = "x86_64")]
let Some(x) =
very_very_very_very_very_very_very_very_very_very_very_very_long_expression_in_assign_rhs()
else {
return;
};
}