From 54a4a629adb83f5428a8763ae509de7df3bc7a21 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 13 Dec 2020 17:32:28 +0200 Subject: [PATCH 1/9] Fix for issue #3847 - handle list item one-line multi post-comments --- src/formatting/lists.rs | 19 +++++++++++-- tests/source/issue-3847.rs | 55 ++++++++++++++++++++++++++++++++++++++ tests/target/issue-3847.rs | 53 ++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 tests/source/issue-3847.rs create mode 100644 tests/target/issue-3847.rs diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index 0532730a5ab..44a9825cdf8 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -444,7 +444,22 @@ where }; let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1); let offset = formatting.shape.indent + overhead; - let comment_shape = Shape::legacy(width, offset); + + /* >>>>>> [DBO] REPLACE next */ + //let comment_shape = Shape::legacy(width, offset); + // If `normalize_comments` is NOT set, second (and on) one-line comment should + // be formatted without the item's line indentation, since moving the separator + // to the front of the item make ithese post-comments unrelated to the item. + let comment_start_trimmed = comment.trim_start(); + let comment_shape = if comment_start_trimmed.starts_with("//") + && comment_start_trimmed.contains("\n") + && !formatting.config.normalize_comments() + { + formatting.shape + } else { + Shape::legacy(width, offset) + }; + /* <<<<<< [DBO] ADD */ // Use block-style only for the last item or multiline comments. let block_style = !formatting.ends_with_newline && last @@ -452,7 +467,7 @@ where || comment.trim().len() > width; rewrite_comment( - comment.trim_start(), + comment_start_trimmed, // >>> [DBO] REPLACED "comment.trim_start()," <<<< block_style, comment_shape, formatting.config, diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs new file mode 100644 index 00000000000..c3b059b4591 --- /dev/null +++ b/tests/source/issue-3847.rs @@ -0,0 +1,55 @@ +// Tests for multi one-line post-comments of list item +// (related cases when `normalize_comments` is set are already included in other test files). + +// Original cases from issue #3847 +type T1 = Result< + u32 // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + // diam ac cursus. Aliquam condimentum in erat quis pretium. + // accumsan urna. Cras volutpat sit amet quam. + , + bool, +>; +type T2 = Result< + u32, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + // diam ac cursus. Aliquam condimentum in erat quis pretium. + // accumsan urna. Cras volutpat sit amet quam. + bool, +>; + +// Additional test cases with lists +fn main() { + let a = ["GOOD" // Comment1 + // Comment2 + , + ]; + let b = ["WASBAD" // Comment1 + // Comment2 + , + "CCC", + ]; +} + +// Additional tests with multi-line comments +type T3_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + , + bool, +>; +type T4_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; + +fn main() { + let a = ["WASGOOD1" /* Comment1 + * Comment2 */ + , + "WASGOOD2", /* Comment1 + * Comment2 */ + "CCC", + ]; +} diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs new file mode 100644 index 00000000000..7be5ae05b2e --- /dev/null +++ b/tests/target/issue-3847.rs @@ -0,0 +1,53 @@ +// Tests for multi one-line post-comments of list item +// (related cases when `normalize_comments` is set are already included in other test files). + +// Original cases from issue #3847 +type T1 = Result< + u32, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + // diam ac cursus. Aliquam condimentum in erat quis pretium. + // accumsan urna. Cras volutpat sit amet quam. + bool, +>; +type T2 = Result< + u32, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + // diam ac cursus. Aliquam condimentum in erat quis pretium. + // accumsan urna. Cras volutpat sit amet quam. + bool, +>; + +// Additional test cases with lists +fn main() { + let a = [ + "GOOD", // Comment1 + // Comment2 + ]; + let b = [ + "WASBAD", // Comment1 + // Comment2 + "CCC", + ]; +} + +// Additional tests with multi-line comments +type T3_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T4_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; + +fn main() { + let a = [ + "WASGOOD1", /* Comment1 + * Comment2 */ + "WASGOOD2", /* Comment1 + * Comment2 */ + "CCC", + ]; +} From 54c7fe03de28c839cf585483317aa184e0ce8e25 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 20 Dec 2020 10:57:02 +0200 Subject: [PATCH 2/9] Fix issue #3847 - indentation of list item several one-line post-comments --- src/formatting/lists.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index 6c8fd363387..45ce2fa115d 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -445,21 +445,19 @@ where let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1); let offset = formatting.shape.indent + overhead; - /* >>>>>> [DBO] REPLACE next */ - //let comment_shape = Shape::legacy(width, offset); - // If `normalize_comments` is NOT set, second (and on) one-line comment should - // be formatted without the item's line indentation, since moving the separator - // to the front of the item make ithese post-comments unrelated to the item. + // If `normalize_comments` is not set, second and on one-line comments should + // be formatted without the item's line indentation, since the separator + // is moved right after the list item, making these (non-first) post-comments unrelated + // to the item. let comment_start_trimmed = comment.trim_start(); - let comment_shape = if comment_start_trimmed.starts_with("//") + let comment_shape = if !formatting.config.normalize_comments() + && comment_start_trimmed.starts_with("//") && comment_start_trimmed.contains("\n") - && !formatting.config.normalize_comments() { formatting.shape } else { Shape::legacy(width, offset) }; - /* <<<<<< [DBO] ADD */ // Use block-style only for the last item or multiline comments. let block_style = !formatting.ends_with_newline && last @@ -467,7 +465,7 @@ where || comment.trim().len() > width; rewrite_comment( - comment_start_trimmed, // >>> [DBO] REPLACED "comment.trim_start()," <<<< + comment_start_trimmed, block_style, comment_shape, formatting.config, From 51b347f1a01e0b6e7afeb83a2248a406e20b314f Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 20 Dec 2020 10:58:55 +0200 Subject: [PATCH 3/9] modify comment --- src/formatting/lists.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index 45ce2fa115d..11ef64da556 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -447,8 +447,8 @@ where // If `normalize_comments` is not set, second and on one-line comments should // be formatted without the item's line indentation, since the separator - // is moved right after the list item, making these (non-first) post-comments unrelated - // to the item. + // is moved right after the list item, making these (non-first) post-comments + // unrelated to the item. let comment_start_trimmed = comment.trim_start(); let comment_shape = if !formatting.config.normalize_comments() && comment_start_trimmed.starts_with("//") From 52449c0cb38c4fc4ff44f194fd38efeefe11566f Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 27 Dec 2020 18:45:07 +0200 Subject: [PATCH 4/9] Add handling of first one-line block-comment --- src/formatting/lists.rs | 18 ++++++++++++++---- tests/source/issue-3847.rs | 32 ++++++++++++++++++++++++++++++++ tests/target/issue-3847.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index 11ef64da556..fa8782dbe6e 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -450,14 +450,24 @@ where // is moved right after the list item, making these (non-first) post-comments // unrelated to the item. let comment_start_trimmed = comment.trim_start(); - let comment_shape = if !formatting.config.normalize_comments() + let first_comment_single_line = if !formatting.config.normalize_comments() && comment_start_trimmed.starts_with("//") - && comment_start_trimmed.contains("\n") { - formatting.shape + true + } else if comment_start_trimmed.starts_with("/*") { + match comment_start_trimmed.find("*/") { + Some(i) if !comment_start_trimmed[..i].contains('\n') => true, + _ => false, + } } else { - Shape::legacy(width, offset) + false }; + let comment_shape = + if first_comment_single_line && comment_start_trimmed.contains("\n") { + formatting.shape + } else { + Shape::legacy(width, offset) + }; // Use block-style only for the last item or multiline comments. let block_style = !formatting.ends_with_newline && last diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs index c3b059b4591..aaf5cd3f28a 100644 --- a/tests/source/issue-3847.rs +++ b/tests/source/issue-3847.rs @@ -53,3 +53,35 @@ fn main() { "CCC", ]; } + +// Addtional test with one-line block-comments +type T5_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + , + bool, +>; +type T6_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; + +// Addtional test with mix one-line and multi-linecomments +type T8_good = Result< + u32 // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + bool, +>; +type T9_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs index 7be5ae05b2e..9689c1aa6d3 100644 --- a/tests/target/issue-3847.rs +++ b/tests/target/issue-3847.rs @@ -51,3 +51,34 @@ fn main() { "CCC", ]; } + +// Addtional test with one-line block-comments +type T5_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T6_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; + +// Addtional test with mix one-line and multi-linecomments +type T8_good = Result< + u32, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T9_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; From 8c4ea726117508bdc985aca62a268665f23f6349 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Wed, 30 Dec 2020 13:47:20 +0200 Subject: [PATCH 5/9] Use some common comments functions --- src/formatting/lists.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index fa8782dbe6e..3b7b1ef5c32 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -8,7 +8,7 @@ use unicode_segmentation::UnicodeSegmentation; use crate::config::{lists::*, Config, IndentStyle}; use crate::formatting::{ - comment::{find_comment_end, rewrite_comment, FindUncommented}, + comment::{comment_style, find_comment_end, rewrite_comment, FindUncommented}, rewrite::RewriteContext, shape::{Indent, Shape}, utils::{ @@ -451,10 +451,15 @@ where // unrelated to the item. let comment_start_trimmed = comment.trim_start(); let first_comment_single_line = if !formatting.config.normalize_comments() - && comment_start_trimmed.starts_with("//") + && comment_style(comment_start_trimmed, false).is_line_comment() { true - } else if comment_start_trimmed.starts_with("/*") { + } else if comment_style( + comment_start_trimmed, + formatting.config.normalize_comments(), + ) + .is_block_comment() + { match comment_start_trimmed.find("*/") { Some(i) if !comment_start_trimmed[..i].contains('\n') => true, _ => false, From a5d9af700570ebbdb2192c4bcd7d9e06c76504e4 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Thu, 31 Dec 2020 11:40:18 +0200 Subject: [PATCH 6/9] Add handling of multi-line comment as first comment with debug info --- src/formatting/lists.rs | 163 +++++++++++++++++++++++++++++-------- tests/source/issue-3847.rs | 40 ++++++++- tests/target/issue-3847.rs | 39 ++++++++- 3 files changed, 203 insertions(+), 39 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index 3b7b1ef5c32..e05e309131d 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -445,49 +445,142 @@ where let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1); let offset = formatting.shape.indent + overhead; - // If `normalize_comments` is not set, second and on one-line comments should + // If `normalize_comments` is not set, second and on comments should // be formatted without the item's line indentation, since the separator // is moved right after the list item, making these (non-first) post-comments // unrelated to the item. let comment_start_trimmed = comment.trim_start(); - let first_comment_single_line = if !formatting.config.normalize_comments() - && comment_style(comment_start_trimmed, false).is_line_comment() - { - true - } else if comment_style( + debug!( + "** [DBO] write_list: is_line_comment={:?}, comment_start_trimmed={:?};", + comment_style(comment_start_trimmed, false).is_line_comment(), comment_start_trimmed, - formatting.config.normalize_comments(), - ) - .is_block_comment() - { - match comment_start_trimmed.find("*/") { - Some(i) if !comment_start_trimmed[..i].contains('\n') => true, - _ => false, - } - } else { - false - }; - let comment_shape = - if first_comment_single_line && comment_start_trimmed.contains("\n") { - formatting.shape + ); + // Find if first comment is single line and the end of multi-line block comment + let (first_comment_single_line, first_comment_end) = + if !formatting.config.normalize_comments() + && comment_style(comment_start_trimmed, false).is_line_comment() + { + (true, None) + } else if comment_style( + comment_start_trimmed, + formatting.config.normalize_comments(), + ) + .is_block_comment() + { + debug!("** [DBO] write_list: block comment;"); + match find_comment_end(&comment_start_trimmed) { + Some(i) => { + debug!("** [DBO] write_list: block comment end={:?};", i); + if comment_start_trimmed[..i].contains('\n') { + (false, Some(i)) + } else { + (true, None) + } + } + _ => (false, None), + } } else { - Shape::legacy(width, offset) + (false, None) }; - - // Use block-style only for the last item or multiline comments. - let block_style = !formatting.ends_with_newline && last - || comment.trim().contains('\n') - || comment.trim().len() > width; - - rewrite_comment( - comment_start_trimmed, - block_style, - comment_shape, - formatting.config, - ) + debug!( + "** [DBO] write_list: first_comment_single_line={:?}, first_comment_end={:?}, formatting.shape={:?}, Shape::legacy(width, offset)={:?};", + first_comment_single_line, + first_comment_end, + formatting.shape, + Shape::legacy(width, offset), + ); + // Properly indent comments + match (first_comment_single_line, first_comment_end) { + (_, None) => { + let comment_shape = + if first_comment_single_line && comment_start_trimmed.contains("\n") { + formatting.shape + } else { + Shape::legacy(width, offset) + }; + debug!("** [DBO] write_list: comment_shape={:?};", comment_shape); + // Use block-style only for the last item or multiline comments. + let block_style = !formatting.ends_with_newline && last + || comment.trim().contains('\n') + || comment.trim().len() > width; + rewrite_comment( + comment_start_trimmed, + block_style, + comment_shape, + formatting.config, + ) + } + (false, Some(comment_end)) => { + // Separate indentation for first multi-line block comment + let formatted_first_comment = rewrite_comment( + &comment_start_trimmed[..comment_end], + true, + Shape::legacy(width, offset), + formatting.config, + )?; + debug!( + "** [DBO] write_list: formatted_first_comment={:?};", + formatted_first_comment + ); + let second_comment_start = comment_start_trimmed[comment_end..] + .find(|c: char| !c.is_whitespace()) + .map_or(None, |i| Some(i + comment_end)); + debug!( + "** [DBO] write_list: second_comment_start={:?};", + second_comment_start + ); + let formatted_all_comments = match second_comment_start { + Some(i) => { + let second_comment = comment_start_trimmed[i..].to_string(); + // Use block-style only for the last item or multiline comments. + let block_style = !formatting.ends_with_newline && last + || second_comment.trim().contains('\n') + || second_comment.trim().len() > width; + let formatted = rewrite_comment( + &second_comment, + block_style, + formatting.shape, + formatting.config, + ); + debug!( + "** [DBO] write_list: formatted second comment={:?};", + formatted + ); + // Fined number of new lines between the first and other comments + // and combine the comments. + let comment_with_newlines = match formatted { + None => String::new(), + Some(c) => { + let newline_count = comment_start_trimmed[comment_end..i] + .split('\n') + .count(); + let gap = if newline_count > 1 { + formatting + .shape + .indent + .to_string_with_newline(formatting.config) + .repeat(newline_count - 1) + } else { + String::new() + }; + format!("{}{}{}", formatted_first_comment, gap, c) + } + }; + comment_with_newlines + } + _ => formatted_first_comment, + }; + Some(formatted_all_comments) + } + (_, _) => unreachable!(), + } }; let mut formatted_comment = rewrite_post_comment(&mut item_max_width)?; + debug!( + "** [DBO] write_list: formatted_comment1={:?};", + formatted_comment + ); // Multiline comments are not included in a previous "indentation group". // Each multiline comment is considered as a separate group. @@ -495,6 +588,10 @@ where item_max_width = None; formatted_comment = rewrite_post_comment(&mut item_max_width)?; } + debug!( + "** [DBO] write_list: formatted_comment2={:?};", + formatted_comment + ); if !starts_with_newline(comment) { if formatting.align_comments { diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs index aaf5cd3f28a..375e5e84efe 100644 --- a/tests/source/issue-3847.rs +++ b/tests/source/issue-3847.rs @@ -29,7 +29,7 @@ fn main() { ]; } -// Additional tests with multi-line comments +// Tests with one multi-line block comment type T3_good = Result< u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam * diam ac cursus. Aliquam condimentum in erat quis pretium. @@ -54,7 +54,7 @@ fn main() { ]; } -// Addtional test with one-line block-comments +// Tests with one-line block-comments type T5_good = Result< u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ @@ -69,7 +69,7 @@ type T6_good = Result< bool, >; -// Addtional test with mix one-line and multi-linecomments +// Tests with mix one-line and multi-linecomments - one-line is first type T8_good = Result< u32 // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam @@ -85,3 +85,37 @@ type T9_good = Result< // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam bool, >; +type T9_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; + +// Tests with mix one-line and multi-linecomments - multi-line is first +type T10_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + , + bool, +>; +type T11_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T12_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs index 9689c1aa6d3..ec0414153ba 100644 --- a/tests/target/issue-3847.rs +++ b/tests/target/issue-3847.rs @@ -28,7 +28,7 @@ fn main() { ]; } -// Additional tests with multi-line comments +// Tests with one multi-line block comment type T3_good = Result< u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam * diam ac cursus. Aliquam condimentum in erat quis pretium. @@ -52,7 +52,7 @@ fn main() { ]; } -// Addtional test with one-line block-comments +// Tests with one-line block-comments type T5_good = Result< u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ @@ -66,7 +66,7 @@ type T6_good = Result< bool, >; -// Addtional test with mix one-line and multi-linecomments +// Tests with mix one-line and multi-linecomments - one-line is first type T8_good = Result< u32, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam @@ -82,3 +82,36 @@ type T9_good = Result< // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam bool, >; +type T9_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; + +// Tests with mix one-line and multi-linecomments - multi-line is first +type T10_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T11_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T12_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; From 2d32877789d67838c286b75f55fcee18b23fd763 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Thu, 31 Dec 2020 14:50:14 +0200 Subject: [PATCH 7/9] Some enhancements and remove preserve of empty lines after first multiline comment --- src/formatting/lists.rs | 64 +++++--------------------------------- tests/source/issue-3847.rs | 50 ++++++++++++++++++++++++++--- tests/target/issue-3847.rs | 35 +++++++++++++++++++++ 3 files changed, 88 insertions(+), 61 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index e05e309131d..b131a0507aa 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -450,11 +450,6 @@ where // is moved right after the list item, making these (non-first) post-comments // unrelated to the item. let comment_start_trimmed = comment.trim_start(); - debug!( - "** [DBO] write_list: is_line_comment={:?}, comment_start_trimmed={:?};", - comment_style(comment_start_trimmed, false).is_line_comment(), - comment_start_trimmed, - ); // Find if first comment is single line and the end of multi-line block comment let (first_comment_single_line, first_comment_end) = if !formatting.config.normalize_comments() @@ -467,10 +462,8 @@ where ) .is_block_comment() { - debug!("** [DBO] write_list: block comment;"); match find_comment_end(&comment_start_trimmed) { Some(i) => { - debug!("** [DBO] write_list: block comment end={:?};", i); if comment_start_trimmed[..i].contains('\n') { (false, Some(i)) } else { @@ -482,13 +475,7 @@ where } else { (false, None) }; - debug!( - "** [DBO] write_list: first_comment_single_line={:?}, first_comment_end={:?}, formatting.shape={:?}, Shape::legacy(width, offset)={:?};", - first_comment_single_line, - first_comment_end, - formatting.shape, - Shape::legacy(width, offset), - ); + // Properly indent comments match (first_comment_single_line, first_comment_end) { (_, None) => { @@ -498,7 +485,6 @@ where } else { Shape::legacy(width, offset) }; - debug!("** [DBO] write_list: comment_shape={:?};", comment_shape); // Use block-style only for the last item or multiline comments. let block_style = !formatting.ends_with_newline && last || comment.trim().contains('\n') @@ -518,17 +504,9 @@ where Shape::legacy(width, offset), formatting.config, )?; - debug!( - "** [DBO] write_list: formatted_first_comment={:?};", - formatted_first_comment - ); let second_comment_start = comment_start_trimmed[comment_end..] .find(|c: char| !c.is_whitespace()) .map_or(None, |i| Some(i + comment_end)); - debug!( - "** [DBO] write_list: second_comment_start={:?};", - second_comment_start - ); let formatted_all_comments = match second_comment_start { Some(i) => { let second_comment = comment_start_trimmed[i..].to_string(); @@ -541,32 +519,12 @@ where block_style, formatting.shape, formatting.config, - ); - debug!( - "** [DBO] write_list: formatted second comment={:?};", - formatted - ); - // Fined number of new lines between the first and other comments - // and combine the comments. - let comment_with_newlines = match formatted { - None => String::new(), - Some(c) => { - let newline_count = comment_start_trimmed[comment_end..i] - .split('\n') - .count(); - let gap = if newline_count > 1 { - formatting - .shape - .indent - .to_string_with_newline(formatting.config) - .repeat(newline_count - 1) - } else { - String::new() - }; - format!("{}{}{}", formatted_first_comment, gap, c) - } - }; - comment_with_newlines + )?; + let indent = formatting + .shape + .indent + .to_string_with_newline(formatting.config); + format!("{}{}{}", formatted_first_comment, indent, formatted) } _ => formatted_first_comment, }; @@ -577,10 +535,6 @@ where }; let mut formatted_comment = rewrite_post_comment(&mut item_max_width)?; - debug!( - "** [DBO] write_list: formatted_comment1={:?};", - formatted_comment - ); // Multiline comments are not included in a previous "indentation group". // Each multiline comment is considered as a separate group. @@ -588,10 +542,6 @@ where item_max_width = None; formatted_comment = rewrite_post_comment(&mut item_max_width)?; } - debug!( - "** [DBO] write_list: formatted_comment2={:?};", - formatted_comment - ); if !starts_with_newline(comment) { if formatting.align_comments { diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs index 375e5e84efe..545c00de149 100644 --- a/tests/source/issue-3847.rs +++ b/tests/source/issue-3847.rs @@ -81,8 +81,9 @@ type T9_good = Result< u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam * diam ac cursus. Aliquam condimentum in erat quis pretium. - * accumsan urna. Cras volutpat sit amet quam. */, + * accumsan urna. Cras volutpat sit amet quam. */ // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + , bool, >; type T9_good = Result< @@ -106,16 +107,57 @@ type T10_good = Result< type T11_good = Result< u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam * diam ac cursus. Aliquam condimentum in erat quis pretium. - * accumsan urna. Cras volutpat sit amet quam. */, - /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */, // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam bool, >; type T12_good = Result< u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam * diam ac cursus. Aliquam condimentum in erat quis pretium. - * accumsan urna. Cras volutpat sit amet quam. */, + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + , + bool, +>; +type T12_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */, + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T13_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. */ /*Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + , + bool, +>; + +// Tests with mix one-line and multi-linecomments - +// multi-line is first with newline between comments +type T14_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */, + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T15_good = Result< + u32 /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + , bool, >; diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs index ec0414153ba..15be78be51c 100644 --- a/tests/target/issue-3847.rs +++ b/tests/target/issue-3847.rs @@ -115,3 +115,38 @@ type T12_good = Result< // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam bool, >; +type T12_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T13_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. */ + /*Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + bool, +>; + +// Tests with mix one-line and multi-linecomments - +// multi-line is first with newline between comments +type T14_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T15_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; From 0543a4af21cdaa68ddb015313d3600e6725b2ff9 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Mon, 4 Jan 2021 12:11:56 +0200 Subject: [PATCH 8/9] Some refactoring and added comments and test cases --- src/formatting/lists.rs | 88 +++++++++++++++++++++++--------------- tests/source/issue-3847.rs | 17 ++++++++ tests/target/issue-3847.rs | 14 ++++++ 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index b131a0507aa..d8304ee08b2 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -421,6 +421,30 @@ where result.push_str(formatting.separator); } + // Note about post-comments indentation: + // In the original code the the item separator may follow some comments that + // may span over some lines. E.g.: + // item1 /* 1st comment */ + // /* 2nd comment */, + // item2, + // + // In this case, rustfmt move the separator right after the item: + // item1, /* 1st comment */ + // /* 2nd comment */ + // item2, + // + // In this code, only the 1st comment is regarded as post comment of item1. + // 2nd comment is regarded as pre-comment of item2, therefore the output + // of formatting this code is: + // item1, /* 1st comment */ + // /* 2nd comment */ + // item2, + // + // i.e. 2nd comment is now indented as pre-comment. + // + // This why in the code below the first post-comment is indented differently + // then the other comments. + if tactic != DefinitiveListTactic::Horizontal && item.post_comment.is_some() { let comment = item.post_comment.as_ref().unwrap(); let overhead = last_line_width(&result) + first_line_width(comment.trim()); @@ -444,57 +468,57 @@ where }; let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1); let offset = formatting.shape.indent + overhead; - - // If `normalize_comments` is not set, second and on comments should - // be formatted without the item's line indentation, since the separator - // is moved right after the list item, making these (non-first) post-comments - // unrelated to the item. let comment_start_trimmed = comment.trim_start(); - // Find if first comment is single line and the end of multi-line block comment + + // Find if first comment is single line and the end of the first comment + // when it is a multi-line block comment (since the first post-comment + // is added to the same line of the list item, its indentation is important + // only when it is a multiline comment). + let style = comment_style( + comment_start_trimmed, + formatting.config.normalize_comments(), + ); let (first_comment_single_line, first_comment_end) = - if !formatting.config.normalize_comments() - && comment_style(comment_start_trimmed, false).is_line_comment() - { + if !formatting.config.normalize_comments() && style.is_line_comment() { + // Line comment (not normalizaed) (true, None) - } else if comment_style( - comment_start_trimmed, - formatting.config.normalize_comments(), - ) - .is_block_comment() - { + } else if style.is_block_comment() { match find_comment_end(&comment_start_trimmed) { Some(i) => { if comment_start_trimmed[..i].contains('\n') { + // Multiline-comment (may be because of normalization) (false, Some(i)) } else { + // One line coment (Block or normalizaed Line) (true, None) } } - _ => (false, None), + _ => (false, None), // Unknow comment end } } else { - (false, None) + (false, None) // Unexpected case - non-block comment with normalization }; - // Properly indent comments + // Closure for formatting post-comment with specific Shape + let rewrite_post_comment_with_shape = |cmt: &str, shape: Shape| { + // Use block-style only for the last item or multiline comments. + let block_style = !formatting.ends_with_newline && last + || cmt.trim().contains('\n') + || cmt.trim().len() > width; + rewrite_comment(&cmt, block_style, shape, formatting.config) + }; + + // Properly indent first and other comments match (first_comment_single_line, first_comment_end) { (_, None) => { + // First comment not multiline - same indentation for all comments let comment_shape = if first_comment_single_line && comment_start_trimmed.contains("\n") { formatting.shape } else { Shape::legacy(width, offset) }; - // Use block-style only for the last item or multiline comments. - let block_style = !formatting.ends_with_newline && last - || comment.trim().contains('\n') - || comment.trim().len() > width; - rewrite_comment( - comment_start_trimmed, - block_style, - comment_shape, - formatting.config, - ) + rewrite_post_comment_with_shape(comment_start_trimmed, comment_shape) } (false, Some(comment_end)) => { // Separate indentation for first multi-line block comment @@ -510,15 +534,9 @@ where let formatted_all_comments = match second_comment_start { Some(i) => { let second_comment = comment_start_trimmed[i..].to_string(); - // Use block-style only for the last item or multiline comments. - let block_style = !formatting.ends_with_newline && last - || second_comment.trim().contains('\n') - || second_comment.trim().len() > width; - let formatted = rewrite_comment( + let formatted = rewrite_post_comment_with_shape( &second_comment, - block_style, formatting.shape, - formatting.config, )?; let indent = formatting .shape diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs index 545c00de149..b09b64da4a3 100644 --- a/tests/source/issue-3847.rs +++ b/tests/source/issue-3847.rs @@ -161,3 +161,20 @@ type T15_good = Result< , bool, >; + +// Tests with first comment is not in same line of item +type T16_good = Result< + u32 + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */, + bool, +>; +type T17_good = Result< +u32 +/* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ +/* diam ac cursus. Aliquam condimentum in erat quis pretium. */ +/* accumsan urna. Cras volutpat sit amet quam. */ +, +bool, +>; diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs index 15be78be51c..267c1956fda 100644 --- a/tests/target/issue-3847.rs +++ b/tests/target/issue-3847.rs @@ -150,3 +150,17 @@ type T15_good = Result< // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam bool, >; + +// Tests with first comment is not in same line of item +type T16_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T17_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; From b713afdc41524de595353de5d9e6f0c514e0f5ae Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Mon, 4 Jan 2021 14:54:00 +0200 Subject: [PATCH 9/9] Fix for the case when the first post-comment is in new line --- src/formatting/lists.rs | 24 ++++++++++++++++-------- tests/source/issue-3847.rs | 25 +++++++++++++++++++++++++ tests/target/issue-3847.rs | 22 ++++++++++++++++++++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/formatting/lists.rs b/src/formatting/lists.rs index d8304ee08b2..1dcd7044d5d 100644 --- a/src/formatting/lists.rs +++ b/src/formatting/lists.rs @@ -424,26 +424,29 @@ where // Note about post-comments indentation: // In the original code the the item separator may follow some comments that // may span over some lines. E.g.: - // item1 /* 1st comment */ - // /* 2nd comment */, + // item1 /* 1st comment line 1 + // * line 2 */ + // /* 2nd comment */, // item2, // - // In this case, rustfmt move the separator right after the item: - // item1, /* 1st comment */ + // In this case, rustfmt moves the separator right after the item: + // item1, /* 1st comment line 1 + // * line 2 */ // /* 2nd comment */ // item2, // // In this code, only the 1st comment is regarded as post comment of item1. // 2nd comment is regarded as pre-comment of item2, therefore the output - // of formatting this code is: - // item1, /* 1st comment */ + // of another round of formatting this code is: + // item1, /* 1st comment line 1 + // * line 2 */ // /* 2nd comment */ // item2, // // i.e. 2nd comment is now indented as pre-comment. // - // This why in the code below the first post-comment is indented differently - // then the other comments. + // This why in the code below a first multiline post-comment is indented + // differently then the other post-comments. if tactic != DefinitiveListTactic::Horizontal && item.post_comment.is_some() { let comment = item.post_comment.as_ref().unwrap(); @@ -792,6 +795,11 @@ pub(crate) fn get_comment_end( find_comment_end(&post_snippet[i..]).unwrap() + i, separator_index + 1, ), + // Comment is preceeded by new line and followed by a separator. + (Some(i), Some(_)) if separator_index > i => cmp::max( + find_comment_end(&post_snippet[i..]).unwrap() + i, + separator_index + 1, + ), // Potential *single* line comment. (_, Some(j)) if j > separator_index => j + 1, _ => post_snippet.len(), diff --git a/tests/source/issue-3847.rs b/tests/source/issue-3847.rs index b09b64da4a3..e4440efa23a 100644 --- a/tests/source/issue-3847.rs +++ b/tests/source/issue-3847.rs @@ -178,3 +178,28 @@ u32 , bool, >; +type T18_good = Result< + u32 + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */, + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T19_good = Result< + u32 + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */, + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T20_good = Result< + u32 + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */, + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; diff --git a/tests/target/issue-3847.rs b/tests/target/issue-3847.rs index 267c1956fda..5a1b3618368 100644 --- a/tests/target/issue-3847.rs +++ b/tests/target/issue-3847.rs @@ -164,3 +164,25 @@ type T17_good = Result< /* accumsan urna. Cras volutpat sit amet quam. */ bool, >; +type T18_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + /* diam ac cursus. Aliquam condimentum in erat quis pretium. */ + /* accumsan urna. Cras volutpat sit amet quam. */ + bool, +>; +type T19_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>; +type T20_good = Result< + u32, /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + * diam ac cursus. Aliquam condimentum in erat quis pretium. + * accumsan urna. Cras volutpat sit amet quam. */ + /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam */ + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam + bool, +>;