From 29c0ab77ba4e3a196227ee1d61a2ae57e35625c0 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 21 May 2017 19:56:13 +0900 Subject: [PATCH 1/4] Implement combining openings and closings --- src/expr.rs | 83 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 9c4f48bbb7e..f0db88ae566 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1334,18 +1334,21 @@ impl Rewrite for ast::Arm { let pats_str = format!("{}{}", pats_str, guard_str); - let body = match body.node { + let (mut extend, body) = match body.node { ast::ExprKind::Block(ref block) if !is_unsafe_block(block) && is_simple_block(block, context.codemap) && context.config.wrap_match_arms() => { if let ast::StmtKind::Expr(ref expr) = block.stmts[0].node { - expr + (false, &**expr) } else { - &**body + (false, &**body) } } - _ => &**body, + ast::ExprKind::Call(_, ref args) => (args.len() == 1, &**body), + ast::ExprKind::Closure(..) => (true, &**body), + _ => (false, &**body), }; + extend &= context.config.fn_call_style == IndentStyle::Block; let comma = arm_comma(&context.config, body); let alt_block_sep = String::from("\n") + @@ -1371,6 +1374,7 @@ impl Rewrite for ast::Arm { Some(ref body_str) if (!body_str.contains('\n') && body_str.len() <= arm_shape.width) || !context.config.wrap_match_arms() || + (extend && first_line_width(body_str) <= arm_shape.width) || is_block => { let block_sep = match context.config.control_brace_style() { ControlBraceStyle::AlwaysNextLine if is_block => alt_block_sep.as_str(), @@ -1611,9 +1615,7 @@ pub fn rewrite_call(context: &RewriteContext, let closure = |callee_max_width| rewrite_call_inner(context, callee, callee_max_width, args, span, shape); - // 2 is for parens - let max_width = try_opt!(shape.width.checked_sub(2)); - binary_search(1, max_width, closure) + binary_search(1, shape.width, closure) } fn rewrite_call_inner(context: &RewriteContext, @@ -1635,25 +1637,43 @@ fn rewrite_call_inner(context: &RewriteContext, .rewrite(context, callee_shape) .ok_or(Ordering::Greater)?; - // 4 = `( )`, 2 = `()` + // 2 = `( `, 1 = `(` let paren_overhead = if context.config.spaces_within_parens() { - 4 - } else { 2 + } else { + 1 }; let used_width = extra_offset(&callee_str, shape); let one_line_width = shape .width - .checked_sub(used_width + paren_overhead) + .checked_sub(used_width + 2 * paren_overhead) .ok_or(Ordering::Greater)?; + // Try combining openings and closings + if args.len() == 1 && context.config.fn_call_style() == IndentStyle::Block { + let expr = &*args[0]; + match expr.node { + ast::ExprKind::Struct(..) | + ast::ExprKind::Call(..) | + ast::ExprKind::Closure(..) => { + let max_width = min(one_line_width, context.config.fn_call_width()); + let shape = Shape::legacy(max_width, shape.block().indent); + if let Some(expr_str) = expr.rewrite(context, shape) { + if first_line_width(&expr_str) <= max_width { + return Ok(format!("{}({})", callee_str, expr_str)); + } + } + } + _ => (), + } + } + let nested_shape = match context.config.fn_call_style() { IndentStyle::Block => shape.block().block_left(context.config.tab_spaces()), - // 1 = ( IndentStyle::Visual => { shape - .visual_indent(used_width + 1) - .sub_width(used_width + paren_overhead) + .visual_indent(used_width + paren_overhead) + .sub_width(used_width + 2 * paren_overhead) } } .ok_or(Ordering::Greater)?; @@ -1664,8 +1684,12 @@ fn rewrite_call_inner(context: &RewriteContext, let list_str = rewrite_call_args(context, args, span, nested_shape, one_line_width) .ok_or(Ordering::Less)?; + let arg_one_line_budget = min(one_line_width, context.config.fn_call_width()); let result = if context.config.fn_call_style() == IndentStyle::Visual || - (!list_str.contains('\n') && list_str.chars().last().unwrap_or(' ') != ',') { + (((can_be_overflowed(args) && + first_line_width(&list_str) <= arg_one_line_budget) || + !list_str.contains('\n')) && + list_str.chars().last().unwrap_or(' ') != ',') { if context.config.spaces_within_parens() && list_str.len() > 0 { format!("{}( {} )", callee_str, list_str) } else { @@ -1682,6 +1706,15 @@ fn rewrite_call_inner(context: &RewriteContext, Ok(result) } +fn can_be_overflowed(args: &[ptr::P]) -> bool { + match args.last().map(|x| &x.node) { + Some(&ast::ExprKind::Closure(..)) | + Some(&ast::ExprKind::Block(..)) | + Some(&ast::ExprKind::Match(..)) if args.len() > 1 => true, + _ => false, + } +} + fn rewrite_call_args(context: &RewriteContext, args: &[ptr::P], span: Span, @@ -1703,12 +1736,7 @@ fn rewrite_call_args(context: &RewriteContext, // Try letting the last argument overflow to the next line with block // indentation. If its first line fits on one line with the other arguments, // we format the function arguments horizontally. - let overflow_last = match args.last().map(|x| &x.node) { - Some(&ast::ExprKind::Closure(..)) | - Some(&ast::ExprKind::Block(..)) | - Some(&ast::ExprKind::Match(..)) if arg_count > 1 => true, - _ => false, - }; + let overflow_last = can_be_overflowed(args); let mut orig_last = None; let mut placeholder = None; @@ -1716,11 +1744,16 @@ fn rewrite_call_args(context: &RewriteContext, // Replace the last item with its first line to see if it fits with // first arguments. if overflow_last { - let nested_shape = Shape { - indent: shape.indent.block_only(), - ..shape + let last_arg = args.last().unwrap(); + let arg_shape = match last_arg.node { + ast::ExprKind::Closure(..) if context.config.fn_call_style == IndentStyle::Block => { + let mut arg_shape = shape.block(); + arg_shape.indent.block_indent -= context.config.tab_spaces; + arg_shape + } + _ => shape.block(), }; - let rewrite = args.last().unwrap().rewrite(context, nested_shape); + let rewrite = args.last().unwrap().rewrite(context, arg_shape); if let Some(rewrite) = rewrite { let rewrite_first_line = Some(rewrite[..first_line_width(&rewrite)].to_owned()); From 2c15204f0c4a0b0d5f97460a23f10fa090f5aef9 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 21 May 2017 19:56:37 +0900 Subject: [PATCH 2/4] Update tests --- src/expr.rs | 6 +- tests/source/configs-fn_call_style-block.rs | 51 +++++++++++++ tests/source/expr-block.rs | 85 +++++++++++++++++++++ tests/target/closure-block-inside-macro.rs | 18 ++--- tests/target/configs-fn_call_style-block.rs | 61 +++++++++++++-- tests/target/expr-block.rs | 72 +++++++++++++++++ 6 files changed, 274 insertions(+), 19 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index f0db88ae566..c1b130fd8f3 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1348,7 +1348,7 @@ impl Rewrite for ast::Arm { ast::ExprKind::Closure(..) => (true, &**body), _ => (false, &**body), }; - extend &= context.config.fn_call_style == IndentStyle::Block; + extend &= context.config.fn_call_style() == IndentStyle::Block; let comma = arm_comma(&context.config, body); let alt_block_sep = String::from("\n") + @@ -1746,9 +1746,9 @@ fn rewrite_call_args(context: &RewriteContext, if overflow_last { let last_arg = args.last().unwrap(); let arg_shape = match last_arg.node { - ast::ExprKind::Closure(..) if context.config.fn_call_style == IndentStyle::Block => { + ast::ExprKind::Closure(..) if context.config.fn_call_style() == IndentStyle::Block => { let mut arg_shape = shape.block(); - arg_shape.indent.block_indent -= context.config.tab_spaces; + arg_shape.indent.block_indent -= context.config.tab_spaces(); arg_shape } _ => shape.block(), diff --git a/tests/source/configs-fn_call_style-block.rs b/tests/source/configs-fn_call_style-block.rs index 35030189b8b..6708966fa69 100644 --- a/tests/source/configs-fn_call_style-block.rs +++ b/tests/source/configs-fn_call_style-block.rs @@ -45,3 +45,54 @@ fn query(conn: &Connection) -> Result<()> { Ok(()) } + +// #1449 +fn future_rayon_wait_1_thread() { + // run with only 1 worker thread; this would deadlock if we couldn't make progress + let mut result = None; + ThreadPool::new(Configuration::new().num_threads(1)) + .unwrap() + .install( + || { + scope( + |s| { + use std::sync::mpsc::channel; + let (tx, rx) = channel(); + let a = s.spawn_future(lazy(move || Ok::(rx.recv().unwrap()))); + // ^^^^ FIXME: why is this needed? + let b = s.spawn_future(a.map(|v| v + 1)); + let c = s.spawn_future(b.map(|v| v + 1)); + s.spawn(move |_| tx.send(20).unwrap()); + result = Some(c.rayon_wait().unwrap()); + }, + ); + }, + ); + assert_eq!(result, Some(22)); +} + +// #1494 +impl Cursor { + fn foo() { + self.cur_type() + .num_template_args() + .or_else(|| { + let n: c_int = unsafe { clang_Cursor_getNumTemplateArguments(self.x) }; + + if n >= 0 { + Some(n as u32) + } else { + debug_assert_eq!(n, -1); + None + } + }) + .or_else(|| { + let canonical = self.canonical(); + if canonical != *self { + canonical.num_template_args() + } else { + None + } + }); + } +} diff --git a/tests/source/expr-block.rs b/tests/source/expr-block.rs index ad959f8ee01..bf5d23a7d9e 100644 --- a/tests/source/expr-block.rs +++ b/tests/source/expr-block.rs @@ -143,3 +143,88 @@ fn foo() { DefinitiveListTactic::Horizontal } } + +fn combine_block() { + foo( + Bar { + x: value, + y: value2, + }, + ); + + let opt = Some( + Struct( + long_argument_one, + long_argument_two, + long_argggggggg, + ), + ); + + do_thing( + |param| { + action(); + foo(param) + }, + ); + + do_thing( + x, + |param| { + action(); + foo(param) + }, + ); + + Ok( + some_function( + lllllllllong_argument_one, + lllllllllong_argument_two, + lllllllllllllllllllllllllllllong_argument_three, + ), + ); + + foo( + thing, + bar( + param2, + pparam1param1param1param1param1param1param1param1param1param1aram1, + param3, + ), + ); + + foo.map_or( + || { + Ok( + SomeStruct { + f1: 0, + f2: 0, + f3: 0, + }, + ) + }, + ); + + match opt { + Some(x) => somefunc(anotherfunc( + long_argument_one, + long_argument_two, + long_argument_three, + )), + None => Ok(SomeStruct { + f1: long_argument_one, + f2: long_argument_two, + f3: long_argument_three, + }), + }; + + match x { + y => func( + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ), + _ => func( + x, + yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy, + zzz, + ), + } +} diff --git a/tests/target/closure-block-inside-macro.rs b/tests/target/closure-block-inside-macro.rs index b58527eb8fb..5d1e1de0f68 100644 --- a/tests/target/closure-block-inside-macro.rs +++ b/tests/target/closure-block-inside-macro.rs @@ -1,15 +1,13 @@ // rustfmt-fn_call_style: Block // #1547 -fuzz_target!( - |data: &[u8]| { - if let Some(first) = data.first() { - let index = *first as usize; - if index >= ENCODINGS.len() { - return; - } - let encoding = ENCODINGS[index]; - dispatch_test(encoding, &data[1..]); +fuzz_target!(|data: &[u8]| { + if let Some(first) = data.first() { + let index = *first as usize; + if index >= ENCODINGS.len() { + return; } + let encoding = ENCODINGS[index]; + dispatch_test(encoding, &data[1..]); } -); +}); diff --git a/tests/target/configs-fn_call_style-block.rs b/tests/target/configs-fn_call_style-block.rs index d7f65635379..ed3dd76c04d 100644 --- a/tests/target/configs-fn_call_style-block.rs +++ b/tests/target/configs-fn_call_style-block.rs @@ -13,18 +13,20 @@ fn main() { "elit", ); // #1501 - let hyper = Arc::new( - Client::with_connector(HttpsConnector::new(TlsClient::new())), - ); + let hyper = Arc::new(Client::with_connector(HttpsConnector::new( + TlsClient::new(), + ))); } // #1521 impl Foo { fn map_pixel_to_coords(&self, point: &Vector2i, view: &View) -> Vector2f { unsafe { - Vector2f::from_raw( - ffi::sfRenderTexture_mapPixelToCoords(self.render_texture, point.raw(), view.raw()), - ) + Vector2f::from_raw(ffi::sfRenderTexture_mapPixelToCoords( + self.render_texture, + point.raw(), + view.raw(), + )) } } } @@ -58,3 +60,50 @@ fn query(conn: &Connection) -> Result<()> { Ok(()) } + +// #1449 +fn future_rayon_wait_1_thread() { + // run with only 1 worker thread; this would deadlock if we couldn't make progress + let mut result = None; + ThreadPool::new(Configuration::new().num_threads(1)) + .unwrap() + .install(|| { + scope(|s| { + use std::sync::mpsc::channel; + let (tx, rx) = channel(); + let a = s.spawn_future(lazy(move || Ok::(rx.recv().unwrap()))); + // ^^^^ FIXME: why is this needed? + let b = s.spawn_future(a.map(|v| v + 1)); + let c = s.spawn_future(b.map(|v| v + 1)); + s.spawn(move |_| tx.send(20).unwrap()); + result = Some(c.rayon_wait().unwrap()); + }); + }); + assert_eq!(result, Some(22)); +} + +// #1494 +impl Cursor { + fn foo() { + self.cur_type() + .num_template_args() + .or_else(|| { + let n: c_int = unsafe { clang_Cursor_getNumTemplateArguments(self.x) }; + + if n >= 0 { + Some(n as u32) + } else { + debug_assert_eq!(n, -1); + None + } + }) + .or_else(|| { + let canonical = self.canonical(); + if canonical != *self { + canonical.num_template_args() + } else { + None + } + }); + } +} diff --git a/tests/target/expr-block.rs b/tests/target/expr-block.rs index b6f35f65aec..1f3082f3068 100644 --- a/tests/target/expr-block.rs +++ b/tests/target/expr-block.rs @@ -213,3 +213,75 @@ fn foo() { DefinitiveListTactic::Horizontal } } + +fn combine_block() { + foo(Bar { + x: value, + y: value2, + }); + + let opt = Some(Struct( + long_argument_one, + long_argument_two, + long_argggggggg, + )); + + do_thing(|param| { + action(); + foo(param) + }); + + do_thing(x, |param| { + action(); + foo(param) + }); + + Ok(some_function( + lllllllllong_argument_one, + lllllllllong_argument_two, + lllllllllllllllllllllllllllllong_argument_three, + )); + + foo( + thing, + bar( + param2, + pparam1param1param1param1param1param1param1param1param1param1aram1, + param3, + ), + ); + + foo.map_or(|| { + Ok(SomeStruct { + f1: 0, + f2: 0, + f3: 0, + }) + }); + + match opt { + Some(x) => somefunc(anotherfunc( + long_argument_one, + long_argument_two, + long_argument_three, + )), + None => Ok(SomeStruct { + f1: long_argument_one, + f2: long_argument_two, + f3: long_argument_three, + }), + }; + + match x { + y => func( + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + ), + _ => { + func( + x, + yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy, + zzz, + ) + } + } +} From b4cd9584b380b196d9f599916180136c66694d71 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Tue, 23 May 2017 11:20:29 +0900 Subject: [PATCH 3/4] Implement combining for tuple and block --- src/expr.rs | 248 ++++++++++++-------- src/types.rs | 4 +- src/utils.rs | 43 ++-- tests/source/expr-block.rs | 16 ++ tests/target/configs-fn_call_style-block.rs | 6 +- tests/target/expr-block.rs | 21 ++ 6 files changed, 221 insertions(+), 117 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index c1b130fd8f3..bfd38f86c62 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -96,9 +96,7 @@ fn format_expr(expr: &ast::Expr, expr.span, shape) } - ast::ExprKind::Tup(ref items) => { - rewrite_tuple(context, items.iter().map(|x| &**x), expr.span, shape) - } + ast::ExprKind::Tup(ref items) => rewrite_tuple(context, items, expr.span, shape), ast::ExprKind::While(ref cond, ref block, label) => { ControlFlow::new_while(None, cond, block, label, expr.span).rewrite(context, shape) } @@ -1345,7 +1343,9 @@ impl Rewrite for ast::Arm { } } ast::ExprKind::Call(_, ref args) => (args.len() == 1, &**body), - ast::ExprKind::Closure(..) => (true, &**body), + ast::ExprKind::Closure(..) | + ast::ExprKind::Struct(..) | + ast::ExprKind::Tup(..) => (true, &**body), _ => (false, &**body), }; extend &= context.config.fn_call_style() == IndentStyle::Block; @@ -1612,8 +1612,9 @@ pub fn rewrite_call(context: &RewriteContext, -> Option where R: Rewrite { - let closure = - |callee_max_width| rewrite_call_inner(context, callee, callee_max_width, args, span, shape); + let closure = |callee_max_width| { + rewrite_call_inner(context, callee, callee_max_width, args, span, shape, false) + }; binary_search(1, shape.width, closure) } @@ -1623,7 +1624,8 @@ fn rewrite_call_inner(context: &RewriteContext, max_callee_width: usize, args: &[ptr::P], span: Span, - shape: Shape) + shape: Shape, + force_trailing_comma: bool) -> Result where R: Rewrite { @@ -1649,80 +1651,40 @@ fn rewrite_call_inner(context: &RewriteContext, .checked_sub(used_width + 2 * paren_overhead) .ok_or(Ordering::Greater)?; - // Try combining openings and closings - if args.len() == 1 && context.config.fn_call_style() == IndentStyle::Block { - let expr = &*args[0]; - match expr.node { - ast::ExprKind::Struct(..) | - ast::ExprKind::Call(..) | - ast::ExprKind::Closure(..) => { - let max_width = min(one_line_width, context.config.fn_call_width()); - let shape = Shape::legacy(max_width, shape.block().indent); - if let Some(expr_str) = expr.rewrite(context, shape) { - if first_line_width(&expr_str) <= max_width { - return Ok(format!("{}({})", callee_str, expr_str)); - } - } - } - _ => (), - } - } - - let nested_shape = match context.config.fn_call_style() { - IndentStyle::Block => shape.block().block_left(context.config.tab_spaces()), - IndentStyle::Visual => { - shape - .visual_indent(used_width + paren_overhead) - .sub_width(used_width + 2 * paren_overhead) - } - } - .ok_or(Ordering::Greater)?; + let nested_shape = shape_from_fn_call_style(context, + shape, + used_width + 2 * paren_overhead, + used_width + paren_overhead) + .ok_or(Ordering::Greater)?; let span_lo = context.codemap.span_after(span, "("); let span = mk_sp(span_lo, span.hi); - let list_str = rewrite_call_args(context, args, span, nested_shape, one_line_width) - .ok_or(Ordering::Less)?; - + let list_str = rewrite_call_args(context, + args, + span, + nested_shape, + one_line_width, + force_trailing_comma) + .ok_or(Ordering::Less)?; let arg_one_line_budget = min(one_line_width, context.config.fn_call_width()); - let result = if context.config.fn_call_style() == IndentStyle::Visual || - (((can_be_overflowed(args) && - first_line_width(&list_str) <= arg_one_line_budget) || - !list_str.contains('\n')) && - list_str.chars().last().unwrap_or(' ') != ',') { - if context.config.spaces_within_parens() && list_str.len() > 0 { - format!("{}( {} )", callee_str, list_str) - } else { - format!("{}({})", callee_str, list_str) - } - } else { - format!("{}(\n{}{}\n{})", - callee_str, - nested_shape.indent.to_string(context.config), - list_str, - shape.block().indent.to_string(context.config)) - }; - - Ok(result) -} - -fn can_be_overflowed(args: &[ptr::P]) -> bool { - match args.last().map(|x| &x.node) { - Some(&ast::ExprKind::Closure(..)) | - Some(&ast::ExprKind::Block(..)) | - Some(&ast::ExprKind::Match(..)) if args.len() > 1 => true, - _ => false, - } + Ok(format!("{}{}", + callee_str, + wrap_args_with_parens(context, + &list_str, + is_extendable(args), + arg_one_line_budget, + shape, + nested_shape))) } fn rewrite_call_args(context: &RewriteContext, args: &[ptr::P], span: Span, shape: Shape, - one_line_width: usize) + one_line_width: usize, + force_trailing_comma: bool) -> Option { - let arg_count = args.len(); - let items = itemize_list(context.codemap, args.iter(), ")", @@ -1736,7 +1698,7 @@ fn rewrite_call_args(context: &RewriteContext, // Try letting the last argument overflow to the next line with block // indentation. If its first line fits on one line with the other arguments, // we format the function arguments horizontally. - let overflow_last = can_be_overflowed(args); + let overflow_last = can_be_overflowed(context, args); let mut orig_last = None; let mut placeholder = None; @@ -1744,14 +1706,15 @@ fn rewrite_call_args(context: &RewriteContext, // Replace the last item with its first line to see if it fits with // first arguments. if overflow_last { - let last_arg = args.last().unwrap(); - let arg_shape = match last_arg.node { - ast::ExprKind::Closure(..) if context.config.fn_call_style() == IndentStyle::Block => { - let mut arg_shape = shape.block(); - arg_shape.indent.block_indent -= context.config.tab_spaces(); - arg_shape + let arg_shape = if context.config.fn_call_style() == IndentStyle::Block && + is_extendable(args) { + Shape { + width: context.config.fn_call_width(), + indent: shape.block().indent.block_unindent(context.config), + offset: 0, } - _ => shape.block(), + } else { + shape.block() }; let rewrite = args.last().unwrap().rewrite(context, arg_shape); @@ -1759,8 +1722,8 @@ fn rewrite_call_args(context: &RewriteContext, let rewrite_first_line = Some(rewrite[..first_line_width(&rewrite)].to_owned()); placeholder = Some(rewrite); - swap(&mut item_vec[arg_count - 1].item, &mut orig_last); - item_vec[arg_count - 1].item = rewrite_first_line; + swap(&mut item_vec[args.len() - 1].item, &mut orig_last); + item_vec[args.len() - 1].item = rewrite_first_line; } } @@ -1778,10 +1741,10 @@ fn rewrite_call_args(context: &RewriteContext, // succeeded and its first line fits with the other arguments. match (overflow_last, tactic, placeholder) { (true, DefinitiveListTactic::Horizontal, placeholder @ Some(..)) => { - item_vec[arg_count - 1].item = placeholder; + item_vec[args.len() - 1].item = placeholder; } (true, _, _) => { - item_vec[arg_count - 1].item = orig_last; + item_vec[args.len() - 1].item = orig_last; } (false, _, _) => {} } @@ -1789,9 +1752,10 @@ fn rewrite_call_args(context: &RewriteContext, let mut fmt = ListFormatting { tactic: tactic, separator: ",", - trailing_separator: if context.inside_macro || - context.config.fn_call_style() == IndentStyle::Visual || - arg_count <= 1 { + trailing_separator: if force_trailing_comma { + SeparatorTactic::Always + } else if context.inside_macro || context.config.fn_call_style() == IndentStyle::Visual || + args.len() <= 1 { SeparatorTactic::Never } else { context.config.trailing_comma() @@ -1807,8 +1771,8 @@ fn rewrite_call_args(context: &RewriteContext, // and not rewriting macro. Some(ref s) if context.config.fn_call_style() == IndentStyle::Block && !context.inside_macro && - (!s.contains('\n') && - (s.len() > one_line_width || s.len() > context.config.fn_call_width())) => { + (first_line_width(s) > one_line_width || + first_line_width(s) > context.config.fn_call_width()) => { fmt.trailing_separator = SeparatorTactic::Vertical; write_list(&item_vec, &fmt) } @@ -1816,6 +1780,72 @@ fn rewrite_call_args(context: &RewriteContext, } } +fn can_be_overflowed(context: &RewriteContext, args: &[ptr::P]) -> bool { + match args.last().map(|x| &x.node) { + Some(&ast::ExprKind::Block(..)) | + Some(&ast::ExprKind::Match(..)) => { + (context.config.fn_call_style() == IndentStyle::Block && args.len() == 1) || + (context.config.fn_call_style() == IndentStyle::Visual && args.len() > 1) + } + Some(&ast::ExprKind::Closure(..)) => { + context.config.fn_call_style() == IndentStyle::Block || + context.config.fn_call_style() == IndentStyle::Visual && args.len() > 1 + } + Some(&ast::ExprKind::Call(..)) | + Some(&ast::ExprKind::Struct(..)) => { + context.config.fn_call_style() == IndentStyle::Block && args.len() == 1 + } + Some(&ast::ExprKind::Tup(..)) => context.config.fn_call_style() == IndentStyle::Block, + _ => false, + } +} + +fn is_extendable(args: &[ptr::P]) -> bool { + if args.len() == 1 { + match args[0].node { + ast::ExprKind::Block(..) | + ast::ExprKind::Call(..) | + ast::ExprKind::Closure(..) | + ast::ExprKind::Match(..) | + ast::ExprKind::Struct(..) | + ast::ExprKind::Tup(..) => true, + _ => false, + } + } else if args.len() > 1 { + match args[args.len() - 1].node { + ast::ExprKind::Closure(..) | + ast::ExprKind::Tup(..) => true, + _ => false, + } + } else { + false + } +} + +fn wrap_args_with_parens(context: &RewriteContext, + args_str: &str, + is_extendable: bool, + one_line_budget: usize, + shape: Shape, + nested_shape: Shape) + -> String { + if context.config.fn_call_style() == IndentStyle::Visual || + (context.inside_macro && !args_str.contains('\n')) || + ((is_extendable || !args_str.contains('\n')) && + first_line_width(&args_str) <= one_line_budget) { + if context.config.spaces_within_parens() && args_str.len() > 0 { + format!("( {} )", args_str) + } else { + format!("({})", args_str) + } + } else { + format!("(\n{}{}\n{})", + nested_shape.indent.to_string(context.config), + args_str, + shape.block().indent.to_string(context.config)) + } +} + fn rewrite_paren(context: &RewriteContext, subexpr: &ast::Expr, shape: Shape) -> Option { debug!("rewrite_paren, shape: {:?}", shape); // 1 is for opening paren, 2 is for opening+closing, we want to keep the closing @@ -1995,17 +2025,28 @@ fn rewrite_field(context: &RewriteContext, field: &ast::Field, shape: Shape) -> } } -pub fn rewrite_tuple<'a, I>(context: &RewriteContext, - mut items: I, - span: Span, - shape: Shape) - -> Option +fn shape_from_fn_call_style(context: &RewriteContext, + shape: Shape, + overhead: usize, + offset: usize) + -> Option { + match context.config.fn_call_style() { + IndentStyle::Block => Some(shape.block().block_indent(context.config.tab_spaces())), + IndentStyle::Visual => shape.visual_indent(offset).sub_width(overhead), + } +} + +pub fn rewrite_tuple_type<'a, I>(context: &RewriteContext, + mut items: I, + span: Span, + shape: Shape) + -> Option where I: ExactSizeIterator, ::Item: Deref, ::Target: Rewrite + Spanned + 'a { - debug!("rewrite_tuple {:?}", shape); // In case of length 1, need a trailing comma + debug!("rewrite_tuple_type {:?}", shape); if items.len() == 1 { // 3 = "(" + ",)" let nested_shape = try_opt!(shape.sub_width(3)).visual_indent(1); @@ -2039,6 +2080,29 @@ pub fn rewrite_tuple<'a, I>(context: &RewriteContext, } } +pub fn rewrite_tuple(context: &RewriteContext, + items: &[ptr::P], + span: Span, + shape: Shape) + -> Option { + debug!("rewrite_tuple {:?}", shape); + // Use old `rewrite_tuple` + if context.config.fn_call_style() == IndentStyle::Visual { + return rewrite_tuple_type(context, items.iter().map(|x| &**x), span, shape); + } + + // We use the same rule as funcation call for rewriting tuple with multiple expressions. + // 1 = "," + rewrite_call_inner(context, + &String::new(), + shape.width.checked_sub(1).unwrap_or(0), + items, + span, + shape, + items.len() == 1) + .ok() +} + pub fn rewrite_unary_prefix(context: &RewriteContext, prefix: &str, rewrite: &R, diff --git a/src/types.rs b/src/types.rs index 570755d4441..8f99be11d1a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -22,7 +22,7 @@ use codemap::SpanUtils; use lists::{format_item_list, itemize_list, format_fn_args}; use rewrite::{Rewrite, RewriteContext}; use utils::{extra_offset, format_mutability, colon_spaces, wrap_str}; -use expr::{rewrite_unary_prefix, rewrite_pair, rewrite_tuple}; +use expr::{rewrite_unary_prefix, rewrite_pair, rewrite_tuple_type}; use config::TypeDensity; use itertools::Itertools; @@ -662,7 +662,7 @@ impl Rewrite for ast::Ty { }) } ast::TyKind::Tup(ref items) => { - rewrite_tuple(context, items.iter().map(|x| &**x), self.span, shape) + rewrite_tuple_type(context, items.iter().map(|x| &**x), self.span, shape) } ast::TyKind::Path(ref q_self, ref path) => { rewrite_path(context, PathContext::Type, q_self.as_ref(), path, shape) diff --git a/src/utils.rs b/src/utils.rs index bb9d178b83f..664d538fbb9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -299,29 +299,32 @@ pub fn wrap_str>(s: S, max_width: usize, shape: Shape) -> Option shape.width { - return None; - } else { - let mut lines = snippet.lines(); - - // The caller of this function has already placed `shape.offset` - // characters on the first line. - let first_line_max_len = try_opt!(max_width.checked_sub(shape.indent.width())); - if lines.next().unwrap().len() > first_line_max_len { + if !snippet.is_empty() { + if !snippet.contains('\n') && snippet.len() > shape.width { return None; - } + } else { + let mut lines = snippet.lines(); - // The other lines must fit within the maximum width. - if lines.any(|line| line.len() > max_width) { - return None; - } + // The caller of this function has already placed `shape.offset` + // characters on the first line. + let first_line_max_len = try_opt!(max_width.checked_sub(shape.indent.width())); + if lines.next().unwrap().len() > first_line_max_len { + 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.indent.width() + 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.indent.width() + shape.width { + return None; + } } } } diff --git a/tests/source/expr-block.rs b/tests/source/expr-block.rs index bf5d23a7d9e..14207b5adb9 100644 --- a/tests/source/expr-block.rs +++ b/tests/source/expr-block.rs @@ -152,6 +152,22 @@ fn combine_block() { }, ); + foo((Bar { + x: value, + y: value2, + },)); + + foo((1, 2, 3, Bar { + x: value, + y: value2, + })); + + foo((1, 2, 3, |x| { + let y = x + 1; + let z = y + 1; + z + })); + let opt = Some( Struct( long_argument_one, diff --git a/tests/target/configs-fn_call_style-block.rs b/tests/target/configs-fn_call_style-block.rs index ed3dd76c04d..af8238defe6 100644 --- a/tests/target/configs-fn_call_style-block.rs +++ b/tests/target/configs-fn_call_style-block.rs @@ -13,9 +13,9 @@ fn main() { "elit", ); // #1501 - let hyper = Arc::new(Client::with_connector(HttpsConnector::new( - TlsClient::new(), - ))); + let hyper = Arc::new(Client::with_connector( + HttpsConnector::new(TlsClient::new()), + )); } // #1521 diff --git a/tests/target/expr-block.rs b/tests/target/expr-block.rs index 1f3082f3068..6e4d2f109ac 100644 --- a/tests/target/expr-block.rs +++ b/tests/target/expr-block.rs @@ -220,6 +220,27 @@ fn combine_block() { y: value2, }); + foo((Bar { + x: value, + y: value2, + },)); + + foo(( + 1, + 2, + 3, + Bar { + x: value, + y: value2, + }, + )); + + foo((1, 2, 3, |x| { + let y = x + 1; + let z = y + 1; + z + })); + let opt = Some(Struct( long_argument_one, long_argument_two, From f83c22f24f2f497c454531e101cf63bf825e3bd5 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Tue, 23 May 2017 12:03:04 +0900 Subject: [PATCH 4/4] Add trailing comma to a single arg in multiline --- src/expr.rs | 41 +++++++++++-------- ...figs-fn_call_style-block-trailing-comma.rs | 1 + tests/source/expr-block.rs | 28 +++++++++++++ ...figs-fn_call_style-block-trailing-comma.rs | 3 ++ tests/target/configs-fn_call_style-block.rs | 2 +- tests/target/expr-block.rs | 22 +++++++++- 6 files changed, 79 insertions(+), 18 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index bfd38f86c62..ad0d654175d 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1655,24 +1655,24 @@ fn rewrite_call_inner(context: &RewriteContext, shape, used_width + 2 * paren_overhead, used_width + paren_overhead) - .ok_or(Ordering::Greater)?; + .ok_or(Ordering::Greater)?; let span_lo = context.codemap.span_after(span, "("); let span = mk_sp(span_lo, span.hi); - let list_str = rewrite_call_args(context, - args, - span, - nested_shape, - one_line_width, - force_trailing_comma) - .ok_or(Ordering::Less)?; + let (extendable, list_str) = rewrite_call_args(context, + args, + span, + nested_shape, + one_line_width, + force_trailing_comma) + .ok_or(Ordering::Less)?; let arg_one_line_budget = min(one_line_width, context.config.fn_call_width()); Ok(format!("{}{}", callee_str, wrap_args_with_parens(context, &list_str, - is_extendable(args), + extendable, arg_one_line_budget, shape, nested_shape))) @@ -1684,7 +1684,7 @@ fn rewrite_call_args(context: &RewriteContext, shape: Shape, one_line_width: usize, force_trailing_comma: bool) - -> Option { + -> Option<(bool, String)> { let items = itemize_list(context.codemap, args.iter(), ")", @@ -1717,12 +1717,12 @@ fn rewrite_call_args(context: &RewriteContext, shape.block() }; let rewrite = args.last().unwrap().rewrite(context, arg_shape); + swap(&mut item_vec[args.len() - 1].item, &mut orig_last); if let Some(rewrite) = rewrite { let rewrite_first_line = Some(rewrite[..first_line_width(&rewrite)].to_owned()); placeholder = Some(rewrite); - swap(&mut item_vec[args.len() - 1].item, &mut orig_last); item_vec[args.len() - 1].item = rewrite_first_line; } } @@ -1765,18 +1765,27 @@ fn rewrite_call_args(context: &RewriteContext, config: context.config, }; + let args_in_single_line = + item_vec + .iter() + .rev() + .skip(1) + .all(|item| item.item.as_ref().map_or(false, |s| !s.contains('\n'))); + match write_list(&item_vec, &fmt) { // If arguments do not fit in a single line and do not contain newline, // try to put it on the next line. Try this only when we are in block mode // and not rewriting macro. Some(ref s) if context.config.fn_call_style() == IndentStyle::Block && !context.inside_macro && - (first_line_width(s) > one_line_width || + (!can_be_overflowed(context, args) && args.len() == 1 && s.contains('\n') || + first_line_width(s) > one_line_width || first_line_width(s) > context.config.fn_call_width()) => { fmt.trailing_separator = SeparatorTactic::Vertical; - write_list(&item_vec, &fmt) + fmt.tactic = DefinitiveListTactic::Vertical; + write_list(&item_vec, &fmt).map(|rw| (false, rw)) } - rewrite @ _ => rewrite, + rewrite @ _ => rewrite.map(|rw| (args_in_single_line && is_extendable(args), rw)), } } @@ -2091,7 +2100,7 @@ pub fn rewrite_tuple(context: &RewriteContext, return rewrite_tuple_type(context, items.iter().map(|x| &**x), span, shape); } - // We use the same rule as funcation call for rewriting tuple with multiple expressions. + // We use the same rule as funcation call for rewriting tuple. // 1 = "," rewrite_call_inner(context, &String::new(), @@ -2100,7 +2109,7 @@ pub fn rewrite_tuple(context: &RewriteContext, span, shape, items.len() == 1) - .ok() + .ok() } pub fn rewrite_unary_prefix(context: &RewriteContext, diff --git a/tests/source/configs-fn_call_style-block-trailing-comma.rs b/tests/source/configs-fn_call_style-block-trailing-comma.rs index 3f3d1dec2ea..6f613fb10bb 100644 --- a/tests/source/configs-fn_call_style-block-trailing-comma.rs +++ b/tests/source/configs-fn_call_style-block-trailing-comma.rs @@ -4,4 +4,5 @@ // rustfmt should not add trailing comma when rewriting macro. See #1528. fn a() { panic!("this is a long string that goes past the maximum line length causing rustfmt to insert a comma here:"); + foo(oooptoptoptoptptooptoptoptoptptooptoptoptoptptoptoptoptoptpt()); } diff --git a/tests/source/expr-block.rs b/tests/source/expr-block.rs index 14207b5adb9..27f7ff67da9 100644 --- a/tests/source/expr-block.rs +++ b/tests/source/expr-block.rs @@ -191,6 +191,19 @@ fn combine_block() { }, ); + do_thing( + x, + ( + 1, + 2, + 3, + |param| { + action(); + foo(param) + }, + ), + ); + Ok( some_function( lllllllllong_argument_one, @@ -226,6 +239,21 @@ fn combine_block() { long_argument_two, long_argument_three, )), + Some(x) => |x| { + let y = x + 1; + let z = y + 1; + z + }, + Some(x) => (1, 2, |x| { + let y = x + 1; + let z = y + 1; + z + }), + Some(x) => SomeStruct { + f1: long_argument_one, + f2: long_argument_two, + f3: long_argument_three, + }, None => Ok(SomeStruct { f1: long_argument_one, f2: long_argument_two, diff --git a/tests/target/configs-fn_call_style-block-trailing-comma.rs b/tests/target/configs-fn_call_style-block-trailing-comma.rs index 3f3d1dec2ea..ebdf41d0e3b 100644 --- a/tests/target/configs-fn_call_style-block-trailing-comma.rs +++ b/tests/target/configs-fn_call_style-block-trailing-comma.rs @@ -4,4 +4,7 @@ // rustfmt should not add trailing comma when rewriting macro. See #1528. fn a() { panic!("this is a long string that goes past the maximum line length causing rustfmt to insert a comma here:"); + foo( + oooptoptoptoptptooptoptoptoptptooptoptoptoptptoptoptoptoptpt(), + ); } diff --git a/tests/target/configs-fn_call_style-block.rs b/tests/target/configs-fn_call_style-block.rs index af8238defe6..cd19d8bb838 100644 --- a/tests/target/configs-fn_call_style-block.rs +++ b/tests/target/configs-fn_call_style-block.rs @@ -36,7 +36,7 @@ fn issue1420() { r#" # Getting started ... - "# + "#, ) .running(waltz) } diff --git a/tests/target/expr-block.rs b/tests/target/expr-block.rs index 6e4d2f109ac..b4d86ed82f5 100644 --- a/tests/target/expr-block.rs +++ b/tests/target/expr-block.rs @@ -102,7 +102,7 @@ fn arrays() { Weighted { weight: 1, item: 1 }, Weighted { weight: x, item: 2 }, Weighted { weight: 1, item: 3 }, - ] + ], ); let z = [ @@ -257,6 +257,11 @@ fn combine_block() { foo(param) }); + do_thing(x, (1, 2, 3, |param| { + action(); + foo(param) + })); + Ok(some_function( lllllllllong_argument_one, lllllllllong_argument_two, @@ -286,6 +291,21 @@ fn combine_block() { long_argument_two, long_argument_three, )), + Some(x) => |x| { + let y = x + 1; + let z = y + 1; + z + }, + Some(x) => (1, 2, |x| { + let y = x + 1; + let z = y + 1; + z + }), + Some(x) => SomeStruct { + f1: long_argument_one, + f2: long_argument_two, + f3: long_argument_three, + }, None => Ok(SomeStruct { f1: long_argument_one, f2: long_argument_two,