Merge pull request #1017 from marcusklaas/tweak-if-else

Format non-statement if-else expressions on a single line

Author: nrc

src/chains.rs

@@ -354,27 +354,13 @@ fn rewrite_chain_subexpr(expr: &ast::Expr,
         }
         ast::ExprKind::Field(_, ref field) => {
             let s = format!(".{}", field.node);
-            if s.len() <= width {
-                Some(s)
-            } else {
-                None
-            }
+            if s.len() <= width { Some(s) } else { None }
         }
         ast::ExprKind::TupField(_, ref field) => {
             let s = format!(".{}", field.node);
-            if s.len() <= width {
-                Some(s)
-            } else {
-                None
-            }
-        }
-        ast::ExprKind::Try(_) => {
-            if width >= 1 {
-                Some("?".into())
-            } else {
-                None
-            }
+            if s.len() <= width { Some(s) } else { None }
         }
+        ast::ExprKind::Try(_) => if width >= 1 { Some("?".into()) } else { None },
         _ => unreachable!(),
     }
 }

src/config.rs

@@ -381,7 +381,9 @@ create_config! {
     chain_indent: BlockIndentStyle, BlockIndentStyle::Tabbed, "Indentation of chain";
     chains_overflow_last: bool, true, "Allow last call in method chain to break the line";
     reorder_imports: bool, false, "Reorder import statements alphabetically";
-    single_line_if_else: bool, false, "Put else on same line as closing brace for if statements";
+    single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else \
+                                                expressions. A value of zero means always break \
+                                                if-else expressions.";
     format_strings: bool, true, "Format string literals where necessary";
     force_format_strings: bool, false, "Always format string literals";
     take_source_hints: bool, true, "Retain some formatting characteristics from the source code";

src/expr.rs

@@ -111,11 +111,7 @@ pub fn rewrite_use_list(width: usize,
     }
 
     // 2 = ::
-    let path_separation_w = if !path_str.is_empty() {
-        2
-    } else {
-        0
-    };
+    let path_separation_w = if !path_str.is_empty() { 2 } else { 0 };
     // 1 = {
     let supp_indent = path_str.len() + path_separation_w + 1;
     // 1 = }
@@ -140,11 +136,7 @@ pub fn rewrite_use_list(width: usize,
     // potentially move "self" to the front of the vector without touching
     // the rest of the items.
     let has_self = move_self_to_front(&mut items);
-    let first_index = if has_self {
-        0
-    } else {
-        1
-    };
+    let first_index = if has_self { 0 } else { 1 };
 
     if context.config.reorder_imports {
         items[1..].sort_by(|a, b| a.item.cmp(&b.item));

src/imports.rs

@@ -139,11 +139,7 @@ impl<'a> FmtVisitor<'a> {
                 // FIXME(#21): we're dropping potential comments in between the
                 // function keywords here.
                 let vis = format_visibility(&item.vis);
-                let mut_str = if is_mutable {
-                    "mut "
-                } else {
-                    ""
-                };
+                let mut_str = if is_mutable { "mut " } else { "" };
                 let prefix = format!("{}static {}{}: ", vis, mut_str, item.ident);
                 let offset = self.block_indent + prefix.len();
                 // 1 = ;
@@ -261,11 +257,7 @@ impl<'a> FmtVisitor<'a> {
         if self.config.fn_single_line && is_simple_block_stmt(block, codemap) {
             let rewrite = {
                 if let Some(ref e) = block.expr {
-                    let suffix = if semicolon_for_expr(e) {
-                        ";"
-                    } else {
-                        ""
-                    };
+                    let suffix = if semicolon_for_expr(e) { ";" } else { "" };
 
                     e.rewrite(&self.get_context(),
                                  self.config.max_width - self.block_indent.width(),
@@ -1274,11 +1266,7 @@ fn rewrite_fn_base(context: &RewriteContext,
         .rewrite(&context, context.config.max_width - indent.width(), indent));
 
     let multi_line_ret_str = ret_str.contains('\n');
-    let ret_str_len = if multi_line_ret_str {
-        0
-    } else {
-        ret_str.len()
-    };
+    let ret_str_len = if multi_line_ret_str { 0 } else { ret_str.len() };
 
     // Args.
     let (mut one_line_budget, mut multi_line_budget, mut arg_indent) =

src/items.rs

@@ -208,11 +208,7 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
         let first = i == 0;
         let last = iter.peek().is_none();
         let separate = !last || trailing_separator;
-        let item_sep_len = if separate {
-            sep_len
-        } else {
-            0
-        };
+        let item_sep_len = if separate { sep_len } else { 0 };
 
         // Item string may be multi-line. Its length (used for block comment alignment)
         // Should be only the length of the last line.

src/lists.rs

@@ -93,13 +93,7 @@ impl<'a> FmtVisitor<'a> {
 
         fn replace_chars(string: &str) -> String {
             string.chars()
-                .map(|ch| {
-                    if ch.is_whitespace() {
-                        ch
-                    } else {
-                        'X'
-                    }
-                })
+                .map(|ch| { if ch.is_whitespace() { ch } else { 'X' } })
                 .collect()
         }
 

src/missed_spans.rs

@@ -126,11 +126,7 @@ impl Rewrite for Pat {
             PatKind::Struct(ref path, ref fields, elipses) => {
                 let path = try_opt!(rewrite_path(context, true, None, path, width, offset));
 
-                let (elipses_str, terminator) = if elipses {
-                    (", ..", "..")
-                } else {
-                    ("", "}")
-                };
+                let (elipses_str, terminator) = if elipses { (", ..", "..") } else { ("", "}") };
 
                 // 5 = `{` plus space before and after plus `}` plus space before.
                 let budget = try_opt!(width.checked_sub(path.len() + 5 + elipses_str.len()));

src/patterns.rs

@@ -187,11 +187,7 @@ fn rewrite_segment(expr_context: bool,
 
             let next_span_lo = param_list.last().unwrap().get_span().hi + BytePos(1);
             let list_lo = context.codemap.span_after(codemap::mk_sp(*span_lo, span_hi), "<");
-            let separator = if expr_context {
-                "::"
-            } else {
-                ""
-            };
+            let separator = if expr_context { "::" } else { "" };
 
             // 1 for <
             let extra_offset = 1 + separator.len();

src/types.rs

@@ -77,11 +77,7 @@ pub fn format_visibility(vis: &Visibility) -> Cow<'static, str> {
         Visibility::Crate(_) => Cow::from("pub(crate) "),
         Visibility::Restricted { ref path, .. } => {
             let Path { global, ref segments, .. } = **path;
-            let prefix = if global {
-                "::"
-            } else {
-                ""
-            };
+            let prefix = if global { "::" } else { "" };
             let mut segments_iter = segments.iter().map(|seg| seg.identifier.name.as_str());
 
             Cow::from(format!("pub({}{}) ", prefix, segments_iter.join("::")))

src/utils.rs

@@ -77,11 +77,7 @@ impl<'a> FmtVisitor<'a> {
         // Check if this block has braces.
         let snippet = self.snippet(b.span);
         let has_braces = snippet.starts_with("{") || snippet.starts_with("unsafe");
-        let brace_compensation = if has_braces {
-            BytePos(1)
-        } else {
-            BytePos(0)
-        };
+        let brace_compensation = if has_braces { BytePos(1) } else { BytePos(0) };
 
         self.last_pos = self.last_pos + brace_compensation;
         self.block_indent = self.block_indent.block_indent(self.config);

src/visitor.rs

@@ -21,7 +21,7 @@ enum_trailing_comma = true
 report_todo = "Always"
 report_fixme = "Never"
 reorder_imports = false
-single_line_if_else = false
+single_line_if_else_max_width = 0
 format_strings = true
 chains_overflow_last = true
 take_source_hints = true

tests/config/small_tabs.toml

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chain_base_indent: Inherit
 // Test chain formatting with block indented base
 

tests/source/chains-block-indented-base.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chains_overflow_last: false
 // Test chain formatting without overflowing the last item.
 

tests/source/chains-no-overflow.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chain_indent: Visual
 // rustfmt-chain_base_indent: Visual
 // Test chain formatting.

tests/source/chains-visual.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // Test chain formatting.
 
 fn main() {

tests/source/chains.rs

@@ -11,21 +11,19 @@ fn main() {
         };
 
     let loooooooooooooong_name = |field| {
-             // TODO(#27): format comments.
+             // format comments.
              if field.node.attrs.len() > 0 { field.node.attrs[0].span.lo
              } else {
                  field.span.lo
              }};
 
-    let block_me = |field| if true_story() { 1 } else { 2 };
-
     let unblock_me = |trivial| {
                          closure()
                      };
 
     let empty = |arg|    {};
 
-    let simple = |arg| { /* TODO(#27): comment formatting */ foo(arg) };
+    let simple = |arg| { /*  comment formatting */ foo(arg) };
 
     let test = |  | { do_something(); do_something_else(); };
 

tests/source/closure.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: AlwaysNextLine
 
 fn main() {

tests/source/else-if-brace-style-always-next-line.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: AlwaysSameLine
 
 fn main() {

tests/source/else-if-brace-style-always-same-line.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: ClosingNextLine
 
 fn main() {

tests/source/else-if-brace-style-closing-next-line.rs

@@ -255,3 +255,20 @@ fn ranges() {
     // the expr below won't compile for some reason...
     // let a = 0 ... ;
 }
+
+fn if_else() {
+  let exact = diff /
+            (if size == 0 {
+    1
+} else {
+    size
+});
+
+            let cx = tp1.x +
+                 any * radius *
+                 if anticlockwise {
+            1.0
+      } else {
+           -1.0
+       };
+}

tests/source/expr.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-wrap_comments: true
 // rustfmt-hard_tabs: true
 

tests/source/hard-tabs.rs

@@ -1,4 +1,4 @@
-// rustfmt-single_line_if_else: true
+// rustfmt-single_line_if_else_max_width: 100
 
 // Format if-else expressions on a single line, when possible.
 

tests/source/single-line-if-else.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chain_base_indent: Inherit
 // Test chain formatting with block indented base
 

tests/target/chains-block-indented-base.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chains_overflow_last: false
 // Test chain formatting without overflowing the last item.
 

tests/target/chains-no-overflow.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-chain_indent: Visual
 // rustfmt-chain_base_indent: Visual
 // Test chain formatting.

tests/target/chains-visual.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // Test chain formatting.
 
 fn main() {

tests/target/chains.rs

@@ -18,26 +18,20 @@ fn main() {
     };
 
     let loooooooooooooong_name = |field| {
-        // TODO(#27): format comments.
+        // format comments.
         if field.node.attrs.len() > 0 {
             field.node.attrs[0].span.lo
         } else {
             field.span.lo
         }
     };
 
-    let block_me = |field| if true_story() {
-        1
-    } else {
-        2
-    };
-
     let unblock_me = |trivial| closure();
 
     let empty = |arg| {};
 
     let simple = |arg| {
-        // TODO(#27): comment formatting
+        // comment formatting
         foo(arg)
     };
 

tests/target/closure.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: AlwaysNextLine
 
 fn main() {

tests/target/else-if-brace-style-always-next-line.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: AlwaysSameLine
 
 fn main() {

tests/target/else-if-brace-style-always-same-line.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-else_if_brace_style: ClosingNextLine
 
 fn main() {

tests/target/else-if-brace-style-closing-next-line.rs

@@ -62,11 +62,7 @@ fn foo() -> bool {
             tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 {
     }
 
-    let test = if true {
-        5
-    } else {
-        3
-    };
+    let test = if true { 5 } else { 3 };
 
     if cond() {
         something();
@@ -92,11 +88,7 @@ fn bar() {
     syntactically_correct(loop {
                               sup('?');
                           },
-                          if cond {
-                              0
-                          } else {
-                              1
-                          });
+                          if cond { 0 } else { 1 });
 
     let third = ..10;
     let infi_range = ..;
@@ -277,3 +269,9 @@ fn ranges() {
     // the expr below won't compile for some reason...
     // let a = 0 ... ;
 }
+
+fn if_else() {
+    let exact = diff / (if size == 0 { 1 } else { size });
+
+    let cx = tp1.x + any * radius * if anticlockwise { 1.0 } else { -1.0 };
+}

tests/target/expr.rs

@@ -1,3 +1,4 @@
+// rustfmt-single_line_if_else_max_width: 0
 // rustfmt-wrap_comments: true
 // rustfmt-hard_tabs: true
 

tests/target/hard-tabs.rs

@@ -1,4 +1,4 @@
-// rustfmt-single_line_if_else: true
+// rustfmt-single_line_if_else_max_width: 100
 
 // Format if-else expressions on a single line, when possible.