]> git.lizzy.rs Git - rust.git/blobdiff - src/chains.rs
chains: refactor block formatting
[rust.git] / src / chains.rs
index 2f05f4a2c4674bef38b29fcd802555815eb26e0e..ff3633a8cffdf06cdca4f15501fd3c6735895128 100644 (file)
 use macros::convert_try_mac;
 use rewrite::{Rewrite, RewriteContext};
 use shape::Shape;
-use utils::{
-    first_line_width, last_line_extendable, last_line_width, mk_sp, trimmed_last_line_width,
-    wrap_str,
-};
+use spanned::Spanned;
+use utils::{first_line_width, last_line_extendable, last_line_width, mk_sp, wrap_str};
 
 use std::borrow::Cow;
 use std::cmp::min;
-use std::iter;
 
 use syntax::codemap::Span;
 use syntax::{ast, ptr};
 
 pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -> Option<String> {
     debug!("rewrite_chain {:?}", shape);
-    let total_span = expr.span;
-    let (parent, subexpr_list) = make_subexpr_list(expr, context);
+    let chain = Chain::from_ast(expr, context);
+    // If this is just an expression with some `?`s, then format it trivially and
+    // return early.
+    if chain.children.is_empty() {
+        let rewrite = chain.parent.expr.rewrite(context, shape.sub_width(chain.parent.tries)?)?;
+        return Some(format!("{}{}", rewrite, "?".repeat(chain.parent.tries)));
+    }
+
+    match context.config.indent_style() {
+        IndentStyle::Block => rewrite_chain_block(chain, context, shape),
+        IndentStyle::Visual => rewrite_chain_visual(chain, context, shape),
+    }
+}
+
+// An expression plus trailing `?`s to be formatted together.
+#[derive(Debug)]
+struct ChainItem {
+    expr: ast::Expr,
+    tries: usize,
+}
+
+#[derive(Debug)]
+struct Chain {
+    parent: ChainItem,
+    // TODO do we need to clone the exprs?
+    children: Vec<ChainItem>,
+}
+
+impl Chain {
+    fn from_ast(expr: &ast::Expr, context: &RewriteContext) -> Chain {
+        let mut subexpr_list = make_subexpr_list(expr, context);
+
+        // Un-parse the expression tree into ChainItems
+        let mut children = vec![];
+        let mut sub_tries = 0;
+        loop {
+            if subexpr_list.is_empty() {
+                break;
+            }
+
+            let subexpr = subexpr_list.pop().unwrap();
+            match subexpr.node {
+                ast::ExprKind::Try(_) => sub_tries += 1,
+                _ => {
+                    children.push(ChainItem {
+                        expr: subexpr.clone(),
+                        tries: sub_tries,
+                    });
+                    sub_tries = 0;
+                }
+            }
+        }
 
-    // Bail out if the chain is just try sugar, i.e., an expression followed by
-    // any number of `?`s.
-    if chain_only_try(&subexpr_list) {
-        return rewrite_try(&parent, subexpr_list.len(), context, shape);
+        Chain {
+            parent: children.remove(0),
+            children,
+        }
     }
-    let suffix_try_num = subexpr_list.iter().take_while(|e| is_try(e)).count();
-    let prefix_try_num = subexpr_list.iter().rev().take_while(|e| is_try(e)).count();
+}
 
+fn rewrite_chain_block(chain: Chain, context: &RewriteContext, shape: Shape) -> Option<String> {
+    debug!("rewrite_chain_block {:?} {:?}", chain, shape);
     // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`.
-    let parent_shape = if is_block_expr(context, &parent, "\n") {
-        match context.config.indent_style() {
-            IndentStyle::Visual => shape.visual_indent(0),
-            IndentStyle::Block => shape,
+    // Root is the parent plus any other chain items placed on the first line to
+    // avoid an orphan. E.g.,
+    // ```
+    // foo.bar
+    //     .baz()
+    // ```
+    // If `bar` were not part of the root, then baz would be orphaned and 'float'.
+    let mut root_rewrite = chain.parent.expr
+        .rewrite(context, shape)
+        .map(|parent_rw| parent_rw + &"?".repeat(chain.parent.tries))?;
+
+    let mut children: &[_] = &chain.children;
+    let mut root_ends_with_block = is_block_expr(context, &chain.parent.expr, &root_rewrite);
+    let tab_width = context.config.tab_spaces().saturating_sub(shape.offset);
+
+    while root_rewrite.len() <= tab_width && !root_rewrite.contains('\n') {
+        let item = &children[0];
+        let shape = shape.offset_left(root_rewrite.len())?;
+        match rewrite_chain_subexpr(&item.expr, context, shape) {
+            Some(rewrite) => {
+                root_rewrite.push_str(&rewrite);
+                root_rewrite.push_str(&"?".repeat(item.tries));
+            }
+            None => break,
+        }
+
+        root_ends_with_block = is_block_expr(context, &item.expr, &root_rewrite);
+
+        children = &children[1..];
+        if children.is_empty() {
+            return Some(root_rewrite);
+        }
+    }
+
+    // Separate out the last item in the chain for special treatment below.
+    let last = &children[children.len() - 1];
+    children = &children[..children.len() - 1];
+
+    // Decide how to layout the rest of the chain.
+    let child_shape = if root_ends_with_block {
+        shape
+    } else {
+        shape.block_indent(context.config.tab_spaces())
+    }.with_max_width(context.config);
+
+    let mut rewrites: Vec<String> = Vec::with_capacity(children.len());
+    rewrites.push(root_rewrite);
+    let mut is_block_like = Vec::with_capacity(children.len());
+    is_block_like.push(root_ends_with_block);
+    for item in children {
+        let rewrite = rewrite_chain_subexpr(&item.expr, context, child_shape)?;
+        is_block_like.push(is_block_expr(context, &item.expr, &rewrite));
+        rewrites.push(format!("{}{}", rewrite, "?".repeat(item.tries)));
+    }
+
+    // Total of all items excluding the last.
+    let extend_last_subexpr = last_line_extendable(&rewrites[rewrites.len() - 1]);
+    let almost_total = if extend_last_subexpr {
+        last_line_width(&rewrites[rewrites.len() - 1])
+    } else {
+        rewrites.iter().fold(0, |a, b| a + b.len())
+    } + last.tries;
+    let one_line_budget = if rewrites.len() == 1 {
+        shape.width
+    } else {
+        min(shape.width, context.config.width_heuristics().chain_width)
+    };
+    let all_in_one_line = rewrites.iter().all(|s| !s.contains('\n'))
+        && almost_total < one_line_budget;
+    let last_shape = if all_in_one_line {
+        shape.sub_width(last.tries)?
+    } else {
+        child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries)?
+    };
+
+    // Rewrite the last child. The last child of a chain requires special treatment. We need to
+    // know whether 'overflowing' the last child make a better formatting:
+    //
+    // A chain with overflowing the last child:
+    // ```
+    // parent.child1.child2.last_child(
+    //     a,
+    //     b,
+    //     c,
+    // )
+    // ```
+    //
+    // A chain without overflowing the last child (in vertical layout):
+    // ```
+    // parent
+    //     .child1
+    //     .child2
+    //     .last_child(a, b, c)
+    // ```
+    //
+    // In particular, overflowing is effective when the last child is a method with a multi-lined
+    // block-like argument (e.g. closure):
+    // ```
+    // parent.child1.child2.last_child(|a, b, c| {
+    //     let x = foo(a, b, c);
+    //     let y = bar(a, b, c);
+    //
+    //     // ...
+    //
+    //     result
+    // })
+    // ```
+
+    let mut last_subexpr_str = None;
+    let mut fits_single_line = false;
+    if all_in_one_line || extend_last_subexpr {
+        // First we try to 'overflow' the last child and see if it looks better than using
+        // vertical layout.
+        if let Some(shape) = last_shape.offset_left(almost_total) {
+            if let Some(rw) = rewrite_chain_subexpr(&last.expr, context, shape) {
+                // We allow overflowing here only if both of the following conditions match:
+                // 1. The entire chain fits in a single line except the last child.
+                // 2. `last_child_str.lines().count() >= 5`.
+                let line_count = rw.lines().count();
+                let could_fit_single_line = almost_total + first_line_width(&rw) <= one_line_budget;
+                if fits_single_line && line_count >= 5 {
+                    last_subexpr_str = Some(rw);
+                    fits_single_line = true;
+                } else {
+                    // We could not know whether overflowing is better than using vertical layout,
+                    // just by looking at the overflowed rewrite. Now we rewrite the last child
+                    // on its own line, and compare two rewrites to choose which is better.
+                    match rewrite_chain_subexpr(&last.expr, context, last_shape) {
+                        Some(ref new_rw) if !could_fit_single_line => {
+                            last_subexpr_str = Some(new_rw.clone());
+                        }
+                        Some(ref new_rw) if new_rw.lines().count() >= line_count => {
+                            last_subexpr_str = Some(rw);
+                            fits_single_line = could_fit_single_line;
+                        }
+                        new_rw @ Some(..) => {
+                            last_subexpr_str = new_rw;
+                        }
+                        _ => {
+                            last_subexpr_str = Some(rw);
+                            fits_single_line = could_fit_single_line;
+                        }
+                    }
+                }
+            }
         }
+    }
+
+    last_subexpr_str = last_subexpr_str.or_else(|| rewrite_chain_subexpr(&last.expr, context, last_shape));
+    rewrites.push(format!("{}{}", last_subexpr_str?, "?".repeat(last.tries)));
+
+    // We should never look at this, since we only look at the block-ness of the
+    // previous item in the chain.
+    is_block_like.push(false);
+
+    let connector = if fits_single_line && all_in_one_line {
+        // Yay, we can put everything on one line.
+        Cow::from("")
+    } else {
+        // Use new lines.
+        if *context.force_one_line_chain.borrow() {
+            return None;
+        }
+        child_shape.indent.to_string_with_newline(context.config)
+    };
+
+    let result = join_rewrites(&rewrites, &is_block_like, &connector);
+    Some(result)
+}
+
+fn rewrite_chain_visual(mut chain: Chain, context: &RewriteContext, shape: Shape) -> Option<String> {
+    let last = chain.children.pop().unwrap();
+
+    // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`.
+    let parent_shape = if is_block_expr(context, &chain.parent.expr, "\n") {
+        shape.visual_indent(0)
     } else {
         shape
     };
-    let parent_rewrite = parent
+    let parent_rewrite = chain.parent.expr
         .rewrite(context, parent_shape)
-        .map(|parent_rw| parent_rw + &"?".repeat(prefix_try_num))?;
+        .map(|parent_rw| parent_rw + &"?".repeat(chain.parent.tries))?;
     let parent_rewrite_contains_newline = parent_rewrite.contains('\n');
-    let is_small_parent = parent_rewrite.len() <= context.config.tab_spaces();
+
+    let other_child_shape = shape.visual_indent(0).with_max_width(context.config);
 
     // Decide how to layout the rest of the chain. `extend` is true if we can
     // put the first non-parent item on the same line as the parent.
-    let (nested_shape, extend) = if !parent_rewrite_contains_newline && is_continuable(&parent) {
-        (
-            chain_indent(context, shape.add_offset(parent_rewrite.len())),
-            context.config.indent_style() == IndentStyle::Visual || is_small_parent,
-        )
-    } else if is_block_expr(context, &parent, &parent_rewrite) {
-        match context.config.indent_style() {
-            // Try to put the first child on the same line with parent's last line
-            IndentStyle::Block => (parent_shape.block_indent(context.config.tab_spaces()), true),
-            // The parent is a block, so align the rest of the chain with the closing
-            // brace.
-            IndentStyle::Visual => (parent_shape, false),
-        }
-    } else {
-        (
-            chain_indent(context, shape.add_offset(parent_rewrite.len())),
-            false,
-        )
-    };
-
-    let other_child_shape = nested_shape.with_max_width(context.config);
+    let extend = !parent_rewrite_contains_newline && is_continuable(&chain.parent.expr);
 
     let first_child_shape = if extend {
         let overhead = last_line_width(&parent_rewrite);
-        let offset = trimmed_last_line_width(&parent_rewrite) + prefix_try_num;
-        match context.config.indent_style() {
-            IndentStyle::Visual => parent_shape.offset_left(overhead)?,
-            IndentStyle::Block => parent_shape.offset_left(offset)?,
-        }
+        parent_shape.offset_left(overhead)?
     } else {
         other_child_shape
     };
@@ -149,27 +345,19 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
         first_child_shape, other_child_shape
     );
 
-    let child_shape_iter = Some(first_child_shape)
-        .into_iter()
-        .chain(iter::repeat(other_child_shape));
-    let subexpr_num = subexpr_list.len();
-    let last_subexpr = &subexpr_list[suffix_try_num];
-    let subexpr_list = &subexpr_list[suffix_try_num..subexpr_num - prefix_try_num];
-    let iter = subexpr_list.iter().skip(1).rev().zip(child_shape_iter);
-    let mut rewrites = iter.map(|(e, shape)| rewrite_chain_subexpr(e, total_span, context, shape))
-        .collect::<Option<Vec<_>>>()?;
+    let mut rewrites: Vec<String> = Vec::with_capacity(chain.children.len());
+    for (i, item) in chain.children.iter().enumerate() {
+        let shape = if i == 0 {
+            first_child_shape
+        } else {
+            other_child_shape
+        };
+        let rewrite = rewrite_chain_subexpr(&item.expr, context, shape)?;
+        rewrites.push(format!("{}{}", rewrite, "?".repeat(item.tries)));
+    }
 
     // Total of all items excluding the last.
-    let extend_last_subexpr = if is_small_parent {
-        rewrites.len() == 1 && last_line_extendable(&rewrites[0])
-    } else {
-        rewrites.is_empty() && last_line_extendable(&parent_rewrite)
-    };
-    let almost_total = if extend_last_subexpr {
-        last_line_width(&parent_rewrite)
-    } else {
-        rewrites.iter().fold(0, |a, b| a + b.len()) + parent_rewrite.len()
-    } + suffix_try_num;
+    let almost_total = rewrites.iter().fold(0, |a, b| a + b.len()) + parent_rewrite.len() + last.tries;
     let one_line_budget = if rewrites.is_empty() {
         shape.width
     } else {
@@ -178,11 +366,8 @@ 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 last_shape = if rewrites.is_empty() {
-        first_child_shape
-    } else {
-        other_child_shape
-    }.sub_width(shape.rhs_overhead(context.config) + suffix_try_num)?;
+    let last_shape =
+        other_child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries)?;
 
     // Rewrite the last child. The last child of a chain requires special treatment. We need to
     // know whether 'overflowing' the last child make a better formatting:
@@ -217,41 +402,47 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
     // })
     // ```
 
-    // `rewrite_last` rewrites the last child on its own line. We use a closure here instead of
-    // directly calling `rewrite_chain_subexpr()` to avoid exponential blowup.
-    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_subexpr {
+    let mut last_subexpr_str = None;
+    let mut fits_single_line = false;
+    if all_in_one_line {
         // First we try to 'overflow' the last child and see if it looks better than using
         // vertical layout.
-        parent_shape.offset_left(almost_total).map(|shape| {
-            if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) {
+        if let Some(shape) = parent_shape.offset_left(almost_total) {
+            if let Some(rw) = rewrite_chain_subexpr(&last.expr, context, shape) {
                 // We allow overflowing here only if both of the following conditions match:
-                // 1. The entire chain fits in a single line expect the last child.
+                // 1. The entire chain fits in a single line except the last child.
                 // 2. `last_child_str.lines().count() >= 5`.
                 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 {
-                    (Some(rw), true)
+                let could_fit_single_line = almost_total + first_line_width(&rw) <= one_line_budget;
+                if could_fit_single_line && line_count >= 5 {
+                    last_subexpr_str = Some(rw);
+                    fits_single_line = true;
                 } else {
                     // We could not know whether overflowing is better than using vertical layout,
                     // just by looking at the overflowed rewrite. Now we rewrite the last child
                     // on its own line, and compare two rewrites to choose which is better.
-                    match rewrite_last() {
-                        Some(ref new_rw) if !fits_single_line => (Some(new_rw.clone()), false),
+                    match rewrite_chain_subexpr(&last.expr, context, last_shape) {
+                        Some(ref new_rw) if !could_fit_single_line => {
+                            last_subexpr_str = Some(new_rw.clone());
+                        }
                         Some(ref new_rw) if new_rw.lines().count() >= line_count => {
-                            (Some(rw), fits_single_line)
+                            last_subexpr_str = Some(rw);
+                            fits_single_line = could_fit_single_line;
+                        }
+                        new_rw @ Some(..) => {
+                            last_subexpr_str = new_rw;
+                        }
+                        _ => {
+                            last_subexpr_str = Some(rw);
+                            fits_single_line = could_fit_single_line;
                         }
-                        new_rw @ Some(..) => (new_rw, false),
-                        _ => (Some(rw), fits_single_line),
                     }
                 }
-            } else {
-                (rewrite_last(), false)
             }
-        })?
-    } else {
-        (rewrite_last(), false)
-    };
+        }
+    } 
+
+    last_subexpr_str = last_subexpr_str.or_else(|| rewrite_chain_subexpr(&last.expr, context, last_shape));
     rewrites.push(last_subexpr_str?);
 
     let connector = if fits_single_line && !parent_rewrite_contains_newline {
@@ -262,73 +453,32 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
         if *context.force_one_line_chain.borrow() {
             return None;
         }
-        nested_shape.indent.to_string_with_newline(context.config)
+        other_child_shape.indent.to_string_with_newline(context.config)
     };
 
-    let first_connector = if is_small_parent || fits_single_line
-        || last_line_extendable(&parent_rewrite)
-        || context.config.indent_style() == IndentStyle::Visual
-    {
-        ""
-    } else {
-        &connector
-    };
-
-    let result = if is_small_parent && rewrites.len() > 1 {
-        let second_connector = if fits_single_line || rewrites[1] == "?"
-            || last_line_extendable(&rewrites[0])
-            || context.config.indent_style() == IndentStyle::Visual
-        {
-            ""
-        } else {
-            &connector
-        };
-        format!(
-            "{}{}{}{}{}",
-            parent_rewrite,
-            first_connector,
-            rewrites[0],
-            second_connector,
-            join_rewrites(&rewrites[1..], &connector)
-        )
-    } else {
-        format!(
-            "{}{}{}",
-            parent_rewrite,
-            first_connector,
-            join_rewrites(&rewrites, &connector)
-        )
-    };
-    let result = format!("{}{}", result, "?".repeat(suffix_try_num));
-    if context.config.indent_style() == IndentStyle::Visual {
-        wrap_str(result, context.config.max_width(), shape)
-    } else {
-        Some(result)
-    }
+    let result = format!("{}{}{}",
+        parent_rewrite,
+        join_rewrites_vis(&rewrites, &connector),
+        "?".repeat(last.tries),
+    );
+    wrap_str(result, context.config.max_width(), shape)
 }
 
-// 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 {
-            true
-        } else {
-            false
+fn join_rewrites(rewrites: &[String], is_block_like: &[bool], connector: &str) -> String {
+    let mut rewrite_iter = rewrites.iter();
+    let mut result = rewrite_iter.next().unwrap().clone();
+
+    for (rewrite, prev_is_block_like) in rewrite_iter.zip(is_block_like.iter()) {
+        if rewrite != "?" && !prev_is_block_like {
+            result.push_str(connector);
         }
-    })
-}
+        result.push_str(&rewrite);
+    }
 
-fn rewrite_try(
-    expr: &ast::Expr,
-    try_count: usize,
-    context: &RewriteContext,
-    shape: Shape,
-) -> Option<String> {
-    let sub_expr = expr.rewrite(context, shape.sub_width(try_count)?)?;
-    Some(format!("{}{}", sub_expr, "?".repeat(try_count)))
+    result
 }
 
-fn join_rewrites(rewrites: &[String], connector: &str) -> String {
+fn join_rewrites_vis(rewrites: &[String], connector: &str) -> String {
     let mut rewrite_iter = rewrites.iter();
     let mut result = rewrite_iter.next().unwrap().clone();
 
@@ -346,7 +496,9 @@ fn join_rewrites(rewrites: &[String], connector: &str) -> String {
 // parens, braces, and brackets in its idiomatic formatting.
 fn is_block_expr(context: &RewriteContext, expr: &ast::Expr, repr: &str) -> bool {
     match expr.node {
-        ast::ExprKind::Mac(..) | ast::ExprKind::Call(..) => {
+        ast::ExprKind::Mac(..)
+        | ast::ExprKind::Call(..)
+        | ast::ExprKind::MethodCall(..) => {
             context.use_block_indent() && repr.contains('\n')
         }
         ast::ExprKind::Struct(..)
@@ -361,31 +513,24 @@ fn is_block_expr(context: &RewriteContext, expr: &ast::Expr, repr: &str) -> bool
         ast::ExprKind::Paren(ref expr)
         | ast::ExprKind::Binary(_, _, ref expr)
         | ast::ExprKind::Index(_, ref expr)
-        | ast::ExprKind::Unary(_, ref expr) => is_block_expr(context, expr, repr),
+        | ast::ExprKind::Unary(_, ref expr)
+        | ast::ExprKind::Closure(_, _, _, _, ref expr, _) 
+        | ast::ExprKind::Try(ref expr)
+        | ast::ExprKind::Yield(Some(ref expr)) => is_block_expr(context, expr, repr),
         _ => false,
     }
 }
 
-// Returns the root of the chain and a Vec of the prefixes of the rest of the chain.
-// E.g., for input `a.b.c` we return (`a`, [`a.b.c`, `a.b`])
-fn make_subexpr_list(expr: &ast::Expr, context: &RewriteContext) -> (ast::Expr, Vec<ast::Expr>) {
+// Returns a Vec of the prefixes of the chain.
+// E.g., for input `a.b.c` we return [`a.b.c`, `a.b`, 'a']
+fn make_subexpr_list(expr: &ast::Expr, context: &RewriteContext) -> Vec<ast::Expr> {
     let mut subexpr_list = vec![expr.clone()];
 
     while let Some(subexpr) = pop_expr_chain(subexpr_list.last().unwrap(), context) {
         subexpr_list.push(subexpr.clone());
     }
 
-    let parent = subexpr_list.pop().unwrap();
-    (parent, subexpr_list)
-}
-
-fn chain_indent(context: &RewriteContext, shape: Shape) -> Shape {
-    match context.config.indent_style() {
-        IndentStyle::Visual => shape.visual_indent(0),
-        IndentStyle::Block => shape
-            .block_indent(context.config.tab_spaces())
-            .with_max_width(context.config),
-    }
+    subexpr_list
 }
 
 // Returns the expression's subexpression, if it exists. When the subexpr
@@ -419,7 +564,6 @@ fn convert_try(expr: &ast::Expr, context: &RewriteContext) -> ast::Expr {
 // `.c`.
 fn rewrite_chain_subexpr(
     expr: &ast::Expr,
-    span: Span,
     context: &RewriteContext,
     shape: Shape,
 ) -> Option<String> {
@@ -433,14 +577,14 @@ fn rewrite_chain_subexpr(
 
     match expr.node {
         ast::ExprKind::MethodCall(ref segment, ref expressions) => {
-            let types = match segment.parameters {
+            let types = match segment.args {
                 Some(ref params) => match **params {
-                    ast::PathParameters::AngleBracketed(ref data) => &data.types[..],
+                    ast::GenericArgs::AngleBracketed(ref data) => &data.args[..],
                     _ => &[],
                 },
                 _ => &[],
             };
-            rewrite_method_call(segment.ident, types, expressions, span, context, shape)
+            rewrite_method_call(segment.ident, types, expressions, expr.span, context, shape)
         }
         ast::ExprKind::Field(ref nested, ref field) => {
             let space = if is_tup_field_access(expr) && is_tup_field_access(nested) {
@@ -472,16 +616,9 @@ fn is_continuable(expr: &ast::Expr) -> bool {
     }
 }
 
-fn is_try(expr: &ast::Expr) -> bool {
-    match expr.node {
-        ast::ExprKind::Try(..) => true,
-        _ => false,
-    }
-}
-
 fn rewrite_method_call(
     method_name: ast::Ident,
-    types: &[ptr::P<ast::Ty>],
+    types: &[ast::GenericArg],
     args: &[ptr::P<ast::Expr>],
     span: Span,
     context: &RewriteContext,
@@ -495,14 +632,9 @@ fn rewrite_method_call(
             .map(|ty| ty.rewrite(context, shape))
             .collect::<Option<Vec<_>>>()?;
 
-        let type_str =
-            if context.config.spaces_within_parens_and_brackets() && !type_list.is_empty() {
-                format!("::< {} >", type_list.join(", "))
-            } else {
-                format!("::<{}>", type_list.join(", "))
-            };
+        let type_str = format!("::<{}>", type_list.join(", "));
 
-        (types.last().unwrap().span.hi(), type_str)
+        (types.last().unwrap().span().hi(), type_str)
     };
 
     let callee_str = format!(".{}{}", method_name, type_str);