Add "type_separator" option to control placement of "+" in types

Fixes rust-lang#4979
Fixes rust-lang#2055

Author: ulyssa

Configurations.md

@@ -2573,6 +2573,56 @@ fn main() {
 }
 ```
 
+## `type_separator`
+
+Where to put an operator when a type-level expression goes multiline.
+
+- **Default value**: `"Front"`
+- **Possible values**: `"Front"`, `"Back"`
+- **Stable**: No
+
+#### `"Front"` (default):
+
+```rust
+pub trait Foo:
+    Add
+    + AddAssign
+    + Clone
+    + Copy
+    + Debug
+    + Default
+    + Eq
+    + Hash
+    + Ord
+    + PartialEq
+    + PartialOrd
+    + Sized
+{
+    //
+}
+```
+
+#### `"Back"`:
+
+```rust
+pub trait Foo:
+    Add +
+    AddAssign +
+    Clone +
+    Copy +
+    Debug +
+    Default +
+    Eq +
+    Hash +
+    Ord +
+    PartialEq +
+    PartialOrd +
+    Sized
+{
+    //
+}
+```
+
 ## `use_small_heuristics`
 
 This option can be used to simplify the management and bulk updates of the granular width configuration settings ([`fn_call_width`](#fn_call_width), [`attr_fn_like_width`](#attr_fn_like_width), [`struct_lit_width`](#struct_lit_width), [`struct_variant_width`](#struct_variant_width), [`array_width`](#array_width), [`chain_width`](#chain_width), [`single_line_if_else_max_width`](#single_line_if_else_max_width)), that respectively control when formatted constructs are multi-lined/vertical based on width.

src/comment.rs

@@ -159,6 +159,26 @@ pub(crate) fn combine_strs_with_missing_comments(
     span: Span,
     shape: Shape,
     allow_extend: bool,
+) -> Option<String> {
+    combine_lines_with_missing_comments(
+        context,
+        prev_str,
+        next_str,
+        span,
+        shape,
+        false,
+        allow_extend,
+    )
+}
+
+pub(crate) fn combine_lines_with_missing_comments(
+    context: &RewriteContext<'_>,
+    prev_str: &str,
+    next_str: &str,
+    span: Span,
+    shape: Shape,
+    allow_multiline_join: bool,
+    allow_extend: bool,
 ) -> Option<String> {
     trace!(
         "combine_strs_with_missing_comments `{}` `{}` {:?} {:?}",
@@ -171,7 +191,8 @@ pub(crate) fn combine_strs_with_missing_comments(
     let mut result =
         String::with_capacity(prev_str.len() + next_str.len() + shape.indent.width() + 128);
     result.push_str(prev_str);
-    let mut allow_one_line = !prev_str.contains('\n') && !next_str.contains('\n');
+    let contains_nl = prev_str.contains('\n') || next_str.contains('\n');
+    let mut allow_line_join = allow_multiline_join || !contains_nl;
     let first_sep =
         if prev_str.is_empty() || next_str.is_empty() || trimmed_last_line_width(prev_str) == 0 {
             ""
@@ -221,14 +242,16 @@ pub(crate) fn combine_strs_with_missing_comments(
     result.push_str(&first_sep);
     result.push_str(&missing_comment);
 
-    let second_sep = if missing_comment.is_empty() || next_str.is_empty() {
+    let skip_second_sep =
+        missing_comment.is_empty() || next_str.is_empty() || next_str.starts_with("\n");
+    let second_sep = if skip_second_sep {
         Cow::from("")
     } else if missing_comment.starts_with("//") {
         indent.to_string_with_newline(config)
     } else {
         one_line_width += missing_comment.len() + first_sep.len() + 1;
-        allow_one_line &= !missing_comment.starts_with("//") && !missing_comment.contains('\n');
-        if prefer_same_line && allow_one_line && one_line_width <= shape.width {
+        allow_line_join &= !missing_comment.starts_with("//") && !missing_comment.contains('\n');
+        if prefer_same_line && allow_line_join && one_line_width <= shape.width {
             Cow::from(" ")
         } else {
             indent.to_string_with_newline(config)

src/config/mod.rs

@@ -102,6 +102,8 @@ create_config! {
     spaces_around_ranges: bool, false, false, "Put spaces around the  .. and ..= range operators";
     binop_separator: SeparatorPlace, SeparatorPlace::Front, false,
         "Where to put a binary operator when a binary expression goes multiline";
+    type_separator: SeparatorPlace, SeparatorPlace::Front, false,
+        "Where to put the operator when a type-level expression goes multiline";
 
     // Misc.
     remove_nested_parens: bool, true, true, "Remove nested parens";
@@ -589,6 +591,7 @@ space_before_colon = false
 space_after_colon = true
 spaces_around_ranges = false
 binop_separator = "Front"
+type_separator = "Front"
 remove_nested_parens = true
 combine_control_expr = true
 overflow_delimited_expr = false

src/expr.rs

@@ -1941,18 +1941,36 @@ pub(crate) fn rewrite_assign_rhs_with_comments<S: Into<String>, R: Rewrite>(
 ) -> Option<String> {
     let lhs = lhs.into();
     let contains_comment = contains_comment(context.snippet(between_span));
-    let shape = if contains_comment {
-        shape.block_left(context.config.tab_spaces())?
-    } else {
-        shape
-    };
-    let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?;
 
-    if contains_comment {
-        let rhs = rhs.trim_start();
-        combine_strs_with_missing_comments(context, &lhs, &rhs, between_span, shape, allow_extend)
-    } else {
-        Some(lhs + &rhs)
+    match (contains_comment, rhs_tactics) {
+        (true, RhsTactics::ForceNextLineWithoutIndent) => {
+            let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?;
+            let rhs = rhs.trim_start();
+
+            combine_strs_with_missing_comments(
+                context,
+                &lhs,
+                &rhs,
+                between_span,
+                shape.block_left(context.config.tab_spaces())?,
+                allow_extend,
+            )
+        }
+        (true, RhsTactics::Default | RhsTactics::AllowOverflow) => {
+            let shape = shape.block_left(context.config.tab_spaces())?;
+            let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?;
+            let rhs = rhs.trim_start();
+
+            combine_strs_with_missing_comments(
+                context,
+                &lhs,
+                &rhs,
+                between_span,
+                shape,
+                allow_extend,
+            )
+        }
+        (false, _) => rewrite_assign_rhs_with(context, lhs, ex, shape, rhs_tactics),
     }
 }
 
@@ -2014,7 +2032,7 @@ fn shape_from_rhs_tactic(
     match rhs_tactic {
         RhsTactics::ForceNextLineWithoutIndent => shape
             .with_max_width(context.config)
-            .sub_width(shape.indent.width()),
+            .sub_width(shape.indent.block_indent(context.config).width()),
         RhsTactics::Default | RhsTactics::AllowOverflow => {
             Shape::indented(shape.indent.block_indent(context.config), context.config)
                 .sub_width(shape.rhs_overhead(context.config))

src/items.rs

@@ -1041,24 +1041,47 @@ 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.
+        let combine_comment_span =
+            |lo: BytePos, hi: BytePos, lhs: String, rhs: &str| -> Option<String> {
+                let span = mk_sp(lo, hi);
+
+                if contains_comment(context.snippet(span)) {
+                    combine_strs_with_missing_comments(context, &lhs, rhs, span, shape, true)
+                } else {
+                    Some(lhs + rhs)
+                }
+            };
+
         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;
-            }
+            // Recover the comment before the colon.
+            let comment_lo = generics.span.hi();
+            let comment_hi = context.snippet_provider.span_before(item.span, ":");
+            result = combine_comment_span(comment_lo, comment_hi, result, ":")?;
+
+            // Recover the comment after the colon.
+            let comment_lo = context.snippet_provider.span_after(item.span, ":");
+            let comment_hi = generic_bounds[0].span().lo();
+            let comment_span = mk_sp(comment_lo, comment_hi);
 
-            result = rewrite_assign_rhs_with(
+            result = rewrite_assign_rhs_with_comments(
                 context,
-                result + ":",
+                &result,
                 generic_bounds,
                 shape,
                 RhsTactics::ForceNextLineWithoutIndent,
+                comment_span,
+                true,
             )?;
+
+            // Recover the comment following the bounds.
+            let comment_lo = generic_bounds[generic_bounds.len() - 1].span().hi();
+            let comment_hi = if generics.where_clause.predicates.is_empty() {
+                body_lo - BytePos(1)
+            } else {
+                generics.where_clause.span.lo()
+            };
+
+            result = combine_comment_span(comment_lo, comment_hi, result, "")?;
         }
 
         // Rewrite where-clause.
@@ -1067,9 +1090,9 @@ pub(crate) fn format_trait(
 
             let where_budget = context.budget(last_line_width(&result));
             let pos_before_where = if generic_bounds.is_empty() {
-                generics.where_clause.span.lo()
+                generics.span.hi()
             } else {
-                generic_bounds[generic_bounds.len() - 1].span().hi()
+                generics.where_clause.span.lo()
             };
             let option = WhereClauseOption::snuggled(&generics_str);
             let where_clause_str = rewrite_where_clause(
@@ -1079,7 +1102,7 @@ pub(crate) fn format_trait(
                 Shape::legacy(where_budget, offset.block_only()),
                 where_on_new_line,
                 "{",
-                None,
+                Some(body_lo),
                 pos_before_where,
                 option,
             )?;
@@ -1094,25 +1117,13 @@ pub(crate) fn format_trait(
                 result.push_str(&where_indent.to_string_with_newline(context.config));
             }
             result.push_str(&where_clause_str);
-        } else {
+        } else if generic_bounds.is_empty() {
             let item_snippet = context.snippet(item.span);
             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);
-                if comment_lo < comment_hi {
-                    match recover_missing_comment_in_span(
-                        mk_sp(comment_lo, comment_hi),
-                        Shape::indented(offset, context.config),
-                        context,
-                        last_line_width(&result),
-                    ) {
-                        Some(ref missing_comment) if !missing_comment.is_empty() => {
-                            result.push_str(missing_comment);
-                        }
-                        _ => (),
-                    }
-                }
+                result = combine_comment_span(comment_lo, comment_hi, result, "")?;
             }
         }