From 4720462e6761306a72de1f62523d72698d4e77c9 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 12:34:09 +0900 Subject: [PATCH 1/7] Add a test for #2200 --- tests/source/chains.rs | 21 +++++++++++++++++++++ tests/target/chains.rs | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tests/source/chains.rs b/tests/source/chains.rs index f14a0293123c..0141326269ba 100644 --- a/tests/source/chains.rs +++ b/tests/source/chains.rs @@ -185,3 +185,24 @@ fn issue2126() { } } } + +// #2200 +impl Foo { + pub fn from_ast(diagnostic: &::errors::Handler, + attrs: &[ast::Attribute]) -> Attributes { + let other_attrs = attrs.iter().filter_map(|attr| { + attr.with_desugared_doc(|attr| { + if attr.check_name("doc") { + if let Some(mi) = attr.meta() { + if let Some(value) = mi.value_str() { + doc_strings.push(DocFragment::Include(line, + attr.span, + filename, + contents)); + } + } + } + }) + }).collect(); + } +} diff --git a/tests/target/chains.rs b/tests/target/chains.rs index 00855de4f981..a88440a9d783 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -213,3 +213,25 @@ fn issue2126() { } } } + +// #2200 +impl Foo { + pub fn from_ast(diagnostic: &::errors::Handler, attrs: &[ast::Attribute]) -> Attributes { + let other_attrs = attrs + .iter() + .filter_map(|attr| { + attr.with_desugared_doc(|attr| { + if attr.check_name("doc") { + if let Some(mi) = attr.meta() { + if let Some(value) = mi.value_str() { + doc_strings.push( + DocFragment::Include(line, attr.span, filename, contents), + ); + } + } + } + }) + }) + .collect(); + } +} From 4436508712a9dc593a75155c36cd9652f4b283fe Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:29:06 +0900 Subject: [PATCH 2/7] Fix a test target --- tests/target/chains.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/target/chains.rs b/tests/target/chains.rs index a88440a9d783..61ba04347e0d 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -224,9 +224,12 @@ impl Foo { if attr.check_name("doc") { if let Some(mi) = attr.meta() { if let Some(value) = mi.value_str() { - doc_strings.push( - DocFragment::Include(line, attr.span, filename, contents), - ); + doc_strings.push(DocFragment::Include( + line, + attr.span, + filename, + contents, + )); } } } From be19bab9dee0a4e54638327a3b072ceb0868484f Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:29:38 +0900 Subject: [PATCH 3/7] Take into account the rhs overhead when rewriting the last element of chain --- src/chains.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index a35770427b50..cb8e8d62988a 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -166,18 +166,11 @@ 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 = { - let last_shape = if rewrites.len() == 0 { - first_child_shape - } else { - other_child_shape - }; - match context.config.indent_style() { - IndentStyle::Visual => last_shape.sub_width(shape.rhs_overhead(context.config))?, - IndentStyle::Block => last_shape, - } - }; - let last_shape = last_shape.sub_width(suffix_try_num)?; + let last_shape = if rewrites.len() == 0 { + first_child_shape + } else { + other_child_shape + }.sub_width(shape.rhs_overhead(context.config) + suffix_try_num)?; // 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: From 94a770a777d3995e80938a31ec51c6e85d7ab694 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:32:31 +0900 Subject: [PATCH 4/7] Use correct shape when rewriting the last arg with overflowing --- src/expr.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 812ebd5cf3a2..96132229c903 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1814,6 +1814,10 @@ where let used_width = extra_offset(callee_str, shape); let one_line_width = shape.width.checked_sub(used_width + 2 * paren_overhead)?; + // 1 = "(" or ")" + let one_line_shape = shape + .offset_left(last_line_width(callee_str) + 1)? + .sub_width(1)?; let nested_shape = shape_from_indent_style( context, shape, @@ -1828,6 +1832,7 @@ where context, args, args_span, + one_line_shape, nested_shape, one_line_width, args_max_width, @@ -1867,7 +1872,8 @@ fn rewrite_call_args<'a, T>( context: &RewriteContext, args: &[&T], span: Span, - shape: Shape, + one_line_shape: Shape, + nested_shape: Shape, one_line_width: usize, args_max_width: usize, force_trailing_comma: bool, @@ -1882,7 +1888,7 @@ where ",", |item| item.span().lo(), |item| item.span().hi(), - |item| item.rewrite(context, shape), + |item| item.rewrite(context, nested_shape), span.lo(), span.hi(), true, @@ -1896,7 +1902,8 @@ where context, &mut item_vec, &args[..], - shape, + one_line_shape, + nested_shape, one_line_width, args_max_width, ); @@ -1912,7 +1919,7 @@ where context.config.trailing_comma() }, separator_place: SeparatorPlace::Back, - shape: shape, + shape: nested_shape, ends_with_newline: context.use_block_indent() && tactic == DefinitiveListTactic::Vertical, preserve_newline: false, config: context.config, @@ -1927,7 +1934,8 @@ fn try_overflow_last_arg<'a, T>( context: &RewriteContext, item_vec: &mut Vec, args: &[&T], - shape: Shape, + one_line_shape: Shape, + nested_shape: Shape, one_line_width: usize, args_max_width: usize, ) -> DefinitiveListTactic @@ -1945,7 +1953,7 @@ where context.force_one_line_chain = true; } } - last_arg_shape(&context, item_vec, shape, args_max_width).and_then(|arg_shape| { + last_arg_shape(&context, item_vec, one_line_shape, args_max_width).and_then(|arg_shape| { rewrite_last_arg_with_overflow(&context, args, &mut item_vec[args.len() - 1], arg_shape) }) } else { From af663d8f627420d111b307ed057918dc44b1a7f0 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:36:51 +0900 Subject: [PATCH 5/7] Ignore fn_call_width when rewriting a call with a single non-call arg --- src/expr.rs | 52 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 96132229c903..d51d355dbdd7 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1953,7 +1953,7 @@ where context.force_one_line_chain = true; } } - last_arg_shape(&context, item_vec, one_line_shape, args_max_width).and_then(|arg_shape| { + last_arg_shape(args, item_vec, one_line_shape, args_max_width).and_then(|arg_shape| { rewrite_last_arg_with_overflow(&context, args, &mut item_vec[args.len() - 1], arg_shape) }) } else { @@ -2000,26 +2000,32 @@ where tactic } -fn last_arg_shape( - context: &RewriteContext, +/// Returns a shape for the last argument which is going to be overflowed. +fn last_arg_shape( + lists: &[&T], items: &[ListItem], shape: Shape, args_max_width: usize, -) -> Option { - let overhead = items.iter().rev().skip(1).fold(0, |acc, i| { - acc + i.item.as_ref().map_or(0, |s| first_line_width(s)) +) -> Option +where + T: Rewrite + Spanned + ToExpr, +{ + let is_nested_call = lists + .iter() + .next() + .and_then(|item| item.to_expr()) + .map_or(false, is_nested_call); + if items.len() == 1 && !is_nested_call { + return Some(shape); + } + let offset = items.iter().rev().skip(1).fold(0, |acc, i| { + // 2 = ", " + acc + 2 + i.inner_as_ref().len() }); - let max_width = min(args_max_width, shape.width); - let arg_indent = if context.use_block_indent() { - shape.block().indent.block_unindent(context.config) - } else { - shape.block().indent - }; - Some(Shape { - width: max_width.checked_sub(overhead)?, - indent: arg_indent, - offset: 0, - }) + Shape { + width: min(args_max_width, shape.width), + ..shape + }.offset_left(offset) } fn rewrite_last_arg_with_overflow<'a, T>( @@ -2101,6 +2107,18 @@ pub fn can_be_overflowed_expr(context: &RewriteContext, expr: &ast::Expr, args_l } } +fn is_nested_call(expr: &ast::Expr) -> bool { + match expr.node { + ast::ExprKind::Call(..) | ast::ExprKind::Mac(..) => true, + ast::ExprKind::AddrOf(_, ref expr) + | ast::ExprKind::Box(ref expr) + | ast::ExprKind::Try(ref expr) + | ast::ExprKind::Unary(_, ref expr) + | ast::ExprKind::Cast(ref expr, _) => is_nested_call(expr), + _ => false, + } +} + pub fn wrap_args_with_parens( context: &RewriteContext, args_str: &str, From 8b53d7806ccb066e7041bcaccc61421a20a4829c Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:37:51 +0900 Subject: [PATCH 6/7] Cargo fmt --- src/bin/rustfmt.rs | 7 ++++--- src/chains.rs | 5 ++--- src/comment.rs | 4 +--- src/expr.rs | 8 ++------ src/items.rs | 20 ++++++-------------- src/visitor.rs | 9 ++------- 6 files changed, 17 insertions(+), 36 deletions(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index 41454c853ed0..001f2c8e0f70 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -87,9 +87,10 @@ impl CliOptions { if let Ok(write_mode) = WriteMode::from_str(write_mode) { options.write_mode = Some(write_mode); } else { - return Err(FmtError::from( - format!("Invalid write-mode: {}", write_mode), - )); + return Err(FmtError::from(format!( + "Invalid write-mode: {}", + write_mode + ))); } } diff --git a/src/chains.rs b/src/chains.rs index cb8e8d62988a..576d036d47be 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -147,9 +147,8 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - 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::>>()?; + let mut rewrites = iter.map(|(e, shape)| rewrite_chain_subexpr(e, total_span, context, shape)) + .collect::>>()?; // Total of all items excluding the last. let extend_last_subexpr = last_line_extendable(&parent_rewrite) && rewrites.is_empty(); diff --git a/src/comment.rs b/src/comment.rs index 2511e5df216a..2e3dadd20660 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -224,9 +224,7 @@ pub fn rewrite_comment( // we should stop now. let num_bare_lines = orig.lines() .map(|line| line.trim()) - .filter(|l| { - !(l.starts_with('*') || l.starts_with("//") || l.starts_with("/*")) - }) + .filter(|l| !(l.starts_with('*') || l.starts_with("//") || l.starts_with("/*"))) .count(); if num_bare_lines > 0 && !config.normalize_comments() { return Some(orig.to_owned()); diff --git a/src/expr.rs b/src/expr.rs index d51d355dbdd7..ad3bb053de91 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -292,9 +292,7 @@ pub fn format_expr( }; expr_rw - .and_then(|expr_str| { - recover_comment_removed(expr_str, expr.span, context) - }) + .and_then(|expr_str| recover_comment_removed(expr_str, expr.span, context)) .and_then(|expr_str| { let attrs = outer_attributes(&expr.attrs); let attrs_str = attrs.rewrite(context, shape)?; @@ -1925,9 +1923,7 @@ where config: context.config, }; - write_list(&item_vec, &fmt).map(|args_str| { - (tactic != DefinitiveListTactic::Vertical, args_str) - }) + write_list(&item_vec, &fmt).map(|args_str| (tactic != DefinitiveListTactic::Vertical, args_str)) } fn try_overflow_last_arg<'a, T>( diff --git a/src/items.rs b/src/items.rs index da110eef9340..6285811e79e2 100644 --- a/src/items.rs +++ b/src/items.rs @@ -817,8 +817,7 @@ fn format_impl_ref_and_type( IndentStyle::Visual => new_line_offset + trait_ref_overhead, IndentStyle::Block => new_line_offset, }; - result.push_str(&*self_ty - .rewrite(context, Shape::legacy(budget, type_offset))?); + result.push_str(&*self_ty.rewrite(context, Shape::legacy(budget, type_offset))?); Some(result) } else { unreachable!(); @@ -1578,9 +1577,7 @@ fn rewrite_static( lhs, &**expr, Shape::legacy(remaining_width, offset.block_only()), - ).and_then(|res| { - recover_comment_removed(res, static_parts.span, context) - }) + ).and_then(|res| recover_comment_removed(res, static_parts.span, context)) .map(|s| if s.ends_with(';') { s } else { s + ";" }) } else { Some(format!("{}{};", prefix, ty_str)) @@ -2096,18 +2093,14 @@ fn rewrite_args( generics_str_contains_newline: bool, ) -> Option { let mut arg_item_strs = args.iter() - .map(|arg| { - arg.rewrite(context, Shape::legacy(multi_line_budget, arg_indent)) - }) + .map(|arg| arg.rewrite(context, Shape::legacy(multi_line_budget, arg_indent))) .collect::>>()?; // Account for sugary self. // FIXME: the comment for the self argument is dropped. This is blocked // on rust issue #27522. let min_args = explicit_self - .and_then(|explicit_self| { - rewrite_explicit_self(explicit_self, args, context) - }) + .and_then(|explicit_self| rewrite_explicit_self(explicit_self, args, context)) .map_or(1, |self_str| { arg_item_strs[0] = self_str; 2 @@ -2326,9 +2319,8 @@ fn rewrite_generics( ) -> Option { let g_shape = generics_shape_from_config(context.config, shape, 0)?; let one_line_width = shape.width.checked_sub(2).unwrap_or(0); - rewrite_generics_inner(context, generics, g_shape, one_line_width, span).or_else(|| { - rewrite_generics_inner(context, generics, g_shape, 0, span) - }) + rewrite_generics_inner(context, generics, g_shape, one_line_width, span) + .or_else(|| rewrite_generics_inner(context, generics, g_shape, 0, span)) } fn rewrite_generics_inner( diff --git a/src/visitor.rs b/src/visitor.rs index 79bade0cc20b..00d62aeeff70 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -109,9 +109,7 @@ impl<'a> FmtVisitor<'a> { if self.config.remove_blank_lines_at_start_or_end_of_block() { if let Some(first_stmt) = b.stmts.first() { let attr_lo = inner_attrs - .and_then(|attrs| { - inner_attributes(attrs).first().map(|attr| attr.span.lo()) - }) + .and_then(|attrs| inner_attributes(attrs).first().map(|attr| attr.span.lo())) .or_else(|| { // Attributes for an item in a statement position // do not belong to the statement. (rust-lang/rust#34459) @@ -872,10 +870,7 @@ fn rewrite_first_group_attrs( for derive in derives { derive_args.append(&mut get_derive_args(context, derive)?); } - return Some(( - derives.len(), - format_derive(context, &derive_args, shape)?, - )); + return Some((derives.len(), format_derive(context, &derive_args, shape)?)); } } // Rewrite the first attribute. From a6d94b9842fad07c62063851fb154e3ebbe35484 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 29 Nov 2017 17:37:56 +0900 Subject: [PATCH 7/7] Update tests --- tests/target/chains-visual.rs | 20 ++++++++++---------- tests/target/configs-fn_call_indent-block.rs | 6 +++--- tests/target/issue-913.rs | 6 +++--- tests/target/match.rs | 12 ++++++------ tests/target/pattern.rs | 7 ++++--- 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/tests/target/chains-visual.rs b/tests/target/chains-visual.rs index abb81d8119c0..507f91113cc7 100644 --- a/tests/target/chains-visual.rs +++ b/tests/target/chains-visual.rs @@ -28,13 +28,13 @@ fn main() { }); some_fuuuuuuuuunction().method_call_a(aaaaa, bbbbb, |c| { - let x = c; - x - }) + let x = c; + x + }) .method_call_b(aaaaa, bbbbb, |c| { - let x = c; - x - }); + let x = c; + x + }); fffffffffffffffffffffffffffffffffff(a, { SCRIPT_TASK_ROOT.with(|root| { @@ -42,10 +42,10 @@ fn main() { }); }); - let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum = xxxxxxx.map(|x| x + 5) - .map(|x| x / 2) - .fold(0, - |acc, x| acc + x); + let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum = + xxxxxxx.map(|x| x + 5) + .map(|x| x / 2) + .fold(0, |acc, x| acc + x); aaaaaaaaaaaaaaaa.map(|x| { x += 1; diff --git a/tests/target/configs-fn_call_indent-block.rs b/tests/target/configs-fn_call_indent-block.rs index 905d34a09780..303d6419a657 100644 --- a/tests/target/configs-fn_call_indent-block.rs +++ b/tests/target/configs-fn_call_indent-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(), + ))); // chain let x = yooooooooooooo diff --git a/tests/target/issue-913.rs b/tests/target/issue-913.rs index 1bfd1cd00413..2158b70a46ff 100644 --- a/tests/target/issue-913.rs +++ b/tests/target/issue-913.rs @@ -10,9 +10,9 @@ mod client { }; let next_state = match self.state { - State::V5( - v5::State::Command(v5::comand::State::WriteVersion(ref mut response)), - ) => { + State::V5(v5::State::Command(v5::comand::State::WriteVersion( + ref mut response, + ))) => { // The pattern cannot be formatted in a way that the match stays // within the column limit. The rewrite should therefore be // skipped. diff --git a/tests/target/match.rs b/tests/target/match.rs index a14aadf0862c..83ee4f97b73e 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -255,12 +255,12 @@ fn issue507() { fn issue508() { match s.type_id() { - Some( - NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLCanvasElement)), - ) => true, - Some( - NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLObjectElement)), - ) => s.has_object_data(), + Some(NodeTypeId::Element(ElementTypeId::HTMLElement( + HTMLElementTypeId::HTMLCanvasElement, + ))) => true, + Some(NodeTypeId::Element(ElementTypeId::HTMLElement( + HTMLElementTypeId::HTMLObjectElement, + ))) => s.has_object_data(), Some(NodeTypeId::Element(_)) => false, } } diff --git a/tests/target/pattern.rs b/tests/target/pattern.rs index 0287e423fa96..6c62affb848f 100644 --- a/tests/target/pattern.rs +++ b/tests/target/pattern.rs @@ -47,9 +47,10 @@ fn main() { impl<'a, 'b> ResolveGeneratedContentFragmentMutator<'a, 'b> { fn mutate_fragment(&mut self, fragment: &mut Fragment) { match **info { - GeneratedContentInfo::ContentItem( - ContentItem::Counter(ref counter_name, counter_style), - ) => {} + GeneratedContentInfo::ContentItem(ContentItem::Counter( + ref counter_name, + counter_style, + )) => {} } } }