Format array using overflow module

This commit applies heuristics used for function calls to array
and vice versa.

diff --git a/src/expr.rs b/src/expr.rs
index 10f2cd2aba77079ab3b75f708c0a3b6a433086e1..c719f9938ee995c8bb43992f30692deea44b9681 100644 (file)
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -13,6 +13,7 @@
 
 use config::lists::*;
 use syntax::codemap::{BytePos, CodeMap, Span};
+use syntax::parse::token::DelimToken;
 use syntax::{ast, ptr};
 
 use chains::rewrite_chain;
@@ -64,14 +65,13 @@ pub fn format_expr(
 
     let expr_rw = match expr.node {
         ast::ExprKind::Array(ref expr_vec) => rewrite_array(
+            "",
             &ptr_vec_to_ref_vec(expr_vec),
-            mk_sp(
-                context.snippet_provider.span_after(expr.span, "["),
-                expr.span.hi(),
-            ),
+            expr.span,
             context,
             shape,
-            false,
+            None,
+            None,
         ),
         ast::ExprKind::Lit(ref l) => rewrite_literal(context, l, shape),
         ast::ExprKind::Call(ref callee, ref args) => {
@@ -422,147 +422,23 @@ pub fn rewrite_pair<LHS, RHS>(
 }
 
 pub fn rewrite_array<T: Rewrite + Spanned + ToExpr>(
+    name: &str,
     exprs: &[&T],
     span: Span,
     context: &RewriteContext,
     shape: Shape,
-    trailing_comma: bool,
+    force_separator_tactic: Option<SeparatorTactic>,
+    delim_token: Option<DelimToken>,
 ) -> Option<String> {
-    let bracket_size = if context.config.spaces_within_parens_and_brackets() {
-        2 // "[ "
-    } else {
-        1 // "["
-    };
-
-    let nested_shape = match context.config.indent_style() {
-        IndentStyle::Block => shape
-            .block()
-            .block_indent(context.config.tab_spaces())
-            .with_max_width(context.config)
-            .sub_width(1)?,
-        IndentStyle::Visual => shape
-            .visual_indent(bracket_size)
-            .sub_width(bracket_size * 2)?,
-    };
-
-    let items = itemize_list(
-        context.snippet_provider,
-        exprs.iter(),
-        "]",
-        ",",
-        |item| item.span().lo(),
-        |item| item.span().hi(),
-        |item| item.rewrite(context, nested_shape),
-        span.lo(),
-        span.hi(),
-        false,
-    ).collect::<Vec<_>>();
-
-    if items.is_empty() {
-        if context.config.spaces_within_parens_and_brackets() {
-            return Some("[ ]".to_string());
-        } else {
-            return Some("[]".to_string());
-        }
-    }
-
-    let tactic = array_tactic(context, shape, nested_shape, exprs, &items, bracket_size);
-    let ends_with_newline = tactic.ends_with_newline(context.config.indent_style());
-
-    let fmt = ListFormatting {
-        tactic,
-        separator: ",",
-        trailing_separator: if trailing_comma {
-            SeparatorTactic::Always
-        } else if context.inside_macro() && !exprs.is_empty() {
-            let ends_with_bracket = context.snippet(span).ends_with(']');
-            let bracket_offset = if ends_with_bracket { 1 } else { 0 };
-            let snippet = context.snippet(mk_sp(span.lo(), span.hi() - BytePos(bracket_offset)));
-            let last_char_index = snippet.rfind(|c: char| !c.is_whitespace())?;
-            if &snippet[last_char_index..last_char_index + 1] == "," {
-                SeparatorTactic::Always
-            } else {
-                SeparatorTactic::Never
-            }
-        } else if context.config.indent_style() == IndentStyle::Visual {
-            SeparatorTactic::Never
-        } else {
-            SeparatorTactic::Vertical
-        },
-        separator_place: SeparatorPlace::Back,
-        shape: nested_shape,
-        ends_with_newline,
-        preserve_newline: false,
-        config: context.config,
-    };
-    let list_str = write_list(&items, &fmt)?;
-
-    let result = if context.config.indent_style() == IndentStyle::Visual
-        || tactic == DefinitiveListTactic::Horizontal
-    {
-        if context.config.spaces_within_parens_and_brackets() && !list_str.is_empty() {
-            format!("[ {} ]", list_str)
-        } else {
-            format!("[{}]", list_str)
-        }
-    } else {
-        format!(
-            "[{}{}{}]",
-            nested_shape.indent.to_string_with_newline(context.config),
-            list_str,
-            shape.block().indent.to_string_with_newline(context.config)
-        )
-    };
-
-    Some(result)
-}
-
-fn array_tactic<T: Rewrite + Spanned + ToExpr>(
-    context: &RewriteContext,
-    shape: Shape,
-    nested_shape: Shape,
-    exprs: &[&T],
-    items: &[ListItem],
-    bracket_size: usize,
-) -> DefinitiveListTactic {
-    let has_long_item = items
-        .iter()
-        .any(|li| li.item.as_ref().map(|s| s.len() > 10).unwrap_or(false));
-
-    match context.config.indent_style() {
-        IndentStyle::Block => {
-            let tactic = match shape.width.checked_sub(2 * bracket_size) {
-                Some(width) => {
-                    let tactic = ListTactic::LimitedHorizontalVertical(
-                        context.config.width_heuristics().array_width,
-                    );
-                    definitive_tactic(items, tactic, Separator::Comma, width)
-                }
-                None => DefinitiveListTactic::Vertical,
-            };
-            if tactic == DefinitiveListTactic::Vertical && !has_long_item
-                && is_every_expr_simple(exprs)
-            {
-                DefinitiveListTactic::Mixed
-            } else {
-                tactic
-            }
-        }
-        IndentStyle::Visual => {
-            if has_long_item || items.iter().any(ListItem::is_multiline) {
-                definitive_tactic(
-                    items,
-                    ListTactic::LimitedHorizontalVertical(
-                        context.config.width_heuristics().array_width,
-                    ),
-                    Separator::Comma,
-                    nested_shape.width,
-                )
-            } else {
-                DefinitiveListTactic::Mixed
-            }
-        }
-    }
+    overflow::rewrite_with_square_brackets(
+        context,
+        name,
+        exprs,
+        shape,
+        span,
+        force_separator_tactic,
+        delim_token,
+    )
 }
 
 fn rewrite_empty_block(
@@ -1489,7 +1365,7 @@ fn is_simple_expr(expr: &ast::Expr) -> bool {
     }
 }
 
-fn is_every_expr_simple<T: ToExpr>(lists: &[&T]) -> bool {
+pub fn is_every_expr_simple<T: ToExpr>(lists: &[&T]) -> bool {
     lists
         .iter()
         .all(|arg| arg.to_expr().map_or(false, is_simple_expr))

diff --git a/src/lists.rs b/src/lists.rs
index fdb022db077ff7c4610980f92dee2cc6106de16b..05b1a7ce2963be91e8575f935ecf49fb3b741877 100644 (file)
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -101,7 +101,7 @@ pub fn is_multiline(&self) -> bool {
                 .map_or(false, |s| s.contains('\n'))
     }
 
-    pub fn has_comment(&self) -> bool {
+    pub fn has_single_line_comment(&self) -> bool {
         self.pre_comment
             .as_ref()
             .map_or(false, |comment| comment.trim_left().starts_with("//"))
@@ -110,6 +110,10 @@ pub fn has_comment(&self) -> bool {
                 .map_or(false, |comment| comment.trim_left().starts_with("//"))
     }
 
+    pub fn has_comment(&self) -> bool {
+        self.pre_comment.is_some() || self.post_comment.is_some()
+    }
+
     pub fn from_str<S: Into<String>>(s: S) -> ListItem {
         ListItem {
             pre_comment: None,
@@ -164,7 +168,7 @@ pub fn definitive_tactic<I, T>(
     let pre_line_comments = items
         .clone()
         .into_iter()
-        .any(|item| item.as_ref().has_comment());
+        .any(|item| item.as_ref().has_single_line_comment());
 
     let limit = match tactic {
         _ if pre_line_comments => return DefinitiveListTactic::Vertical,
@@ -266,11 +270,15 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
                     result.push_str(indent_str);
                     line_len = 0;
                     if formatting.ends_with_newline {
-                        if last {
-                            separate = true;
-                        } else {
-                            trailing_separator = true;
-                        }
+                        trailing_separator = true;
+                    }
+                }
+
+                if last && formatting.ends_with_newline {
+                    match formatting.trailing_separator {
+                        SeparatorTactic::Always => separate = true,
+                        SeparatorTactic::Vertical if result.contains('\n') => separate = true,
+                        _ => (),
                     }
                 }
 

diff --git a/src/macros.rs b/src/macros.rs
index 0387fb3a96356193ff473f7e6eb6cb751c063dcd..c65cc74b5f4877497c0e649e970a06b40d778838 100644 (file)
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -256,6 +256,7 @@ pub fn rewrite_macro_inner(
         DelimToken::Bracket => {
             // Handle special case: `vec![expr; expr]`
             if vec_with_semi {
+                let mac_shape = shape.offset_left(macro_name.len())?;
                 let (lbr, rbr) = if context.config.spaces_within_parens_and_brackets() {
                     ("[ ", " ]")
                 } else {
@@ -287,25 +288,35 @@ pub fn rewrite_macro_inner(
                 // If we are rewriting `vec!` macro or other special macros,
                 // then we can rewrite this as an usual array literal.
                 // Otherwise, we must preserve the original existence of trailing comma.
-                if FORCED_BRACKET_MACROS.contains(&macro_name.as_str()) {
+                let macro_name = &macro_name.as_str();
+                let mut force_trailing_comma = if trailing_comma {
+                    Some(SeparatorTactic::Always)
+                } else {
+                    Some(SeparatorTactic::Never)
+                };
+                if FORCED_BRACKET_MACROS.contains(macro_name) {
                     context.inside_macro.replace(false);
-                    trailing_comma = false;
+                    if context.use_block_indent() {
+                        force_trailing_comma = Some(SeparatorTactic::Vertical);
+                    };
                 }
                 // Convert `MacroArg` into `ast::Expr`, as `rewrite_array` only accepts the latter.
-                let sp = mk_sp(
-                    context
-                        .snippet_provider
-                        .span_after(mac.span, original_style.opener()),
-                    mac.span.hi() - BytePos(1),
-                );
                 let arg_vec = &arg_vec.iter().map(|e| &*e).collect::<Vec<_>>()[..];
-                let rewrite = rewrite_array(arg_vec, sp, context, mac_shape, trailing_comma)?;
+                let rewrite = rewrite_array(
+                    macro_name,
+                    arg_vec,
+                    mac.span,
+                    context,
+                    shape,
+                    force_trailing_comma,
+                    Some(original_style),
+                )?;
                 let comma = match position {
                     MacroPosition::Item => ";",
                     _ => "",
                 };
 
-                Some(format!("{}{}{}", macro_name, rewrite, comma))
+                Some(format!("{}{}", rewrite, comma))
             }
         }
         DelimToken::Brace => {

diff --git a/src/overflow.rs b/src/overflow.rs
index 9d586c0ffa4a442ffc218e0e0973fcaf7cbd5afa..6383d032273bdf4e4bcab637137b8d5ffd9f9417 100644 (file)
--- a/src/overflow.rs
+++ b/src/overflow.rs
@@ -14,10 +14,11 @@
 use config::lists::*;
 use syntax::ast;
 use syntax::codemap::Span;
+use syntax::parse::token::DelimToken;
 
 use closures;
 use codemap::SpanUtils;
-use expr::{is_nested_call, maybe_get_args_offset, ToExpr};
+use expr::{is_every_expr_simple, is_nested_call, maybe_get_args_offset, ToExpr};
 use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator};
 use rewrite::{Rewrite, RewriteContext};
 use shape::Shape;
@@ -26,6 +27,8 @@
 
 use std::cmp::min;
 
+const SHORT_ITEM_THRESHOLD: usize = 10;
+
 pub fn rewrite_with_parens<T>(
     context: &RewriteContext,
     ident: &str,
@@ -48,6 +51,7 @@ pub fn rewrite_with_parens<T>(
         ")",
         item_max_width,
         force_separator_tactic,
+        None,
     ).rewrite(shape)
 }
 
@@ -71,6 +75,38 @@ pub fn rewrite_with_angle_brackets<T>(
         ">",
         context.config.max_width(),
         None,
+        None,
+    ).rewrite(shape)
+}
+
+pub fn rewrite_with_square_brackets<T>(
+    context: &RewriteContext,
+    name: &str,
+    items: &[&T],
+    shape: Shape,
+    span: Span,
+    force_separator_tactic: Option<SeparatorTactic>,
+    delim_token: Option<DelimToken>,
+) -> Option<String>
+where
+    T: Rewrite + ToExpr + Spanned,
+{
+    let (lhs, rhs) = match delim_token {
+        Some(DelimToken::Paren) => ("(", ")"),
+        Some(DelimToken::Brace) => ("{", "}"),
+        _ => ("[", "]"),
+    };
+    Context::new(
+        context,
+        items,
+        name,
+        shape,
+        span,
+        lhs,
+        rhs,
+        context.config.width_heuristics().array_width,
+        force_separator_tactic,
+        Some(("[", "]")),
     ).rewrite(shape)
 }
 
@@ -86,6 +122,7 @@ struct Context<'a, T: 'a> {
     item_max_width: usize,
     one_line_width: usize,
     force_separator_tactic: Option<SeparatorTactic>,
+    custom_delims: Option<(&'a str, &'a str)>,
 }
 
 impl<'a, T: 'a + Rewrite + ToExpr + Spanned> Context<'a, T> {
@@ -99,6 +136,7 @@ pub fn new(
         suffix: &'static str,
         item_max_width: usize,
         force_separator_tactic: Option<SeparatorTactic>,
+        custom_delims: Option<(&'a str, &'a str)>,
     ) -> Context<'a, T> {
         // 2 = `( `, 1 = `(`
         let paren_overhead = if context.config.spaces_within_parens_and_brackets() {
@@ -135,6 +173,7 @@ pub fn new(
             item_max_width,
             one_line_width,
             force_separator_tactic,
+            custom_delims,
         }
     }
 
@@ -301,6 +340,8 @@ fn try_overflow_last_item(&self, list_items: &mut Vec<ListItem>) -> DefinitiveLi
                             if one_line {
                                 tactic = DefinitiveListTactic::SpecialMacro(num_args_before);
                             };
+                        } else if is_every_expr_simple(self.items) && no_long_items(list_items) {
+                            tactic = DefinitiveListTactic::Mixed;
                         }
                     }
                 }
@@ -339,13 +380,20 @@ fn rewrite_items(&self) -> Option<(bool, String)> {
                 tactic
             } else if !self.context.use_block_indent() {
                 SeparatorTactic::Never
+            } else if tactic == DefinitiveListTactic::Mixed {
+                // We are using mixed layout because everything did not fit within a single line.
+                SeparatorTactic::Always
             } else {
                 self.context.config.trailing_comma()
             },
             separator_place: SeparatorPlace::Back,
             shape: self.nested_shape,
-            ends_with_newline: self.context.use_block_indent()
-                && tactic == DefinitiveListTactic::Vertical,
+            ends_with_newline: match tactic {
+                DefinitiveListTactic::Vertical | DefinitiveListTactic::Mixed => {
+                    self.context.use_block_indent()
+                }
+                _ => false,
+            },
             preserve_newline: false,
             config: self.context.config,
         };
@@ -363,6 +411,10 @@ fn wrap_items(&self, items_str: &str, shape: Shape, is_extendable: bool) -> Stri
             ..shape
         };
 
+        let (prefix, suffix) = match self.custom_delims {
+            Some((lhs, rhs)) => (lhs, rhs),
+            _ => (self.prefix, self.suffix),
+        };
         let paren_overhead = paren_overhead(self.context);
         let fits_one_line = items_str.len() + paren_overhead <= shape.width;
         let extend_width = if items_str.is_empty() {
@@ -381,7 +433,7 @@ fn wrap_items(&self, items_str: &str, shape: Shape, is_extendable: bool) -> Stri
             self.ident.len() + items_str.len() + 2 + indent_str.len() + nested_indent_str.len(),
         );
         result.push_str(self.ident);
-        result.push_str(self.prefix);
+        result.push_str(prefix);
         if !self.context.use_block_indent()
             || (self.context.inside_macro() && !items_str.contains('\n') && fits_one_line)
             || (is_extendable && extend_width <= shape.width)
@@ -400,7 +452,7 @@ fn wrap_items(&self, items_str: &str, shape: Shape, is_extendable: bool) -> Stri
             }
             result.push_str(&indent_str);
         }
-        result.push_str(self.suffix);
+        result.push_str(suffix);
         result
     }
 
@@ -488,3 +540,8 @@ fn shape_from_indent_style(
         }
     }
 }
+
+fn no_long_items(list: &[ListItem]) -> bool {
+    list.iter()
+        .all(|item| !item.has_comment() && item.inner_as_ref().len() <= SHORT_ITEM_THRESHOLD)
+}