Block indenting for struct lit patterns

Now follows struct_lit_style (and most other struct_lit_ options).

Required a fair bit of refactoring and bug fixes.

Fixes #1311

rfc-rustfmt.toml

@@ -1,4 +1,4 @@
 fn_args_layout = "Block"
 array_layout = "Block"
 where_style = "Rfc"
-generics_indent = "Tabbed"
+generics_indent = "Block"

src/config.rs

@@ -320,7 +320,7 @@ create_config! {
     tab_spaces: usize, 4, "Number of spaces per tab";
     fn_call_width: usize, 60,
         "Maximum width of the args of a function call before falling back to vertical formatting";
-    struct_lit_width: usize, 16,
+    struct_lit_width: usize, 18,
         "Maximum width in the body of a struct lit before falling back to vertical formatting";
     struct_variant_width: usize, 35,
         "Maximum width in the body of a struct variant before falling back to vertical formatting";
@@ -380,7 +380,7 @@ create_config! {
     wrap_match_arms: bool, true, "Wrap multiline match arms in blocks";
     match_block_trailing_comma: bool, false,
         "Put a trailing comma after a block based match arm (non-block arms are not affected)";
-    closure_block_indent_threshold: isize, 5, "How many lines a closure must have before it is \
+    closure_block_indent_threshold: isize, 7, "How many lines a closure must have before it is \
                                                block indented. -1 means never use block indent.";
     space_before_type_annotation: bool, false,
         "Leave a space before the colon in a type annotation";

src/expr.rs

@@ -19,7 +19,8 @@ use {Indent, Shape, Spanned};
 use codemap::SpanUtils;
 use rewrite::{Rewrite, RewriteContext};
 use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
-            DefinitiveListTactic, definitive_tactic, ListItem, format_item_list};
+            DefinitiveListTactic, definitive_tactic, ListItem, format_item_list,
+            struct_lit_shape, struct_lit_tactic, shape_for_tactic, struct_lit_formatting};
 use string::{StringFormat, rewrite_string};
 use utils::{extra_offset, last_line_width, wrap_str, binary_search, first_line_width,
             semicolon_for_stmt, trimmed_last_line_width, left_most_sub_expr, stmt_expr};
@@ -658,8 +659,7 @@ impl Rewrite for ast::Stmt {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
         let result = match self.node {
             ast::StmtKind::Local(ref local) => {
-                local.rewrite(context,
-                              Shape::legacy(context.config.max_width, shape.indent))
+                local.rewrite(context, shape)
             }
             ast::StmtKind::Expr(ref ex) |
             ast::StmtKind::Semi(ref ex) => {
@@ -893,7 +893,6 @@ impl<'a> Rewrite for ControlFlow<'a> {
             block_width
         };
 
-        // TODO this .block() - not what we want if we are actually visually indented
         let block_shape = Shape { width: block_width, ..shape };
         let block_str = try_opt!(self.block.rewrite(context, block_shape));
 
@@ -1120,8 +1119,9 @@ fn rewrite_match(context: &RewriteContext,
     }
 
     // `match `cond` {`
-    let cond_budget = try_opt!(shape.width.checked_sub(8));
-    let cond_str = try_opt!(cond.rewrite(context, Shape::legacy(cond_budget, shape.indent + 6)));
+    let cond_shape = try_opt!(shape.shrink_left(6));
+    let cond_shape = try_opt!(cond_shape.sub_width(2));
+    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::AlwaysSameLine => " ",
@@ -1563,7 +1563,11 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
     let callee = callee.borrow();
     // FIXME using byte lens instead of char lens (and probably all over the
     // place too)
-    let callee_str = match callee.rewrite(context, Shape { width: max_callee_width, ..shape }) {
+    let callee_str = match callee.rewrite(context,
+                                          Shape {
+                                              width: max_callee_width,
+                                              ..shape
+                                          }) {
         Some(string) => {
             if !string.contains('\n') && string.len() > max_callee_width {
                 panic!("{:?} {}", string, max_callee_width);
@@ -1731,115 +1735,71 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
     let path_shape = try_opt!(shape.sub_width(2));
     let path_str = try_opt!(rewrite_path(context, PathContext::Expr, None, path, path_shape));
 
-    // Foo { a: Foo } - indent is +3, width is -5.
-    let h_shape = shape.sub_width(path_str.len() + 5);
-    let v_shape = match context.config.struct_lit_style {
-        IndentStyle::Visual => {
-            try_opt!(try_opt!(shape.shrink_left(path_str.len() + 3)).sub_width(2))
-        }
-        IndentStyle::Block => {
-            let shape = shape.block_indent(context.config.tab_spaces);
-            Shape {
-                width: try_opt!(context.config.max_width.checked_sub(shape.indent.width())),
-                ..shape
-            }
-        }
-    };
+    if fields.len() == 0 && base.is_none() {
+        return Some(format!("{} {{}}", path_str));
+    }
 
     let field_iter = fields.into_iter()
         .map(StructLitField::Regular)
         .chain(base.into_iter().map(StructLitField::Base));
 
+    // Foo { a: Foo } - indent is +3, width is -5.
+    let (h_shape, v_shape) = try_opt!(struct_lit_shape(shape, context, path_str.len() + 3, 2));
+
+    let span_lo = |item: &StructLitField| match *item {
+        StructLitField::Regular(field) => field.span.lo,
+        StructLitField::Base(expr) => {
+            let last_field_hi = fields.last().map_or(span.lo, |field| field.span.hi);
+            let snippet = context.snippet(mk_sp(last_field_hi, expr.span.lo));
+            let pos = snippet.find_uncommented("..").unwrap();
+            last_field_hi + BytePos(pos as u32)
+        }
+    };
+    let span_hi = |item: &StructLitField| match *item {
+        StructLitField::Regular(field) => field.span.hi,
+        StructLitField::Base(expr) => expr.span.hi,
+    };
+    let rewrite = |item: &StructLitField| match *item {
+        StructLitField::Regular(field) => {
+            // The 1 taken from the v_budget is for the comma.
+            rewrite_field(context, field, try_opt!(v_shape.sub_width(1)))
+        }
+        StructLitField::Base(expr) => {
+            // 2 = ..
+            expr.rewrite(context, try_opt!(v_shape.shrink_left(2))).map(|s| format!("..{}", s))
+        }
+    };
+
     let items = itemize_list(context.codemap,
                              field_iter,
                              "}",
-                             |item| match *item {
-                                 StructLitField::Regular(field) => field.span.lo,
-                                 StructLitField::Base(expr) => {
-        let last_field_hi = fields.last().map_or(span.lo, |field| field.span.hi);
-        let snippet = context.snippet(mk_sp(last_field_hi, expr.span.lo));
-        let pos = snippet.find_uncommented("..").unwrap();
-        last_field_hi + BytePos(pos as u32)
-    }
-                             },
-                             |item| match *item {
-                                 StructLitField::Regular(field) => field.span.hi,
-                                 StructLitField::Base(expr) => expr.span.hi,
-                             },
-                             |item| {
-        match *item {
-            StructLitField::Regular(field) => {
-                // The 1 taken from the v_budget is for the comma.
-                rewrite_field(context, field, try_opt!(v_shape.sub_width(1)))
-            }
-            StructLitField::Base(expr) => {
-                // 2 = ..
-                expr.rewrite(context, try_opt!(v_shape.shrink_left(2))).map(|s| format!("..{}", s))
-            }
-        }
-    },
+                             span_lo,
+                             span_hi,
+                             rewrite,
                              context.codemap.span_after(span, "{"),
                              span.hi);
     let item_vec = items.collect::<Vec<_>>();
 
-    let tactic = if let Some(h_shape) = h_shape {
-        let mut prelim_tactic = match (context.config.struct_lit_style, fields.len()) {
-            (IndentStyle::Visual, 1) => ListTactic::HorizontalVertical,
-            _ => context.config.struct_lit_multiline_style.to_list_tactic(),
-        };
+    let tactic = struct_lit_tactic(h_shape, context, &item_vec);
+    let nested_shape = shape_for_tactic(tactic, h_shape, v_shape);
+    let fmt = struct_lit_formatting(nested_shape, tactic, context, base.is_some());
 
-        if prelim_tactic == ListTactic::HorizontalVertical && fields.len() > 1 {
-            prelim_tactic = ListTactic::LimitedHorizontalVertical(context.config.struct_lit_width);
-        }
-
-        definitive_tactic(&item_vec, prelim_tactic, h_shape.width)
-    } else {
-        DefinitiveListTactic::Vertical
-    };
-
-    let nested_shape = match tactic {
-        DefinitiveListTactic::Horizontal => h_shape.unwrap(),
-        _ => v_shape,
-    };
-
-    let ends_with_newline = context.config.struct_lit_style != IndentStyle::Visual &&
-                            tactic == DefinitiveListTactic::Vertical;
-
-    let fmt = ListFormatting {
-        tactic: tactic,
-        separator: ",",
-        trailing_separator: if base.is_some() {
-            SeparatorTactic::Never
-        } else {
-            context.config.trailing_comma
-        },
-        shape: nested_shape,
-        ends_with_newline: ends_with_newline,
-        config: context.config,
-    };
     let fields_str = try_opt!(write_list(&item_vec, &fmt));
+    let fields_str = if context.config.struct_lit_style == IndentStyle::Block &&
+                        (fields_str.contains('\n') ||
+                         context.config.struct_lit_multiline_style == MultilineStyle::ForceMulti ||
+                         fields_str.len() > h_shape.map(|s| s.width).unwrap_or(0)) {
+        format!("\n{}{}\n{}",
+                v_shape.indent.to_string(context.config),
+                fields_str,
+                shape.indent.to_string(context.config))
+    } else {
+        // One liner or visual indent.
+        format!(" {} ", fields_str)
+    };
 
-    // Empty struct.
-    if fields_str.is_empty() {
-        return Some(format!("{} {{}}", path_str));
-    }
+    Some(format!("{} {{{}}}", path_str, fields_str))
 
-    // One liner or visual indent.
-    if context.config.struct_lit_style == IndentStyle::Visual ||
-       (context.config.struct_lit_multiline_style != MultilineStyle::ForceMulti &&
-        !fields_str.contains('\n') &&
-        fields_str.len() <= h_shape.map(|s| s.width).unwrap_or(0)) {
-        return Some(format!("{} {{ {} }}", path_str, fields_str));
-    }
-
-    // Multiple lines.
-    let inner_indent = v_shape.indent.to_string(context.config);
-    let outer_indent = shape.indent.to_string(context.config);
-    Some(format!("{} {{\n{}{}\n{}}}",
-                 path_str,
-                 inner_indent,
-                 fields_str,
-                 outer_indent))
     // FIXME if context.config.struct_lit_style == Visual, but we run out
     // of space, we should fall back to BlockIndent.
 }
@@ -1996,7 +1956,7 @@ pub fn rewrite_assign_rhs<S: Into<String>>(context: &RewriteContext,
     let max_width = try_opt!(shape.width.checked_sub(last_line_width + 1));
     let rhs = ex.rewrite(context,
                          Shape::offset(max_width,
-                                       shape.indent.block_only(),
+                                       shape.indent,
                                        shape.indent.alignment + last_line_width + 1));
 
     fn count_line_breaks(src: &str) -> usize {

src/items.rs

@@ -37,12 +37,11 @@ impl Rewrite for ast::Local {
                shape.width,
                shape.indent);
         let mut result = "let ".to_owned();
-        let pattern_offset = shape.indent + result.len();
-        // 1 = ;
-        let pattern_width = try_opt!(shape.width.checked_sub(pattern_offset.width() + 1));
 
-        let pat_str = try_opt!(self.pat.rewrite(&context,
-                                                Shape::legacy(pattern_width, pattern_offset)));
+        let pat_shape = try_opt!(shape.offset_left(result.len()));
+        // 1 = ;
+        let pat_shape = try_opt!(pat_shape.sub_width(1));
+        let pat_str = try_opt!(self.pat.rewrite(&context, pat_shape));
         result.push_str(&pat_str);
 
         // String that is placed within the assignment pattern and expression.
@@ -71,12 +70,13 @@ impl Rewrite for ast::Local {
 
         if let Some(ref ex) = self.init {
             // 1 = trailing semicolon;
-            let budget = try_opt!(shape.width.checked_sub(shape.indent.block_only().width() + 1));
+            //let budget = try_opt!(shape.width.checked_sub(shape.indent.block_only().width() + 1));
+            let nested_shape = try_opt!(shape.sub_width(1));
 
             result = try_opt!(rewrite_assign_rhs(&context,
                                                  result,
                                                  ex,
-                                                 Shape::legacy(budget, shape.indent.block_only())));
+                                                 nested_shape));
         }
 
         result.push(';');

src/lib.rs

@@ -330,6 +330,14 @@ impl Shape {
              })
     }
 
+    pub fn offset_left(&self, width: usize) -> Option<Shape> {
+        Some(Shape {
+                 width: try_opt!(self.width.checked_sub(width)),
+                 indent: self.indent,
+                 offset: self.offset + width,
+             })
+    }
+
     pub fn used_width(&self) -> usize {
         self.indent.block_indent + self.offset
     }

src/lists.rs

@@ -15,7 +15,8 @@ use syntax::codemap::{self, CodeMap, BytePos};
 
 use {Indent, Shape};
 use comment::{FindUncommented, rewrite_comment, find_comment_end};
-use config::Config;
+use config::{Config, IndentStyle};
+use rewrite::RewriteContext;
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
 /// Formatting tactic for lists. This will be cast down to a
@@ -502,3 +503,81 @@ fn comment_len(comment: Option<&str>) -> usize {
         None => 0,
     }
 }
+
+// Compute horizontal and vertical shapes for a struct-lit-like thing.
+pub fn struct_lit_shape(shape: Shape,
+                        context: &RewriteContext,
+                        prefix_width: usize,
+                        suffix_width: usize)
+                        -> Option<(Option<Shape>, Shape)> {
+    let v_shape = match context.config.struct_lit_style {
+        IndentStyle::Visual => {
+            try_opt!(try_opt!(shape.shrink_left(prefix_width)).sub_width(suffix_width))
+        }
+        IndentStyle::Block => {
+            let shape = shape.block_indent(context.config.tab_spaces);
+            Shape {
+                width: try_opt!(context.config.max_width.checked_sub(shape.indent.width())),
+                ..shape
+            }
+        }
+    };
+    let h_shape = shape.sub_width(prefix_width + suffix_width);
+    Some((h_shape, v_shape))
+}
+
+// Compute the tactic for the internals of a struct-lit-like thing.
+pub fn struct_lit_tactic(h_shape: Option<Shape>,
+                         context: &RewriteContext,
+                         items: &[ListItem])
+                         -> DefinitiveListTactic {
+    if let Some(h_shape) = h_shape {
+        let mut prelim_tactic = match (context.config.struct_lit_style, items.len()) {
+            (IndentStyle::Visual, 1) => ListTactic::HorizontalVertical,
+            _ => context.config.struct_lit_multiline_style.to_list_tactic(),
+        };
+
+        if prelim_tactic == ListTactic::HorizontalVertical && items.len() > 1 {
+            prelim_tactic = ListTactic::LimitedHorizontalVertical(context.config.struct_lit_width);
+        }
+
+        definitive_tactic(items, prelim_tactic, h_shape.width)
+    } else {
+        DefinitiveListTactic::Vertical
+    }
+}
+
+// Given a tactic and possible shapes for horizontal and vertical layout,
+// come up with the actual shape to use.
+pub fn shape_for_tactic(tactic: DefinitiveListTactic,
+                        h_shape: Option<Shape>,
+                        v_shape: Shape)
+                        -> Shape {
+    match tactic {
+        DefinitiveListTactic::Horizontal => h_shape.unwrap(),
+        _ => v_shape,
+    }
+}
+
+// Create a ListFormatting object for formatting the internals of a
+// struct-lit-like thing, that is a series of fields.
+pub fn struct_lit_formatting<'a>(shape: Shape,
+                                 tactic: DefinitiveListTactic,
+                                 context: &'a RewriteContext,
+                                 force_no_trailing_comma: bool)
+                                 -> ListFormatting<'a> {
+    let ends_with_newline = context.config.struct_lit_style != IndentStyle::Visual &&
+                            tactic == DefinitiveListTactic::Vertical;
+    ListFormatting {
+        tactic: tactic,
+        separator: ",",
+        trailing_separator: if force_no_trailing_comma {
+            SeparatorTactic::Never
+        } else {
+            context.config.trailing_comma
+        },
+        shape: shape,
+        ends_with_newline: ends_with_newline,
+        config: context.config,
+    }
+}

src/patterns.rs

@@ -10,9 +10,11 @@
 
 use Shape;
 use codemap::SpanUtils;
+use config::{IndentStyle, MultilineStyle};
 use rewrite::{Rewrite, RewriteContext};
 use utils::{wrap_str, format_mutability};
-use lists::{format_item_list, itemize_list, ListItem};
+use lists::{format_item_list, itemize_list, ListItem, struct_lit_shape, struct_lit_tactic,
+            shape_for_tactic, struct_lit_formatting, write_list};
 use expr::{rewrite_unary_prefix, rewrite_pair};
 use types::{rewrite_path, PathContext};
 use super::Spanned;
@@ -112,48 +114,7 @@ impl Rewrite for Pat {
                 wrap_str(result, context.config.max_width, shape)
             }
             PatKind::Struct(ref path, ref fields, elipses) => {
-                let path = try_opt!(rewrite_path(context, PathContext::Expr, None, path, shape));
-
-                let (elipses_str, terminator) = if elipses { (", ..", "..") } else { ("", "}") };
-
-                // 5 = `{` plus space before and after plus `}` plus space before.
-                let budget = try_opt!(shape.width.checked_sub(path.len() + 5 + elipses_str.len()));
-                // FIXME Using visual indenting, should use block or visual to match
-                // struct lit preference (however, in practice I think it is rare
-                // for struct patterns to be multi-line).
-                // 3 = `{` plus space before and after.
-                let offset = shape.indent + path.len() + 3;
-
-                let items =
-                    itemize_list(context.codemap,
-                                 fields.iter(),
-                                 terminator,
-                                 |f| f.span.lo,
-                                 |f| f.span.hi,
-                                 |f| f.node.rewrite(context, Shape::legacy(budget, offset)),
-                                 context.codemap.span_after(self.span, "{"),
-                                 self.span.hi);
-                let mut field_string = try_opt!(format_item_list(items,
-                                                                 Shape::legacy(budget, offset),
-                                                                 context.config));
-                if elipses {
-                    if field_string.contains('\n') {
-                        field_string.push_str(",\n");
-                        field_string.push_str(&offset.to_string(context.config));
-                        field_string.push_str("..");
-                    } else {
-                        if !field_string.is_empty() {
-                            field_string.push_str(", ");
-                        }
-                        field_string.push_str("..");
-                    }
-                }
-
-                if field_string.is_empty() {
-                    Some(format!("{} {{}}", path))
-                } else {
-                    Some(format!("{} {{ {} }}", path, field_string))
-                }
+                rewrite_struct_pat(path, fields, elipses, self.span, context, shape)
             }
             // FIXME(#819) format pattern macros.
             PatKind::Mac(..) => {
@@ -163,6 +124,72 @@ impl Rewrite for Pat {
     }
 }
 
+fn rewrite_struct_pat(path: &ast::Path,
+                      fields: &[codemap::Spanned<ast::FieldPat>],
+                      elipses: bool,
+                      span: Span,
+                      context: &RewriteContext,
+                      shape: Shape)
+                      -> Option<String> {
+    let path_shape = try_opt!(shape.sub_width(2));
+    let path_str = try_opt!(rewrite_path(context, PathContext::Expr, None, path, path_shape));
+
+    if fields.len() == 0 && !elipses {
+        return Some(format!("{} {{}}", path_str));
+    }
+
+    let (elipses_str, terminator) = if elipses { (", ..", "..") } else { ("", "}") };
+
+    // 3 = ` { `, 2 = ` }`.
+    let (h_shape, v_shape) =
+        try_opt!(struct_lit_shape(shape, context, path_str.len() + 3, elipses_str.len() + 2));
+
+    let items = itemize_list(context.codemap,
+                             fields.iter(),
+                             terminator,
+                             |f| f.span.lo,
+                             |f| f.span.hi,
+                             |f| f.node.rewrite(context, v_shape),
+                             context.codemap.span_after(span, "{"),
+                             span.hi);
+    let item_vec = items.collect::<Vec<_>>();
+
+    let tactic = struct_lit_tactic(h_shape, context, &item_vec);
+    let nested_shape = shape_for_tactic(tactic, h_shape, v_shape);
+    let fmt = struct_lit_formatting(nested_shape, tactic, context, false);
+
+    let mut fields_str = try_opt!(write_list(&item_vec, &fmt));
+
+    if elipses {
+        if fields_str.contains('\n') {
+            fields_str.push_str("\n");
+            fields_str.push_str(&nested_shape.indent.to_string(context.config));
+            fields_str.push_str("..");
+        } else {
+            if !fields_str.is_empty() {
+                fields_str.push_str(", ");
+            }
+            fields_str.push_str("..");
+        }
+    }
+
+
+    let fields_str = if context.config.struct_lit_style == IndentStyle::Block &&
+                        (fields_str.contains('\n') ||
+                         context.config.struct_lit_multiline_style == MultilineStyle::ForceMulti ||
+                         fields_str.len() > h_shape.map(|s| s.width).unwrap_or(0)) {
+        format!("\n{}{}\n{}",
+                v_shape.indent.to_string(context.config),
+                fields_str,
+                shape.indent.to_string(context.config))
+    } else {
+        // One liner or visual indent.
+        format!(" {} ", fields_str)
+    };
+
+    Some(format!("{} {{{}}}", path_str, fields_str))
+}
+
 impl Rewrite for FieldPat {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
         let pat = self.pat.rewrite(context, shape);
@@ -176,7 +203,6 @@ impl Rewrite for FieldPat {
     }
 }
 
-
 enum TuplePatField<'a> {
     Pat(&'a ptr::P<ast::Pat>),
     Dotdot(Span),

tests/source/closure.rs

@@ -81,3 +81,11 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
         }
     }
 }
+
+fn foo() {
+    lifetimes_iter___map(|lasdfasfd| {
+        let hi = if l.bounds.is_empty() {
+            l.lifetime.span.hi
+        };
+    });
+}

tests/target/closure.rs

@@ -98,3 +98,11 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
         }
     }
 }
+
+fn foo() {
+    lifetimes_iter___map(|lasdfasfd| {
+                             let hi = if l.bounds.is_empty() {
+                                 l.lifetime.span.hi
+                             };
+                         });
+}