Merge pull request #1769 from topecongiro/comment-vertical-alignment

Comment vertical alignment

src/bin/cargo-fmt.rs

@@ -142,14 +142,14 @@ fn get_fmt_args() -> Vec<String> {
 
 #[derive(Debug)]
 enum TargetKind {
-    Lib, // dylib, staticlib, lib
-    Bin, // bin
-    Example, // example file
-    Test, // test file
-    Bench, // bench file
+    Lib,         // dylib, staticlib, lib
+    Bin,         // bin
+    Example,     // example file
+    Test,        // test file
+    Bench,       // bench file
     CustomBuild, // build script
-    ProcMacro, // a proc macro implementation
-    Other, // plugin,...
+    ProcMacro,   // a proc macro implementation
+    Other,       // plugin,...
 }
 
 impl TargetKind {

src/expr.rs

@@ -1298,9 +1298,7 @@ fn rewrite_cond(
                 label_string,
                 self.keyword,
                 between_kwd_cond_comment.as_ref().map_or(
-                    if pat_expr_string.is_empty() ||
-                        pat_expr_string.starts_with('\n')
-                    {
+                    if pat_expr_string.is_empty() || pat_expr_string.starts_with('\n') {
                         ""
                     } else {
                         " "
@@ -1707,7 +1705,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
             ends_with_newline: false,
             config: context.config,
         };
-        let pats_str = try_opt!(write_list(items, &fmt));
+        let pats_str = try_opt!(write_list(&items, &fmt));
 
         let guard_shape = if pats_str.contains('\n') {
             shape.with_max_width(context.config)

src/items.rs

@@ -478,7 +478,7 @@ fn format_variant_list(
             config: self.config,
         };
 
-        let list = try_opt!(write_list(items, &fmt));
+        let list = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
         result.push_str(&list);
         result.push('\n');
         Some(result)
@@ -2539,7 +2539,7 @@ fn rewrite_where_clause_rfc_style(
         ends_with_newline: true,
         config: context.config,
     };
-    let preds_str = try_opt!(write_list(items, &fmt));
+    let preds_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
     Some(format!(
         "{}where\n{}{}",

src/lists.rs

@@ -17,7 +17,7 @@
 use comment::{find_comment_end, rewrite_comment, FindUncommented};
 use config::{Config, IndentStyle};
 use rewrite::RewriteContext;
-use utils::mk_sp;
+use utils::{first_line_width, last_line_width, mk_sp};
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
 /// Formatting tactic for lists. This will be cast down to a
@@ -86,8 +86,12 @@ pub struct ListItem {
 }
 
 impl ListItem {
+    pub fn inner_as_ref(&self) -> &str {
+        self.item.as_ref().map_or("", |s| &*s)
+    }
+
     pub fn is_multiline(&self) -> bool {
-        self.item.as_ref().map_or(false, |s| s.contains('\n')) || self.pre_comment.is_some() ||
+        self.inner_as_ref().contains('\n') || self.pre_comment.is_some() ||
             self.post_comment
                 .as_ref()
                 .map_or(false, |s| s.contains('\n'))
@@ -157,7 +161,7 @@ pub fn definitive_tactic<I, T>(items: I, tactic: ListTactic, width: usize) -> De
 // TODO: add unit tests
 pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
 where
-    I: IntoIterator<Item = T>,
+    I: IntoIterator<Item = T> + Clone,
     T: AsRef<ListItem>,
 {
     let tactic = formatting.tactic;
@@ -167,7 +171,9 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
     // will be a trailing separator.
     let trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic);
     let mut result = String::new();
+    let cloned_items = items.clone();
     let mut iter = items.into_iter().enumerate().peekable();
+    let mut item_max_width: Option<usize> = None;
 
     let mut line_len = 0;
     let indent_str = &formatting.shape.indent.to_string(formatting.config);
@@ -238,6 +244,7 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
             } else {
                 result.push(' ');
             }
+            item_max_width = None;
         }
 
         result.push_str(&inner_item[..]);
@@ -261,36 +268,66 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
         }
 
         if tactic == DefinitiveListTactic::Vertical && item.post_comment.is_some() {
-            // 1 = space between item and comment.
-            let width = formatting
-                .shape
-                .width
-                .checked_sub(item_last_line_width + 1)
-                .unwrap_or(1);
-            let mut offset = formatting.shape.indent;
-            offset.alignment += item_last_line_width + 1;
             let comment = item.post_comment.as_ref().unwrap();
+            let overhead = last_line_width(&result) + first_line_width(comment.trim());
 
-            debug!("Width = {}, offset = {:?}", 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;
+            let rewrite_post_comment = |item_max_width: &mut Option<usize>| {
+                if item_max_width.is_none() && !last && !inner_item.contains('\n') {
+                    *item_max_width = Some(max_width_of_item_with_post_comment(
+                        &cloned_items,
+                        i,
+                        overhead,
+                        formatting.config.max_width(),
+                    ));
+                }
+                let overhead = if let &mut Some(max_width) = item_max_width {
+                    max_width + 2
+                } else {
+                    // 1 = space between item and comment.
+                    item_last_line_width + 1
+                };
+                let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1);
+                let offset = formatting.shape.indent + overhead;
+                let comment_shape = Shape::legacy(width, offset);
 
-            let formatted_comment = try_opt!(rewrite_comment(
-                comment,
-                block_style,
-                Shape::legacy(width, offset),
-                formatting.config,
-            ));
+                // 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, block_style, comment_shape, formatting.config)
+            };
+
+            let mut formatted_comment = try_opt!(rewrite_post_comment(&mut item_max_width));
 
             if !formatted_comment.starts_with('\n') {
-                result.push(' ');
+                let mut comment_alignment =
+                    post_comment_alignment(item_max_width, inner_item.len());
+                if first_line_width(&formatted_comment) + last_line_width(&result) +
+                    comment_alignment + 1 > formatting.config.max_width()
+                {
+                    item_max_width = None;
+                    formatted_comment = try_opt!(rewrite_post_comment(&mut item_max_width));
+                    comment_alignment = post_comment_alignment(item_max_width, inner_item.len());
+                }
+                for _ in 0..(comment_alignment + 1) {
+                    result.push(' ');
+                }
+                // An additional space for the missing trailing comma
+                if last && item_max_width.is_some() && !separate {
+                    result.push(' ');
+                }
+            }
+            if formatted_comment.contains('\n') {
+                item_max_width = None;
             }
             result.push_str(&formatted_comment);
+        } else {
+            item_max_width = None;
         }
 
         if !last && tactic == DefinitiveListTactic::Vertical && item.new_lines {
+            item_max_width = None;
             result.push('\n');
         }
     }
@@ -298,6 +335,44 @@ pub fn write_list<I, T>(items: I, formatting: &ListFormatting) -> Option<String>
     Some(result)
 }
 
+fn max_width_of_item_with_post_comment<I, T>(
+    items: &I,
+    i: usize,
+    overhead: usize,
+    max_budget: usize,
+) -> usize
+where
+    I: IntoIterator<Item = T> + Clone,
+    T: AsRef<ListItem>,
+{
+    let mut max_width = 0;
+    let mut first = true;
+    for item in items.clone().into_iter().skip(i) {
+        let item = item.as_ref();
+        let inner_item_width = item.inner_as_ref().len();
+        if !first &&
+            (item.is_multiline() || !item.post_comment.is_some() ||
+                 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 post_comment_alignment(item_max_width: Option<usize>, inner_item_len: usize) -> usize {
+    item_max_width
+        .and_then(|max_line_width| max_line_width.checked_sub(inner_item_len))
+        .unwrap_or(0)
+}
+
 pub struct ListItems<'a, I, F1, F2, F3>
 where
     I: Iterator,

tests/source/structs.rs

@@ -209,3 +209,41 @@ fn foo() {
     convex_shape.set_point(2, &Vector2f { x: 450.0, y: 100.0 });
     convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 });
 }
+
+// Vertical alignment
+struct Foo {
+    aaaaa: u32, // a
+
+    b: u32, // b
+    cc: u32, // cc
+
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 1
+    yy: u32, // comment2
+    zzz: u32, // comment3
+
+    aaaaaa: u32, // comment4
+    bb: u32, // comment5
+    // separate
+    dd: u32, // comment7
+    c: u32, // comment6
+
+    aaaaaaa: u32, /* multi
+     * line
+     * comment
+     */
+    b: u32, // hi
+
+    do_not_push_this_comment1: u32, // comment1
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    please_do_not_push_this_comment3: u32, // comment3
+
+    do_not_push_this_comment1: u32, // comment1
+    // separate
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    please_do_not_push_this_comment3: u32, // comment3
+
+    do_not_push_this_comment1: u32, // comment1
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    // separate
+    please_do_not_push_this_comment3: u32, // comment3
+}

tests/target/configs-struct_field_align_threshold-20.rs

@@ -76,7 +76,7 @@ pub struct Writebatch<K: Key> {
 struct NewInt<T: Copy>(
     pub i32,
     SomeType, // inline comment
-    T, // sup
+    T,        // sup
 );
 
 struct Qux<
@@ -219,7 +219,7 @@ struct Foo<T>(
 where
     T: PartialEq;
 struct Foo<T>(
-    TTTTTTTTTTTTTTTTT, // Foo
+    TTTTTTTTTTTTTTTTT,        // Foo
     UUUUUUUUUUUUUUUUUUUUUUUU, // Bar
     // Baz
     TTTTTTTTTTTTTTTTTTT,

tests/target/enum.rs

@@ -88,7 +88,7 @@ pub enum GenericEnum<I, T>
     I: Iterator<Item = T>,
 {
     // Pre Comment
-    Left { list: I, root: T }, // Post-comment
+    Left { list: I, root: T },  // Post-comment
     Right { list: I, root: T }, // Post Comment
 }
 

tests/target/fn-args-with-last-line-comment.rs

@@ -3,8 +3,8 @@ pub trait X {
     fn a(&self) -> &'static str;
     fn bcd(
         &self,
-        c: &str, // comment on this arg
-        d: u16, // comment on this arg
+        c: &str,         // comment on this arg
+        d: u16,          // comment on this arg
         e: &Vec<String>, // comment on this arg
     ) -> Box<Q>;
 }

tests/target/fn-simple.rs

@@ -2,7 +2,7 @@
 
 fn simple(
     // pre-comment on a function!?
-    i: i32, // yes, it's possible!
+    i: i32,          // yes, it's possible!
     response: NoWay, // hose
 ) {
     fn op(

tests/target/macros.rs

@@ -29,7 +29,7 @@ fn main() {
     kaas!(
         // comments
         a, // post macro
-        b // another
+        b  // another
     );
 
     trailingcomma!(a, b, c,);

tests/target/multiple.rs

@@ -39,7 +39,7 @@ fn foo() -> Box<Write + 'static>
 }
 
 fn baz<
-    'a: 'b, // comment on 'a
+    'a: 'b,                                                            // comment on 'a
     T: SomsssssssssssssssssssssssssssssssssssssssssssssssssssssseType, // comment on T
 >(
     a: A,
@@ -71,7 +71,7 @@ impl Bar {
     fn foo(
         &mut self,
         a: sdfsdfcccccccccccccccccccccccccccccccccccccccccccccccccc, // comment on a
-        b: sdfasdfsdfasfs, // closing comment
+        b: sdfasdfsdfasfs,                                           // closing comment
     ) -> isize {
     }
 

tests/target/structs.rs

@@ -44,7 +44,7 @@ pub struct Writebatch<K: Key> {
 struct NewInt<T: Copy>(
     pub i32,
     SomeType, // inline comment
-    T, // sup
+    T,        // sup
 );
 
 struct Qux<
@@ -187,7 +187,7 @@ struct Foo<T>(
 where
     T: PartialEq;
 struct Foo<T>(
-    TTTTTTTTTTTTTTTTT, // Foo
+    TTTTTTTTTTTTTTTTT,        // Foo
     UUUUUUUUUUUUUUUUUUUUUUUU, // Bar
     // Baz
     TTTTTTTTTTTTTTTTTTT,
@@ -246,3 +246,41 @@ fn foo() {
     convex_shape.set_point(2, &Vector2f { x: 450.0, y: 100.0 });
     convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 });
 }
+
+// Vertical alignment
+struct Foo {
+    aaaaa: u32, // a
+
+    b: u32,  // b
+    cc: u32, // cc
+
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 1
+    yy: u32,  // comment2
+    zzz: u32, // comment3
+
+    aaaaaa: u32, // comment4
+    bb: u32,     // comment5
+    // separate
+    dd: u32, // comment7
+    c: u32,  // comment6
+
+    aaaaaaa: u32, /* multi
+                   * line
+                   * comment
+                   * */
+    b: u32, // hi
+
+    do_not_push_this_comment1: u32, // comment1
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    please_do_not_push_this_comment3: u32, // comment3
+
+    do_not_push_this_comment1: u32, // comment1
+    // separate
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    please_do_not_push_this_comment3: u32, // comment3
+
+    do_not_push_this_comment1: u32, // comment1
+    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2
+    // separate
+    please_do_not_push_this_comment3: u32, // comment3
+}