Indentation of asssignment comments between lhs and rhs

src/comment.rs

@@ -7,12 +7,12 @@ use lazy_static::lazy_static;
 use regex::Regex;
 use rustc_span::Span;
 
-use crate::config::Config;
+use crate::config::{Config, Version};
 use crate::rewrite::RewriteContext;
 use crate::shape::{Indent, Shape};
 use crate::string::{rewrite_string, StringFormat};
 use crate::utils::{
-    count_newlines, first_line_width, last_line_width, trim_left_preserve_layout,
+    count_newlines, first_line_width, is_single_line, last_line_width, trim_left_preserve_layout,
     trimmed_last_line_width, unicode_str_width,
 };
 use crate::{ErrorKind, FormattingError};
@@ -160,6 +160,20 @@ pub(crate) fn is_last_comment_block(s: &str) -> bool {
     s.trim_end().ends_with("*/")
 }
 
+/// Returns true if the last comment in the first line of the passed comments block
+/// ends on that line.
+fn is_first_comment_line_ends_comment(s: &str) -> bool {
+    if s.starts_with("/*") {
+        // FIXME: mixed block and line comments on the same line are not properly formatted,
+        // e.g. "/* Block comment /* // Line comment" at the end of the line.
+        s.lines().next().map_or(false, |l| l.ends_with("*/"))
+    } else if s.starts_with("//") {
+        true // Comment is "// comment"
+    } else {
+        false
+    }
+}
+
 /// Combine `prev_str` and `next_str` into a single `String`. `span` may contain
 /// comments between two strings. If there are such comments, then that will be
 /// recovered. If `allow_extend` is true and there is no comment between the two
@@ -226,7 +240,15 @@ pub(crate) fn combine_strs_with_missing_comments(
     } else {
         let one_line_width = last_line_width(prev_str) + first_line_width(&missing_comment) + 1;
         if prefer_same_line && one_line_width <= shape.width {
-            Cow::from(" ")
+            // First comment in the comments block can be in same line if it ends in the first line.
+            if context.config.version() == Version::One
+                || is_single_line(&missing_comment)
+                || is_first_comment_line_ends_comment(&missing_comment)
+            {
+                Cow::from(" ")
+            } else {
+                indent.to_string_with_newline(config)
+            }
         } else {
             indent.to_string_with_newline(config)
         }
@@ -240,7 +262,11 @@ pub(crate) fn combine_strs_with_missing_comments(
         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 context.config.version() == Version::One {
+            allow_one_line &= !missing_comment.starts_with("//") && !missing_comment.contains('\n');
+        } else {
+            allow_one_line &= !missing_comment.contains('\n');
+        }
         if prefer_same_line && allow_one_line && one_line_width <= shape.width {
             Cow::from(" ")
         } else {
@@ -1017,27 +1043,57 @@ fn light_rewrite_comment(
     config: &Config,
     is_doc_comment: bool,
 ) -> String {
-    let lines: Vec<&str> = orig
-        .lines()
-        .map(|l| {
-            // This is basically just l.trim(), but in the case that a line starts
-            // with `*` we want to leave one space before it, so it aligns with the
-            // `*` in `/*`.
-            let first_non_whitespace = l.find(|c| !char::is_whitespace(c));
-            let left_trimmed = if let Some(fnw) = first_non_whitespace {
-                if l.as_bytes()[fnw] == b'*' && fnw > 0 {
-                    &l[fnw - 1..]
+    if config.version() == Version::One {
+        let lines: Vec<&str> = orig
+            .lines()
+            .map(|l| {
+                // This is basically just l.trim(), but in the case that a line starts
+                // with `*` we want to leave one space before it, so it aligns with the
+                // `*` in `/*`.
+                let first_non_whitespace = l.find(|c| !char::is_whitespace(c));
+                let left_trimmed = if let Some(fnw) = first_non_whitespace {
+                    if l.as_bytes()[fnw] == b'*' && fnw > 0 {
+                        &l[fnw - 1..]
+                    } else {
+                        &l[fnw..]
+                    }
                 } else {
-                    &l[fnw..]
-                }
-            } else {
-                ""
-            };
+                    ""
+                };
+                // Preserve markdown's double-space line break syntax in doc comment.
+                trim_end_unless_two_whitespaces(left_trimmed, is_doc_comment)
+            })
+            .collect();
+        lines.join(&format!("\n{}", offset.to_string(config)))
+    } else {
+        // Version::Two
+        let mut result = String::with_capacity(orig.len() * 2);
+        let mut lines = orig.lines().peekable();
+
+        // This is basically just l.trim(), but in the case that a line starts with `*`
+        // we want to leave one space before it, so it aligns with the `*` in `/*`.
+        while let Some(full_line) = lines.next() {
+            let line = full_line.trim_start();
+
+            if line.is_empty() {
+                continue;
+            }
+
+            if line.starts_with('*') {
+                result.push(' ');
+            }
+
             // Preserve markdown's double-space line break syntax in doc comment.
-            trim_end_unless_two_whitespaces(left_trimmed, is_doc_comment)
-        })
-        .collect();
-    lines.join(&format!("\n{}", offset.to_string(config)))
+            let trimmed = trim_end_unless_two_whitespaces(line, is_doc_comment);
+            result.push_str(trimmed);
+
+            let is_last = lines.peek().is_none();
+            if !is_last {
+                result.push_str(&offset.to_string_with_newline(config))
+            }
+        }
+        result
+    }
 }
 
 /// Trims comment characters and possibly a single space from the left of a string.

src/expr.rs

@@ -42,7 +42,7 @@ impl Rewrite for ast::Expr {
     }
 }
 
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, PartialEq, Debug)]
 pub(crate) enum ExprType {
     Statement,
     SubExpression,
@@ -171,11 +171,19 @@ pub(crate) fn format_expr(
         ast::ExprKind::Path(ref qself, ref path) => {
             rewrite_path(context, PathContext::Expr, qself, path, shape)
         }
-        ast::ExprKind::Assign(ref lhs, ref rhs, _) => {
-            rewrite_assignment(context, lhs, rhs, None, shape)
+        ast::ExprKind::Assign(ref lhs, ref rhs, op_span) => {
+            if context.config.version() == Version::One {
+                rewrite_assignment_for_op(context, lhs, rhs, None, shape)
+            } else {
+                rewrite_assignment_for_op_span(context, lhs, rhs, op_span, shape)
+            }
         }
         ast::ExprKind::AssignOp(ref op, ref lhs, ref rhs) => {
-            rewrite_assignment(context, lhs, rhs, Some(op), shape)
+            if context.config.version() == Version::One {
+                rewrite_assignment_for_op(context, lhs, rhs, Some(op), shape)
+            } else {
+                rewrite_assignment_for_op_span(context, lhs, rhs, op.span, shape)
+            }
         }
         ast::ExprKind::Continue(ref opt_label) => {
             let id_str = match *opt_label {
@@ -1898,7 +1906,7 @@ impl<'ast> RhsAssignKind<'ast> {
     }
 }
 
-fn rewrite_assignment(
+fn rewrite_assignment_for_op(
     context: &RewriteContext<'_>,
     lhs: &ast::Expr,
     rhs: &ast::Expr,
@@ -1910,19 +1918,60 @@ fn rewrite_assignment(
         None => "=",
     };
 
-    // 1 = space between lhs and operator.
-    let lhs_shape = shape.sub_width(operator_str.len() + 1)?;
-    let lhs_str = format!("{} {}", lhs.rewrite(context, lhs_shape)?, operator_str);
+    rewrite_assignment(context, lhs, rhs, operator_str, None, shape)
+}
 
-    rewrite_assign_rhs(
+fn rewrite_assignment_for_op_span(
+    context: &RewriteContext<'_>,
+    lhs: &ast::Expr,
+    rhs: &ast::Expr,
+    op_span: Span,
+    shape: Shape,
+) -> Option<String> {
+    rewrite_assignment(
         context,
-        lhs_str,
+        lhs,
         rhs,
-        &RhsAssignKind::Expr(&rhs.kind, rhs.span),
+        context.snippet(op_span),
+        Some(mk_sp(op_span.hi(), rhs.span.lo())),
         shape,
     )
 }
 
+fn rewrite_assignment(
+    context: &RewriteContext<'_>,
+    lhs: &ast::Expr,
+    rhs: &ast::Expr,
+    operator_str: &str,
+    comment_span: Option<Span>,
+    shape: Shape,
+) -> Option<String> {
+    // 1 = space between lhs and operator.
+    let lhs_shape = shape.sub_width(operator_str.len() + 1)?;
+    let lhs_str = format!("{} {}", lhs.rewrite(context, lhs_shape)?, operator_str);
+
+    if context.config.version() == Version::One {
+        rewrite_assign_rhs(
+            context,
+            lhs_str,
+            rhs,
+            &RhsAssignKind::Expr(&rhs.kind, rhs.span),
+            shape,
+        )
+    } else {
+        rewrite_assign_rhs_with_comments(
+            context,
+            lhs_str,
+            rhs,
+            shape,
+            &RhsAssignKind::Expr(&rhs.kind, rhs.span),
+            RhsTactics::Default,
+            comment_span?,
+            true,
+        )
+    }
+}
+
 /// Controls where to put the rhs.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub(crate) enum RhsTactics {

src/items.rs

@@ -113,24 +113,59 @@ impl Rewrite for ast::Local {
         result.push_str(&infix);
 
         if let Some((init, _els)) = self.kind.init_else_opt() {
-            // 1 = trailing semicolon;
-            let nested_shape = shape.sub_width(1)?;
+            let lhs_to_expr_span = if let Some(ref ty) = self.ty {
+                ty.span.between(init.span)
+            } else {
+                self.pat.span.between(init.span)
+            };
 
-            result = rewrite_assign_rhs(
-                context,
-                result,
-                init,
-                &RhsAssignKind::Expr(&init.kind, init.span),
-                nested_shape,
-            )?;
-            // todo else
+            result = rewrite_initializer_expr(context, init, result, lhs_to_expr_span, shape)?;
         }
+        // todo else of kind.init_else_opt
 
         result.push(';');
         Some(result)
     }
 }
 
+fn rewrite_initializer_expr(
+    context: &RewriteContext<'_>,
+    init: &ast::Expr,
+    lhs: String,
+    lhs_to_rhs_span: Span,
+    shape: Shape,
+) -> Option<String> {
+    if context.config.version() == Version::One {
+        // 1 = trailing semicolon;
+        let nested_shape = shape.sub_width(1)?;
+
+        rewrite_assign_rhs(
+            context,
+            lhs,
+            init,
+            &RhsAssignKind::Expr(&init.kind, init.span),
+            nested_shape,
+        )
+    } else {
+        // Version:Two+
+        let comment_lo = context.snippet_provider.span_after(lhs_to_rhs_span, "=");
+        let comment_span = mk_sp(comment_lo, init.span.lo());
+
+        // 1 = trailing semicolon;
+        let nested_shape = shape.sub_width(1)?;
+        rewrite_assign_rhs_with_comments(
+            context,
+            lhs,
+            init,
+            nested_shape,
+            &RhsAssignKind::Expr(&init.kind, init.span),
+            RhsTactics::Default,
+            comment_span,
+            true,
+        )
+    }
+}
+
 // FIXME convert to using rewrite style rather than visitor
 // FIXME format modules in this style
 #[allow(dead_code)]