From 7aad9199e612fbb9c6a4737f3f2dedf359178833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Tue, 9 Jul 2019 15:57:00 +0200 Subject: [PATCH 01/10] use the lists module for formatting the generic bounds Fix how to extract post comments in a list with separators at the front. Fix extraction of a comment between the type bounds and the opening curly bracket. Support explicit padding passed through the separator when writing a list. --- src/comment.rs | 8 +-- src/items.rs | 28 ++++---- src/lists.rs | 142 +++++++++++++++++++++++++++++-------- src/types.rs | 81 +++++++++++++-------- tests/source/issue-3669.rs | 49 +++++++++++++ tests/target/issue-3669.rs | 51 +++++++++++++ tests/target/trait.rs | 7 +- 7 files changed, 290 insertions(+), 76 deletions(-) create mode 100644 tests/source/issue-3669.rs create mode 100644 tests/target/issue-3669.rs diff --git a/src/comment.rs b/src/comment.rs index 6222c34b1a2..3adeeea0fc6 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -1789,7 +1789,7 @@ mod test { * test3 */"#, false, - Shape::legacy(100, Indent::new(0, 0)), + Shape::legacy(100, Indent::empty()), &wrap_normalize_config).unwrap(); assert_eq!("/// test1\n/// test2\n// test3", comment); @@ -1798,7 +1798,7 @@ mod test { // test2"#, false, - Shape::legacy(100, Indent::new(0, 0)), + Shape::legacy(100, Indent::empty()), &wrap_normalize_config).unwrap(); assert_eq!("// test1\n\n// test2", comment); @@ -1807,7 +1807,7 @@ mod test { //@ test2"#, false, - Shape::legacy(100, Indent::new(0, 0)), + Shape::legacy(100, Indent::empty()), &wrap_normalize_config).unwrap(); assert_eq!("//@ test1\n\n//@ test2", comment); @@ -1819,7 +1819,7 @@ mod test { another bare line! */"#, false, - Shape::legacy(100, Indent::new(0, 0)), + Shape::legacy(100, Indent::empty()), &wrap_config).unwrap(); assert_eq!("// test1\n/*\n a bare line!\n\n another bare line!\n*/", comment); } diff --git a/src/items.rs b/src/items.rs index 73b752dfe78..85fed8de0a0 100644 --- a/src/items.rs +++ b/src/items.rs @@ -994,20 +994,20 @@ pub(crate) fn format_trait( rewrite_generics(context, rewrite_ident(context, item.ident), generics, shape)?; result.push_str(&generics_str); - // FIXME(#2055): rustfmt fails to format when there are comments between trait bounds. if !generic_bounds.is_empty() { - let ident_hi = context - .snippet_provider - .span_after(item.span, &item.ident.as_str()); - let bound_hi = generic_bounds.last().unwrap().span().hi(); - let snippet = context.snippet(mk_sp(ident_hi, bound_hi)); - if contains_comment(snippet) { - return None; - } - + let comment_span = mk_sp(generics.span.hi(), generic_bounds[0].span().lo()); + let after_colon = context.snippet_provider.span_after(comment_span, ":"); + let comment = recover_missing_comment_in_span( + mk_sp(after_colon, comment_span.hi()), + shape, + context, + // 1 = ":" + last_line_width(&result) + 1, + ) + .unwrap_or_default(); result = rewrite_assign_rhs_with( context, - result + ":", + format!("{}:{}", result, comment), generic_bounds, shape, RhsTactics::ForceNextLineWithoutIndent, @@ -1052,7 +1052,11 @@ pub(crate) fn format_trait( if let Some(lo) = item_snippet.find('/') { // 1 = `{` let comment_hi = body_lo - BytePos(1); - let comment_lo = item.span.lo() + BytePos(lo as u32); + let comment_lo = if generic_bounds.is_empty() { + item.span.lo() + BytePos(lo as u32) + } else { + generic_bounds[generic_bounds.len() - 1].span().hi() + }; if comment_lo < comment_hi { match recover_missing_comment_in_span( mk_sp(comment_lo, comment_hi), diff --git a/src/lists.rs b/src/lists.rs index ac83d7082c8..32b1c666e4c 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -5,7 +5,7 @@ use std::iter::Peekable; use syntax::source_map::BytePos; -use crate::comment::{find_comment_end, rewrite_comment, FindUncommented}; +use crate::comment::{find_comment_end, is_last_comment_block, rewrite_comment, FindUncommented}; use crate::config::lists::*; use crate::config::{Config, IndentStyle}; use crate::rewrite::RewriteContext; @@ -32,6 +32,8 @@ pub(crate) struct ListFormatting<'a> { // Whether comments should be visually aligned. align_comments: bool, config: &'a Config, + item_on_newline: Vec, + padding: bool, } impl<'a> ListFormatting<'a> { @@ -47,9 +49,21 @@ impl<'a> ListFormatting<'a> { nested: false, align_comments: true, config, + item_on_newline: Vec::new(), + padding: true, } } + pub(crate) fn padding(mut self, padding: bool) -> Self { + self.padding = padding; + self + } + + pub(crate) fn item_on_newline(mut self, item_on_newline: Vec) -> Self { + self.item_on_newline = item_on_newline; + self + } + pub(crate) fn tactic(mut self, tactic: DefinitiveListTactic) -> Self { self.tactic = tactic; self @@ -281,6 +295,7 @@ where let mut prev_item_is_nested_import = false; let mut line_len = 0; + let mut first_item_on_line = true; let indent_str = &formatting.shape.indent.to_string(formatting.config); while let Some((i, item)) = iter.next() { let item = item.as_ref(); @@ -310,7 +325,7 @@ where } match tactic { - DefinitiveListTactic::Horizontal if !first => { + DefinitiveListTactic::Horizontal if !first && formatting.padding => { result.push(' '); } DefinitiveListTactic::SpecialMacro(num_args_before) => { @@ -328,6 +343,7 @@ where DefinitiveListTactic::Vertical if !first && !inner_item.is_empty() && !result.is_empty() => { + first_item_on_line = true; result.push('\n'); result.push_str(indent_str); } @@ -335,18 +351,22 @@ where let total_width = total_item_width(item) + item_sep_len; // 1 is space between separator and item. - if (line_len > 0 && line_len + 1 + total_width > formatting.shape.width) - || prev_item_had_post_comment - || (formatting.nested - && (prev_item_is_nested_import || (!first && inner_item.contains("::")))) + if (!formatting.item_on_newline.is_empty() && formatting.item_on_newline[i]) + || formatting.item_on_newline.is_empty() + && ((line_len > 0 && line_len + 1 + total_width > formatting.shape.width) + || prev_item_had_post_comment + || (formatting.nested + && (prev_item_is_nested_import + || (!first && inner_item.contains("::"))))) { result.push('\n'); result.push_str(indent_str); line_len = 0; + first_item_on_line = true; if formatting.ends_with_newline { trailing_separator = true; } - } else if line_len > 0 { + } else if formatting.padding && line_len > 0 { result.push(' '); line_len += 1; } @@ -399,8 +419,14 @@ where } if separate && sep_place.is_front() && !first { - result.push_str(formatting.separator.trim()); - result.push(' '); + if formatting.padding { + result.push_str(formatting.separator.trim()); + result.push(' '); + } else if first_item_on_line { + result.push_str(formatting.separator.trim_start()); + } else { + result.push_str(formatting.separator); + } } result.push_str(inner_item); @@ -414,7 +440,12 @@ where formatting.config, )?; - result.push(' '); + if is_last_comment_block(&formatted_comment) { + result.push(' '); + } else { + result.push('\n'); + result.push_str(indent_str); + } result.push_str(&formatted_comment); } @@ -512,6 +543,7 @@ where result.push('\n'); } + first_item_on_line = false; prev_item_had_post_comment = item.post_comment.is_some(); prev_item_is_nested_import = inner_item.contains("::"); } @@ -599,25 +631,20 @@ pub(crate) fn extract_pre_comment(pre_snippet: &str) -> (Option, ListIte } } -pub(crate) fn extract_post_comment( - post_snippet: &str, - comment_end: usize, - separator: &str, -) -> Option { +fn extract_post_comment(post_snippet: &str, comment_end: usize, separator: &str) -> Option { let white_space: &[_] = &[' ', '\t']; // Cleanup post-comment: strip separators and whitespace. let post_snippet = post_snippet[..comment_end].trim(); let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') { post_snippet[1..].trim_matches(white_space) + } else if post_snippet.ends_with(separator) { + // the separator is in front of the next item + post_snippet[..post_snippet.len() - separator.len()].trim_matches(white_space) } else if post_snippet.starts_with(separator) { post_snippet[separator.len()..].trim_matches(white_space) - } - // not comment or over two lines - else if post_snippet.ends_with(',') - && (!post_snippet.trim().starts_with("//") || post_snippet.trim().contains('\n')) - { - post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space) + } else if post_snippet.ends_with(',') && !post_snippet.trim_start().starts_with("//") { + post_snippet[..post_snippet.len() - 1].trim_matches(white_space) } else { post_snippet }; @@ -633,12 +660,7 @@ pub(crate) fn extract_post_comment( } } -pub(crate) fn get_comment_end( - post_snippet: &str, - separator: &str, - terminator: &str, - is_last: bool, -) -> usize { +fn get_comment_end(post_snippet: &str, separator: &str, terminator: &str, is_last: bool) -> usize { if is_last { return post_snippet .find_uncommented(terminator) @@ -686,7 +708,7 @@ pub(crate) fn get_comment_end( // Account for extra whitespace between items. This is fiddly // because of the way we divide pre- and post- comments. -pub(crate) fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool { +fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool { if post_snippet.is_empty() || comment_end == 0 { return false; } @@ -767,7 +789,17 @@ where } #[allow(clippy::too_many_arguments)] -// Creates an iterator over a list's items with associated comments. +/// Creates an iterator over a list's items with associated comments. +/// +/// - inner is the iterator over items +/// - terminator is a string that closes the list, used to get comments after the last item +/// - separator is a string that separates the items +/// - get_lo is a closure to get the lower bound of an item's span +/// - get_hi is a closure to get the upper bound of an item's span +/// - get_item_string is a closure to get the rewritten item as a string +/// - prev_span_end is the BytePos before the first item +/// - next_span_start is the BytePos after the last item +/// - leave_last is a boolean whether or not to rewrite the last item pub(crate) fn itemize_list<'a, T, I, F1, F2, F3>( snippet_provider: &'a SnippetProvider<'_>, inner: I, @@ -918,5 +950,57 @@ pub(crate) fn struct_lit_formatting<'a>( nested: false, align_comments: true, config: context.config, + item_on_newline: Vec::new(), + padding: true, + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_extract_post_comment() { + let data = [ + ( + ", // a comment", + ", // a comment".len(), + ",", + "// a comment", + ), + ( + ": // a comment", + ": // a comment".len(), + ":", + "// a comment", + ), + ( + "// a comment\n +", + "// a comment\n +".len(), + "+", + "// a comment\n", + ), + ( + "+ // a comment\n ", + "+ // a comment\n ".len(), + "+", + "// a comment", + ), + ( + "/* a comment */ ,", + "/* a comment */ ,".len(), + "+", + "/* a comment */", + ), + ]; + + for (i, (post_snippet, comment_end, separator, expected)) in data.iter().enumerate() { + assert_eq!( + extract_post_comment(post_snippet, *comment_end, separator), + Some(expected.to_string()), + "Failed on input {}", + i + ); + } } } diff --git a/src/types.rs b/src/types.rs index d09e8c01e38..2423bab8264 100644 --- a/src/types.rs +++ b/src/types.rs @@ -802,52 +802,77 @@ fn join_bounds( debug_assert!(!items.is_empty()); // Try to join types in a single line + let type_strs: Vec<_> = itemize_list( + context.snippet_provider, + items.iter(), + "", + "+", + |item| item.span().lo(), + |item| item.span().hi(), + |item| item.rewrite(context, shape), + items[0].span().lo(), + items[items.len() - 1].span().hi(), + false, + ) + .collect(); let joiner = match context.config.type_punctuation_density() { TypeDensity::Compressed => "+", TypeDensity::Wide => " + ", }; - let type_strs = items - .iter() - .map(|item| item.rewrite(context, shape)) - .collect::>>()?; - let result = type_strs.join(joiner); + let fmt = ListFormatting::new(shape, context.config) + .padding(false) + .separator(joiner) + .tactic(DefinitiveListTactic::Horizontal); + let result = write_list(&type_strs, &fmt)?; + if items.len() <= 1 || (!result.contains('\n') && result.len() <= shape.width) { return Some(result); } // We need to use multiple lines. - let (type_strs, offset) = if need_indent { + let (type_strs, shape) = if need_indent { // Rewrite with additional indentation. let nested_shape = shape.block_indent(context.config.tab_spaces()); - let type_strs = items - .iter() - .map(|item| item.rewrite(context, nested_shape)) - .collect::>>()?; - (type_strs, nested_shape.indent) + let type_strs: Vec<_> = itemize_list( + context.snippet_provider, + items.iter(), + "", + "+", + |item| item.span().lo(), + |item| item.span().hi(), + |item| item.rewrite(context, nested_shape), + items[0].span().lo(), + items[items.len() - 1].span().hi(), + false, + ) + .collect(); + (type_strs, nested_shape) } else { - (type_strs, shape.indent) + (type_strs, shape) }; - let is_bound_extendable = |s: &str, b: &ast::GenericBound| match b { ast::GenericBound::Outlives(..) => true, ast::GenericBound::Trait(..) => last_line_extendable(s), }; - let mut result = String::with_capacity(128); - result.push_str(&type_strs[0]); - let mut can_be_put_on_the_same_line = is_bound_extendable(&result, &items[0]); let generic_bounds_in_order = is_generic_bounds_in_order(items); - for (bound, bound_str) in items[1..].iter().zip(type_strs[1..].iter()) { - if generic_bounds_in_order && can_be_put_on_the_same_line { - result.push_str(joiner); - } else { - result.push_str(&offset.to_string_with_newline(context.config)); - result.push_str("+ "); - } - result.push_str(bound_str); - can_be_put_on_the_same_line = is_bound_extendable(bound_str, bound); - } - - Some(result) + let mut item_on_newline = if generic_bounds_in_order { + items + .iter() + .zip(type_strs.iter()) + .map(|(bound, bound_str)| !is_bound_extendable(bound_str.inner_as_ref(), bound)) + .collect::>() + } else { + vec![true; items.len()] + }; + item_on_newline.insert(0, false); + let fmt = ListFormatting::new(shape, context.config) + .padding(false) + .item_on_newline(item_on_newline) + .trailing_separator(SeparatorTactic::Always) + .separator_place(SeparatorPlace::Front) + .separator(joiner) + .tactic(DefinitiveListTactic::Mixed); + write_list(&type_strs, &fmt) } pub(crate) fn can_be_overflowed_type( diff --git a/tests/source/issue-3669.rs b/tests/source/issue-3669.rs new file mode 100644 index 00000000000..fe9582ba380 --- /dev/null +++ b/tests/source/issue-3669.rs @@ -0,0 +1,49 @@ +pub trait PCG: self::sealed::Sealed // comment1 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + + Shl // Note(Evrey): Because Rust is drunk. 3 ++ BitAnd // Note(Evrey): Because Rust is drunk. 4 ++ BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Foo: self::sealed::Sealed + + Sized + + Eq + + Hash + + Debug + + Clone + + Default + + Serialize + + for<'a> Deserialize<'a> +{ + type DoubleState: Copy + + ShrAssign + + Shl ++ BitAnd ++ BitOrAssign + + Sub + + Into + + Debug + + Eq + + Hash + + Default + + Serialize + + for<'a> Deserialize<'a>; +} diff --git a/tests/target/issue-3669.rs b/tests/target/issue-3669.rs new file mode 100644 index 00000000000..2bc62c189ca --- /dev/null +++ b/tests/target/issue-3669.rs @@ -0,0 +1,51 @@ +pub trait PCG: + self::sealed::Sealed // comment1 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + + Shl // Note(Evrey): Because Rust is drunk. 3 + + BitAnd // Note(Evrey): Because Rust is drunk. 4 + + BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Foo: + self::sealed::Sealed + + Sized + + Eq + + Hash + + Debug + + Clone + + Default + + Serialize + + for<'a> Deserialize<'a> +{ + type DoubleState: Copy + + ShrAssign + + Shl + + BitAnd + + BitOrAssign + + Sub + + Into + + Debug + + Eq + + Hash + + Default + + Serialize + + for<'a> Deserialize<'a>; +} diff --git a/tests/target/trait.rs b/tests/target/trait.rs index 620046a71b2..57e09759c28 100644 --- a/tests/target/trait.rs +++ b/tests/target/trait.rs @@ -87,10 +87,11 @@ trait Y // comment // #2055 pub trait Foo: // A and C -A + C -// and B + A + + C // and B + B -{} +{ +} // #2158 trait Foo { From ca9ffccf64330b5916f3d2604ef3354fe1f11f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Fri, 12 Jul 2019 15:23:28 +0200 Subject: [PATCH 02/10] add tests with type_punctuation_density set to Compressed and a list of types spanning multiple lines --- tests/source/types_compressed.rs | 7 +++++ tests/target/types_compressed.rs | 44 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 tests/source/types_compressed.rs create mode 100644 tests/target/types_compressed.rs diff --git a/tests/source/types_compressed.rs b/tests/source/types_compressed.rs new file mode 100644 index 00000000000..4254842d422 --- /dev/null +++ b/tests/source/types_compressed.rs @@ -0,0 +1,7 @@ +// rustfmt-type_punctuation_density: Compressed + +pub fn do_something<'a, T: Trait1 + Trait2 + 'a>( + &fooo: u32, +) -> impl Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + 'a + 'a + 'a + 'a + 'a + { +} diff --git a/tests/target/types_compressed.rs b/tests/target/types_compressed.rs new file mode 100644 index 00000000000..d0efdc71a77 --- /dev/null +++ b/tests/target/types_compressed.rs @@ -0,0 +1,44 @@ +// rustfmt-type_punctuation_density: Compressed + +pub fn do_something<'a, T: Trait1+Trait2+'a>( + &fooo: u32, +) -> impl Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +'a+'a+'a+'a+'a { +} From 2f9357bae947bf1a4452846498c37e8e2081d4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Wed, 17 Jul 2019 01:54:18 +0200 Subject: [PATCH 03/10] clarified logic for formatting the type bounds as a list --- src/lists.rs | 46 +++++++++++++++++++++++++++++----------------- src/types.rs | 32 ++++++++++++++++++-------------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index 32b1c666e4c..efa0a355388 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -22,17 +22,19 @@ pub(crate) struct ListFormatting<'a> { trailing_separator: SeparatorTactic, separator_place: SeparatorPlace, shape: Shape, - // Non-expressions, e.g., items, will have a new line at the end of the list. - // Important for comment styles. + /// Non-expressions, e.g., items, will have a new line at the end of the list. + /// Important for comment styles. ends_with_newline: bool, - // Remove newlines between list elements for expressions. + /// Remove newlines between list elements for expressions. preserve_newline: bool, - // Nested import lists get some special handling for the "Mixed" list type + /// Nested import lists get some special handling for the "Mixed" list type. nested: bool, - // Whether comments should be visually aligned. + /// Whether comments should be visually aligned. align_comments: bool, config: &'a Config, - item_on_newline: Vec, + /// The decision of putting an item on a newline is determined by the caller. + custom_list_tactic: Vec, + /// Whether whitespaces should be added around the separator. padding: bool, } @@ -49,7 +51,7 @@ impl<'a> ListFormatting<'a> { nested: false, align_comments: true, config, - item_on_newline: Vec::new(), + custom_list_tactic: Vec::new(), padding: true, } } @@ -59,8 +61,8 @@ impl<'a> ListFormatting<'a> { self } - pub(crate) fn item_on_newline(mut self, item_on_newline: Vec) -> Self { - self.item_on_newline = item_on_newline; + pub(crate) fn custom_list_tactic(mut self, custom_list_tactic: Vec) -> Self { + self.custom_list_tactic = custom_list_tactic; self } @@ -325,6 +327,19 @@ where } match tactic { + _ if !formatting.custom_list_tactic.is_empty() => { + if *formatting + .custom_list_tactic + .get(i) + .expect("invalid custom_list_tactic formatting option") + { + result.push('\n'); + result.push_str(indent_str); + first_item_on_line = true; + } else if formatting.padding && !first_item_on_line { + result.push(' '); + } + } DefinitiveListTactic::Horizontal if !first && formatting.padding => { result.push(' '); } @@ -351,13 +366,10 @@ where let total_width = total_item_width(item) + item_sep_len; // 1 is space between separator and item. - if (!formatting.item_on_newline.is_empty() && formatting.item_on_newline[i]) - || formatting.item_on_newline.is_empty() - && ((line_len > 0 && line_len + 1 + total_width > formatting.shape.width) - || prev_item_had_post_comment - || (formatting.nested - && (prev_item_is_nested_import - || (!first && inner_item.contains("::"))))) + if (line_len > 0 && line_len + 1 + total_width > formatting.shape.width) + || prev_item_had_post_comment + || (formatting.nested + && (prev_item_is_nested_import || (!first && inner_item.contains("::")))) { result.push('\n'); result.push_str(indent_str); @@ -950,7 +962,7 @@ pub(crate) fn struct_lit_formatting<'a>( nested: false, align_comments: true, config: context.config, - item_on_newline: Vec::new(), + custom_list_tactic: Vec::new(), padding: true, } } diff --git a/src/types.rs b/src/types.rs index 2423bab8264..3127a91a96b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -855,23 +855,27 @@ fn join_bounds( ast::GenericBound::Trait(..) => last_line_extendable(s), }; let generic_bounds_in_order = is_generic_bounds_in_order(items); - let mut item_on_newline = if generic_bounds_in_order { - items - .iter() - .zip(type_strs.iter()) - .map(|(bound, bound_str)| !is_bound_extendable(bound_str.inner_as_ref(), bound)) - .collect::>() - } else { - vec![true; items.len()] - }; - item_on_newline.insert(0, false); let fmt = ListFormatting::new(shape, context.config) .padding(false) - .item_on_newline(item_on_newline) - .trailing_separator(SeparatorTactic::Always) - .separator_place(SeparatorPlace::Front) .separator(joiner) - .tactic(DefinitiveListTactic::Mixed); + .trailing_separator(SeparatorTactic::Always) + .separator_place(SeparatorPlace::Front); + let fmt = if generic_bounds_in_order { + let custom_list_tactic = std::iter::once(false) // no newline before the first bound + .chain( + items + .iter() + .zip(type_strs.iter()) + .map(|(bound, bound_str)| { + // putting a newline before the current bound depends on the previous bound + !is_bound_extendable(bound_str.inner_as_ref(), bound) + }), + ) + .collect::>(); + fmt.custom_list_tactic(custom_list_tactic) + } else { + fmt.tactic(DefinitiveListTactic::Vertical) + }; write_list(&type_strs, &fmt) } From 3850fb1479bf5a1082d25d25e59066f9639b167b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Thu, 25 Jul 2019 00:50:19 +0200 Subject: [PATCH 04/10] trailing newlines in post_comments should be removed --- src/lists.rs | 19 +++++++++++---- tests/source/issue-3669.rs | 25 ++++++++++++++++++++ tests/target/issue-3669.rs | 48 +++++++++++++++++++++++++++++--------- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index efa0a355388..b95b7597799 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -644,19 +644,28 @@ pub(crate) fn extract_pre_comment(pre_snippet: &str) -> (Option, ListIte } fn extract_post_comment(post_snippet: &str, comment_end: usize, separator: &str) -> Option { + // leading newlines are important but not when they are trailing let white_space: &[_] = &[' ', '\t']; // Cleanup post-comment: strip separators and whitespace. let post_snippet = post_snippet[..comment_end].trim(); let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') { - post_snippet[1..].trim_matches(white_space) + post_snippet[1..] + .trim_matches(white_space) + .trim_end_matches('\n') } else if post_snippet.ends_with(separator) { // the separator is in front of the next item - post_snippet[..post_snippet.len() - separator.len()].trim_matches(white_space) + post_snippet[..post_snippet.len() - separator.len()] + .trim_matches(white_space) + .trim_end_matches('\n') } else if post_snippet.starts_with(separator) { - post_snippet[separator.len()..].trim_matches(white_space) + post_snippet[separator.len()..] + .trim_matches(white_space) + .trim_end_matches('\n') } else if post_snippet.ends_with(',') && !post_snippet.trim_start().starts_with("//") { - post_snippet[..post_snippet.len() - 1].trim_matches(white_space) + post_snippet[..post_snippet.len() - 1] + .trim_matches(white_space) + .trim_end_matches('\n') } else { post_snippet }; @@ -990,7 +999,7 @@ mod test { "// a comment\n +", "// a comment\n +".len(), "+", - "// a comment\n", + "// a comment", ), ( "+ // a comment\n ", diff --git a/tests/source/issue-3669.rs b/tests/source/issue-3669.rs index fe9582ba380..f9046c3d236 100644 --- a/tests/source/issue-3669.rs +++ b/tests/source/issue-3669.rs @@ -23,6 +23,31 @@ pub trait PCG: self::sealed::Sealed // comment1 + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 } +pub trait Bar: self::sealed::Sealed + // comment1 + Sized + // comment2 + Eq + // comment3 + Hash + // comment4 + Debug + // comment5 + Clone + // comment6 + Default + // comment7 + Serialize + // comment8 + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy + // Note(Evrey): Because Rust is drunk. 1 + ShrAssign + // Note(Evrey): Because Rust is drunk. 2 + Shl + // Note(Evrey): Because Rust is drunk. 3 +BitAnd + // Note(Evrey): Because Rust is drunk. 4 +BitOrAssign + // Note(Evrey): Because Rust is drunk. 5 + Sub + // Note(Evrey): Because Rust is drunk. 6 + Into + // Note(Evrey): Because Rust is drunk. 7 + Debug + // Note(Evrey): Because Rust is drunk. 8 + Eq + // Note(Evrey): Because Rust is drunk. 9 + Hash + // Note(Evrey): Because Rust is drunk. 10 + Default + // Note(Evrey): Because Rust is drunk. 11 + Serialize + // Note(Evrey): Because Rust is drunk. 12 + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + pub trait Foo: self::sealed::Sealed + Sized + Eq diff --git a/tests/target/issue-3669.rs b/tests/target/issue-3669.rs index 2bc62c189ca..32418dae585 100644 --- a/tests/target/issue-3669.rs +++ b/tests/target/issue-3669.rs @@ -9,18 +9,44 @@ pub trait PCG: + Serialize // comment8 + for<'a> Deserialize<'a> // comment9 { - type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 - + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + Shl // Note(Evrey): Because Rust is drunk. 3 - + BitAnd // Note(Evrey): Because Rust is drunk. 4 - + BitOrAssign // Note(Evrey): Because Rust is drunk. 5 - + Sub // Note(Evrey): Because Rust is drunk. 6 - + Into // Note(Evrey): Because Rust is drunk. 7 - + Debug // Note(Evrey): Because Rust is drunk. 8 - + Eq // Note(Evrey): Because Rust is drunk. 9 - + Hash // Note(Evrey): Because Rust is drunk. 10 - + Default // Note(Evrey): Because Rust is drunk. 11 - + Serialize // Note(Evrey): Because Rust is drunk. 12 + + BitAnd // Note(Evrey): Because Rust is drunk. 4 + + BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Bar: + self::sealed::Sealed // comment1 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + + Shl // Note(Evrey): Because Rust is drunk. 3 + + BitAnd // Note(Evrey): Because Rust is drunk. 4 + + BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 } From 58ad27cfd744e5f6f305b0d8e83dbefc0cb466cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Tue, 30 Jul 2019 16:10:03 +0200 Subject: [PATCH 05/10] take into consideration the length taken by the separator when aligning post comments --- src/lists.rs | 356 ++++++++++++++++++++++++++++++++++--- tests/target/issue-3669.rs | 32 ++-- 2 files changed, 344 insertions(+), 44 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index b95b7597799..b3004d98de8 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -275,10 +275,9 @@ where } } -// Format a list of commented items into a string. -pub(crate) fn write_list(items: I, formatting: &ListFormatting<'_>) -> Option +/// Format a list of commented items into a string. +pub(crate) fn write_list(items: &[T], formatting: &ListFormatting<'_>) -> Option where - I: IntoIterator + Clone, T: AsRef, { let tactic = formatting.tactic; @@ -288,14 +287,38 @@ where // will be a trailing separator. let mut trailing_separator = formatting.needs_trailing_separator(); let mut result = String::with_capacity(128); - let cloned_items = items.clone(); - let mut iter = items.into_iter().enumerate().peekable(); + let mut iter = items.iter().enumerate().peekable(); let mut item_max_width: Option = None; let sep_place = SeparatorPlace::from_tactic(formatting.separator_place, tactic, formatting.separator); let mut prev_item_had_post_comment = false; let mut prev_item_is_nested_import = false; + let first_item_len = if let Some(item) = items.iter().nth(0) { + item.as_ref().inner_as_ref().len() + } else { + 0 + }; + let last_item_len = if let Some(item) = items.iter().last() { + item.as_ref().inner_as_ref().len() + } else { + 0 + }; + let middle_item_same_len_first = items + .iter() + // all but first item + .skip(1) + // check if any of the intermediate items share the size of the first item + .map(|item| item.as_ref().inner_as_ref().len()) + .any(|len| len == first_item_len); + let middle_item_same_len_last = items + .iter() + // all but last item + .take(items.len().saturating_sub(1)) + // check if any of the intermediate items share the size of the last item + .map(|item| item.as_ref().inner_as_ref().len()) + .any(|len| len == last_item_len); + let mut line_len = 0; let mut first_item_on_line = true; let indent_str = &formatting.shape.indent.to_string(formatting.config); @@ -472,7 +495,7 @@ where let rewrite_post_comment = |item_max_width: &mut Option| { if item_max_width.is_none() && !last && !inner_item.contains('\n') { *item_max_width = Some(max_width_of_item_with_post_comment( - &cloned_items, + &items, i, overhead, formatting.config.max_width(), @@ -507,8 +530,19 @@ where if !starts_with_newline(comment) { if formatting.align_comments { - let mut comment_alignment = - post_comment_alignment(item_max_width, inner_item.len()); + let mut comment_alignment = post_comment_alignment( + item_max_width, + inner_item.len(), + formatting, + sep_place, + first, + middle_item_same_len_first, + item_max_width.unwrap_or_default() == first_item_len, + last, + middle_item_same_len_last, + item_max_width.unwrap_or_default() == last_item_len, + separate, + ); if first_line_width(&formatted_comment) + last_line_width(&result) + comment_alignment @@ -517,21 +551,22 @@ where { item_max_width = None; formatted_comment = rewrite_post_comment(&mut item_max_width)?; - comment_alignment = - post_comment_alignment(item_max_width, inner_item.len()); - } - for _ in 0..=comment_alignment { - result.push(' '); + comment_alignment = post_comment_alignment( + item_max_width, + inner_item.len(), + formatting, + sep_place, + first, + middle_item_same_len_first, + item_max_width.unwrap_or_default() == first_item_len, + last, + middle_item_same_len_last, + item_max_width.unwrap_or_default() == last_item_len, + separate, + ); } - } - // An additional space for the missing trailing separator (or - // if we skipped alignment above). - if !formatting.align_comments - || (last - && item_max_width.is_some() - && !separate - && !formatting.separator.is_empty()) - { + result.push_str(&" ".repeat(comment_alignment)); + } else { result.push(' '); } } else { @@ -563,19 +598,18 @@ where Some(result) } -fn max_width_of_item_with_post_comment( - items: &I, +fn max_width_of_item_with_post_comment( + items: &[T], i: usize, overhead: usize, max_budget: usize, ) -> usize where - I: IntoIterator + Clone, T: AsRef, { let mut max_width = 0; let mut first = true; - for item in items.clone().into_iter().skip(i) { + for item in items.iter().skip(i) { let item = item.as_ref(); let inner_item_width = item.inner_as_ref().len(); if !first @@ -596,8 +630,74 @@ where max_width } -fn post_comment_alignment(item_max_width: Option, inner_item_len: usize) -> usize { - item_max_width.unwrap_or(0).saturating_sub(inner_item_len) +fn post_comment_alignment( + item_max_width: Option, + inner_item_len: usize, + fmt: &ListFormatting<'_>, + sep_place: SeparatorPlace, + first: bool, + middle_item_same_len_first: bool, + first_item_longest: bool, + last: bool, + middle_item_same_len_last: bool, + last_item_longest: bool, + separate: bool, +) -> usize { + // 1 = whitespace before the post_comment + let alignment = item_max_width.unwrap_or(0).saturating_sub(inner_item_len) + 1; + if item_max_width.is_none() { + return alignment; + } + let alignment = match sep_place { + /* + * Front separator: first item is missing the separator and needs to be compensated + */ + SeparatorPlace::Front => { + match (first, first_item_longest) { + _ if middle_item_same_len_first => { + alignment + if first { fmt.separator.len() } else { 0 } + } + // first item is the longest: others need to minus the (separator + padding) + // to the alignment + (false, true) => { + // FIXME: document the trim the leading whitespace + alignment.saturating_sub(fmt.separator.trim_start().len()) + - if fmt.padding { 1 } else { 0 } + } + // first item is not the longest: first needs to add (searator + padding) + // to the alignment + (true, false) => { + alignment + fmt.separator.trim_start().len() + if fmt.padding { 1 } else { 0 } + } + _ => alignment, + } + } + /* + * Back separator: last item is missing the separator (if it is not trailing) + * and needs to be compensated + */ + SeparatorPlace::Back => { + match (last, last_item_longest) { + _ if middle_item_same_len_last => { + alignment + + if last && !separate { + fmt.separator.len() + } else { + 0 + } + } + // last item is the longest: others need to minus the sep to the alignment + (false, true) if !fmt.needs_trailing_separator() => { + alignment.saturating_sub(fmt.separator.len()) + } + // last item is not the longest: last needs to add sep to the alignment + (true, false) if !separate => alignment + fmt.separator.len(), + _ => alignment, + } + } + }; + // at least 1 for the whitespace before the post_comment + if alignment == 0 { 1 } else { alignment } } pub(crate) struct ListItems<'a, I, F1, F2, F3> @@ -979,6 +1079,206 @@ pub(crate) fn struct_lit_formatting<'a>( #[cfg(test)] mod test { use super::*; + use crate::config::Config; + use crate::shape::{Indent, Shape}; + + #[test] + fn post_comment_alignment() { + let config: Config = Default::default(); + + let data = [ + //separator at the front and first item is the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator("+") + .trailing_separator(SeparatorTactic::Always) + .separator_place(SeparatorPlace::Front) + }, + [ + ("item1aaaaaaaaaaaaaaa", "// len20"), + ("item2aaaaa", "// len10"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaaaaaaaaaaaa // len20 ++ item2aaaaa // len10 ++ item3aaaaaaaaaa // len15"#, + ), + // separator at the front and second item is the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator("+") + .trailing_separator(SeparatorTactic::Always) + .separator_place(SeparatorPlace::Front) + }, + [ + ("item1aaaaa", "// len10"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaa // len10 ++ item2aaaaaaaaaaaaaaa // len20 ++ item3aaaaaaaaaa // len15"#, + ), + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator("+") + .separator_place(SeparatorPlace::Front) + }, + [ + ("item1aaaaaaaaaaaaaaa", "// len20"), + ("item2aaaaa", "// len10"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaaaaaaaaaaaa // len20 ++ item2aaaaa // len10 ++ item3aaaaaaaaaa // len15"#, + ), + // font separator and middle/last items are the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator("+") + .separator_place(SeparatorPlace::Front) + }, + [ + ("item1aaaaa", "// len10"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaaaaaaa", "// len20"), + ], + r#"item1aaaaa // len10 ++ item2aaaaaaaaaaaaaaa // len20 ++ item3aaaaaaaaaaaaaaa // len20"#, + ), + // font separator and first/middle items are the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Front) + }, + [ + ("item1aaaaaaaaaaaaaaa", "// len20"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaaaaaaaaaaaa // len20 ++ item2aaaaaaaaaaaaaaa // len20 ++ item3aaaaaaaaaa // len15"#, + ), + // back separator and first item is the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Back) + }, + [ + ("item1aaaaaaaaaaaaaaa", "// len20"), + ("item2aaaaa", "// len10"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaaaaaaaaaaaa + // len20 +item2aaaaa + // len10 +item3aaaaaaaaaa // len15"#, + ), + // back separator and last item is the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Back) + }, + [ + ("item1aaaaa", "// len10"), + ("item2aaaaaaaaaa", "// len15"), + ("item3aaaaaaaaaaaaaaa", "// len20"), + ], + r#"item1aaaaa + // len10 +item2aaaaaaaaaa + // len15 +item3aaaaaaaaaaaaaaa // len20"#, + ), + // back separator and middle item is the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Back) + }, + [ + ("item1aaaaa", "// len10"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaa + // len10 +item2aaaaaaaaaaaaaaa + // len20 +item3aaaaaaaaaa // len15"#, + ), + // back separator and middle/last items are the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Back) + }, + [ + ("item1aaaaa", "// len10"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaaaaaaa", "// len20"), + ], + r#"item1aaaaa + // len10 +item2aaaaaaaaaaaaaaa + // len20 +item3aaaaaaaaaaaaaaa // len20"#, + ), + // back separator and first/middle items are the longest + ( + { + let shape = Shape::legacy(config.max_width(), Indent::empty()); + ListFormatting::new(shape, &config) + .separator(" +") + .separator_place(SeparatorPlace::Back) + }, + [ + ("item1aaaaaaaaaaaaaaa", "// len20"), + ("item2aaaaaaaaaaaaaaa", "// len20"), + ("item3aaaaaaaaaa", "// len15"), + ], + r#"item1aaaaaaaaaaaaaaa + // len20 +item2aaaaaaaaaaaaaaa + // len20 +item3aaaaaaaaaa // len15"#, + ), + ]; + + for (i, (fmt, items, expected)) in data.into_iter().enumerate() { + let items = items + .into_iter() + .map(|(inner_item, post_comment)| ListItem { + pre_comment_style: ListItemCommentStyle::SameLine, + pre_comment: None, + item: Some(inner_item.to_string()), + post_comment: Some(post_comment.to_string()), + new_lines: false, + }) + .collect::>(); + assert_eq!( + write_list(&items, &fmt), + Some(expected.to_string()), + "failed on item {}", + i + ); + } + } #[test] fn test_extract_post_comment() { diff --git a/tests/target/issue-3669.rs b/tests/target/issue-3669.rs index 32418dae585..06e42ad9075 100644 --- a/tests/target/issue-3669.rs +++ b/tests/target/issue-3669.rs @@ -1,15 +1,15 @@ pub trait PCG: self::sealed::Sealed // comment1 - + Sized // comment2 - + Eq // comment3 - + Hash // comment4 - + Debug // comment5 - + Clone // comment6 - + Default // comment7 - + Serialize // comment8 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + for<'a> Deserialize<'a> // comment9 { - type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + Shl // Note(Evrey): Because Rust is drunk. 3 + BitAnd // Note(Evrey): Because Rust is drunk. 4 @@ -26,16 +26,16 @@ pub trait PCG: pub trait Bar: self::sealed::Sealed // comment1 - + Sized // comment2 - + Eq // comment3 - + Hash // comment4 - + Debug // comment5 - + Clone // comment6 - + Default // comment7 - + Serialize // comment8 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + for<'a> Deserialize<'a> // comment9 { - type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + Shl // Note(Evrey): Because Rust is drunk. 3 + BitAnd // Note(Evrey): Because Rust is drunk. 4 From 62961be93e7e04451a6dd3dfd8885a409d56dcf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Wed, 31 Jul 2019 10:15:00 +0200 Subject: [PATCH 06/10] extract some write_list variables into separate structs to better indicate what they are used for --- src/lists.rs | 345 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 204 insertions(+), 141 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index b3004d98de8..45d5bb86803 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -275,6 +275,175 @@ where } } +struct ItemState { + first: bool, + last: bool, + first_item_on_line: bool, +} + +impl ItemState { + fn new() -> ItemState { + ItemState { + first: false, + last: false, + first_item_on_line: true, + } + } +} + +struct WriteListState { + sep_place: SeparatorPlace, + separate: bool, + trailing_separator: bool, +} + +impl WriteListState { + fn new(formatting: &ListFormatting<'_>) -> WriteListState { + WriteListState { + sep_place: SeparatorPlace::from_tactic( + formatting.separator_place, + formatting.tactic, + formatting.separator, + ), + separate: false, + // Now that we know how we will layout, we can decide for sure if there + // will be a trailing separator. + trailing_separator: formatting.needs_trailing_separator(), + } + } + + fn should_separate(&mut self, item_state: &ItemState) { + self.separate = match self.sep_place { + SeparatorPlace::Front => !item_state.first, + SeparatorPlace::Back => !item_state.last || self.trailing_separator, + }; + } + + fn should_separate_if_front(&self) -> bool { + self.separate && self.sep_place.is_front() + } + + fn should_separate_if_back(&self) -> bool { + self.separate && self.sep_place.is_back() + } +} + +struct PostCommentAlignment { + first_item_len: usize, + middle_item_same_len_first: bool, + last_item_len: usize, + middle_item_same_len_last: bool, +} + +impl PostCommentAlignment { + fn new>(items: &[T]) -> PostCommentAlignment { + let first_item_len = if let Some(item) = items.iter().nth(0) { + item.as_ref().inner_as_ref().len() + } else { + 0 + }; + let last_item_len = if let Some(item) = items.iter().last() { + item.as_ref().inner_as_ref().len() + } else { + 0 + }; + let middle_item_same_len_first = items + .iter() + // all but first item + .skip(1) + // check if any of the intermediate items share the size of the first item + .map(|item| item.as_ref().inner_as_ref().len()) + .any(|len| len == first_item_len); + let middle_item_same_len_last = items + .iter() + // all but last item + .take(items.len().saturating_sub(1)) + // check if any of the intermediate items share the size of the last item + .map(|item| item.as_ref().inner_as_ref().len()) + .any(|len| len == last_item_len); + PostCommentAlignment { + first_item_len, + middle_item_same_len_first, + last_item_len, + middle_item_same_len_last, + } + } + + fn compute( + &self, + item_state: &ItemState, + item_max_width: Option, + inner_item_len: usize, + fmt: &ListFormatting<'_>, + write_list_state: &WriteListState, + ) -> usize { + // 1 = whitespace before the post_comment + let alignment = item_max_width.unwrap_or(0).saturating_sub(inner_item_len) + 1; + if item_max_width.is_none() { + return alignment; + } + let first_item_longest = item_max_width.unwrap() == self.first_item_len; + let last_item_longest = item_max_width.unwrap() == self.last_item_len; + let alignment = match write_list_state.sep_place { + /* + * Front separator: first item is missing the separator and needs to be compensated + */ + SeparatorPlace::Front => { + match (item_state.first, first_item_longest) { + _ if self.middle_item_same_len_first => { + alignment + + if item_state.first { + fmt.separator.len() + } else { + 0 + } + } + // first item is the longest: others need to minus the (separator + padding) + // to the alignment + (false, true) => { + // FIXME: document the trim the leading whitespace + alignment.saturating_sub(fmt.separator.trim_start().len()) + - if fmt.padding { 1 } else { 0 } + } + // first item is not the longest: first needs to add (searator + padding) + // to the alignment + (true, false) => { + alignment + + fmt.separator.trim_start().len() + + if fmt.padding { 1 } else { 0 } + } + _ => alignment, + } + } + /* + * Back separator: last item is missing the separator (if it is not trailing) + * and needs to be compensated + */ + SeparatorPlace::Back => { + match (item_state.last, last_item_longest) { + _ if self.middle_item_same_len_last => { + alignment + + if item_state.last && !write_list_state.separate { + fmt.separator.len() + } else { + 0 + } + } + // last item is the longest: others need to minus the sep to the alignment + (false, true) if !fmt.needs_trailing_separator() => { + alignment.saturating_sub(fmt.separator.len()) + } + // last item is not the longest: last needs to add sep to the alignment + (true, false) if !write_list_state.separate => alignment + fmt.separator.len(), + _ => alignment, + } + } + }; + // at least 1 for the whitespace before the post_comment + if alignment == 0 { 1 } else { alignment } + } +} + /// Format a list of commented items into a string. pub(crate) fn write_list(items: &[T], formatting: &ListFormatting<'_>) -> Option where @@ -283,55 +452,29 @@ where let tactic = formatting.tactic; let sep_len = formatting.separator.len(); - // Now that we know how we will layout, we can decide for sure if there - // will be a trailing separator. - let mut trailing_separator = formatting.needs_trailing_separator(); let mut result = String::with_capacity(128); let mut iter = items.iter().enumerate().peekable(); let mut item_max_width: Option = None; - let sep_place = - SeparatorPlace::from_tactic(formatting.separator_place, tactic, formatting.separator); let mut prev_item_had_post_comment = false; let mut prev_item_is_nested_import = false; - let first_item_len = if let Some(item) = items.iter().nth(0) { - item.as_ref().inner_as_ref().len() - } else { - 0 - }; - let last_item_len = if let Some(item) = items.iter().last() { - item.as_ref().inner_as_ref().len() - } else { - 0 - }; - let middle_item_same_len_first = items - .iter() - // all but first item - .skip(1) - // check if any of the intermediate items share the size of the first item - .map(|item| item.as_ref().inner_as_ref().len()) - .any(|len| len == first_item_len); - let middle_item_same_len_last = items - .iter() - // all but last item - .take(items.len().saturating_sub(1)) - // check if any of the intermediate items share the size of the last item - .map(|item| item.as_ref().inner_as_ref().len()) - .any(|len| len == last_item_len); + let mut item_state = ItemState::new(); + let mut write_list_state = WriteListState::new(formatting); + let pca = PostCommentAlignment::new(items); let mut line_len = 0; - let mut first_item_on_line = true; let indent_str = &formatting.shape.indent.to_string(formatting.config); while let Some((i, item)) = iter.next() { let item = item.as_ref(); let inner_item = item.item.as_ref()?; - let first = i == 0; - let last = iter.peek().is_none(); - let mut separate = match sep_place { - SeparatorPlace::Front => !first, - SeparatorPlace::Back => !last || trailing_separator, + item_state.first = i == 0; + item_state.last = iter.peek().is_none(); + write_list_state.should_separate(&item_state); + let item_sep_len = if write_list_state.separate { + sep_len + } else { + 0 }; - let item_sep_len = if separate { sep_len } else { 0 }; // Item string may be multi-line. Its length (used for block comment alignment) // should be only the length of the last line. @@ -358,12 +501,12 @@ where { result.push('\n'); result.push_str(indent_str); - first_item_on_line = true; - } else if formatting.padding && !first_item_on_line { + item_state.first_item_on_line = true; + } else if formatting.padding && !item_state.first_item_on_line { result.push(' '); } } - DefinitiveListTactic::Horizontal if !first && formatting.padding => { + DefinitiveListTactic::Horizontal if !item_state.first && formatting.padding => { result.push(' '); } DefinitiveListTactic::SpecialMacro(num_args_before) => { @@ -379,9 +522,9 @@ where } } DefinitiveListTactic::Vertical - if !first && !inner_item.is_empty() && !result.is_empty() => + if !item_state.first && !inner_item.is_empty() && !result.is_empty() => { - first_item_on_line = true; + item_state.first_item_on_line = true; result.push('\n'); result.push_str(indent_str); } @@ -392,22 +535,24 @@ where if (line_len > 0 && line_len + 1 + total_width > formatting.shape.width) || prev_item_had_post_comment || (formatting.nested - && (prev_item_is_nested_import || (!first && inner_item.contains("::")))) + && (prev_item_is_nested_import + || (!item_state.first && inner_item.contains("::")))) { result.push('\n'); result.push_str(indent_str); line_len = 0; - first_item_on_line = true; + item_state.first_item_on_line = true; if formatting.ends_with_newline { - trailing_separator = true; + write_list_state.trailing_separator = true; } } else if formatting.padding && line_len > 0 { result.push(' '); line_len += 1; } - if last && formatting.ends_with_newline { - separate = formatting.trailing_separator != SeparatorTactic::Never; + if item_state.last && formatting.ends_with_newline { + write_list_state.separate = + formatting.trailing_separator != SeparatorTactic::Never; } line_len += total_width; @@ -453,11 +598,11 @@ where item_max_width = None; } - if separate && sep_place.is_front() && !first { + if write_list_state.should_separate_if_front() && !item_state.first { if formatting.padding { result.push_str(formatting.separator.trim()); result.push(' '); - } else if first_item_on_line { + } else if item_state.first_item_on_line { result.push_str(formatting.separator.trim_start()); } else { result.push_str(formatting.separator); @@ -484,7 +629,7 @@ where result.push_str(&formatted_comment); } - if separate && sep_place.is_back() { + if write_list_state.should_separate_if_back() { result.push_str(formatting.separator); } @@ -493,7 +638,7 @@ where let overhead = last_line_width(&result) + first_line_width(comment.trim()); let rewrite_post_comment = |item_max_width: &mut Option| { - if item_max_width.is_none() && !last && !inner_item.contains('\n') { + if item_max_width.is_none() && !item_state.last && !inner_item.contains('\n') { *item_max_width = Some(max_width_of_item_with_post_comment( &items, i, @@ -514,7 +659,7 @@ where let comment_shape = Shape::legacy(width, offset); // Use block-style only for the last item or multiline comments. - let block_style = !formatting.ends_with_newline && last + let block_style = !formatting.ends_with_newline && item_state.last || comment.trim().contains('\n') || comment.trim().len() > width; @@ -530,18 +675,12 @@ where if !starts_with_newline(comment) { if formatting.align_comments { - let mut comment_alignment = post_comment_alignment( + let mut comment_alignment = pca.compute( + &item_state, item_max_width, inner_item.len(), formatting, - sep_place, - first, - middle_item_same_len_first, - item_max_width.unwrap_or_default() == first_item_len, - last, - middle_item_same_len_last, - item_max_width.unwrap_or_default() == last_item_len, - separate, + &write_list_state, ); if first_line_width(&formatted_comment) + last_line_width(&result) @@ -551,18 +690,12 @@ where { item_max_width = None; formatted_comment = rewrite_post_comment(&mut item_max_width)?; - comment_alignment = post_comment_alignment( + comment_alignment = pca.compute( + &item_state, item_max_width, inner_item.len(), formatting, - sep_place, - first, - middle_item_same_len_first, - item_max_width.unwrap_or_default() == first_item_len, - last, - middle_item_same_len_last, - item_max_width.unwrap_or_default() == last_item_len, - separate, + &write_list_state, ); } result.push_str(&" ".repeat(comment_alignment)); @@ -582,7 +715,7 @@ where } if formatting.preserve_newline - && !last + && !item_state.last && tactic == DefinitiveListTactic::Vertical && item.new_lines { @@ -590,7 +723,7 @@ where result.push('\n'); } - first_item_on_line = false; + item_state.first_item_on_line = false; prev_item_had_post_comment = item.post_comment.is_some(); prev_item_is_nested_import = inner_item.contains("::"); } @@ -630,76 +763,6 @@ where max_width } -fn post_comment_alignment( - item_max_width: Option, - inner_item_len: usize, - fmt: &ListFormatting<'_>, - sep_place: SeparatorPlace, - first: bool, - middle_item_same_len_first: bool, - first_item_longest: bool, - last: bool, - middle_item_same_len_last: bool, - last_item_longest: bool, - separate: bool, -) -> usize { - // 1 = whitespace before the post_comment - let alignment = item_max_width.unwrap_or(0).saturating_sub(inner_item_len) + 1; - if item_max_width.is_none() { - return alignment; - } - let alignment = match sep_place { - /* - * Front separator: first item is missing the separator and needs to be compensated - */ - SeparatorPlace::Front => { - match (first, first_item_longest) { - _ if middle_item_same_len_first => { - alignment + if first { fmt.separator.len() } else { 0 } - } - // first item is the longest: others need to minus the (separator + padding) - // to the alignment - (false, true) => { - // FIXME: document the trim the leading whitespace - alignment.saturating_sub(fmt.separator.trim_start().len()) - - if fmt.padding { 1 } else { 0 } - } - // first item is not the longest: first needs to add (searator + padding) - // to the alignment - (true, false) => { - alignment + fmt.separator.trim_start().len() + if fmt.padding { 1 } else { 0 } - } - _ => alignment, - } - } - /* - * Back separator: last item is missing the separator (if it is not trailing) - * and needs to be compensated - */ - SeparatorPlace::Back => { - match (last, last_item_longest) { - _ if middle_item_same_len_last => { - alignment - + if last && !separate { - fmt.separator.len() - } else { - 0 - } - } - // last item is the longest: others need to minus the sep to the alignment - (false, true) if !fmt.needs_trailing_separator() => { - alignment.saturating_sub(fmt.separator.len()) - } - // last item is not the longest: last needs to add sep to the alignment - (true, false) if !separate => alignment + fmt.separator.len(), - _ => alignment, - } - } - }; - // at least 1 for the whitespace before the post_comment - if alignment == 0 { 1 } else { alignment } -} - pub(crate) struct ListItems<'a, I, F1, F2, F3> where I: Iterator, From 9865ce04aef8bbf893e2f48f97d9a40308af024c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Wed, 31 Jul 2019 11:49:14 +0200 Subject: [PATCH 07/10] move item_max_width into the PostCommentAlignment struct --- src/lists.rs | 150 ++++++++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 66 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index 45d5bb86803..5d14df7577b 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -276,6 +276,7 @@ where } struct ItemState { + ith: usize, first: bool, last: bool, first_item_on_line: bool, @@ -284,6 +285,7 @@ struct ItemState { impl ItemState { fn new() -> ItemState { ItemState { + ith: 0, first: false, last: false, first_item_on_line: true, @@ -329,6 +331,7 @@ impl WriteListState { } struct PostCommentAlignment { + item_max_width: Option, first_item_len: usize, middle_item_same_len_first: bool, last_item_len: usize, @@ -362,6 +365,7 @@ impl PostCommentAlignment { .map(|item| item.as_ref().inner_as_ref().len()) .any(|len| len == last_item_len); PostCommentAlignment { + item_max_width: None, first_item_len, middle_item_same_len_first, last_item_len, @@ -369,21 +373,74 @@ impl PostCommentAlignment { } } + fn update_item_max_width>( + &mut self, + items: &[T], + formatting: &ListFormatting<'_>, + item_state: &ItemState, + inner_item: &str, + overhead: usize, + ) { + if self.item_max_width.is_none() && !item_state.last && !inner_item.contains('\n') { + self.item_max_width = Some(PostCommentAlignment::max_width_of_item_with_post_comment( + &items, + item_state.ith, + overhead, + formatting.config.max_width(), + )); + } + } + + fn max_width_of_item_with_post_comment( + items: &[T], + i: usize, + overhead: usize, + max_budget: usize, + ) -> usize + where + T: AsRef, + { + let mut max_width = 0; + let mut first = true; + for item in items.iter().skip(i) { + let item = item.as_ref(); + let inner_item_width = item.inner_as_ref().len(); + if !first + && (item.is_different_group() + || item.post_comment.is_none() + || inner_item_width + overhead > max_budget) + { + return max_width; + } + if max_width < inner_item_width { + max_width = inner_item_width; + } + if item.new_lines { + return max_width; + } + first = false; + } + max_width + } + fn compute( &self, item_state: &ItemState, - item_max_width: Option, inner_item_len: usize, fmt: &ListFormatting<'_>, write_list_state: &WriteListState, ) -> usize { // 1 = whitespace before the post_comment - let alignment = item_max_width.unwrap_or(0).saturating_sub(inner_item_len) + 1; - if item_max_width.is_none() { + let alignment = self + .item_max_width + .unwrap_or(0) + .saturating_sub(inner_item_len) + + 1; + if self.item_max_width.is_none() { return alignment; } - let first_item_longest = item_max_width.unwrap() == self.first_item_len; - let last_item_longest = item_max_width.unwrap() == self.last_item_len; + let first_item_longest = self.item_max_width.unwrap() == self.first_item_len; + let last_item_longest = self.item_max_width.unwrap() == self.last_item_len; let alignment = match write_list_state.sep_place { /* * Front separator: first item is missing the separator and needs to be compensated @@ -450,28 +507,27 @@ where T: AsRef, { let tactic = formatting.tactic; - let sep_len = formatting.separator.len(); let mut result = String::with_capacity(128); let mut iter = items.iter().enumerate().peekable(); - let mut item_max_width: Option = None; let mut prev_item_had_post_comment = false; let mut prev_item_is_nested_import = false; let mut item_state = ItemState::new(); let mut write_list_state = WriteListState::new(formatting); - let pca = PostCommentAlignment::new(items); + let mut pca = PostCommentAlignment::new(items); let mut line_len = 0; let indent_str = &formatting.shape.indent.to_string(formatting.config); while let Some((i, item)) = iter.next() { let item = item.as_ref(); let inner_item = item.item.as_ref()?; + item_state.ith = i; item_state.first = i == 0; item_state.last = iter.peek().is_none(); write_list_state.should_separate(&item_state); let item_sep_len = if write_list_state.separate { - sep_len + formatting.separator.len() } else { 0 }; @@ -595,7 +651,7 @@ where result.push(' '); } } - item_max_width = None; + pca.item_max_width = None; } if write_list_state.should_separate_if_front() && !item_state.first { @@ -637,18 +693,10 @@ where let comment = item.post_comment.as_ref().unwrap(); let overhead = last_line_width(&result) + first_line_width(comment.trim()); - let rewrite_post_comment = |item_max_width: &mut Option| { - if item_max_width.is_none() && !item_state.last && !inner_item.contains('\n') { - *item_max_width = Some(max_width_of_item_with_post_comment( - &items, - i, - overhead, - formatting.config.max_width(), - )); - } + let rewrite_post_comment = |item_max_width: Option| { let overhead = if starts_with_newline(comment) { 0 - } else if let Some(max_width) = *item_max_width { + } else if let Some(max_width) = item_max_width { max_width + 2 } else { // 1 = space between item and comment. @@ -671,28 +719,30 @@ where ) }; - let mut formatted_comment = rewrite_post_comment(&mut item_max_width)?; + pca.update_item_max_width(items, formatting, &item_state, inner_item, overhead); + let mut formatted_comment = rewrite_post_comment(pca.item_max_width)?; if !starts_with_newline(comment) { if formatting.align_comments { - let mut comment_alignment = pca.compute( - &item_state, - item_max_width, - inner_item.len(), - formatting, - &write_list_state, - ); + let mut comment_alignment = + pca.compute(&item_state, inner_item.len(), formatting, &write_list_state); if first_line_width(&formatted_comment) + last_line_width(&result) + comment_alignment + 1 > formatting.config.max_width() { - item_max_width = None; - formatted_comment = rewrite_post_comment(&mut item_max_width)?; + pca.item_max_width = None; + pca.update_item_max_width( + items, + formatting, + &item_state, + inner_item, + overhead, + ); + formatted_comment = rewrite_post_comment(pca.item_max_width)?; comment_alignment = pca.compute( &item_state, - item_max_width, inner_item.len(), formatting, &write_list_state, @@ -707,11 +757,11 @@ where result.push_str(indent_str); } if formatted_comment.contains('\n') { - item_max_width = None; + pca.item_max_width = None; } result.push_str(&formatted_comment); } else { - item_max_width = None; + pca.item_max_width = None; } if formatting.preserve_newline @@ -719,7 +769,7 @@ where && tactic == DefinitiveListTactic::Vertical && item.new_lines { - item_max_width = None; + pca.item_max_width = None; result.push('\n'); } @@ -731,38 +781,6 @@ where Some(result) } -fn max_width_of_item_with_post_comment( - items: &[T], - i: usize, - overhead: usize, - max_budget: usize, -) -> usize -where - T: AsRef, -{ - let mut max_width = 0; - let mut first = true; - for item in items.iter().skip(i) { - let item = item.as_ref(); - let inner_item_width = item.inner_as_ref().len(); - if !first - && (item.is_different_group() - || item.post_comment.is_none() - || inner_item_width + overhead > max_budget) - { - return max_width; - } - if max_width < inner_item_width { - max_width = inner_item_width; - } - if item.new_lines { - return max_width; - } - first = false; - } - max_width -} - pub(crate) struct ListItems<'a, I, F1, F2, F3> where I: Iterator, From db09dacf8d0409bb763ce33140ab2ea02d2b4048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Wed, 31 Jul 2019 22:01:42 +0200 Subject: [PATCH 08/10] add documentation and clarify some code about list stringification --- src/lists.rs | 166 +++++++++++++++++++++++++++------------------------ 1 file changed, 88 insertions(+), 78 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index 5d14df7577b..14ec18a1219 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -275,33 +275,31 @@ where } } -struct ItemState { +/// Stores the state of the ongoing stringification of a list item. +struct WriteListItemState { + /// The offset of the item in the list. ith: usize, + /// Whether or not the item is the first in the list. first: bool, + /// Whether or not the item is the last in the list. last: bool, + /// Whether or not the item is the first to be written on the current line. first_item_on_line: bool, + /// The placement of the separator + sep_place: SeparatorPlace, + /// Whether or not the separator should be written. + separate: bool, + /// Whether or not the separator can be trailing. + trailing_separator: bool, } -impl ItemState { - fn new() -> ItemState { - ItemState { +impl WriteListItemState { + fn new(formatting: &ListFormatting<'_>) -> WriteListItemState { + WriteListItemState { ith: 0, first: false, last: false, first_item_on_line: true, - } - } -} - -struct WriteListState { - sep_place: SeparatorPlace, - separate: bool, - trailing_separator: bool, -} - -impl WriteListState { - fn new(formatting: &ListFormatting<'_>) -> WriteListState { - WriteListState { sep_place: SeparatorPlace::from_tactic( formatting.separator_place, formatting.tactic, @@ -314,27 +312,36 @@ impl WriteListState { } } - fn should_separate(&mut self, item_state: &ItemState) { + /// Set if a separator should be written for the current item. + fn should_separate(&mut self) { self.separate = match self.sep_place { - SeparatorPlace::Front => !item_state.first, - SeparatorPlace::Back => !item_state.last || self.trailing_separator, + SeparatorPlace::Front => !self.first, + SeparatorPlace::Back => !self.last || self.trailing_separator, }; } + /// Returns true if a separtor should be placed in front of the current item. fn should_separate_if_front(&self) -> bool { self.separate && self.sep_place.is_front() } + /// Returns true if a separtor should be placed at the back of the current item. fn should_separate_if_back(&self) -> bool { self.separate && self.sep_place.is_back() } } +/// Align comments written after items. struct PostCommentAlignment { + /// The largest item width in a group of items. item_max_width: Option, + /// The length of the first item. first_item_len: usize, + /// Whether or not an item that is not the first has the same length as the first item. middle_item_same_len_first: bool, + /// The length of the last item. last_item_len: usize, + /// Whether or not an item that is not the last has the same length as the last item. middle_item_same_len_last: bool, } @@ -373,11 +380,12 @@ impl PostCommentAlignment { } } + /// Get the length of the largest item starting at the ith-item. fn update_item_max_width>( &mut self, items: &[T], formatting: &ListFormatting<'_>, - item_state: &ItemState, + item_state: &WriteListItemState, inner_item: &str, overhead: usize, ) { @@ -423,30 +431,35 @@ impl PostCommentAlignment { max_width } - fn compute( + /// Computes the number whitespaces to insert after an item so that its post comment is aligned + /// with others. + /// + /// The alignment logic is complicated by the fact that the alignment is based on the longest + /// item, without the additional space taken by the separator. For the first and last items + /// where the separator may be missing, some compensation needs to be computed. + fn alignment( &self, - item_state: &ItemState, + item_state: &WriteListItemState, inner_item_len: usize, fmt: &ListFormatting<'_>, - write_list_state: &WriteListState, ) -> usize { // 1 = whitespace before the post_comment - let alignment = self - .item_max_width - .unwrap_or(0) - .saturating_sub(inner_item_len) - + 1; if self.item_max_width.is_none() { - return alignment; + return 1; } - let first_item_longest = self.item_max_width.unwrap() == self.first_item_len; - let last_item_longest = self.item_max_width.unwrap() == self.last_item_len; - let alignment = match write_list_state.sep_place { + let item_max_width = self.item_max_width.unwrap(); + let alignment = item_max_width.saturating_sub(inner_item_len) + 1; + let first_item_longest = item_max_width == self.first_item_len; + let last_item_longest = item_max_width == self.last_item_len; + + let alignment = match item_state.sep_place { /* * Front separator: first item is missing the separator and needs to be compensated */ SeparatorPlace::Front => { match (item_state.first, first_item_longest) { + // in case a middle item has the same length as the first, then all items have + // the correct alignment: only the first item needs to account for the separator _ if self.middle_item_same_len_first => { alignment + if item_state.first { @@ -457,11 +470,13 @@ impl PostCommentAlignment { } // first item is the longest: others need to minus the (separator + padding) // to the alignment - (false, true) => { - // FIXME: document the trim the leading whitespace - alignment.saturating_sub(fmt.separator.trim_start().len()) - - if fmt.padding { 1 } else { 0 } - } + (false, true) => alignment + .saturating_sub(if item_state.first_item_on_line { + fmt.separator.trim_start().len() + } else { + fmt.separator.len() + }) + .saturating_sub(if fmt.padding { 1 } else { 0 }), // first item is not the longest: first needs to add (searator + padding) // to the alignment (true, false) => { @@ -480,7 +495,7 @@ impl PostCommentAlignment { match (item_state.last, last_item_longest) { _ if self.middle_item_same_len_last => { alignment - + if item_state.last && !write_list_state.separate { + + if item_state.last && !item_state.separate { fmt.separator.len() } else { 0 @@ -491,7 +506,7 @@ impl PostCommentAlignment { alignment.saturating_sub(fmt.separator.len()) } // last item is not the longest: last needs to add sep to the alignment - (true, false) if !write_list_state.separate => alignment + fmt.separator.len(), + (true, false) if !item_state.separate => alignment + fmt.separator.len(), _ => alignment, } } @@ -507,47 +522,36 @@ where T: AsRef, { let tactic = formatting.tactic; + let indent_str = &formatting.shape.indent.to_string(formatting.config); let mut result = String::with_capacity(128); let mut iter = items.iter().enumerate().peekable(); + + // Mixed tactic state let mut prev_item_had_post_comment = false; let mut prev_item_is_nested_import = false; + let mut line_len = 0; - let mut item_state = ItemState::new(); - let mut write_list_state = WriteListState::new(formatting); + let mut item_state = WriteListItemState::new(formatting); let mut pca = PostCommentAlignment::new(items); - let mut line_len = 0; - let indent_str = &formatting.shape.indent.to_string(formatting.config); while let Some((i, item)) = iter.next() { let item = item.as_ref(); let inner_item = item.item.as_ref()?; + + if !item.is_substantial() { + continue; + } + item_state.ith = i; item_state.first = i == 0; item_state.last = iter.peek().is_none(); - write_list_state.should_separate(&item_state); - let item_sep_len = if write_list_state.separate { + item_state.should_separate(); + let item_sep_len = if item_state.separate { formatting.separator.len() } else { 0 }; - - // Item string may be multi-line. Its length (used for block comment alignment) - // should be only the length of the last line. - let item_last_line = if item.is_multiline() { - inner_item.lines().last().unwrap_or("") - } else { - inner_item.as_ref() - }; - let mut item_last_line_width = item_last_line.len() + item_sep_len; - if item_last_line.starts_with(&**indent_str) { - item_last_line_width -= indent_str.len(); - } - - if !item.is_substantial() { - continue; - } - match tactic { _ if !formatting.custom_list_tactic.is_empty() => { if *formatting @@ -599,7 +603,7 @@ where line_len = 0; item_state.first_item_on_line = true; if formatting.ends_with_newline { - write_list_state.trailing_separator = true; + item_state.trailing_separator = true; } } else if formatting.padding && line_len > 0 { result.push(' '); @@ -607,11 +611,12 @@ where } if item_state.last && formatting.ends_with_newline { - write_list_state.separate = - formatting.trailing_separator != SeparatorTactic::Never; + item_state.separate = formatting.trailing_separator != SeparatorTactic::Never; } line_len += total_width; + prev_item_had_post_comment = item.post_comment.is_some(); + prev_item_is_nested_import = inner_item.contains("::"); } _ => {} } @@ -654,7 +659,7 @@ where pca.item_max_width = None; } - if write_list_state.should_separate_if_front() && !item_state.first { + if item_state.should_separate_if_front() && !item_state.first { if formatting.padding { result.push_str(formatting.separator.trim()); result.push(' '); @@ -685,7 +690,7 @@ where result.push_str(&formatted_comment); } - if write_list_state.should_separate_if_back() { + if item_state.should_separate_if_back() { result.push_str(formatting.separator); } @@ -699,6 +704,18 @@ where } else if let Some(max_width) = item_max_width { max_width + 2 } else { + // Item string may be multi-line. Its length (used for block comment alignment) + // should be only the length of the last line. + let item_last_line = if item.is_multiline() { + inner_item.lines().last().unwrap_or("") + } else { + inner_item.as_ref() + }; + let mut item_last_line_width = item_last_line.len() + item_sep_len; + if item_last_line.starts_with(&**indent_str) { + item_last_line_width -= indent_str.len(); + } + // 1 = space between item and comment. item_last_line_width + 1 }; @@ -725,11 +742,10 @@ where if !starts_with_newline(comment) { if formatting.align_comments { let mut comment_alignment = - pca.compute(&item_state, inner_item.len(), formatting, &write_list_state); + pca.alignment(&item_state, inner_item.len(), formatting); if first_line_width(&formatted_comment) + last_line_width(&result) + comment_alignment - + 1 > formatting.config.max_width() { pca.item_max_width = None; @@ -741,12 +757,8 @@ where overhead, ); formatted_comment = rewrite_post_comment(pca.item_max_width)?; - comment_alignment = pca.compute( - &item_state, - inner_item.len(), - formatting, - &write_list_state, - ); + comment_alignment = + pca.alignment(&item_state, inner_item.len(), formatting); } result.push_str(&" ".repeat(comment_alignment)); } else { @@ -774,8 +786,6 @@ where } item_state.first_item_on_line = false; - prev_item_had_post_comment = item.post_comment.is_some(); - prev_item_is_nested_import = inner_item.contains("::"); } Some(result) From a601c5efd9063c76234c8a6c990e508560a5e2d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Sat, 21 Sep 2019 01:23:19 +0200 Subject: [PATCH 09/10] fix indent in types_compressed.rs test following #3731 --- tests/target/types_compressed.rs | 76 ++++++++++++++++---------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/tests/target/types_compressed.rs b/tests/target/types_compressed.rs index d0efdc71a77..ff3bcf66c54 100644 --- a/tests/target/types_compressed.rs +++ b/tests/target/types_compressed.rs @@ -3,42 +3,42 @@ pub fn do_something<'a, T: Trait1+Trait2+'a>( &fooo: u32, ) -> impl Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +'a+'a+'a+'a+'a { + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +Foo + +'a+'a+'a+'a+'a { } From 6e2202ef2bde958a09a44b0685ae22f50394fae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Thu, 26 Sep 2019 01:01:15 +0200 Subject: [PATCH 10/10] gated the handling of comment in types list --- src/items.rs | 32 ++++--- src/types.rs | 75 ++++++++++++++++- .../type_compressed-one.rs} | 1 + .../source/issue-3669/type_compressed-two.rs | 8 ++ .../version-one.rs} | 2 + tests/source/issue-3669/version-two.rs | 84 +++++++++++++++++++ .../target/issue-3669/type_compressed-one.rs | 45 ++++++++++ .../target/issue-3669/type_compressed-two.rs | 45 ++++++++++ tests/target/issue-3669/version-one.rs | 77 +++++++++++++++++ .../version-two.rs} | 11 +++ tests/target/trait.rs | 7 +- tests/target/types_compressed.rs | 44 ---------- 12 files changed, 369 insertions(+), 62 deletions(-) rename tests/source/{types_compressed.rs => issue-3669/type_compressed-one.rs} (94%) create mode 100644 tests/source/issue-3669/type_compressed-two.rs rename tests/source/{issue-3669.rs => issue-3669/version-one.rs} (99%) create mode 100644 tests/source/issue-3669/version-two.rs create mode 100644 tests/target/issue-3669/type_compressed-one.rs create mode 100644 tests/target/issue-3669/type_compressed-two.rs create mode 100644 tests/target/issue-3669/version-one.rs rename tests/target/{issue-3669.rs => issue-3669/version-two.rs} (97%) delete mode 100644 tests/target/types_compressed.rs diff --git a/src/items.rs b/src/items.rs index 2f1280150c2..20b0cda0cf8 100644 --- a/src/items.rs +++ b/src/items.rs @@ -991,16 +991,28 @@ pub(crate) fn format_trait( result.push_str(&generics_str); if !generic_bounds.is_empty() { - let comment_span = mk_sp(generics.span.hi(), generic_bounds[0].span().lo()); - let after_colon = context.snippet_provider.span_after(comment_span, ":"); - let comment = recover_missing_comment_in_span( - mk_sp(after_colon, comment_span.hi()), - shape, - context, - // 1 = ":" - last_line_width(&result) + 1, - ) - .unwrap_or_default(); + let comment = if context.config.version() == Version::Two { + let comment_span = mk_sp(generics.span.hi(), generic_bounds[0].span().lo()); + let after_colon = context.snippet_provider.span_after(comment_span, ":"); + recover_missing_comment_in_span( + mk_sp(after_colon, comment_span.hi()), + shape, + context, + // 1 = ":" + last_line_width(&result) + 1, + ) + .unwrap_or_default() + } else { + let ident_hi = context + .snippet_provider + .span_after(item.span, &item.ident.as_str()); + let bound_hi = generic_bounds.last().unwrap().span().hi(); + let snippet = context.snippet(mk_sp(ident_hi, bound_hi)); + if contains_comment(snippet) { + return None; + } + String::new() + }; result = rewrite_assign_rhs_with( context, format!("{}:{}", result, comment), diff --git a/src/types.rs b/src/types.rs index b0369a08967..4fb745f06bf 100644 --- a/src/types.rs +++ b/src/types.rs @@ -499,7 +499,11 @@ fn rewrite_bounded_lifetime( "{}{}{}", result, colon, - join_bounds(context, shape.sub_width(overhead)?, bounds, true)? + if context.config.version() == Version::One { + join_bounds_v1(context, shape.sub_width(overhead)?, bounds, true)? + } else { + join_bounds_v2(context, shape.sub_width(overhead)?, bounds, true)? + } ); Some(result) } @@ -542,7 +546,11 @@ impl Rewrite for ast::GenericBounds { return Some(String::new()); } - join_bounds(context, shape, self, true) + if context.config.version() == Version::One { + join_bounds_v1(context, shape, self, true) + } else { + join_bounds_v2(context, shape, self, true) + } } } @@ -748,7 +756,7 @@ impl Rewrite for ast::Ty { let rw = if context.config.version() == Version::One { it.rewrite(context, shape) } else { - join_bounds(context, shape, it, false) + join_bounds_v2(context, shape, it, false) }; rw.map(|it_str| { let space = if it_str.is_empty() { "" } else { " " }; @@ -827,7 +835,66 @@ fn is_generic_bounds_in_order(generic_bounds: &[ast::GenericBound]) -> bool { } } -fn join_bounds( +fn join_bounds_v1( + context: &RewriteContext<'_>, + shape: Shape, + items: &[ast::GenericBound], + need_indent: bool, +) -> Option { + debug_assert!(!items.is_empty()); + + // Try to join types in a single line + let joiner = match context.config.type_punctuation_density() { + TypeDensity::Compressed => "+", + TypeDensity::Wide => " + ", + }; + let type_strs = items + .iter() + .map(|item| item.rewrite(context, shape)) + .collect::>>()?; + let result = type_strs.join(joiner); + if items.len() <= 1 || (!result.contains('\n') && result.len() <= shape.width) { + return Some(result); + } + + // We need to use multiple lines. + let (type_strs, offset) = if need_indent { + // Rewrite with additional indentation. + let nested_shape = shape + .block_indent(context.config.tab_spaces()) + .with_max_width(context.config); + let type_strs = items + .iter() + .map(|item| item.rewrite(context, nested_shape)) + .collect::>>()?; + (type_strs, nested_shape.indent) + } else { + (type_strs, shape.indent) + }; + + let is_bound_extendable = |s: &str, b: &ast::GenericBound| match b { + ast::GenericBound::Outlives(..) => true, + ast::GenericBound::Trait(..) => last_line_extendable(s), + }; + let mut result = String::with_capacity(128); + result.push_str(&type_strs[0]); + let mut can_be_put_on_the_same_line = is_bound_extendable(&result, &items[0]); + let generic_bounds_in_order = is_generic_bounds_in_order(items); + for (bound, bound_str) in items[1..].iter().zip(type_strs[1..].iter()) { + if generic_bounds_in_order && can_be_put_on_the_same_line { + result.push_str(joiner); + } else { + result.push_str(&offset.to_string_with_newline(context.config)); + result.push_str("+ "); + } + result.push_str(bound_str); + can_be_put_on_the_same_line = is_bound_extendable(bound_str, bound); + } + + Some(result) +} + +fn join_bounds_v2( context: &RewriteContext<'_>, shape: Shape, items: &[ast::GenericBound], diff --git a/tests/source/types_compressed.rs b/tests/source/issue-3669/type_compressed-one.rs similarity index 94% rename from tests/source/types_compressed.rs rename to tests/source/issue-3669/type_compressed-one.rs index 4254842d422..a6a4a9180a0 100644 --- a/tests/source/types_compressed.rs +++ b/tests/source/issue-3669/type_compressed-one.rs @@ -1,3 +1,4 @@ +// rustfmt-version: One // rustfmt-type_punctuation_density: Compressed pub fn do_something<'a, T: Trait1 + Trait2 + 'a>( diff --git a/tests/source/issue-3669/type_compressed-two.rs b/tests/source/issue-3669/type_compressed-two.rs new file mode 100644 index 00000000000..ce4a5c8920e --- /dev/null +++ b/tests/source/issue-3669/type_compressed-two.rs @@ -0,0 +1,8 @@ +// rustfmt-version: Two +// rustfmt-type_punctuation_density: Compressed + +pub fn do_something<'a, T: Trait1 + Trait2 + 'a>( + &fooo: u32, +) -> impl Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + Foo + 'a + 'a + 'a + 'a + 'a + { +} diff --git a/tests/source/issue-3669.rs b/tests/source/issue-3669/version-one.rs similarity index 99% rename from tests/source/issue-3669.rs rename to tests/source/issue-3669/version-one.rs index f9046c3d236..f2f1956b066 100644 --- a/tests/source/issue-3669.rs +++ b/tests/source/issue-3669/version-one.rs @@ -1,3 +1,5 @@ +// rustfmt-version: One + pub trait PCG: self::sealed::Sealed // comment1 + Sized // comment2 + Eq // comment3 diff --git a/tests/source/issue-3669/version-two.rs b/tests/source/issue-3669/version-two.rs new file mode 100644 index 00000000000..c8e873df7de --- /dev/null +++ b/tests/source/issue-3669/version-two.rs @@ -0,0 +1,84 @@ +// rustfmt-version: Two + +pub trait PCG: self::sealed::Sealed // comment1 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + + Shl // Note(Evrey): Because Rust is drunk. 3 ++ BitAnd // Note(Evrey): Because Rust is drunk. 4 ++ BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Bar: self::sealed::Sealed + // comment1 + Sized + // comment2 + Eq + // comment3 + Hash + // comment4 + Debug + // comment5 + Clone + // comment6 + Default + // comment7 + Serialize + // comment8 + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy + // Note(Evrey): Because Rust is drunk. 1 + ShrAssign + // Note(Evrey): Because Rust is drunk. 2 + Shl + // Note(Evrey): Because Rust is drunk. 3 +BitAnd + // Note(Evrey): Because Rust is drunk. 4 +BitOrAssign + // Note(Evrey): Because Rust is drunk. 5 + Sub + // Note(Evrey): Because Rust is drunk. 6 + Into + // Note(Evrey): Because Rust is drunk. 7 + Debug + // Note(Evrey): Because Rust is drunk. 8 + Eq + // Note(Evrey): Because Rust is drunk. 9 + Hash + // Note(Evrey): Because Rust is drunk. 10 + Default + // Note(Evrey): Because Rust is drunk. 11 + Serialize + // Note(Evrey): Because Rust is drunk. 12 + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Foo: self::sealed::Sealed + + Sized + + Eq + + Hash + + Debug + + Clone + + Default + + Serialize + + for<'a> Deserialize<'a> +{ + type DoubleState: Copy + + ShrAssign + + Shl ++ BitAnd ++ BitOrAssign + + Sub + + Into + + Debug + + Eq + + Hash + + Default + + Serialize + + for<'a> Deserialize<'a>; +} + +// #2055 +pub trait Foo: +// A and C +A + C +// and B + + B +{} diff --git a/tests/target/issue-3669/type_compressed-one.rs b/tests/target/issue-3669/type_compressed-one.rs new file mode 100644 index 00000000000..46e4a2e5fe9 --- /dev/null +++ b/tests/target/issue-3669/type_compressed-one.rs @@ -0,0 +1,45 @@ +// rustfmt-version: One +// rustfmt-type_punctuation_density: Compressed + +pub fn do_something<'a, T: Trait1+Trait2+'a>( + &fooo: u32, +) -> impl Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + Foo + + 'a+'a+'a+'a+'a { +} diff --git a/tests/target/issue-3669/type_compressed-two.rs b/tests/target/issue-3669/type_compressed-two.rs new file mode 100644 index 00000000000..224293f81cb --- /dev/null +++ b/tests/target/issue-3669/type_compressed-two.rs @@ -0,0 +1,45 @@ +// rustfmt-version: Two +// rustfmt-type_punctuation_density: Compressed + +pub fn do_something<'a, T: Trait1+Trait2+'a>( + &fooo: u32, +) -> impl Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++Foo ++'a+'a+'a+'a+'a { +} diff --git a/tests/target/issue-3669/version-one.rs b/tests/target/issue-3669/version-one.rs new file mode 100644 index 00000000000..173ec092b1a --- /dev/null +++ b/tests/target/issue-3669/version-one.rs @@ -0,0 +1,77 @@ +// rustfmt-version: One + +pub trait PCG: self::sealed::Sealed // comment1 + + Sized // comment2 + + Eq // comment3 + + Hash // comment4 + + Debug // comment5 + + Clone // comment6 + + Default // comment7 + + Serialize // comment8 + + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy // Note(Evrey): Because Rust is drunk. 1 + + ShrAssign // Note(Evrey): Because Rust is drunk. 2 + + Shl // Note(Evrey): Because Rust is drunk. 3 ++ BitAnd // Note(Evrey): Because Rust is drunk. 4 ++ BitOrAssign // Note(Evrey): Because Rust is drunk. 5 + + Sub // Note(Evrey): Because Rust is drunk. 6 + + Into // Note(Evrey): Because Rust is drunk. 7 + + Debug // Note(Evrey): Because Rust is drunk. 8 + + Eq // Note(Evrey): Because Rust is drunk. 9 + + Hash // Note(Evrey): Because Rust is drunk. 10 + + Default // Note(Evrey): Because Rust is drunk. 11 + + Serialize // Note(Evrey): Because Rust is drunk. 12 + + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Bar: self::sealed::Sealed + // comment1 + Sized + // comment2 + Eq + // comment3 + Hash + // comment4 + Debug + // comment5 + Clone + // comment6 + Default + // comment7 + Serialize + // comment8 + for<'a> Deserialize<'a> // comment9 +{ + type DoubleState: Copy + // Note(Evrey): Because Rust is drunk. 1 + ShrAssign + // Note(Evrey): Because Rust is drunk. 2 + Shl + // Note(Evrey): Because Rust is drunk. 3 +BitAnd + // Note(Evrey): Because Rust is drunk. 4 +BitOrAssign + // Note(Evrey): Because Rust is drunk. 5 + Sub + // Note(Evrey): Because Rust is drunk. 6 + Into + // Note(Evrey): Because Rust is drunk. 7 + Debug + // Note(Evrey): Because Rust is drunk. 8 + Eq + // Note(Evrey): Because Rust is drunk. 9 + Hash + // Note(Evrey): Because Rust is drunk. 10 + Default + // Note(Evrey): Because Rust is drunk. 11 + Serialize + // Note(Evrey): Because Rust is drunk. 12 + for<'a> Deserialize<'a>; // Note(Evrey): Because Rust is drunk. 13 +} + +pub trait Foo: + self::sealed::Sealed + + Sized + + Eq + + Hash + + Debug + + Clone + + Default + + Serialize + + for<'a> Deserialize<'a> +{ + type DoubleState: Copy + + ShrAssign + + Shl + + BitAnd + + BitOrAssign + + Sub + + Into + + Debug + + Eq + + Hash + + Default + + Serialize + + for<'a> Deserialize<'a>; +} diff --git a/tests/target/issue-3669.rs b/tests/target/issue-3669/version-two.rs similarity index 97% rename from tests/target/issue-3669.rs rename to tests/target/issue-3669/version-two.rs index 06e42ad9075..082fa8164e3 100644 --- a/tests/target/issue-3669.rs +++ b/tests/target/issue-3669/version-two.rs @@ -1,3 +1,5 @@ +// rustfmt-version: Two + pub trait PCG: self::sealed::Sealed // comment1 + Sized // comment2 @@ -75,3 +77,12 @@ pub trait Foo: + Serialize + for<'a> Deserialize<'a>; } + +// #2055 +pub trait Foo: +// A and C + A + + C // and B + + B +{ +} diff --git a/tests/target/trait.rs b/tests/target/trait.rs index 57e09759c28..620046a71b2 100644 --- a/tests/target/trait.rs +++ b/tests/target/trait.rs @@ -87,11 +87,10 @@ trait Y // comment // #2055 pub trait Foo: // A and C - A - + C // and B +A + C +// and B + B -{ -} +{} // #2158 trait Foo { diff --git a/tests/target/types_compressed.rs b/tests/target/types_compressed.rs deleted file mode 100644 index ff3bcf66c54..00000000000 --- a/tests/target/types_compressed.rs +++ /dev/null @@ -1,44 +0,0 @@ -// rustfmt-type_punctuation_density: Compressed - -pub fn do_something<'a, T: Trait1+Trait2+'a>( - &fooo: u32, -) -> impl Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +Foo - +'a+'a+'a+'a+'a { -}