diff --git a/src/config.rs b/src/config.rs index 341f1d45afd..1af346265de 100644 --- a/src/config.rs +++ b/src/config.rs @@ -79,4 +79,5 @@ create_config! { reorder_imports: bool, // Alphabetically, case sensitive. expr_indent_style: BlockIndentStyle, closure_indent_style: BlockIndentStyle, + single_line_if_else: bool, } diff --git a/src/default.toml b/src/default.toml index 5792079a407..0af42981522 100644 --- a/src/default.toml +++ b/src/default.toml @@ -15,3 +15,4 @@ report_fixme = "Never" reorder_imports = false expr_indent_style = "Tabbed" closure_indent_style = "Visual" +single_line_if_else = false diff --git a/src/expr.rs b/src/expr.rs index f1279d93a5a..14659815d72 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -20,7 +20,7 @@ use types::rewrite_path; use items::{span_lo_for_arg, span_hi_for_arg, rewrite_fn_input}; use syntax::{ast, ptr}; -use syntax::codemap::{Pos, Span, BytePos, mk_sp}; +use syntax::codemap::{CodeMap, Pos, Span, BytePos, mk_sp}; use syntax::visit::Visitor; impl Rewrite for ast::Expr { @@ -80,7 +80,8 @@ impl Rewrite for ast::Expr { else_block.as_ref().map(|e| &**e), None, width, - offset) + offset, + true) } ast::Expr_::ExprIfLet(ref pat, ref cond, ref if_block, ref else_block) => { rewrite_if_else(context, @@ -89,7 +90,8 @@ impl Rewrite for ast::Expr { else_block.as_ref().map(|e| &**e), Some(pat), width, - offset) + offset, + true) } // We reformat it ourselves because rustc gives us a bad span // for ranges, see rust#27162 @@ -412,10 +414,11 @@ fn rewrite_range(context: &RewriteContext, fn rewrite_if_else(context: &RewriteContext, cond: &ast::Expr, if_block: &ast::Block, - else_block: Option<&ast::Expr>, + else_block_opt: Option<&ast::Expr>, pat: Option<&ast::Pat>, width: usize, - offset: usize) + offset: usize, + allow_single_line: bool) -> Option { // 3 = "if ", 2 = " {" let pat_expr_string = try_opt!(rewrite_pat_expr(context, @@ -423,22 +426,102 @@ fn rewrite_if_else(context: &RewriteContext, cond, "let ", " =", - width - 3 - 2, + try_opt!(width.checked_sub(3 + 2)), offset + 3)); + // Try to format if-else on single line. + if allow_single_line && context.config.single_line_if_else { + let trial = single_line_if_else(context, &pat_expr_string, if_block, else_block_opt, width); + + if trial.is_some() { + return trial; + } + } + let if_block_string = try_opt!(if_block.rewrite(context, width, offset)); let mut result = format!("if {} {}", pat_expr_string, if_block_string); - if let Some(else_block) = else_block { - let else_block_string = try_opt!(else_block.rewrite(context, width, offset)); + if let Some(else_block) = else_block_opt { + let rewrite = match else_block.node { + // If the else expression is another if-else expression, prevent it + // from being formatted on a single line. + ast::Expr_::ExprIfLet(ref pat, ref cond, ref if_block, ref else_block) => { + rewrite_if_else(context, + cond, + if_block, + else_block.as_ref().map(|e| &**e), + Some(pat), + width, + offset, + false) + } + ast::Expr_::ExprIf(ref cond, ref if_block, ref else_block) => { + rewrite_if_else(context, + cond, + if_block, + else_block.as_ref().map(|e| &**e), + None, + width, + offset, + false) + } + _ => else_block.rewrite(context, width, offset), + }; result.push_str(" else "); - result.push_str(&else_block_string); + result.push_str(&&try_opt!(rewrite)); } Some(result) } +fn single_line_if_else(context: &RewriteContext, + pat_expr_str: &str, + if_node: &ast::Block, + else_block_opt: Option<&ast::Expr>, + width: usize) + -> Option { + let else_block = try_opt!(else_block_opt); + let fixed_cost = "if { } else { }".len(); + + if let ast::ExprBlock(ref else_node) = else_block.node { + if !is_simple_block(if_node, context.codemap) || + !is_simple_block(else_node, context.codemap) || pat_expr_str.contains('\n') { + return None; + } + + let new_width = try_opt!(width.checked_sub(pat_expr_str.len() + fixed_cost)); + let if_expr = if_node.expr.as_ref().unwrap(); + let if_str = try_opt!(if_expr.rewrite(context, new_width, 0)); + + let new_width = try_opt!(new_width.checked_sub(if_str.len())); + let else_expr = else_node.expr.as_ref().unwrap(); + let else_str = try_opt!(else_expr.rewrite(context, new_width, 0)); + + // FIXME: this check shouldn't be necessary. Rewrites should either fail + // or wrap to a newline when the object does not fit the width. + let fits_line = fixed_cost + pat_expr_str.len() + if_str.len() + else_str.len() <= width; + + if fits_line && !if_str.contains('\n') && !else_str.contains('\n') { + return Some(format!("if {} {{ {} }} else {{ {} }}", pat_expr_str, if_str, else_str)); + } + } + + None +} + +// Checks that a block contains no statements, an expression and no comments. +fn is_simple_block(block: &ast::Block, codemap: &CodeMap) -> bool { + if !block.stmts.is_empty() || block.expr.is_none() { + return false; + } + + let snippet = codemap.span_to_snippet(block.span).unwrap(); + + // FIXME: fails when either // or /* is contained in a string literal. + !snippet.contains("//") && !snippet.contains("/*") +} + fn rewrite_match(context: &RewriteContext, cond: &ast::Expr, arms: &[ast::Arm], @@ -830,7 +913,7 @@ fn rewrite_paren(context: &RewriteContext, debug!("rewrite_paren, width: {}, offset: {}", width, offset); // 1 is for opening paren, 2 is for opening+closing, we want to keep the closing // paren on the same line as the subexpr. - let subexpr_str = subexpr.rewrite(context, width-2, offset+1); + let subexpr_str = subexpr.rewrite(context, try_opt!(width.checked_sub(2)), offset + 1); debug!("rewrite_paren, subexpr_str: `{:?}`", subexpr_str); subexpr_str.map(|s| format!("({})", s)) } diff --git a/tests/config/small_tabs.toml b/tests/config/small_tabs.toml index 2aff85506a4..1b4ab5cb7e9 100644 --- a/tests/config/small_tabs.toml +++ b/tests/config/small_tabs.toml @@ -15,3 +15,4 @@ report_fixme = "Never" reorder_imports = false expr_indent_style = "Tabbed" closure_indent_style = "Visual" +single_line_if_else = false diff --git a/tests/source/expr.rs b/tests/source/expr.rs index d304093ebb2..85ee7727451 100644 --- a/tests/source/expr.rs +++ b/tests/source/expr.rs @@ -43,6 +43,8 @@ some_ridiculously_loooooooooooooooooooooong_function(10000 * 30000000000 + 40000 + 2 + 3 { } + let test = if true { 5 } else { 3 }; + if cond() { something(); } else if different_cond() { diff --git a/tests/source/single-line-if-else.rs b/tests/source/single-line-if-else.rs new file mode 100644 index 00000000000..42629ab8e37 --- /dev/null +++ b/tests/source/single-line-if-else.rs @@ -0,0 +1,50 @@ +// rustfmt-single_line_if_else: true + +// Format if-else expressions on a single line, when possible. + +fn main() { + let a = if 1 > 2 { + unreachable!() + } else { + 10 + }; + + let a = if x { 1 } else if y { 2 } else { 3 }; + + let b = if cond() { + 5 + } else { + // Brief comment. + 10 + }; + + let c = if cond() { + statement(); + + 5 + } else { + 10 + }; + + let d = if let Some(val) = turbo + { "cool" } else { + "beans" }; + + if cond() { statement(); } else { other_statement(); } + + if true { + do_something() + } + + let x = if veeeeeeeeery_loooooong_condition() { aaaaaaaaaaaaaaaaaaaaaaaaaaa } else { bbbbbbbbbb }; + + let x = if veeeeeeeeery_loooooong_condition() { aaaaaaaaaaaaaaaaaaaaaaaaa } else { + bbbbbbbbbb }; + + funk(if test() { + 1 + } else { + 2 + }, + arg2); +} diff --git a/tests/target/expr.rs b/tests/target/expr.rs index df5e98e4f1b..45c654b738f 100644 --- a/tests/target/expr.rs +++ b/tests/target/expr.rs @@ -61,6 +61,12 @@ fn foo() -> bool { 1 + 2 + 3 { } + let test = if true { + 5 + } else { + 3 + }; + if cond() { something(); } else if different_cond() { diff --git a/tests/target/single-line-if-else.rs b/tests/target/single-line-if-else.rs new file mode 100644 index 00000000000..27b6d773da5 --- /dev/null +++ b/tests/target/single-line-if-else.rs @@ -0,0 +1,52 @@ +// rustfmt-single_line_if_else: true + +// Format if-else expressions on a single line, when possible. + +fn main() { + let a = if 1 > 2 { unreachable!() } else { 10 }; + + let a = if x { + 1 + } else if y { + 2 + } else { + 3 + }; + + let b = if cond() { + 5 + } else { + // Brief comment. + 10 + }; + + let c = if cond() { + statement(); + + 5 + } else { + 10 + }; + + let d = if let Some(val) = turbo { "cool" } else { "beans" }; + + if cond() { + statement(); + } else { + other_statement(); + } + + if true { + do_something() + } + + let x = if veeeeeeeeery_loooooong_condition() { + aaaaaaaaaaaaaaaaaaaaaaaaaaa + } else { + bbbbbbbbbb + }; + + let x = if veeeeeeeeery_loooooong_condition() { aaaaaaaaaaaaaaaaaaaaaaaaa } else { bbbbbbbbbb }; + + funk(if test() { 1 } else { 2 }, arg2); +}