Merge pull request #2042 from topecongiro/refactoring

Refactorings

Author: nrc

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,46 +43,20 @@
 ///           .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.
-/// Visual:
-/// ```
-/// let a = foo.bar
-///            .baz()
-///            .qux
-/// ```
-/// Inherit:
+/// Block:
 /// ```
 /// let a = foo.bar
-/// .baz()
-/// .qux
+///     .baz()
+///     .qux
 /// ```
-/// Tabbed:
+/// Visual:
 /// ```
 /// let a = foo.bar
-///     .baz()
-///     .qux
+///            .baz()
+///            .qux
 /// ```
 
 use shape::Shape;
@@ -182,13 +165,23 @@ 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) {
                 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 || extend_last_subexr) {
                     (Some(rw), true)
                 } else {
                     match rewrite_last() {
@@ -229,38 +222,37 @@ 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,
             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));
-    wrap_str(result, context.config.max_width(), shape)
-}
-
-fn is_extendable_parent(context: &RewriteContext, parent_str: &str) -> bool {
-    context.config.chain_indent() == IndentStyle::Block && last_line_extendable(parent_str)
+    if context.config.chain_indent() == IndentStyle::Visual {
+        wrap_str(result, context.config.max_width(), shape)
+    } else {
+        Some(result)
+    }
 }
 
 // True if the chain is only `?`s.
@@ -288,17 +280,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[..]);
     }
 
@@ -431,32 +420,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<ast::Ty>],

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));
             }
         }
     }

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')
-        }
-        for _ in 0..num_spaces {
-            indent.push(' ')
+        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)
         }
-        indent
     }
 }
 

src/types.rs

@@ -550,7 +550,7 @@ impl Rewrite for ast::TyParamBounds {
         let strs = self.iter()
             .map(|b| b.rewrite(context, shape))
             .collect::<Option<Vec<_>>>()?;
-        join_bounds(context, shape, &strs).rewrite(context, shape)
+        Some(join_bounds(context, shape, &strs))
     }
 }
 
@@ -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)
                             )?
                         )
                     }

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.
@@ -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]
@@ -390,45 +396,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.
-pub fn wrap_str<S: AsRef<str>>(s: S, max_width: usize, shape: Shape) -> Option<S> {
-    {
-        let snippet = s.as_ref();
-
-        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;
-                }
-            }
-        }
+// 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<String> {
+    if is_valid_str(&s, max_width, shape) {
+        Some(s)
+    } else {
+        None
     }
-
-    Some(s)
 }
 
-impl Rewrite for String {
-    fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
-        wrap_str(self, context.config.max_width(), shape).map(ToOwned::to_owned)
+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;
+        }
     }
+    true
 }
 
 #[inline]