Skip to content

Commit 262feb3

Browse files
authored
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.
1 parent b636723 commit 262feb3

File tree

5 files changed

+196
-3
lines changed

5 files changed

+196
-3
lines changed

src/items.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ impl Rewrite for ast::Local {
7575
false,
7676
)?
7777
};
78+
let let_kw_offset = result.len() - "let ".len();
7879

7980
// 4 = "let ".len()
8081
let pat_shape = shape.offset_left(4)?;
@@ -127,8 +128,15 @@ impl Rewrite for ast::Local {
127128

128129
if let Some(block) = else_block {
129130
let else_kw_span = init.span.between(block.span);
131+
// Strip attributes and comments to check if newline is needed before the else
132+
// keyword from the initializer part. (#5901)
133+
let init_str = if context.config.version() == Version::Two {
134+
&result[let_kw_offset..]
135+
} else {
136+
result.as_str()
137+
};
130138
let force_newline_else = pat_str.contains('\n')
131-
|| !same_line_else_kw_and_brace(&result, context, else_kw_span, nested_shape);
139+
|| !same_line_else_kw_and_brace(init_str, context, else_kw_span, nested_shape);
132140
let else_kw = rewrite_else_kw_with_comments(
133141
force_newline_else,
134142
true,
@@ -146,11 +154,16 @@ impl Rewrite for ast::Local {
146154
std::cmp::min(shape.width, context.config.single_line_let_else_max_width());
147155

148156
// If available_space hits zero we know for sure this will be a multi-lined block
149-
let available_space = max_width.saturating_sub(result.len());
157+
let assign_str_with_else_kw = if context.config.version() == Version::Two {
158+
&result[let_kw_offset..]
159+
} else {
160+
result.as_str()
161+
};
162+
let available_space = max_width.saturating_sub(assign_str_with_else_kw.len());
150163

151164
let allow_single_line = !force_newline_else
152165
&& available_space > 0
153-
&& allow_single_line_let_else_block(&result, block);
166+
&& allow_single_line_let_else_block(assign_str_with_else_kw, block);
154167

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

tests/source/let_else.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,23 @@ fn with_trailing_try_operator() {
160160
// Maybe this is a workaround?
161161
let Ok(Some(next_bucket)) = ranking_rules[cur_ranking_rule_index].next_bucket(ctx, logger, &ranking_rule_universes[cur_ranking_rule_index]) else { return };
162162
}
163+
164+
fn issue5901() {
165+
#[cfg(target_os = "linux")]
166+
let Some(x) = foo else { todo!() };
167+
168+
#[cfg(target_os = "linux")]
169+
// Some comments between attributes and let-else statement
170+
let Some(x) = foo else { todo!() };
171+
172+
#[cfg(target_os = "linux")]
173+
#[cfg(target_arch = "x86_64")]
174+
let Some(x) = foo else { todo!() };
175+
176+
// The else block will be multi-lined because attributes and comments before `let`
177+
// are included when calculating max width
178+
#[cfg(target_os = "linux")]
179+
#[cfg(target_arch = "x86_64")]
180+
// Some comments between attributes and let-else statement
181+
let Some(x) = foo() else { todo!() };
182+
}

tests/source/let_else_v2.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// rustfmt-version: Two
2+
// rustfmt-single_line_let_else_max_width: 100
3+
4+
fn issue5901() {
5+
#[cfg(target_os = "linux")]
6+
let Some(x) = foo else { todo!() };
7+
8+
#[cfg(target_os = "linux")]
9+
// Some comments between attributes and let-else statement
10+
let Some(x) = foo else { todo!() };
11+
12+
#[cfg(target_os = "linux")]
13+
#[cfg(target_arch = "x86_64")]
14+
let Some(x) = foo else { todo!() };
15+
16+
// The else block is multi-lined
17+
#[cfg(target_os = "linux")]
18+
let Some(x) = foo else { return; };
19+
20+
// The else block will be single-lined because attributes and comments before `let`
21+
// are no longer included when calculating max width
22+
#[cfg(target_os = "linux")]
23+
#[cfg(target_arch = "x86_64")]
24+
// Some comments between attributes and let-else statement
25+
let Some(x) = foo else { todo!() };
26+
27+
// Some more test cases for v2 formatting with attributes
28+
29+
#[cfg(target_os = "linux")]
30+
#[cfg(target_arch = "x86_64")]
31+
let Some(x) = opt
32+
// pre else keyword line-comment
33+
else { return; };
34+
35+
#[cfg(target_os = "linux")]
36+
#[cfg(target_arch = "x86_64")]
37+
let Some(x) = opt else
38+
// post else keyword line-comment
39+
{ return; };
40+
41+
#[cfg(target_os = "linux")]
42+
#[cfg(target_arch = "x86_64")]
43+
let Foo {x: Bar(..), y: FooBar(..), z: Baz(..)} = opt else {
44+
return;
45+
};
46+
47+
#[cfg(target_os = "linux")]
48+
#[cfg(target_arch = "x86_64")]
49+
let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) = opt else {
50+
return;
51+
};
52+
53+
#[cfg(target_os = "linux")]
54+
#[cfg(target_arch = "x86_64")]
55+
let Some(x) = very_very_very_very_very_very_very_very_very_very_very_very_long_expression_in_assign_rhs() else { return; };
56+
}

tests/target/let_else.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,34 @@ fn with_trailing_try_operator() {
252252
return;
253253
};
254254
}
255+
256+
fn issue5901() {
257+
#[cfg(target_os = "linux")]
258+
let Some(x) = foo
259+
else {
260+
todo!()
261+
};
262+
263+
#[cfg(target_os = "linux")]
264+
// Some comments between attributes and let-else statement
265+
let Some(x) = foo
266+
else {
267+
todo!()
268+
};
269+
270+
#[cfg(target_os = "linux")]
271+
#[cfg(target_arch = "x86_64")]
272+
let Some(x) = foo
273+
else {
274+
todo!()
275+
};
276+
277+
// The else block will be multi-lined because attributes and comments before `let`
278+
// are included when calculating max width
279+
#[cfg(target_os = "linux")]
280+
#[cfg(target_arch = "x86_64")]
281+
// Some comments between attributes and let-else statement
282+
let Some(x) = foo() else {
283+
todo!()
284+
};
285+
}

tests/target/let_else_v2.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// rustfmt-version: Two
2+
// rustfmt-single_line_let_else_max_width: 100
3+
4+
fn issue5901() {
5+
#[cfg(target_os = "linux")]
6+
let Some(x) = foo else { todo!() };
7+
8+
#[cfg(target_os = "linux")]
9+
// Some comments between attributes and let-else statement
10+
let Some(x) = foo else { todo!() };
11+
12+
#[cfg(target_os = "linux")]
13+
#[cfg(target_arch = "x86_64")]
14+
let Some(x) = foo else { todo!() };
15+
16+
// The else block is multi-lined
17+
#[cfg(target_os = "linux")]
18+
let Some(x) = foo else {
19+
return;
20+
};
21+
22+
// The else block will be single-lined because attributes and comments before `let`
23+
// are no longer included when calculating max width
24+
#[cfg(target_os = "linux")]
25+
#[cfg(target_arch = "x86_64")]
26+
// Some comments between attributes and let-else statement
27+
let Some(x) = foo else { todo!() };
28+
29+
// Some more test cases for v2 formatting with attributes
30+
31+
#[cfg(target_os = "linux")]
32+
#[cfg(target_arch = "x86_64")]
33+
let Some(x) = opt
34+
// pre else keyword line-comment
35+
else {
36+
return;
37+
};
38+
39+
#[cfg(target_os = "linux")]
40+
#[cfg(target_arch = "x86_64")]
41+
let Some(x) = opt else
42+
// post else keyword line-comment
43+
{
44+
return;
45+
};
46+
47+
#[cfg(target_os = "linux")]
48+
#[cfg(target_arch = "x86_64")]
49+
let Foo {
50+
x: Bar(..),
51+
y: FooBar(..),
52+
z: Baz(..),
53+
} = opt
54+
else {
55+
return;
56+
};
57+
58+
#[cfg(target_os = "linux")]
59+
#[cfg(target_arch = "x86_64")]
60+
let Some(Ok((Message::ChangeColor(super::color::Color::Rgb(r, g, b)), Point { x, y, z }))) =
61+
opt
62+
else {
63+
return;
64+
};
65+
66+
#[cfg(target_os = "linux")]
67+
#[cfg(target_arch = "x86_64")]
68+
let Some(x) =
69+
very_very_very_very_very_very_very_very_very_very_very_very_long_expression_in_assign_rhs()
70+
else {
71+
return;
72+
};
73+
}

0 commit comments

Comments
 (0)