From 35cd72d9890d4800886de4fd3e4a4f245c3931d2 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 21:48:05 +0900 Subject: [PATCH 01/14] Use correct budget for the last child of chain --- src/chains.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/chains.rs b/src/chains.rs index 74c43d783b7..c5a1644b67a 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -182,7 +182,17 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - let all_in_one_line = !parent_rewrite_contains_newline && rewrites.iter().all(|s| !s.contains('\n')) && almost_total < one_line_budget; - let rewrite_last = || rewrite_chain_subexpr(last_subexpr, total_span, context, nested_shape); + let last_shape = if rewrites.is_empty() { + // We only have a single child. + first_child_shape + } else { + match context.config.chain_indent() { + IndentStyle::Visual => other_child_shape.sub_width(shape.rhs_overhead(context.config))?, + IndentStyle::Block => other_child_shape, + } + }; + let last_shape = last_shape.sub_width(suffix_try_num)?; + let rewrite_last = || rewrite_chain_subexpr(last_subexpr, total_span, context, last_shape); let (last_subexpr_str, fits_single_line) = if all_in_one_line || extend_last_subexr { parent_shape.offset_left(almost_total).map(|shape| { if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) { From 02ef2ee8de4c46be7a125c965e064d7a662cdc55 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 21:48:45 +0900 Subject: [PATCH 02/14] Fix a typo --- src/chains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chains.rs b/src/chains.rs index c5a1644b67a..d5016d8db26 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -198,7 +198,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) { let line_count = rw.lines().count(); let fits_single_line = almost_total + first_line_width(&rw) <= one_line_budget; - if (line_count >= 5 && fits_single_line) || extend_last_subexr { + if fits_single_line && (line_count >= 5 && fits_single_line || extend_last_subexr) { (Some(rw), true) } else { match rewrite_last() { From 2b6e50436c6bd8826067a08688c74755d57b7b75 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 21:51:15 +0900 Subject: [PATCH 03/14] Run wrap_str() only when chain_indent is set to "Visual" --- src/chains.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/chains.rs b/src/chains.rs index d5016d8db26..30a9444d4ae 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -266,7 +266,11 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - ) }; let result = format!("{}{}", result, repeat_try(suffix_try_num)); - wrap_str(result, context.config.max_width(), shape) + if context.config.chain_indent() == IndentStyle::Visual { + wrap_str(result, context.config.max_width(), shape) + } else { + Some(result) + } } fn is_extendable_parent(context: &RewriteContext, parent_str: &str) -> bool { From dde0cdabe04ec5773e36d12b20649dc8f8b0f55c Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 21:58:29 +0900 Subject: [PATCH 04/14] Remove String::rewrite() --- src/utils.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index a719525d4d9..4b8ceadbf20 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -15,7 +15,7 @@ use syntax::ast::{self, Attribute, MetaItem, MetaItemKind, NestedMetaItem, Neste Path, Visibility}; use syntax::codemap::{BytePos, Span, NO_EXPANSION}; -use rewrite::{Rewrite, RewriteContext}; +use rewrite::RewriteContext; use shape::Shape; // When we get scoped annotations, we should have rustfmt::skip. @@ -425,12 +425,6 @@ pub fn wrap_str>(s: S, max_width: usize, shape: Shape) -> Option Option { - wrap_str(self, context.config.max_width(), shape).map(ToOwned::to_owned) - } -} - #[inline] pub fn colon_spaces(before: bool, after: bool) -> &'static str { match (before, after) { From dd5ed5393098dc4a39558f119d7a2d862dc211ae Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:01:44 +0900 Subject: [PATCH 05/14] Remove calling rewrite() against String --- src/expr.rs | 4 +--- src/types.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index f41feb52631..dbaf1cceefc 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -718,9 +718,7 @@ fn rewrite_closure_block( if block_str.matches('\n').count() <= block_threshold as usize && !need_block_indent(&block_str, shape) { - if let Some(block_str) = block_str.rewrite(context, shape) { - return Some(format!("{} {}", prefix, block_str)); - } + return Some(format!("{} {}", prefix, block_str)); } } } diff --git a/src/types.rs b/src/types.rs index c17112c9404..1b70b5448d9 100644 --- a/src/types.rs +++ b/src/types.rs @@ -550,7 +550,7 @@ impl Rewrite for ast::TyParamBounds { let strs = self.iter() .map(|b| b.rewrite(context, shape)) .collect::>>()?; - join_bounds(context, shape, &strs).rewrite(context, shape) + Some(join_bounds(context, shape, &strs)) } } From c046a261a80277441a59d339a3b5afb3a92d3d29 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:04:02 +0900 Subject: [PATCH 06/14] Change the signature of wrap_str() --- src/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index 4b8ceadbf20..e4ed4b884e2 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -392,7 +392,7 @@ macro_rules! skip_out_of_file_lines_range_visitor { // Wraps string-like values in an Option. Returns Some when the string adheres // to the Rewrite constraints defined for the Rewrite trait and else otherwise. -pub fn wrap_str>(s: S, max_width: usize, shape: Shape) -> Option { +pub fn wrap_str(s: String, max_width: usize, shape: Shape) -> Option { { let snippet = s.as_ref(); From d38b3acee148f1ee5b4b7284bc68f89e5c09bbfc Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:14:33 +0900 Subject: [PATCH 07/14] Simplify wrap_str() --- src/utils.rs | 56 +++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index e4ed4b884e2..649a6070fd2 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -390,39 +390,37 @@ macro_rules! skip_out_of_file_lines_range_visitor { } } -// Wraps string-like values in an Option. Returns Some when the string adheres -// to the Rewrite constraints defined for the Rewrite trait and else otherwise. +// Wraps String in an Option. Returns Some when the string adheres to the +// Rewrite constraints defined for the Rewrite trait and else otherwise. pub fn wrap_str(s: String, max_width: usize, shape: Shape) -> Option { - { - let snippet = s.as_ref(); + if is_valid_str(&s, max_width, shape) { + Some(s) + } else { + None + } +} - if !snippet.is_empty() { - if !snippet.contains('\n') && snippet.len() > shape.width { - return None; - } else { - let mut lines = snippet.lines(); - - if lines.next().unwrap().len() > shape.width { - return None; - } - - // The other lines must fit within the maximum width. - if lines.any(|line| line.len() > max_width) { - return None; - } - - // `width` is the maximum length of the last line, excluding - // indentation. - // A special check for the last line, since the caller may - // place trailing characters on this line. - if snippet.lines().rev().next().unwrap().len() > shape.used_width() + shape.width { - return None; - } - } +fn is_valid_str(snippet: &str, max_width: usize, shape: Shape) -> bool { + if !snippet.is_empty() { + // First line must fits with `shape.width`. + if first_line_width(snippet) > shape.width { + return false; + } + // If the snippet does not include newline, we are done. + if first_line_width(snippet) == snippet.len() { + return true; + } + // The other lines must fit within the maximum width. + if snippet.lines().skip(1).any(|line| line.len() > max_width) { + return false; + } + // A special check for the last line, since the caller may + // place trailing characters on this line. + if last_line_width(snippet) > shape.used_width() + shape.width { + return false; } } - - Some(s) + true } #[inline] From ed7ceebfe07c674933b002beda88391ad5cd6066 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:17:01 +0900 Subject: [PATCH 08/14] Faster last_line_extendable() --- src/utils.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 649a6070fd2..3106df9ed8b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -171,12 +171,18 @@ pub fn trimmed_last_line_width(s: &str) -> usize { #[inline] pub fn last_line_extendable(s: &str) -> bool { - s.lines().last().map_or(false, |s| { - s.ends_with("\"#") - || s.trim() - .chars() - .all(|c| c == ')' || c == ']' || c == '}' || c == '?') - }) + if s.ends_with("\"#") { + return true; + } + for c in s.chars().rev() { + match c { + ')' | ']' | '}' | '?' => continue, + '\n' => break, + _ if c.is_whitespace() => continue, + _ => return false, + } + } + true } #[inline] From 1097a431bf758efe8b95e8cd7de1df140af3402b Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:21:53 +0900 Subject: [PATCH 09/14] Change return type of Indent::to_string() to Cow<'static, str> --- src/shape.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index b5d57997e87..fffbe2b9913 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::borrow::Cow; use std::ops::{Add, Sub}; use Config; @@ -21,6 +22,10 @@ pub struct Indent { pub alignment: usize, } +// INDENT_BUFFER.len() = 80 +const INDENT_BUFFER_LEN: usize = 80; +const INDENT_BUFFER: &str = + " "; impl Indent { pub fn new(block_indent: usize, alignment: usize) -> Indent { Indent { @@ -68,21 +73,25 @@ impl Indent { self.block_indent + self.alignment } - pub fn to_string(&self, config: &Config) -> String { + pub fn to_string(&self, config: &Config) -> Cow<'static, str> { let (num_tabs, num_spaces) = if config.hard_tabs() { (self.block_indent / config.tab_spaces(), self.alignment) } else { (0, self.width()) }; let num_chars = num_tabs + num_spaces; - let mut indent = String::with_capacity(num_chars); - for _ in 0..num_tabs { - indent.push('\t') + if num_tabs == 0 && num_chars <= INDENT_BUFFER_LEN { + Cow::from(&INDENT_BUFFER[0..num_chars]) + } else { + let mut indent = String::with_capacity(num_chars); + for _ in 0..num_tabs { + indent.push('\t') + } + for _ in 0..num_spaces { + indent.push(' ') + } + Cow::from(indent) } - for _ in 0..num_spaces { - indent.push(' ') - } - indent } } From 427b4a831da4f0186049c627069c2376c370a76e Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:29:53 +0900 Subject: [PATCH 10/14] Get rid of choose_first_connector() --- src/chains.rs | 47 ++++++++--------------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 30a9444d4ae..289d05cf879 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -239,16 +239,15 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - connector.as_str() }; - let subexpr_num = subexpr_list.len(); let result = if is_small_parent && rewrites.len() > 1 { - let second_connector = choose_first_connector( - context, - &rewrites[0], - &rewrites[1], - &connector, - &subexpr_list[..subexpr_num - 1], - false, - ); + let second_connector = if fits_single_line || rewrites[1] == "?" + || last_line_extendable(&rewrites[0]) + || context.config.chain_indent() == IndentStyle::Visual + { + "" + } else { + &connector + }; format!( "{}{}{}{}{}", parent_rewrite, @@ -273,10 +272,6 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - } } -fn is_extendable_parent(context: &RewriteContext, parent_str: &str) -> bool { - context.config.chain_indent() == IndentStyle::Block && last_line_extendable(parent_str) -} - // True if the chain is only `?`s. fn chain_only_try(exprs: &[ast::Expr]) -> bool { exprs.iter().all(|e| if let ast::ExprKind::Try(_) = e.node { @@ -445,32 +440,6 @@ fn is_try(expr: &ast::Expr) -> bool { } } -fn choose_first_connector<'a>( - context: &RewriteContext, - parent_str: &str, - first_child_str: &str, - connector: &'a str, - subexpr_list: &[ast::Expr], - extend: bool, -) -> &'a str { - if subexpr_list.is_empty() { - "" - } else if extend || subexpr_list.last().map_or(false, is_try) - || is_extendable_parent(context, parent_str) - { - // 1 = ";", being conservative here. - if last_line_width(parent_str) + first_line_width(first_child_str) + 1 - <= context.config.max_width() - { - "" - } else { - connector - } - } else { - connector - } -} - fn rewrite_method_call( method_name: ast::Ident, types: &[ptr::P], From 7359d3ad34d09539fa6340cecf8d8bb56e4ceb72 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:30:12 +0900 Subject: [PATCH 11/14] Simplify join_rewrites() --- src/chains.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 289d05cf879..b55ce0982e8 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -254,14 +254,14 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - first_connector, rewrites[0], second_connector, - join_rewrites(&rewrites[1..], &subexpr_list[..subexpr_num - 1], &connector) + join_rewrites(&rewrites[1..], &connector) ) } else { format!( "{}{}{}", parent_rewrite, first_connector, - join_rewrites(&rewrites, subexpr_list, &connector) + join_rewrites(&rewrites, &connector) ) }; let result = format!("{}{}", result, repeat_try(suffix_try_num)); @@ -297,17 +297,14 @@ fn rewrite_try( Some(format!("{}{}", sub_expr, repeat_try(try_count))) } -fn join_rewrites(rewrites: &[String], subexps: &[ast::Expr], connector: &str) -> String { +fn join_rewrites(rewrites: &[String], connector: &str) -> String { let mut rewrite_iter = rewrites.iter(); let mut result = rewrite_iter.next().unwrap().clone(); - let mut subexpr_iter = subexps.iter().rev(); - subexpr_iter.next(); - for (rewrite, expr) in rewrite_iter.zip(subexpr_iter) { - match expr.node { - ast::ExprKind::Try(_) => (), - _ => result.push_str(connector), - }; + for rewrite in rewrite_iter { + if rewrite != "?" { + result.push_str(connector); + } result.push_str(&rewrite[..]); } From 923a7bc1d9fb7303eab061a1e2463794e03f0bcb Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:31:38 +0900 Subject: [PATCH 12/14] Update doc comments in chains.rs --- src/chains.rs | 47 +++++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index b55ce0982e8..570f78e5b32 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -24,6 +24,15 @@ /// alignment). /// E.g., `let foo = { aaaa; bbb; ccc }.bar.baz();`, we would layout for the /// following values of `chain_indent`: +/// Block: +/// ``` +/// let foo = { +/// aaaa; +/// bbb; +/// ccc +/// }.bar +/// .baz(); +/// ``` /// Visual: /// ``` /// let foo = { @@ -34,47 +43,21 @@ /// .bar /// .baz(); /// ``` -/// Inherit: -/// ``` -/// let foo = { -/// aaaa; -/// bbb; -/// ccc -/// } -/// .bar -/// .baz(); -/// ``` -/// Tabbed: -/// ``` -/// let foo = { -/// aaaa; -/// bbb; -/// ccc -/// } -/// .bar -/// .baz(); -/// ``` /// /// If the first item in the chain is a block expression, we align the dots with /// the braces. +/// Block: +/// ``` +/// let a = foo.bar +/// .baz() +/// .qux +/// ``` /// Visual: /// ``` /// let a = foo.bar /// .baz() /// .qux /// ``` -/// Inherit: -/// ``` -/// let a = foo.bar -/// .baz() -/// .qux -/// ``` -/// Tabbed: -/// ``` -/// let a = foo.bar -/// .baz() -/// .qux -/// ``` use shape::Shape; use config::IndentStyle; From eb8566f3ee3caf39b3610df96219588ce2fffb21 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 7 Oct 2017 22:38:32 +0900 Subject: [PATCH 13/14] Run cargo fmt rustfmt removes trailing comma from a function call. This could be a bug. --- src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types.rs b/src/types.rs index 1b70b5448d9..d73e8653121 100644 --- a/src/types.rs +++ b/src/types.rs @@ -660,7 +660,7 @@ impl Rewrite for ast::Ty { mut_str, mt.ty.rewrite( context, - Shape::legacy(budget, shape.indent + 1 + mut_len), + Shape::legacy(budget, shape.indent + 1 + mut_len) )? ) } From c7250d18b1bf4a91cb95535135b3eacf096d8906 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 9 Oct 2017 22:44:00 +0900 Subject: [PATCH 14/14] Fix a typo --- src/chains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chains.rs b/src/chains.rs index 570f78e5b32..38d015322ce 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -181,7 +181,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) { let line_count = rw.lines().count(); let fits_single_line = almost_total + first_line_width(&rw) <= one_line_budget; - if fits_single_line && (line_count >= 5 && fits_single_line || extend_last_subexr) { + if fits_single_line && (line_count >= 5 || extend_last_subexr) { (Some(rw), true) } else { match rewrite_last() {