Merge pull request #1840 from topecongiro/match-with-max-width

Match with max width

src/expr.rs

@@ -24,7 +24,7 @@ use config::{Config, ControlBraceStyle, IndentStyle, MultilineStyle, Style};
 use items::{span_hi_for_arg, span_lo_for_arg};
 use lists::{definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting,
             struct_lit_shape, struct_lit_tactic, write_list, DefinitiveListTactic, ListFormatting,
-            ListItem, ListTactic, SeparatorTactic};
+            ListItem, ListTactic, Separator, SeparatorTactic};
 use macros::{rewrite_macro, MacroPosition};
 use patterns::{can_be_overflowed_pat, TuplePatField};
 use rewrite::{Rewrite, RewriteContext};
@@ -488,7 +488,7 @@ where
                 Some(width) => {
                     let tactic =
                         ListTactic::LimitedHorizontalVertical(context.config.array_width());
-                    definitive_tactic(&items, tactic, width)
+                    definitive_tactic(&items, tactic, Separator::Comma, width)
                 }
                 None => DefinitiveListTactic::Vertical,
             }
@@ -497,6 +497,7 @@ where
             definitive_tactic(
                 &items,
                 ListTactic::LimitedHorizontalVertical(context.config.array_width()),
+                Separator::Comma,
                 nested_shape.width,
             )
         } else {
@@ -594,7 +595,12 @@ fn rewrite_closure_fn_decl(
         .width
         .checked_sub(ret_str.len() + 1)
         .unwrap_or(0);
-    let tactic = definitive_tactic(&item_vec, ListTactic::HorizontalVertical, horizontal_budget);
+    let tactic = definitive_tactic(
+        &item_vec,
+        ListTactic::HorizontalVertical,
+        Separator::Comma,
+        horizontal_budget,
+    );
     let arg_shape = match tactic {
         DefinitiveListTactic::Horizontal => try_opt!(arg_shape.sub_width(ret_str.len() + 1)),
         _ => arg_shape,
@@ -1530,17 +1536,25 @@ fn rewrite_match(
         return None;
     }
 
-    // 6 = `match `, 2 = ` {`
+    // Do not take the rhs overhead from the upper expressions into account
+    // when rewriting match condition.
+    let new_width = try_opt!(context.config.max_width().checked_sub(shape.used_width()));
+    let cond_shape = Shape {
+        width: new_width,
+        ..shape
+    };
+    // 6 = `match `
     let cond_shape = match context.config.control_style() {
-        Style::Legacy => try_opt!(shape.shrink_left(6).and_then(|s| s.sub_width(2))),
+        Style::Legacy => try_opt!(cond_shape.shrink_left(6)),
-        Style::Rfc => try_opt!(shape.offset_left(6).and_then(|s| s.sub_width(2))),
+        Style::Rfc => try_opt!(cond_shape.offset_left(6)),
     };
     let cond_str = try_opt!(cond.rewrite(context, cond_shape));
     let alt_block_sep = String::from("\n") + &shape.indent.block_only().to_string(context.config);
     let block_sep = match context.config.control_brace_style() {
         ControlBraceStyle::AlwaysNextLine => &alt_block_sep,
         _ if last_line_extendable(&cond_str) => " ",
-        _ if cond_str.contains('\n') => &alt_block_sep,
+        // 2 = ` {`
+        _ if cond_str.contains('\n') || cond_str.len() + 2 > cond_shape.width => &alt_block_sep,
         _ => " ",
     };
 
@@ -1673,7 +1687,12 @@ fn rewrite_match_pattern(
     );
 
     let items: Vec<_> = pat_strs.into_iter().map(ListItem::from_str).collect();
-    let tactic = definitive_tactic(&items, ListTactic::HorizontalVertical, pat_shape.width);
+    let tactic = definitive_tactic(
+        &items,
+        ListTactic::HorizontalVertical,
+        Separator::VerticalBar,
+        pat_shape.width,
+    );
     let fmt = ListFormatting {
         tactic: tactic,
         separator: " |",
@@ -2221,6 +2240,7 @@ where
     let tactic = definitive_tactic(
         &*item_vec,
         ListTactic::LimitedHorizontalVertical(args_max_width),
+        Separator::Comma,
         one_line_width,
     );
 
@@ -2761,6 +2781,7 @@ where
     let tactic = definitive_tactic(
         &item_vec,
         ListTactic::HorizontalVertical,
+        Separator::Comma,
         nested_shape.width,
     );
     let fmt = ListFormatting {

src/imports.rs

@@ -17,7 +17,7 @@ use Shape;
 use codemap::SpanUtils;
 use config::IndentStyle;
 use lists::{definitive_tactic, itemize_list, write_list, DefinitiveListTactic, ListFormatting,
-            ListItem, SeparatorTactic};
+            ListItem, Separator, SeparatorTactic};
 use rewrite::{Rewrite, RewriteContext};
 use types::{rewrite_path, PathContext};
 use utils;
@@ -451,6 +451,7 @@ fn rewrite_use_list(
     let tactic = definitive_tactic(
         &items[first_index..],
         context.config.imports_layout(),
+        Separator::Comma,
         remaining_width,
     );
 

src/items.rs

@@ -23,7 +23,7 @@ use config::{BraceStyle, Config, Density, IndentStyle, ReturnIndent, Style};
 use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs,
            rewrite_call_inner, ExprType};
 use lists::{definitive_tactic, itemize_list, write_list, DefinitiveListTactic, ListFormatting,
-            ListItem, ListTactic, SeparatorTactic};
+            ListItem, ListTactic, Separator, SeparatorTactic};
 use rewrite::{Rewrite, RewriteContext};
 use types::join_bounds;
 use utils::{colon_spaces, contains_skip, end_typaram, format_defaultness, format_mutability,
@@ -2244,6 +2244,7 @@ fn rewrite_args(
     let tactic = definitive_tactic(
         &arg_items,
         context.config.fn_args_density().to_list_tactic(),
+        Separator::Comma,
         one_line_budget,
     );
     let budget = match tactic {
@@ -2426,7 +2427,12 @@ where
 {
     let item_vec = items.collect::<Vec<_>>();
 
-    let tactic = definitive_tactic(&item_vec, ListTactic::HorizontalVertical, one_line_budget);
+    let tactic = definitive_tactic(
+        &item_vec,
+        ListTactic::HorizontalVertical,
+        Separator::Comma,
+        one_line_budget,
+    );
     let fmt = ListFormatting {
         tactic: tactic,
         separator: ",",
@@ -2639,7 +2645,12 @@ fn rewrite_where_clause(
     let item_vec = items.collect::<Vec<_>>();
     // FIXME: we don't need to collect here if the where_layout isn't
     // HorizontalVertical.
-    let tactic = definitive_tactic(&item_vec, context.config.where_layout(), budget);
+    let tactic = definitive_tactic(
+        &item_vec,
+        context.config.where_layout(),
+        Separator::Comma,
+        budget,
+    );
 
     let mut comma_tactic = context.config.trailing_comma();
     // Kind of a hack because we don't usually have trailing commas in where clauses.

src/lists.rs

@@ -135,7 +135,30 @@ impl DefinitiveListTactic {
     }
 }
 
-pub fn definitive_tactic<I, T>(items: I, tactic: ListTactic, width: usize) -> DefinitiveListTactic
+/// The type of separator for lists.
+#[derive(Eq, PartialEq, Debug)]
+pub enum Separator {
+    Comma,
+    VerticalBar,
+}
+
+impl Separator {
+    pub fn len(&self) -> usize {
+        match *self {
+            // 2 = `, `
+            Separator::Comma => 2,
+            // 3 = ` | `
+            Separator::VerticalBar => 3,
+        }
+    }
+}
+
+pub fn definitive_tactic<I, T>(
+    items: I,
+    tactic: ListTactic,
+    sep: Separator,
+    width: usize,
+) -> DefinitiveListTactic
 where
     I: IntoIterator<Item = T> + Clone,
     T: AsRef<ListItem>,
@@ -155,8 +178,7 @@ where
     };
 
     let (sep_count, total_width) = calculate_width(items.clone());
-    let sep_len = ", ".len(); // FIXME: make more generic?
+    let total_sep_len = sep.len() * sep_count.checked_sub(1).unwrap_or(0);
-    let total_sep_len = sep_len * sep_count.checked_sub(1).unwrap_or(0);
     let real_total = total_width + total_sep_len;
 
     if real_total <= limit && !pre_line_comments &&
@@ -640,7 +662,7 @@ pub fn struct_lit_tactic(
             (IndentStyle::Visual, 1) => ListTactic::HorizontalVertical,
             _ => context.config.struct_lit_multiline_style().to_list_tactic(),
         };
-        definitive_tactic(items, prelim_tactic, h_shape.width)
+        definitive_tactic(items, prelim_tactic, Separator::Comma, h_shape.width)
     } else {
         DefinitiveListTactic::Vertical
     }

src/types.rs

@@ -22,7 +22,7 @@ use codemap::SpanUtils;
 use config::{IndentStyle, Style, TypeDensity};
 use expr::{rewrite_pair, rewrite_tuple, rewrite_unary_prefix, wrap_args_with_parens};
 use items::{format_generics_item_list, generics_shape_from_config};
-use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTactic,
+use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTactic, Separator,
             SeparatorTactic};
 use rewrite::{Rewrite, RewriteContext};
 use utils::{colon_spaces, extra_offset, format_mutability, last_line_width, mk_sp, wrap_str};
@@ -348,7 +348,12 @@ where
 
     let item_vec: Vec<_> = items.collect();
 
-    let tactic = definitive_tactic(&*item_vec, ListTactic::HorizontalVertical, budget);
+    let tactic = definitive_tactic(
+        &*item_vec,
+        ListTactic::HorizontalVertical,
+        Separator::Comma,
+        budget,
+    );
 
     let fmt = ListFormatting {
         tactic: tactic,

src/vertical.rs

@@ -20,7 +20,7 @@ use codemap::SpanUtils;
 use comment::contains_comment;
 use expr::rewrite_field;
 use items::{rewrite_struct_field, rewrite_struct_field_prefix};
-use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTactic};
+use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTactic, Separator};
 use rewrite::{Rewrite, RewriteContext};
 use utils::{contains_skip, mk_sp};
 
@@ -221,7 +221,12 @@ fn rewrite_aligned_items_inner<T: AlignedItem>(
         span.hi,
     ).collect::<Vec<_>>();
 
-    let tactic = definitive_tactic(&items, ListTactic::HorizontalVertical, one_line_width);
+    let tactic = definitive_tactic(
+        &items,
+        ListTactic::HorizontalVertical,
+        Separator::Comma,
+        one_line_width,
+    );
 
     let fmt = ListFormatting {
         tactic: tactic,

tests/source/match.rs

@@ -388,3 +388,23 @@ fn issue525() {
         TaskState::Failed => "failed",
     });
 }
+
+// #1838, #1839
+fn match_with_near_max_width() {
+    let (this_line_uses_99_characters_and_is_formatted_properly, x012345) = match some_expression   {
+        _ => unimplemented!(),
+    };
+
+    let (should_be_formatted_like_the_line_above_using_100_characters, x0) = match some_expression  {
+        _ => unimplemented!(),
+    };
+
+    let (should_put_the_brace_on_the_next_line_using_101_characters, x0000) = match some_expression
+    {
+        _ => unimplemented!(),
+    };
+    match m {
+        Variant::Tag | Variant::Tag2 | Variant::Tag3 | Variant::Tag4 | Variant::Tag5 | Variant::Tag6 =>
+        {}
+    }
+}

tests/target/match.rs

@@ -426,3 +426,27 @@ fn issue525() {
         },
     );
 }
+
+// #1838, #1839
+fn match_with_near_max_width() {
+    let (this_line_uses_99_characters_and_is_formatted_properly, x012345) = match some_expression {
+        _ => unimplemented!(),
+    };
+
+    let (should_be_formatted_like_the_line_above_using_100_characters, x0) = match some_expression {
+        _ => unimplemented!(),
+    };
+
+    let (should_put_the_brace_on_the_next_line_using_101_characters, x0000) = match some_expression
+    {
+        _ => unimplemented!(),
+    };
+    match m {
+        Variant::Tag |
+        Variant::Tag2 |
+        Variant::Tag3 |
+        Variant::Tag4 |
+        Variant::Tag5 |
+        Variant::Tag6 => {}
+    }
+}