Single line if's for 1 stmt block #6031
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1102,10 +1102,31 @@ impl<'a> Rewrite for ControlFlow<'a> { | |
| }; | ||
| let block_str = { | ||
| let old_val = context.is_if_else_block.replace(self.else_block.is_some()); | ||
| let max_width = if context.config.single_line_simple_if() { | ||
| std::cmp::min( | ||
| shape.width, | ||
| context.config.single_line_simple_if_max_width(), | ||
| ) | ||
| } else { | ||
| shape.width | ||
| }; | ||
| let available_space = max_width.saturating_sub(used_width); | ||
| let allow_single_line = allow_single_line_if(&cond_str, self.block) | ||
| && self.keyword == "if" | ||
| && available_space > 0; | ||
|
|
||
| let mut result = if allow_single_line && context.config.single_line_simple_if() { | ||
| rewrite_block_inner(self.block, None, None, true, context, block_shape)? | ||
| } else { | ||
| rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true)? | ||
| }; | ||
|
|
||
| let block_exceeds_width = result.len() > available_space; | ||
| if allow_single_line && !result.contains('\n') && block_exceeds_width { | ||
| result = rewrite_block_inner(self.block, None, None, false, context, block_shape)?; | ||
| } | ||
| context.is_if_else_block.replace(old_val); | ||
| result | ||
Sjael marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| let mut result = format!("{cond_str}{block_str}"); | ||
|
|
@@ -1158,6 +1179,36 @@ impl<'a> Rewrite for ControlFlow<'a> { | |
| } | ||
| } | ||
|
|
||
| fn allow_single_line_if(result: &str, block: &ast::Block) -> bool { | ||
| if result.contains('\n') { | ||
| return false; | ||
| } | ||
|
Comment on lines
+1207
to
+1209
There was a problem hiding this comment. Does fn main() {
#[allow(unused)]
if true {
println!("case 1");
}
#[allow(unused)]
// What about if there's a comment here?
if true {
println!("case 2 with comment");
}
}There was a problem hiding this comment. We also don't want to allow single lining if the block contains a comment. I think the block rewriting handles that but just a call out that we'd need a test for it. There was a problem hiding this comment.
There was a problem hiding this comment. I ask because I overlooked this case when adding |
||
| if block.stmts.len() == 0 { | ||
| return true; | ||
| } | ||
| if block.stmts.len() == 1 { | ||
| return is_simple_control_flow_stmt(&block.stmts[0]); | ||
| } | ||
| false | ||
| } | ||
|
|
||
| fn is_simple_control_flow_stmt(stmt: &ast::Stmt) -> bool { | ||
| match stmt.kind { | ||
| ast::StmtKind::Expr(ref expr) => match expr.kind { | ||
Sjael marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ast::ExprKind::Continue(..) => true, | ||
| ast::ExprKind::Break(_, ref opt_expr) | ast::ExprKind::Ret(ref opt_expr) => { | ||
| if let Some(_) = *opt_expr { | ||
| false | ||
| } else { | ||
| true | ||
| } | ||
| } | ||
| _ => false, | ||
| }, | ||
| _ => false, | ||
| } | ||
| } | ||
|
|
||
| fn rewrite_label(opt_label: Option<ast::Label>) -> Cow<'static, str> { | ||
| match opt_label { | ||
| Some(label) => Cow::from(format!("{}: ", label.ident)), | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // rustfmt-single_line_simple_if: true | ||
| // rustfmt-unstable_features: true | ||
Sjael marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| fn main() { | ||
| // if statements may be formatted on a single line if they are "short" | ||
| // and only contain a single expression of 'return', 'continue' or 'break' | ||
| if true { continue } | ||
|
|
||
| if true { | ||
| continue | ||
| } | ||
|
|
||
| // Default max width is 50 | ||
| if width == 50_characters_or_shorter { continue } | ||
| if width == 51_characters_long_and_above { return } | ||
|
|
||
| if name == super_duper_really_really_mega_ultra_giga_long_name_with_a_cherry_on_top { return } | ||
|
|
||
| // More than 1 stmt means a new line, it is no longer 'simple' | ||
| if a { let y = 1; return y; } | ||
|
|
||
| // Adds a semicolon to 'return/continue/break' when put on a new line | ||
| // (unless config has trailing_semicolon = false) | ||
| if a { let y = 1; return y } | ||
|
|
||
| // Will not work on fn or method calls (may change) | ||
| if true { do_something() } | ||
|
|
||
| // Will not work on an expression with trailing semicolon pre-format | ||
| if true { return; } | ||
|
|
||
| // Will not single line if there is an else block, even with single expressions | ||
| if true { return } else { break } | ||
|
|
||
| // Will not be single line if returns/breaks with a value | ||
| for i in 0..2{ | ||
| if true { break } | ||
| if true { break 2 } | ||
| if true { return } | ||
| if true { return 3 } | ||
| } | ||
|
|
||
| // Will not be single line if comment is in the block | ||
| if true { | ||
| // nope | ||
| return | ||
| } | ||
| if true { /* nope 2 */ return } | ||
|
|
||
| // Only works on if blocks, not other control flow | ||
| for i in 0..2 { if i == 1 { continue } } | ||
|
|
||
| for i in 0..2 { | ||
| loop { if i == 1 { continue } } | ||
| } | ||
|
|
||
| // New line formatted here as 'loop' != 'return/continue/break' | ||
| if i == 1 { loop { return } } | ||
|
|
||
| // Works on labelled break/continue | ||
| 'gamer: loop { if true{ break 'gamer } } | ||
|
|
||
| 'gamer: loop { if true{ break 'gamer; } } | ||
|
|
||
| let result = 'block: { | ||
| if foo() { break 'block 1 } | ||
| if bar() { break 'block 2; } | ||
| 3 | ||
| }; | ||
|
|
||
| #[allow(unused)] | ||
| // Comments after attributes dont mess it up | ||
| if true { return } | ||
| #[cfg(target_os = "linux")] | ||
| // Comments after attributes dont mess it up | ||
| if name == super_duper_ultra_really_name { return } | ||
| #[cfg(target_os = "linux")] | ||
| /* Multiple lines dont mess this up */ | ||
| /* Multiple lines dont mess this up */ | ||
| if name == super_duper_ultra_really_name { return } | ||
|
|
||
| // Works as intended with nested ifs and indents | ||
| if true { | ||
| if true { continue } | ||
| if true { if true { continue } } | ||
| } else if false { | ||
| if true { if true { if width == 50_characters_or_shorter { continue } if width == 51_characters_long_and_above { return } } } | ||
| } else { | ||
| if true { return; } | ||
| } | ||
|
|
||
| // Works with complex conditions | ||
| if matches!(x, Ok(Some(value))) { continue } | ||
| if matches!(x, Ok(Some(value))) { kick_ball() } | ||
| if matches!(x, Ok(Some(value))) && value.some_method_call(input) { break } | ||
| if matches!(x, Ok(Some(value))) && value.some_method_call(input) { run_fast() } | ||
| if matches!(x, Ok(Some(value))) && value.some_method_call(input) && single_line_if_is_allowed_at_all_ever { return } | ||
| if matches!(x, Ok(Some(value))) && value.some_method_call(input) && single_line_if_is_allowed_at_all_ever { play_catch() } | ||
|
|
||
| // Nested complex conditions | ||
| if true { | ||
| if matches!(x, Ok(Some(value))) { continue } | ||
| if true { if matches!(x, Ok(Some(value))) && value.some_method_call(input) { break } } | ||
| } else if false { | ||
| if true { if true { if matches!(x, Ok(Some(value))) { continue } } } | ||
| } else { | ||
| if true { if true { if matches!(x, Ok(Some(value))) && value.some_method_call(input) && single_line_if_is_allowed_at_all_ever { return } } } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // rustfmt-single_line_if_else_max_width: 0 | ||
|
|
||
| fn main() { | ||
| let a = if 1 > 2 { | ||
| unreachable!() | ||
| } else { | ||
| 10 | ||
| }; | ||
|
|
||
| let a = if x { 1 } else if y { 2 } else { 3 }; | ||
|
|
||
| if true { continue } | ||
|
|
||
| if true { | ||
| continue | ||
| } | ||
|
|
||
| if width == 50_characters_or_shorter { continue } | ||
| if width == 51_characters_long_and_above { return } | ||
|
|
||
| if name == super_duper_really_really_mega_ultra_giga_long_name_with_a_cherry_on_top { return } | ||
|
|
||
| if true { return } else { break } | ||
|
|
||
| let x = if true { return } else { break }; | ||
|
|
||
| let b = if cond() { | ||
| 5 | ||
| } else { | ||
| // Brief comment. | ||
| 10 | ||
| }; | ||
|
|
||
| let c = if cond() { | ||
| statement(); | ||
|
|
||
| 5 | ||
| } else { | ||
| 10 | ||
| }; | ||
|
|
||
| if cond() { statement(); } else { other_statement(); } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.