Determine when new comment lines are needed for itemized blocks

ytmimi · calebcartwright · commit ec46ffd981d3 · 2021-12-01T18:31:10.000-06:00
Fixes 5088

Previously, rustfmt would add a new comment line anytime it reformatted
an itemized block within a comment when ``wrap_comments=true``. This
would lead to rustfmt adding empty comments with trailing whitespace.

Now, new comment lines are only added if the original comment spanned
multiple lines, if the comment needs to be wrapped, or if the comment
originally started with an empty comment line.
diff --git a/src/comment.rs b/src/comment.rs
@@ -519,6 +519,7 @@ struct CommentRewrite<'a> {
     opener: String,
     closer: String,
     line_start: String,
+    style: CommentStyle<'a>,
 }
 
 impl<'a> CommentRewrite<'a> {
@@ -528,10 +529,14 @@ impl<'a> CommentRewrite<'a> {
         shape: Shape,
         config: &'a Config,
     ) -> CommentRewrite<'a> {
-        let (opener, closer, line_start) = if block_style {
-            CommentStyle::SingleBullet.to_str_tuplet()
+        let ((opener, closer, line_start), style) = if block_style {
+            (
+                CommentStyle::SingleBullet.to_str_tuplet(),
+                CommentStyle::SingleBullet,
+            )
         } else {
-            comment_style(orig, config.normalize_comments()).to_str_tuplet()
+            let style = comment_style(orig, config.normalize_comments());
+            (style.to_str_tuplet(), style)
         };
 
         let max_width = shape
@@ -564,6 +569,7 @@ impl<'a> CommentRewrite<'a> {
             opener: opener.to_owned(),
             closer: closer.to_owned(),
             line_start: line_start.to_owned(),
+            style,
         };
         cr.result.push_str(opener);
         cr
@@ -583,6 +589,15 @@ impl<'a> CommentRewrite<'a> {
         result
     }
 
+    /// Check if any characters were written to the result buffer after the start of the comment.
+    /// when calling [`CommentRewrite::new()`] the result buffer is initiazlied with the opening
+    /// characters for the comment.
+    fn buffer_contains_comment(&self) -> bool {
+        // if self.result.len() < self.opener.len() then an empty comment is in the buffer
+        // if self.result.len() > self.opener.len() then a non empty comment is in the buffer
+        self.result.len() != self.opener.len()
+    }
+
     fn finish(mut self) -> String {
         if !self.code_block_buffer.is_empty() {
             // There is a code block that is not properly enclosed by backticks.
@@ -598,7 +613,12 @@ impl<'a> CommentRewrite<'a> {
             // the last few lines are part of an itemized block
             self.fmt.shape = Shape::legacy(self.max_width, self.fmt_indent);
             let item_fmt = ib.create_string_format(&self.fmt);
-            self.result.push_str(&self.comment_line_separator);
+
+            // only push a comment_line_separator for ItemizedBlocks if the comment is not empty
+            if self.buffer_contains_comment() {
+                self.result.push_str(&self.comment_line_separator);
+            }
+
             self.result.push_str(&ib.opener);
             match rewrite_string(
                 &ib.trimmed_block_as_string(),
@@ -632,7 +652,13 @@ impl<'a> CommentRewrite<'a> {
         line: &'a str,
         has_leading_whitespace: bool,
     ) -> bool {
-        let is_last = i == count_newlines(orig);
+        let num_newlines = count_newlines(orig);
+        let is_last = i == num_newlines;
+        let needs_new_comment_line = if self.style.is_block_comment() {
+            num_newlines > 0 || self.buffer_contains_comment()
+        } else {
+            self.buffer_contains_comment()
+        };
 
         if let Some(ref mut ib) = self.item_block {
             if ib.add_line(line) {
@@ -641,7 +667,12 @@ impl<'a> CommentRewrite<'a> {
             self.is_prev_line_multi_line = false;
             self.fmt.shape = Shape::legacy(self.max_width, self.fmt_indent);
             let item_fmt = ib.create_string_format(&self.fmt);
-            self.result.push_str(&self.comment_line_separator);
+
+            // only push a comment_line_separator if we need to start a new comment line
+            if needs_new_comment_line {
+                self.result.push_str(&self.comment_line_separator);
+            }
+
             self.result.push_str(&ib.opener);
             match rewrite_string(
                 &ib.trimmed_block_as_string(),
diff --git a/tests/source/issue-5088/deeply_nested_long_comment_wrap_comments_true.rs b/tests/source/issue-5088/deeply_nested_long_comment_wrap_comments_true.rs
@@ -0,0 +1,33 @@
+// rustfmt-wrap_comments: true
+
+fn main() {
+    {
+        {
+            {
+                {
+                    {
+                        {
+                            {
+                                {
+                                    {
+                                        {
+                                            {
+                                                // - aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc
+
+                                                // * aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc
+
+                                                /* - aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc */
+
+                                                /* * aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc */
+                                            };
+                                        };
+                                    };
+                                };
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
+}
diff --git a/tests/source/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_true.rs b/tests/source/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_true.rs
@@ -0,0 +1,19 @@
+// rustfmt-wrap_comments: true
+
+//
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+//
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+/*
+ * - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/*
+ * - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+
+/*
+ * * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/*
+ * * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
diff --git a/tests/source/issue-5088/very_long_comment_wrap_comments_true.rs b/tests/source/issue-5088/very_long_comment_wrap_comments_true.rs
@@ -0,0 +1,13 @@
+// rustfmt-wrap_comments: true
+
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+/* - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/* - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+
+/* * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/* * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
diff --git a/tests/target/issue-5088/deeply_nested_long_comment_wrap_comments_false.rs b/tests/target/issue-5088/deeply_nested_long_comment_wrap_comments_false.rs
@@ -0,0 +1,33 @@
+// rustfmt-wrap_comments: false
+
+fn main() {
+    {
+        {
+            {
+                {
+                    {
+                        {
+                            {
+                                {
+                                    {
+                                        {
+                                            {
+                                                // - aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc
+
+                                                // * aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc
+
+                                                /* - aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc */
+
+                                                /* * aaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa aaaaaaaaa bbbbbbbbbb bbbbbbbbb bbbbbbbbb ccc cccccccccc ccccccc cccccccc */
+                                            };
+                                        };
+                                    };
+                                };
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
+}
diff --git a/tests/target/issue-5088/deeply_nested_long_comment_wrap_comments_true.rs b/tests/target/issue-5088/deeply_nested_long_comment_wrap_comments_true.rs
@@ -0,0 +1,49 @@
+// rustfmt-wrap_comments: true
+
+fn main() {
+    {
+        {
+            {
+                {
+                    {
+                        {
+                            {
+                                {
+                                    {
+                                        {
+                                            {
+                                                // - aaaa aaaaaaaaa aaaaaaaaa
+                                                //   aaaaaaaaa aaaaaaaaa
+                                                //   bbbbbbbbbb bbbbbbbbb
+                                                //   bbbbbbbbb ccc cccccccccc
+                                                //   ccccccc cccccccc
+
+                                                // * aaaa aaaaaaaaa aaaaaaaaa
+                                                //   aaaaaaaaa aaaaaaaaa
+                                                //   bbbbbbbbbb bbbbbbbbb
+                                                //   bbbbbbbbb ccc cccccccccc
+                                                //   ccccccc cccccccc
+
+                                                /* - aaaa aaaaaaaaa aaaaaaaaa
+                                                 *   aaaaaaaaa aaaaaaaaa
+                                                 *   bbbbbbbbbb bbbbbbbbb
+                                                 *   bbbbbbbbb ccc cccccccccc
+                                                 *   ccccccc cccccccc */
+
+                                                /* * aaaa aaaaaaaaa aaaaaaaaa
+                                                 *   aaaaaaaaa aaaaaaaaa
+                                                 *   bbbbbbbbbb bbbbbbbbb
+                                                 *   bbbbbbbbb ccc cccccccccc
+                                                 *   ccccccc cccccccc */
+                                            };
+                                        };
+                                    };
+                                };
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
+}
diff --git a/tests/target/issue-5088/multi_line_itemized_block_wrap_comments_false.rs b/tests/target/issue-5088/multi_line_itemized_block_wrap_comments_false.rs
@@ -0,0 +1,17 @@
+// rustfmt-wrap_comments: false
+
+// - some itemized block 1
+// - some itemized block 2
+
+// * some itemized block 3
+// * some itemized block 4
+
+/*
+ * - some itemized block 5
+ * - some itemized block 6
+ */
+
+/*
+ * * some itemized block 7
+ * * some itemized block 8
+ */
diff --git a/tests/target/issue-5088/multi_line_itemized_block_wrap_comments_true.rs b/tests/target/issue-5088/multi_line_itemized_block_wrap_comments_true.rs
@@ -0,0 +1,17 @@
+// rustfmt-wrap_comments: true
+
+// - some itemized block 1
+// - some itemized block 2
+
+// * some itemized block 3
+// * some itemized block 4
+
+/*
+ * - some itemized block 5
+ * - some itemized block 6
+ */
+
+/*
+ * * some itemized block 7
+ * * some itemized block 8
+ */
diff --git a/tests/target/issue-5088/multi_line_text_with_itemized_block_wrap_comments_false.rs b/tests/target/issue-5088/multi_line_text_with_itemized_block_wrap_comments_false.rs
@@ -0,0 +1,37 @@
+// rustfmt-wrap_comments: false
+
+// Some text
+// - some itemized block 1
+// - some itemized block 2
+// Some more text
+// - some itemized block 3
+// - some itemized block 4
+// Even more text
+
+// Some text
+// * some itemized block 5
+// * some itemized block 6
+// Some more text
+// * some itemized block 7
+// * some itemized block 8
+// Even more text
+
+/*
+ * Some text
+ * - some itemized block 9
+ * - some itemized block 10
+ * Some more text
+ * - some itemized block 11
+ * - some itemized block 12
+ * Even more text
+ */
+
+/*
+ * Some text
+ * * some itemized block 13
+ * * some itemized block 14
+ * Some more text
+ * * some itemized block 15
+ * * some itemized block 16
+ * Even more text
+ */
diff --git a/tests/target/issue-5088/multi_line_text_with_itemized_block_wrap_comments_true.rs b/tests/target/issue-5088/multi_line_text_with_itemized_block_wrap_comments_true.rs
@@ -0,0 +1,37 @@
+// rustfmt-wrap_comments: true
+
+// Some text
+// - some itemized block 1
+// - some itemized block 2
+// Some more text
+// - some itemized block 3
+// - some itemized block 4
+// Even more text
+
+// Some text
+// * some itemized block 5
+// * some itemized block 6
+// Some more text
+// * some itemized block 7
+// * some itemized block 8
+// Even more text
+
+/*
+ * Some text
+ * - some itemized block 9
+ * - some itemized block 10
+ * Some more text
+ * - some itemized block 11
+ * - some itemized block 12
+ * Even more text
+ */
+
+/*
+ * Some text
+ * * some itemized block 13
+ * * some itemized block 14
+ * Some more text
+ * * some itemized block 15
+ * * some itemized block 16
+ * Even more text
+ */
diff --git a/tests/target/issue-5088/single_line_itemized_block_wrap_comments_false.rs b/tests/target/issue-5088/single_line_itemized_block_wrap_comments_false.rs
@@ -0,0 +1,9 @@
+// rustfmt-wrap_comments: false
+
+// - some itemized block 1
+
+// * some itemized block 2
+
+/* - some itemized block 3 */
+
+/* * some itemized block 4 */
diff --git a/tests/target/issue-5088/single_line_itemized_block_wrap_comments_true.rs b/tests/target/issue-5088/single_line_itemized_block_wrap_comments_true.rs
@@ -0,0 +1,9 @@
+// rustfmt-wrap_comments: true
+
+// - some itemized block 1
+
+// * some itemized block 2
+
+/* - some itemized block 3 */
+
+/* * some itemized block 4 */
diff --git a/tests/target/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_false.rs b/tests/target/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_false.rs
@@ -0,0 +1,19 @@
+// rustfmt-wrap_comments: false
+
+//
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+//
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+// * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+
+/*
+ * - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/*
+ * - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+
+/*
+ * * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
+/*
+ * * Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.*/
diff --git a/tests/target/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_true.rs b/tests/target/issue-5088/start_with_empty_comment_very_long_itemized_block_wrap_comments_true.rs
diff --git a/tests/target/issue-5088/start_with_empty_comment_wrap_comments_false.rs b/tests/target/issue-5088/start_with_empty_comment_wrap_comments_false.rs
diff --git a/tests/target/issue-5088/start_with_empty_comment_wrap_comments_true.rs b/tests/target/issue-5088/start_with_empty_comment_wrap_comments_true.rs
diff --git a/tests/target/issue-5088/very_long_comment_wrap_comments_false.rs b/tests/target/issue-5088/very_long_comment_wrap_comments_false.rs
diff --git a/tests/target/issue-5088/very_long_comment_wrap_comments_true.rs b/tests/target/issue-5088/very_long_comment_wrap_comments_true.rs
