From 4bb31a72315806e2a71dab0874132b988d303e60 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Tue, 21 Mar 2017 11:23:59 +1300
Subject: [PATCH] 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 | 2 +-
src/config.rs | 4 +-
src/expr.rs | 162 +++++++++++++++-------------------------
src/items.rs | 14 ++--
src/lib.rs | 8 ++
src/lists.rs | 81 +++++++++++++++++++-
src/patterns.rs | 114 +++++++++++++++++-----------
tests/source/closure.rs | 8 ++
tests/target/closure.rs | 8 ++
9 files changed, 245 insertions(+), 156 deletions(-)
diff --git a/rfc-rustfmt.toml b/rfc-rustfmt.toml
index 11bfda45956..a46692917b8 100644
--- a/rfc-rustfmt.toml
+++ b/rfc-rustfmt.toml
@@ -1,4 +1,4 @@
fn_args_layout = "Block"
array_layout = "Block"
where_style = "Rfc"
-generics_indent = "Tabbed"
+generics_indent = "Block"
diff --git a/src/config.rs b/src/config.rs
index 91438a00f08..92a5aa7611e 100644
--- a/src/config.rs
+++ b/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";
diff --git a/src/expr.rs b/src/expr.rs
index 9e1237bd7c2..aa4988740dc 100644
--- a/src/expr.rs
+++ b/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 {
diff --git a/src/items.rs b/src/items.rs
index 69acab70f34..06c5552121f 100644
--- a/src/items.rs
+++ b/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(';');
diff --git a/src/lib.rs b/src/lib.rs
index f37466bebfc..aa93061602e 100644
--- a/src/lib.rs
+++ b/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
}
diff --git a/src/lists.rs b/src/lists.rs
index 5c25c8b1cea..2bdb9cee695 100644
--- a/src/lists.rs
+++ b/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,
+ }
+}
diff --git a/src/patterns.rs b/src/patterns.rs
index c1b3de56cfb..18def6bfb64 100644
--- a/src/patterns.rs
+++ b/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),
diff --git a/tests/source/closure.rs b/tests/source/closure.rs
index b95f961405a..c9968a996f4 100644
--- a/tests/source/closure.rs
+++ b/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
+ };
+ });
+}
diff --git a/tests/target/closure.rs b/tests/target/closure.rs
index a3d9cfe863b..ad6b48aeac9 100644
--- a/tests/target/closure.rs
+++ b/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
+ };
+ });
+}