diff --git a/src/config.rs b/src/config.rs index 357f99a6097..341f1d45afd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -78,4 +78,5 @@ create_config! { report_fixme: ReportTactic, reorder_imports: bool, // Alphabetically, case sensitive. expr_indent_style: BlockIndentStyle, + closure_indent_style: BlockIndentStyle, } diff --git a/src/default.toml b/src/default.toml index 035fa21e917..5792079a407 100644 --- a/src/default.toml +++ b/src/default.toml @@ -14,3 +14,4 @@ report_todo = "Always" report_fixme = "Never" reorder_imports = false expr_indent_style = "Tabbed" +closure_indent_style = "Visual" diff --git a/src/expr.rs b/src/expr.rs index 0cdaf060714..af6e9ce1e35 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -17,6 +17,7 @@ use visitor::FmtVisitor; use config::BlockIndentStyle; use comment::{FindUncommented, rewrite_comment}; use types::rewrite_path; +use items::{span_lo_for_arg, span_hi_for_arg, rewrite_fn_input}; use syntax::{ast, ptr}; use syntax::codemap::{Pos, Span, BytePos, mk_sp}; @@ -115,11 +116,98 @@ impl Rewrite for ast::Expr { }; Some(format!("continue{}", id_str)) } + ast::Expr_::ExprClosure(capture, ref fn_decl, ref body) => { + rewrite_closure(capture, fn_decl, body, self.span, context, width, offset) + } _ => context.codemap.span_to_snippet(self.span).ok(), } } } +// This functions is pretty messy because of the wrapping and unwrapping of +// expressions into and from blocks. See rust issue #27872. +fn rewrite_closure(capture: ast::CaptureClause, + fn_decl: &ast::FnDecl, + body: &ast::Block, + span: Span, + context: &RewriteContext, + width: usize, + offset: usize) + -> Option { + let mover = if capture == ast::CaptureClause::CaptureByValue { + "move " + } else { + "" + }; + let offset = offset + mover.len(); + + // 4 = "|| {".len(), which is overconservative when the closure consists of + // a single expression. + let argument_budget = try_opt!(width.checked_sub(4 + mover.len())); + // 1 = | + let argument_offset = offset + 1; + + let arg_items = itemize_list(context.codemap, + fn_decl.inputs.iter(), + "|", + |arg| span_lo_for_arg(arg), + |arg| span_hi_for_arg(arg), + |arg| rewrite_fn_input(arg), + span_after(span, "|", context.codemap), + body.span.lo); + + let fmt = ListFormatting::for_fn(argument_budget, argument_offset); + let prefix = format!("{}|{}|", mover, write_list(&arg_items.collect::>(), &fmt)); + let block_indent = closure_block_indent(context, offset); + + let body_rewrite = if body.stmts.is_empty() { + let expr = body.expr.as_ref().unwrap(); + // All closure bodies are blocks in the eyes of the AST, but we may not + // want to unwrap them when they only contain a single expression. + let inner_expr = match expr.node { + ast::Expr_::ExprBlock(ref inner) if inner.stmts.is_empty() && inner.expr.is_some() => { + inner.expr.as_ref().unwrap() + } + _ => expr, + }; + + // 1 = the separating space between arguments and the body. + let extra_offset = extra_offset(&prefix, offset) + 1; + let rewrite = inner_expr.rewrite(context, width - extra_offset, offset + extra_offset); + + // Checks if rewrite succeeded and fits on a single line. + let accept_rewrite = rewrite.as_ref().map(|result| !result.contains('\n')).unwrap_or(false); + + if accept_rewrite { + rewrite + } else { + if let ast::Expr_::ExprBlock(ref inner_body) = expr.node { + // Closure body is a proper block, with braces and all. + let inner_context = &RewriteContext { block_indent: block_indent, ..*context }; + inner_body.rewrite(inner_context, 0, 0) + } else { + // Closure body is an expression, but not a block. It does not + // fit a single line, so we format it as it were wrapped in a + // block. + let inner_context = &RewriteContext { + block_indent: block_indent + context.config.tab_spaces, + ..*context + }; + let indent = offset + context.config.tab_spaces; + expr.rewrite(inner_context, context.config.max_width - indent, indent) + .map(|result| { + format!("{{\n{}{}\n{}}}", make_indent(indent), result, make_indent(offset)) + }) + } + } + } else { + let inner_context = &RewriteContext { block_indent: block_indent, ..*context }; + body.rewrite(inner_context, 0, 0) + }; + + Some(format!("{} {}", prefix, try_opt!(body_rewrite))) +} + impl Rewrite for ast::Block { fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option { let user_str = context.codemap.span_to_snippet(self.span).unwrap(); @@ -674,21 +762,14 @@ fn rewrite_call(context: &RewriteContext, |item| item.span.lo, |item| item.span.hi, // Take old span when rewrite fails. - |item| item.rewrite(inner_context, remaining_width, offset) - .unwrap_or(context.codemap.span_to_snippet(item.span) - .unwrap()), + |item| { + item.rewrite(inner_context, remaining_width, offset) + .unwrap_or(context.codemap.span_to_snippet(item.span).unwrap()) + }, callee.span.hi + BytePos(1), span.hi); - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: offset, - h_width: remaining_width, - v_width: remaining_width, - ends_with_newline: false, - }; + let fmt = ListFormatting::for_fn(remaining_width, offset); Some(format!("{}({})", callee_str, write_list(&items.collect::>(), &fmt))) } @@ -701,6 +782,15 @@ fn expr_block_indent(context: &RewriteContext, offset: usize) -> usize { } } +// FIXME: Code duplication; is there a better solution? +fn closure_block_indent(context: &RewriteContext, offset: usize) -> usize { + match context.config.closure_indent_style { + BlockIndentStyle::Inherit => context.block_indent, + BlockIndentStyle::Tabbed => context.block_indent + context.config.tab_spaces, + BlockIndentStyle::Visual => offset, + } +} + fn rewrite_paren(context: &RewriteContext, subexpr: &ast::Expr, width: usize, @@ -760,13 +850,13 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, match *item { StructLitField::Regular(ref field) => field.span.lo, // 2 = .. - StructLitField::Base(ref expr) => expr.span.lo - BytePos(2) + StructLitField::Base(ref expr) => expr.span.lo - BytePos(2), } }, |item| { match *item { StructLitField::Regular(ref field) => field.span.hi, - StructLitField::Base(ref expr) => expr.span.hi + StructLitField::Base(ref expr) => expr.span.hi, } }, |item| { @@ -775,7 +865,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, rewrite_field(inner_context, &field, h_budget, indent) .unwrap_or(context.codemap.span_to_snippet(field.span) .unwrap()) - }, + } StructLitField::Base(ref expr) => { // 2 = .. expr.rewrite(inner_context, h_budget - 2, indent + 2) @@ -846,23 +936,15 @@ fn rewrite_tuple_lit(context: &RewriteContext, ")", |item| item.span.lo, |item| item.span.hi, - |item| item.rewrite(context, - context.config.max_width - indent - 1, - indent) - .unwrap_or(context.codemap.span_to_snippet(item.span) - .unwrap()), + |item| { + let inner_width = context.config.max_width - indent - 1; + item.rewrite(context, inner_width, indent) + .unwrap_or(context.codemap.span_to_snippet(item.span).unwrap()) + }, span.lo + BytePos(1), // Remove parens span.hi - BytePos(1)); - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: indent, - h_width: width - 2, - v_width: width - 2, - ends_with_newline: false, - }; + let fmt = ListFormatting::for_fn(width - 2, indent); Some(format!("({})", write_list(&items.collect::>(), &fmt))) } diff --git a/src/imports.rs b/src/imports.rs index 7d7300fb562..a0ba88de6f2 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -117,13 +117,15 @@ pub fn rewrite_use_list(width: usize, "}", |vpi| vpi.span.lo, |vpi| vpi.span.hi, - |vpi| match vpi.node { - ast::PathListItem_::PathListIdent{ name, .. } => { - name.to_string() - } - ast::PathListItem_::PathListMod{ .. } => { - "self".to_owned() - } + |vpi| { + match vpi.node { + ast::PathListItem_::PathListIdent{ name, .. } => { + name.to_string() + } + ast::PathListItem_::PathListMod{ .. } => { + "self".to_owned() + } + } }, span_after(span, "{", context.codemap), span.hi); diff --git a/src/items.rs b/src/items.rs index 79d374f85ec..e853dc90c30 100644 --- a/src/items.rs +++ b/src/items.rs @@ -232,7 +232,7 @@ impl<'a> FmtVisitor<'a> { arg_indent: usize, span: Span) -> String { - let mut arg_item_strs: Vec<_> = args.iter().map(|a| self.rewrite_fn_input(a)).collect(); + let mut arg_item_strs: Vec<_> = args.iter().map(rewrite_fn_input).collect(); // Account for sugary self. // FIXME: the comment for the self argument is dropped. This is blocked // on rust issue #27522. @@ -512,13 +512,12 @@ impl<'a> FmtVisitor<'a> { struct_def.fields.iter(), terminator, |field| { - // Include attributes and doc comments, - // if present - if field.node.attrs.len() > 0 { - field.node.attrs[0].span.lo - } else { - field.span.lo - } + // Include attributes and doc comments, if present + if field.node.attrs.len() > 0 { + field.node.attrs[0].span.lo + } else { + field.span.lo + } }, |field| field.node.ty.span.hi, |field| self.format_field(field), @@ -650,16 +649,14 @@ impl<'a> FmtVisitor<'a> { fn rewrite_generics(&self, generics: &ast::Generics, offset: usize, span: Span) -> String { // FIXME convert bounds to where clauses where they get too big or if // there is a where clause at all. - let mut result = String::new(); let lifetimes: &[_] = &generics.lifetimes; let tys: &[_] = &generics.ty_params; if lifetimes.len() + tys.len() == 0 { - return result; + return String::new(); } let budget = self.config.max_width - offset - 2; // TODO might need to insert a newline if the generics are really long - result.push('<'); // Strings for the generics. // 1 = < @@ -697,20 +694,9 @@ impl<'a> FmtVisitor<'a> { item.item = ty; } - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: offset + 1, - h_width: budget, - v_width: budget, - ends_with_newline: false, - }; - result.push_str(&write_list(&items, &fmt)); + let fmt = ListFormatting::for_fn(budget, offset + 1); - result.push('>'); - - result + format!("<{}>", write_list(&items, &fmt)) } fn rewrite_where_clause(&self, @@ -719,15 +705,10 @@ impl<'a> FmtVisitor<'a> { indent: usize, span_end: BytePos) -> String { - let mut result = String::new(); if where_clause.predicates.len() == 0 { - return result; + return String::new(); } - result.push('\n'); - result.push_str(&make_indent(indent + config.tab_spaces)); - result.push_str("where "); - let context = self.get_context(); // 6 = "where ".len() let offset = indent + config.tab_spaces + 6; @@ -752,11 +733,12 @@ impl<'a> FmtVisitor<'a> { indent: offset, h_width: budget, v_width: budget, - ends_with_newline: false, + ends_with_newline: true, }; - result.push_str(&write_list(&items.collect::>(), &fmt)); - result + format!("\n{}where {}", + make_indent(indent + config.tab_spaces), + write_list(&items.collect::>(), &fmt)) } fn rewrite_return(&self, ret: &ast::FunctionRetTy) -> String { @@ -766,17 +748,21 @@ impl<'a> FmtVisitor<'a> { ast::FunctionRetTy::Return(ref ty) => "-> ".to_owned() + &pprust::ty_to_string(ty), } } +} - // TODO we farm this out, but this could spill over the column limit, so we - // ought to handle it properly. - fn rewrite_fn_input(&self, arg: &ast::Arg) -> String { - if is_named_arg(arg) { - format!("{}: {}", - pprust::pat_to_string(&arg.pat), - pprust::ty_to_string(&arg.ty)) +// TODO we farm this out, but this could spill over the column limit, so we +// ought to handle it properly. +pub fn rewrite_fn_input(arg: &ast::Arg) -> String { + if is_named_arg(arg) { + if let ast::Ty_::TyInfer = arg.ty.node { + pprust::pat_to_string(&arg.pat) } else { - pprust::ty_to_string(&arg.ty) + format!("{}: {}", + pprust::pat_to_string(&arg.pat), + pprust::ty_to_string(&arg.ty)) } + } else { + pprust::ty_to_string(&arg.ty) } } @@ -810,7 +796,7 @@ fn rewrite_explicit_self(explicit_self: &ast::ExplicitSelf, args: &[ast::Arg]) - } } -fn span_lo_for_arg(arg: &ast::Arg) -> BytePos { +pub fn span_lo_for_arg(arg: &ast::Arg) -> BytePos { if is_named_arg(arg) { arg.pat.span.lo } else { @@ -818,6 +804,13 @@ fn span_lo_for_arg(arg: &ast::Arg) -> BytePos { } } +pub fn span_hi_for_arg(arg: &ast::Arg) -> BytePos { + match arg.ty.node { + ast::Ty_::TyInfer if is_named_arg(arg) => arg.pat.span.hi, + _ => arg.ty.span.hi, + } +} + fn is_named_arg(arg: &ast::Arg) -> bool { if let ast::Pat_::PatIdent(_, ident, _) = arg.pat.node { ident.node != token::special_idents::invalid diff --git a/src/lists.rs b/src/lists.rs index eadefe2e0fb..dd1a489b847 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -52,6 +52,20 @@ pub struct ListFormatting<'a> { pub ends_with_newline: bool, } +impl<'a> ListFormatting<'a> { + pub fn for_fn(width: usize, offset: usize) -> ListFormatting<'a> { + ListFormatting { + tactic: ListTactic::HorizontalVertical, + separator: ",", + trailing_separator: SeparatorTactic::Never, + indent: offset, + h_width: width, + v_width: width, + ends_with_newline: false, + } + } +} + pub struct ListItem { pub pre_comment: Option, // Item should include attributes and doc comments diff --git a/src/types.rs b/src/types.rs index 3bdb08cc0d2..fa3d61603b8 100644 --- a/src/types.rs +++ b/src/types.rs @@ -14,7 +14,7 @@ use syntax::ast; use syntax::print::pprust; use syntax::codemap::{self, Span, BytePos, CodeMap}; -use lists::{itemize_list, write_list, ListTactic, SeparatorTactic, ListFormatting}; +use lists::{itemize_list, write_list, ListFormatting}; use rewrite::{Rewrite, RewriteContext}; use utils::{extra_offset, span_after}; @@ -218,16 +218,7 @@ fn rewrite_segment(segment: &ast::PathSegment, let extra_offset = 1 + separator.len(); // 1 for > let list_width = try_opt!(width.checked_sub(extra_offset + 1)); - - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: offset + extra_offset, - h_width: list_width, - v_width: list_width, - ends_with_newline: false, - }; + let fmt = ListFormatting::for_fn(list_width, offset + extra_offset); // update pos *span_lo = next_span_lo; @@ -253,16 +244,8 @@ fn rewrite_segment(segment: &ast::PathSegment, // 2 for () let budget = try_opt!(width.checked_sub(output.len() + 2)); - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - // 1 for ( - indent: offset + 1, - h_width: budget, - v_width: budget, - ends_with_newline: false, - }; + // 1 for ( + let fmt = ListFormatting::for_fn(budget, offset + 1); // update pos *span_lo = data.inputs.last().unwrap().span.hi + BytePos(1); diff --git a/tests/config/small_tabs.toml b/tests/config/small_tabs.toml index b2c7f5fc43f..2aff85506a4 100644 --- a/tests/config/small_tabs.toml +++ b/tests/config/small_tabs.toml @@ -14,3 +14,4 @@ report_todo = "Always" report_fixme = "Never" reorder_imports = false expr_indent_style = "Tabbed" +closure_indent_style = "Visual" diff --git a/tests/source/closure.rs b/tests/source/closure.rs new file mode 100644 index 00000000000..292cef376dd --- /dev/null +++ b/tests/source/closure.rs @@ -0,0 +1,30 @@ +// Closures + +fn main() { + let square = ( |i: i32 | i * i ); + + let commented = |/* first */ a /*argument*/, /* second*/ b: WithType /* argument*/, /* ignored */ _ | + (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbb); + + let commented = |/* first */ a /*argument*/, /* second*/ b: WithType /* argument*/, /* ignored */ _ | + (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb); + + let block_body = move |xxxxxxxxxxxxxxxxxxxxxxxxxxxxx, ref yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy| { + xxxxxxxxxxxxxxxxxxxxxxxxxxxxx + yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + }; + + let loooooooooooooong_name = |field| { + // TODO(#27): format comments. + if field.node.attrs.len() > 0 { field.node.attrs[0].span.lo + } else { + field.span.lo + }}; + + let block_me = |field| if true_story() { 1 } else { 2 }; + + let unblock_me = |trivial| { + closure() + }; + + let empty = |arg| {}; +} diff --git a/tests/target/closure.rs b/tests/target/closure.rs new file mode 100644 index 00000000000..7ebfbf12521 --- /dev/null +++ b/tests/target/closure.rs @@ -0,0 +1,48 @@ +// Closures + +fn main() { + let square = (|i: i32| i * i); + + let commented = |// first + a, // argument + // second + b: WithType, // argument + // ignored + _| (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbb); + + let commented = |// first + a, // argument + // second + b: WithType, // argument + // ignored + _| { + (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) + }; + + let block_body = move |xxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ref yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy| { + xxxxxxxxxxxxxxxxxxxxxxxxxxxxx + yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + }; + + let loooooooooooooong_name = |field| { + // TODO(#27): format comments. + if field.node.attrs.len() > 0 { + field.node.attrs[0].span.lo + } else { + field.span.lo + } + }; + + let block_me = |field| { + if true_story() { + 1 + } else { + 2 + } + }; + + let unblock_me = |trivial| closure(); + + let empty = |arg| {}; +} diff --git a/tests/target/comments-fn.rs b/tests/target/comments-fn.rs index 0b21d84074a..57b6ae63381 100644 --- a/tests/target/comments-fn.rs +++ b/tests/target/comments-fn.rs @@ -11,7 +11,7 @@ fn foo(a: aaaaaaaaaaaaa, // A comment e: eeeeeeeeeeeee /* comment before paren */) -> bar where F: Foo, // COmment after where clause - G: Goo /* final comment */ + G: Goo // final comment { }