From aed558fce435924e6839f46c4cc0f021fe33f097 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Fri, 9 Oct 2015 16:07:14 +1300 Subject: [PATCH] Handle multi-line return types and multi-line tuples Closes #409 --- src/items.rs | 93 ++++++++++++++++++++++++++++------------ src/lists.rs | 12 +++--- src/types.rs | 30 ++++++++----- tests/source/multiple.rs | 10 +++++ tests/target/multiple.rs | 11 +++++ 5 files changed, 112 insertions(+), 44 deletions(-) diff --git a/src/items.rs b/src/items.rs index 816bfac0fb3..caf17257578 100644 --- a/src/items.rs +++ b/src/items.rs @@ -149,7 +149,7 @@ impl<'a> FmtVisitor<'a> { false); match rewrite { - Some(new_fn) => { + Some((new_fn, _)) => { self.buffer.push_str(format_visibility(item.vis)); self.buffer.push_str(&new_fn); self.buffer.push_str(";"); @@ -202,21 +202,23 @@ impl<'a> FmtVisitor<'a> { -> Option { let mut newline_brace = self.newline_for_brace(&generics.where_clause); - let mut result = try_opt!(self.rewrite_fn_base(indent, - ident, - fd, - explicit_self, - generics, - unsafety, - constness, - abi, - vis, - span, - newline_brace, - true)); + let (mut result, force_newline_brace) = try_opt!(self.rewrite_fn_base(indent, + ident, + fd, + explicit_self, + generics, + unsafety, + constness, + abi, + vis, + span, + newline_brace, + true)); if self.config.fn_brace_style != BraceStyle::AlwaysNextLine && !result.contains('\n') { newline_brace = false; + } else if force_newline_brace { + newline_brace = true; } // Prepare for the function body by possibly adding a newline and @@ -243,6 +245,7 @@ impl<'a> FmtVisitor<'a> { // Drop semicolon or it will be interpreted as comment let span = codemap::mk_sp(span.lo, span.hi - BytePos(1)); + // FIXME: silly formatting of the `.0`. let mut result = try_opt!(self.rewrite_fn_base(indent, ident, &sig.decl, @@ -254,7 +257,8 @@ impl<'a> FmtVisitor<'a> { ast::Visibility::Inherited, span, false, - false)); + false)) + .0; // Re-attach semicolon result.push(';'); @@ -262,6 +266,7 @@ impl<'a> FmtVisitor<'a> { Some(result) } + // Return type is (result, force_new_line_for_brace) fn rewrite_fn_base(&mut self, indent: Indent, ident: ast::Ident, @@ -275,7 +280,8 @@ impl<'a> FmtVisitor<'a> { span: Span, newline_brace: bool, has_body: bool) - -> Option { + -> Option<(String, bool)> { + let mut force_new_line_for_brace = false; // FIXME we'll lose any comments in between parts of the function decl, but anyone // who comments there probably deserves what they get. @@ -311,13 +317,22 @@ impl<'a> FmtVisitor<'a> { result.push_str(&generics_str); let context = self.get_context(); + // Note that if the width and indent really matter, we'll re-layout the + // return type later anyway. let ret_str = fd.output .rewrite(&context, self.config.max_width - indent.width(), indent) .unwrap(); + let multi_line_ret_str = ret_str.contains('\n'); + let ret_str_len = if multi_line_ret_str { + 0 + } else { + ret_str.len() + }; + // Args. - let (one_line_budget, multi_line_budget, mut arg_indent) = - self.compute_budgets_for_args(&result, indent, ret_str.len(), newline_brace); + let (mut one_line_budget, multi_line_budget, mut arg_indent) = + self.compute_budgets_for_args(&result, indent, ret_str_len, newline_brace); debug!("rewrite_fn: one_line_budget: {}, multi_line_budget: {}, arg_indent: {:?}", one_line_budget, @@ -343,6 +358,10 @@ impl<'a> FmtVisitor<'a> { result.push('('); } + if multi_line_ret_str { + one_line_budget = 0; + } + // A conservative estimation, to goal is to be over all parens in generics let args_start = generics.ty_params .last() @@ -371,12 +390,18 @@ impl<'a> FmtVisitor<'a> { // over the max width, then put the return type on a new line. // Unless we are formatting args like a block, in which case there // should always be room for the return type. - if (result.contains("\n") || - result.len() + indent.width() + ret_str.len() > self.config.max_width) && - self.config.fn_args_layout != StructLitStyle::Block { + let ret_indent = if (result.contains("\n") || multi_line_ret_str || + result.len() + indent.width() + ret_str_len > + self.config.max_width) && + self.config.fn_args_layout != StructLitStyle::Block { let indent = match self.config.fn_return_indent { ReturnIndent::WithWhereClause => indent + 4, - // TODO: we might want to check that using the arg indent + // Aligning with non-existent args looks silly. + _ if arg_str.len() == 0 => { + force_new_line_for_brace = true; + indent + 4 + } + // FIXME: we might want to check that using the arg indent // doesn't blow our budget, and if it does, then fallback to // the where clause indent. _ => arg_indent, @@ -384,10 +409,24 @@ impl<'a> FmtVisitor<'a> { result.push('\n'); result.push_str(&indent.to_string(self.config)); + indent } else { result.push(' '); + Indent::new(indent.width(), result.len()) + }; + + if multi_line_ret_str { + // Now that we know the proper indent and width, we need to + // re-layout the return type. + + let budget = try_opt!(self.config.max_width.checked_sub(ret_indent.width())); + let ret_str = fd.output + .rewrite(&context, budget, ret_indent) + .unwrap(); + result.push_str(&ret_str); + } else { + result.push_str(&ret_str); } - result.push_str(&ret_str); // Comment between return type and the end of the decl. let snippet_lo = fd.output.span().hi; @@ -426,7 +465,7 @@ impl<'a> FmtVisitor<'a> { span.hi)); result.push_str(&where_clause_str); - Some(result) + Some((result, force_new_line_for_brace)) } fn rewrite_args(&self, @@ -463,7 +502,7 @@ impl<'a> FmtVisitor<'a> { arg_items.push(ListItem::from_str("")); } - // TODO(#21): if there are no args, there might still be a comment, but + // FIXME(#21): if there are no args, there might still be a comment, but // without spans for the comment or parens, there is no chance of // getting it right. You also don't get to put a comment on self, unless // it is explicit. @@ -559,7 +598,7 @@ impl<'a> FmtVisitor<'a> { (0, max_space - used_space, new_indent) } else { // Whoops! bankrupt. - // TODO: take evasive action, perhaps kill the indent or something. + // FIXME: take evasive action, perhaps kill the indent or something. panic!("in compute_budgets_for_args"); } } @@ -731,7 +770,7 @@ impl<'a> FmtVisitor<'a> { Some(result) } ast::VariantKind::StructVariantKind(ref struct_def) => { - // TODO: Should limit the width, as we have a trailing comma + // FIXME: Should limit the width, as we have a trailing comma self.format_struct("", field.node.name, ast::Visibility::Inherited, @@ -968,7 +1007,7 @@ impl<'a> FmtVisitor<'a> { }; let h_budget = self.config.max_width - generics_offset.width() - 2; - // TODO: might need to insert a newline if the generics are really long. + // FIXME: might need to insert a newline if the generics are really long. // Strings for the generics. let context = self.get_context(); diff --git a/src/lists.rs b/src/lists.rs index 0f77ea6a750..c9cc24f18e1 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -78,12 +78,12 @@ pub fn format_item_list(items: I, list_helper(items, width, offset, config, ListTactic::HorizontalVertical) } -fn list_helper(items: I, - width: usize, - offset: Indent, - config: &Config, - tactic: ListTactic) - -> Option +pub fn list_helper(items: I, + width: usize, + offset: Indent, + config: &Config, + tactic: ListTactic) + -> Option where I: Iterator { let item_vec: Vec<_> = items.collect(); diff --git a/src/types.rs b/src/types.rs index 2cbbed92fa5..a706f7ec464 100644 --- a/src/types.rs +++ b/src/types.rs @@ -13,7 +13,7 @@ use syntax::print::pprust; use syntax::codemap::{self, Span, BytePos, CodeMap}; use Indent; -use lists::{format_item_list, itemize_list, format_fn_args}; +use lists::{format_item_list, itemize_list, format_fn_args, list_helper, ListTactic}; use rewrite::{Rewrite, RewriteContext}; use utils::{extra_offset, span_after, format_mutability, wrap_str}; @@ -475,19 +475,27 @@ impl Rewrite for ast::Ty { ty.rewrite(context, budget, offset + 1).map(|ty_str| format!("({})", ty_str)) } ast::TyTup(ref tup_ret) => { - let inner = if let [ref item] = &**tup_ret { - try_opt!(item.rewrite(context, width, offset)) + "," + let budget = try_opt!(width.checked_sub(2)); + if tup_ret.is_empty() { + Some("()".to_string()) + } else if let [ref item] = &**tup_ret { + let inner = try_opt!(item.rewrite(context, budget, offset + 1)); + let ret = format!("({},)", inner); + wrap_str(ret, context.config.max_width, budget, offset + 1) } else { - let rewrites: Option>; - rewrites = tup_ret.iter() - .map(|item| item.rewrite(context, width, offset)) - .collect(); + let items = itemize_list(context.codemap, + tup_ret.iter(), + ")", + |item| item.span.lo, + |item| item.span.hi, + |item| item.rewrite(context, budget, offset + 1), + tup_ret[0].span.lo, + self.span.hi); - try_opt!(rewrites).join(", ") - }; - let ret = format!("({})", inner); - wrap_str(ret, context.config.max_width, width, offset) + list_helper(items, budget, offset + 1, context.config, ListTactic::Mixed) + .map(|s| format!("({})", s)) + } } _ => wrap_str(pprust::ty_to_string(self), context.config.max_width, diff --git a/tests/source/multiple.rs b/tests/source/multiple.rs index dc1fefb9019..4f47aff0544 100644 --- a/tests/source/multiple.rs +++ b/tests/source/multiple.rs @@ -112,3 +112,13 @@ fn main() { let s = expand(a , b); } + +fn deconstruct() -> (SocketAddr, Method, Headers, + RequestUri, HttpVersion, + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA) { +} + +fn deconstruct(foo: Bar) -> (SocketAddr, Method, Headers, + RequestUri, HttpVersion, + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA) { +} diff --git a/tests/target/multiple.rs b/tests/target/multiple.rs index 2ec281ae04e..d7f7a79f580 100644 --- a/tests/target/multiple.rs +++ b/tests/target/multiple.rs @@ -141,3 +141,14 @@ fn main() { abcd abcd"; let s = expand(a, b); } + +fn deconstruct() + -> (SocketAddr, Method, Headers, RequestUri, HttpVersion, + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA) +{ +} + +fn deconstruct(foo: Bar) + -> (SocketAddr, Method, Headers, RequestUri, HttpVersion, + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA) { +}