From 14f416273b850e1551baed294388ff999748fee9 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:19:20 +0900 Subject: [PATCH 1/6] Add format_constness() and last_line_used_width() --- src/utils.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/utils.rs b/src/utils.rs index 47ce5e7fcbe..d0917721898 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -56,6 +56,14 @@ pub fn format_visibility(vis: &Visibility) -> Cow<'static, str> { } } +#[inline] +pub fn format_constness(constness: ast::Constness) -> &'static str { + match constness { + ast::Constness::Const => "const ", + ast::Constness::NotConst => "", + } +} + #[inline] pub fn format_defaultness(defaultness: ast::Defaultness) -> &'static str { match defaultness { @@ -107,6 +115,16 @@ pub fn last_line_width(s: &str) -> usize { } } +// The total used width of the last line. +#[inline] +pub fn last_line_used_width(s: &str, offset: usize) -> usize { + if s.contains('\n') { + last_line_width(s) + } else { + offset + s.len() + } +} + #[inline] pub fn trimmed_last_line_width(s: &str) -> usize { match s.rfind('\n') { From 6ab727e6fffc0e236932e4c55cd20f8f09f687b0 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:20:04 +0900 Subject: [PATCH 2/6] Set where_density default value to Density::Vertical --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 7ffe141e6c1..65292d60b26 100644 --- a/src/config.rs +++ b/src/config.rs @@ -539,7 +539,7 @@ create_config! { // 1. Should we at least try to put the where clause on the same line as the rest of the // function decl? // 2. Currently options `Tall` and `Vertical` produce the same output. - where_density: Density, Density::CompressedIfEmpty, "Density of a where clause"; + where_density: Density, Density::Vertical, "Density of a where clause"; where_layout: ListTactic, ListTactic::Vertical, "Element layout inside a where clause"; where_pred_indent: IndentStyle, IndentStyle::Visual, "Indentation style of a where predicate"; From c67f729205f4472cb688bf947041ccc98568e5d0 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:23:12 +0900 Subject: [PATCH 3/6] Refactoring --- src/items.rs | 60 ++++++++++++++++++++-------------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/src/items.rs b/src/items.rs index d8afbe29b99..5fb0cd8e2cb 100644 --- a/src/items.rs +++ b/src/items.rs @@ -26,8 +26,9 @@ use lists::{definitive_tactic, itemize_list, write_list, DefinitiveListTactic, L ListItem, ListTactic, Separator, SeparatorTactic}; use rewrite::{Rewrite, RewriteContext}; use types::join_bounds; -use utils::{colon_spaces, contains_skip, end_typaram, format_defaultness, format_mutability, - format_unsafety, format_visibility, last_line_width, mk_sp, semicolon_for_expr, +use utils::{colon_spaces, contains_skip, end_typaram, first_line_width, format_abi, + format_constness, format_defaultness, format_mutability, format_unsafety, + format_visibility, last_line_used_width, last_line_width, mk_sp, semicolon_for_expr, stmt_expr, trim_newlines, trimmed_last_line_width, wrap_str}; use vertical::rewrite_with_alignment; use visitor::FmtVisitor; @@ -835,11 +836,7 @@ fn rewrite_trait_ref( result_len: usize, ) -> Option { // 1 = space between generics and trait_ref - let used_space = 1 + polarity_str.len() + if generics_str.contains('\n') { - last_line_width(&generics_str) - } else { - result_len + generics_str.len() - }; + let used_space = 1 + polarity_str.len() + last_line_used_width(generics_str, result_len); let shape = Shape::indented(offset + used_space, context.config); if let Some(trait_ref_str) = trait_ref.rewrite(context, shape) { if !(retry && trait_ref_str.contains('\n')) { @@ -1229,11 +1226,7 @@ fn format_tuple_struct( if fields.is_empty() { // 3 = `();` - let used_width = if result.contains('\n') { - last_line_width(&result) + 3 - } else { - offset.width() + result.len() + 3 - }; + let used_width = last_line_used_width(&result, offset.width()) + 3; if used_width > context.config.max_width() { result.push('\n'); result.push_str(&offset @@ -1790,24 +1783,13 @@ fn rewrite_fn_base( let where_clause = &generics.where_clause; let mut result = String::with_capacity(1024); - // Vis unsafety abi. + // Vis defaultness constness unsafety abi. result.push_str(&*format_visibility(vis)); - - if let ast::Defaultness::Default = defaultness { - result.push_str("default "); - } - - if let ast::Constness::Const = constness { - result.push_str("const "); - } - - result.push_str(::utils::format_unsafety(unsafety)); - + result.push_str(format_defaultness(defaultness)); + result.push_str(format_constness(constness)); + result.push_str(format_unsafety(unsafety)); if abi != abi::Abi::Rust { - result.push_str(&::utils::format_abi( - abi, - context.config.force_explicit_abi(), - )); + result.push_str(&format_abi(abi, context.config.force_explicit_abi())); } // fn foo @@ -1822,9 +1804,17 @@ fn rewrite_fn_base( // 2 = `()` 2 }; - let shape = try_opt!( - Shape::indented(indent + last_line_width(&result), context.config).sub_width(overhead) - ); + let used_width = last_line_used_width(&result, indent.width()); + let one_line_budget = context + .config + .max_width() + .checked_sub(used_width + overhead) + .unwrap_or(0); + let shape = Shape { + width: one_line_budget, + indent: indent, + offset: used_width, + }; let g_span = mk_sp(span.lo, fd.output.span().lo); let generics_str = try_opt!(rewrite_generics(context, generics, shape, g_span)); result.push_str(&generics_str); @@ -2764,7 +2754,7 @@ fn format_generics( let budget = context .config .max_width() - .checked_sub(last_line_width(&result)) + .checked_sub(last_line_used_width(&result, offset.width())) .unwrap_or(0); let where_clause_str = try_opt!(rewrite_where_clause( context, @@ -2786,11 +2776,7 @@ fn format_generics( force_same_line_brace || trimmed_last_line_width(&result) == 1 || brace_style != BraceStyle::AlwaysNextLine }; - let total_used_width = if result.contains('\n') { - last_line_width(&result) - } else { - used_width + result.len() - }; + let total_used_width = last_line_used_width(&result, used_width); let remaining_budget = context .config .max_width() From ec6c2d6e997209a14bd954861fe1a35326d4251e Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:25:26 +0900 Subject: [PATCH 4/6] Refactor compute_budgets_for_args() --- src/items.rs | 104 +++++++++++++++++++++++++++------------------------ 1 file changed, 55 insertions(+), 49 deletions(-) diff --git a/src/items.rs b/src/items.rs index 5fb0cd8e2cb..4c3e650c36a 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1835,21 +1835,15 @@ fn rewrite_fn_base( let ret_str_len = if multi_line_ret_str { 0 } else { ret_str.len() }; // Args. - let (mut one_line_budget, mut multi_line_budget, mut arg_indent) = - try_opt!(compute_budgets_for_args( - context, - &result, - indent, - ret_str_len, - newline_brace, - has_braces, - )); - - if context.config.fn_args_layout() == IndentStyle::Block { - arg_indent = indent.block_indent(context.config); - // 1 = "," - multi_line_budget = context.config.max_width() - (arg_indent.width() + 1); - } + let (one_line_budget, multi_line_budget, mut arg_indent) = try_opt!(compute_budgets_for_args( + context, + &result, + indent, + ret_str_len, + newline_brace, + has_braces, + multi_line_ret_str, + )); debug!( "rewrite_fn_base: one_line_budget: {}, multi_line_budget: {}, arg_indent: {:?}", @@ -1885,10 +1879,6 @@ fn rewrite_fn_base( 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 @@ -1917,11 +1907,8 @@ fn rewrite_fn_base( generics_str.contains('\n'), )); - let multi_line_arg_str = - arg_str.contains('\n') || arg_str.chars().last().map_or(false, |c| c == ','); - let put_args_in_block = match context.config.fn_args_layout() { - IndentStyle::Block => multi_line_arg_str || generics_str.contains('\n'), + IndentStyle::Block => arg_str.contains('\n') || arg_str.len() > one_line_budget, _ => false, } && !fd.inputs.is_empty(); @@ -1936,6 +1923,12 @@ fn rewrite_fn_base( result.push(')'); } else { result.push_str(&arg_str); + let used_width = last_line_used_width(&result, indent.width()) + first_line_width(&ret_str); + // Put the closing brace on the next line if it overflows the max width. + // 1 = `)` + if fd.inputs.len() == 0 && used_width + 1 > context.config.max_width() { + result.push('\n'); + } if context.config.spaces_within_parens() && fd.inputs.len() > 0 { result.push(' ') } @@ -1954,15 +1947,16 @@ fn rewrite_fn_base( } // Return type. - if !ret_str.is_empty() { + if let ast::FunctionRetTy::Ty(..) = fd.output { let ret_should_indent = match context.config.fn_args_layout() { // If our args are block layout then we surely must have space. - IndentStyle::Block if put_args_in_block => false, + IndentStyle::Block if put_args_in_block || fd.inputs.len() == 0 => false, + _ if args_last_line_contains_comment => false, + _ if result.contains('\n') || multi_line_ret_str => true, _ => { - // If we've already gone multi-line, or the return type would push over the max - // width, then put the return type on a new line. With the +1 for the signature - // length an additional space between the closing parenthesis of the argument and - // the arrow '->' is considered. + // If the return type would push over the max width, then put the return type on + // a new line. With the +1 for the signature length an additional space between + // the closing parenthesis of the argument and the arrow '->' is considered. let mut sig_length = result.len() + indent.width() + ret_str_len + 1; // If there is no where clause, take into account the space after the return type @@ -1971,10 +1965,7 @@ fn rewrite_fn_base( sig_length += 2; } - let overlong_sig = sig_length > context.config.max_width(); - - (!args_last_line_contains_comment) && - (result.contains('\n') || multi_line_ret_str || overlong_sig) + sig_length > context.config.max_width() } }; let ret_indent = if ret_should_indent { @@ -2276,6 +2267,7 @@ fn compute_budgets_for_args( ret_str_len: usize, newline_brace: bool, has_braces: bool, + force_vertical_layout: bool, ) -> Option<((usize, usize, Indent))> { debug!( "compute_budgets_for_args {} {:?}, {}, {}", @@ -2285,7 +2277,7 @@ fn compute_budgets_for_args( newline_brace ); // Try keeping everything on the same line. - if !result.contains('\n') { + if !result.contains('\n') && !force_vertical_layout { // 2 = `()`, 3 = `() `, space is before ret_string. let overhead = if ret_str_len == 0 { 2 } else { 3 }; let mut used_space = indent.width() + result.len() + ret_str_len + overhead; @@ -2306,31 +2298,45 @@ fn compute_budgets_for_args( if one_line_budget > 0 { // 4 = "() {".len() - let multi_line_overhead = - indent.width() + result.len() + if newline_brace { 2 } else { 4 }; - let multi_line_budget = - try_opt!(context.config.max_width().checked_sub(multi_line_overhead)); + let (indent, multi_line_budget) = match context.config.fn_args_layout() { + IndentStyle::Block => { + let indent = indent.block_indent(context.config); + let budget = + try_opt!(context.config.max_width().checked_sub(indent.width() + 1)); + (indent, budget) + } + IndentStyle::Visual => { + let indent = indent + result.len() + 1; + let multi_line_overhead = + indent.width() + result.len() + if newline_brace { 2 } else { 4 }; + let budget = + try_opt!(context.config.max_width().checked_sub(multi_line_overhead)); + (indent, budget) + } + }; - return Some(( - one_line_budget, - multi_line_budget, - indent + result.len() + 1, - )); + return Some((one_line_budget, multi_line_budget, indent)); } } // Didn't work. we must force vertical layout and put args on a newline. let new_indent = indent.block_indent(context.config); - // Account for `)` and possibly ` {`. - let used_space = new_indent.width() + if ret_str_len == 0 { 1 } else { 3 }; + let used_space = match context.config.fn_args_layout() { + // 1 = `,` + IndentStyle::Block => new_indent.width() + 1, + // Account for `)` and possibly ` {`. + IndentStyle::Visual => new_indent.width() + if ret_str_len == 0 { 1 } else { 3 }, + }; let max_space = try_opt!(context.config.max_width().checked_sub(used_space)); Some((0, max_space, new_indent)) } -fn newline_for_brace(config: &Config, where_clause: &ast::WhereClause) -> bool { - match config.fn_brace_style() { - BraceStyle::AlwaysNextLine => true, - BraceStyle::SameLineWhere if !where_clause.predicates.is_empty() => true, +fn newline_for_brace(config: &Config, where_clause: &ast::WhereClause, has_body: bool) -> bool { + match (config.fn_brace_style(), config.where_density()) { + (BraceStyle::AlwaysNextLine, _) => true, + (_, Density::Compressed) if where_clause.predicates.len() == 1 => false, + (_, Density::CompressedIfEmpty) if where_clause.predicates.len() == 1 && !has_body => false, + (BraceStyle::SameLineWhere, _) if !where_clause.predicates.is_empty() => true, _ => false, } } From 659d325982c38346a57cd7a999b27b8f478092a0 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:26:22 +0900 Subject: [PATCH 5/6] Implement compressed where clause with Rfc style --- src/items.rs | 57 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/items.rs b/src/items.rs index 4c3e650c36a..f317081f586 100644 --- a/src/items.rs +++ b/src/items.rs @@ -259,12 +259,12 @@ impl<'a> FmtVisitor<'a> { span: Span, block: &ast::Block, ) -> Option { - let mut newline_brace = newline_for_brace(self.config, &generics.where_clause); let context = self.get_context(); let block_snippet = self.snippet(mk_sp(block.span.lo, block.span.hi)); let has_body = !block_snippet[1..block_snippet.len() - 1].trim().is_empty() || !context.config.fn_empty_single_line(); + let mut newline_brace = newline_for_brace(self.config, &generics.where_clause, has_body); let (mut result, force_newline_brace) = try_opt!(rewrite_fn_base( &context, @@ -591,6 +591,7 @@ pub fn format_impl( "{", false, last_line_width(&ref_and_type) == 1, + false, where_span_end, item.span, self_ty.span.hi, @@ -983,6 +984,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent) "{", false, trait_bound_str.is_empty() && last_line_width(&generics_str) == 1, + false, None, item.span, pos_before_where, @@ -1216,6 +1218,7 @@ fn format_tuple_struct( ";", true, false, + false, None, span, body_hi, @@ -1313,6 +1316,7 @@ pub fn rewrite_type_alias( "=", true, true, + false, Some(span.hi), span, generics.span.hi, @@ -2034,22 +2038,22 @@ fn rewrite_fn_base( } let should_compress_where = match context.config.where_density() { - Density::Compressed => !result.contains('\n') || put_args_in_block, + Density::Compressed => !result.contains('\n'), Density::CompressedIfEmpty => !has_body && !result.contains('\n'), _ => false, - } || (put_args_in_block && ret_str.is_empty()); + }; let pos_before_where = match fd.output { ast::FunctionRetTy::Default(..) => args_span.hi, ast::FunctionRetTy::Ty(ref ty) => ty.span.hi, }; + if where_clause.predicates.len() == 1 && should_compress_where { - let budget = try_opt!( - context - .config - .max_width() - .checked_sub(last_line_width(&result)) - ); + let budget = context + .config + .max_width() + .checked_sub(last_line_used_width(&result, indent.width())) + .unwrap_or(0); if let Some(where_clause_str) = rewrite_where_clause( context, where_clause, @@ -2057,22 +2061,16 @@ fn rewrite_fn_base( Shape::legacy(budget, indent), Density::Compressed, "{", - !has_braces, - put_args_in_block && ret_str.is_empty(), + true, + false, // Force where clause on the next line + true, // Compress where Some(span.hi), span, pos_before_where, ) { - if !where_clause_str.contains('\n') { - if last_line_width(&result) + where_clause_str.len() > context.config.max_width() { - result.push('\n'); - } - - result.push_str(&where_clause_str); - - force_new_line_for_brace |= last_line_contains_single_line_comment(&result); - return Some((result, force_new_line_for_brace)); - } + result.push_str(&where_clause_str); + force_new_line_for_brace |= last_line_contains_single_line_comment(&result); + return Some((result, force_new_line_for_brace)); } } @@ -2085,6 +2083,7 @@ fn rewrite_fn_base( "{", !has_braces, put_args_in_block && ret_str.is_empty(), + false, Some(span.hi), span, pos_before_where, @@ -2502,6 +2501,8 @@ fn rewrite_where_clause_rfc_style( suppress_comma: bool, // where clause can be kept on the current line. snuggle: bool, + // copmressed single where clause + compress_where: bool, span_end: Option, span: Span, span_end_before_where: BytePos, @@ -2568,14 +2569,21 @@ fn rewrite_where_clause_rfc_style( } else { "\n".to_owned() + &clause_shape.indent.to_string(context.config) }; + let clause_sep = if compress_where && comment_before.is_empty() && comment_after.is_empty() && + !preds_str.contains('\n') && 6 + preds_str.len() <= shape.width + { + String::from(" ") + } else { + format!("\n{}", clause_shape.indent.to_string(context.config)) + }; Some(format!( - "{}{}{}where{}{}\n{}{}", + "{}{}{}where{}{}{}{}", starting_newline, comment_before, newline_before_where, newline_after_where, comment_after, - clause_shape.indent.to_string(context.config), + clause_sep, preds_str )) } @@ -2589,6 +2597,7 @@ fn rewrite_where_clause( terminator: &str, suppress_comma: bool, snuggle: bool, + compress_where: bool, span_end: Option, span: Span, span_end_before_where: BytePos, @@ -2605,6 +2614,7 @@ fn rewrite_where_clause( terminator, suppress_comma, snuggle, + compress_where, span_end, span, span_end_before_where, @@ -2771,6 +2781,7 @@ fn format_generics( terminator, false, trimmed_last_line_width(&result) == 1, + false, Some(span.hi), span, generics.span.hi, From d2acd9970381a500e99dc2f51f98844a7b0c46cb Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Tue, 1 Aug 2017 22:26:37 +0900 Subject: [PATCH 6/6] Update tests --- tests/target/configs-where_density-compressed.rs | 7 ++----- .../configs-where_density-compressed_if_empty.rs | 3 +-- tests/target/fn-custom-4.rs | 4 +--- tests/target/fn_args_layout-block.rs | 12 ++++-------- tests/target/long-fn-1.rs | 7 +++++++ tests/target/multiple.rs | 6 ++---- 6 files changed, 17 insertions(+), 22 deletions(-) diff --git a/tests/target/configs-where_density-compressed.rs b/tests/target/configs-where_density-compressed.rs index 215c10c2d7a..deddbee459f 100644 --- a/tests/target/configs-where_density-compressed.rs +++ b/tests/target/configs-where_density-compressed.rs @@ -3,13 +3,10 @@ trait Lorem { fn ipsum(dolor: Dolor) -> Sit - where - Dolor: Eq; + where Dolor: Eq; fn ipsum(dolor: Dolor) -> Sit - where - Dolor: Eq, - { + where Dolor: Eq { // body } } diff --git a/tests/target/configs-where_density-compressed_if_empty.rs b/tests/target/configs-where_density-compressed_if_empty.rs index 2a5da551f37..45f22eb602c 100644 --- a/tests/target/configs-where_density-compressed_if_empty.rs +++ b/tests/target/configs-where_density-compressed_if_empty.rs @@ -3,8 +3,7 @@ trait Lorem { fn ipsum(dolor: Dolor) -> Sit - where - Dolor: Eq; + where Dolor: Eq; fn ipsum(dolor: Dolor) -> Sit where diff --git a/tests/target/fn-custom-4.rs b/tests/target/fn-custom-4.rs index c044becc782..297aae5123b 100644 --- a/tests/target/fn-custom-4.rs +++ b/tests/target/fn-custom-4.rs @@ -13,9 +13,7 @@ where } fn qux() -where - X: TTTTTTTTTTTTTTTTTTTTTTTTTTTT, -{ +where X: TTTTTTTTTTTTTTTTTTTTTTTTTTTT { baz(); } diff --git a/tests/target/fn_args_layout-block.rs b/tests/target/fn_args_layout-block.rs index d61fc0b9e7b..c8847768e13 100644 --- a/tests/target/fn_args_layout-block.rs +++ b/tests/target/fn_args_layout-block.rs @@ -86,14 +86,12 @@ where { } -fn foo() - -> ( +fn foo() -> ( Loooooooooooooooooooooong, Reeeeeeeeeeeeeeeeeeeeeeeeturn, iiiiiiiiis, Looooooooooooooooong, -) -{ +) { foo(); } @@ -136,13 +134,11 @@ fn foo ( +fn foo() -> ( Looooooooooooooooooooooooooong, Reeeeeeeeeeeeeeeeeeeeeeeeeeeeeturn, iiiiiiiiiiiiiis, Loooooooooooooooooooooong, -) -{ +) { foo(); } diff --git a/tests/target/long-fn-1.rs b/tests/target/long-fn-1.rs index f94431965c2..c3d44de7c69 100644 --- a/tests/target/long-fn-1.rs +++ b/tests/target/long-fn-1.rs @@ -12,3 +12,10 @@ impl Foo { fn some_inpu(&mut self, input: Input, input_path: Option) -> (Input, Option) { } } + +// #1843 +#[allow(non_snake_case)] +pub extern "C" fn Java_com_exonum_binding_storage_indices_ValueSetIndexProxy_nativeContainsByHash( +) -> bool { + false +} diff --git a/tests/target/multiple.rs b/tests/target/multiple.rs index a7d7957b478..6e61c82e5f6 100644 --- a/tests/target/multiple.rs +++ b/tests/target/multiple.rs @@ -151,16 +151,14 @@ fn main() { let s = expand(a, b); } -fn deconstruct() - -> ( +fn deconstruct() -> ( SocketAddr, Method, Headers, RequestUri, HttpVersion, AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, -) -{ +) { } fn deconstruct(