Merge pull request #1829 from topecongiro/issue-1046

Allow attributes to stay on the same line with fields

Author: nrc

Cargo.lock

@@ -18,7 +18,7 @@ use {Indent, Shape};
 use config::Config;
 use rewrite::RewriteContext;
 use string::{rewrite_string, StringFormat};
-use utils::wrap_str;
+use utils::{first_line_width, last_line_width, wrap_str};
 
 fn is_custom_comment(comment: &str) -> bool {
     if !comment.starts_with("//") {
@@ -136,6 +136,93 @@ fn comment_style(orig: &str, normalize_comments: bool) -> CommentStyle {
     }
 }
 
+pub fn combine_strs_with_missing_comments(
+    context: &RewriteContext,
+    prev_str: &str,
+    next_str: &str,
+    span: Span,
+    shape: Shape,
+    allow_extend: bool,
+) -> Option<String> {
+    let mut allow_one_line = !prev_str.contains('\n') && !next_str.contains('\n');
+    let first_sep = if prev_str.is_empty() || next_str.is_empty() {
+        ""
+    } else {
+        " "
+    };
+    let mut one_line_width =
+        last_line_width(prev_str) + first_line_width(next_str) + first_sep.len();
+
+    let original_snippet = context.snippet(span);
+    let trimmed_snippet = original_snippet.trim();
+    let indent_str = shape.indent.to_string(context.config);
+
+    if trimmed_snippet.is_empty() {
+        if allow_extend && prev_str.len() + first_sep.len() + next_str.len() <= shape.width {
+            return Some(format!("{}{}{}", prev_str, first_sep, next_str));
+        } else {
+            let sep = if prev_str.is_empty() {
+                String::new()
+            } else {
+                String::from("\n") + &indent_str
+            };
+            return Some(format!("{}{}{}", prev_str, sep, next_str));
+        }
+    }
+
+    // We have a missing comment between the first expression and the second expression.
+
+    // Peek the the original source code and find out whether there is a newline between the first
+    // expression and the second expression or the missing comment. We will preserve the orginal
+    // layout whenever possible.
+    let prefer_same_line = if let Some(pos) = original_snippet.chars().position(|c| c == '/') {
+        !original_snippet[..pos].contains('\n')
+    } else {
+        !original_snippet.contains('\n')
+    };
+
+    let missing_comment = try_opt!(rewrite_comment(
+        trimmed_snippet,
+        false,
+        shape,
+        context.config
+    ));
+    one_line_width -= first_sep.len();
+    let first_sep = if prev_str.is_empty() || missing_comment.is_empty() {
+        String::new()
+    } 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 {
+            String::from(" ")
+        } else {
+            format!("\n{}", indent_str)
+        }
+    };
+    let second_sep = if missing_comment.is_empty() || next_str.is_empty() {
+        String::new()
+    } else {
+        if missing_comment.starts_with("//") {
+            format!("\n{}", indent_str)
+        } 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 {
+                String::from(" ")
+            } else {
+                format!("\n{}", indent_str)
+            }
+        }
+    };
+    Some(format!(
+        "{}{}{}{}{}",
+        prev_str,
+        first_sep,
+        missing_comment,
+        second_sep,
+        next_str,
+    ))
+}
+
 pub fn rewrite_comment(
     orig: &str,
     block_style: bool,

src/comment.rs

@@ -19,7 +19,8 @@ use syntax::parse::classify;
 use {Indent, Shape, Spanned};
 use chains::rewrite_chain;
 use codemap::{LineRangeUtils, SpanUtils};
-use comment::{contains_comment, recover_comment_removed, rewrite_comment, FindUncommented};
+use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed,
+              rewrite_comment, FindUncommented};
 use config::{Config, ControlBraceStyle, IndentStyle, MultilineStyle, Style};
 use items::{span_hi_for_arg, span_lo_for_arg};
 use lists::{definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting,
@@ -49,61 +50,6 @@ pub enum ExprType {
     SubExpression,
 }
 
-fn combine_attr_and_expr(
-    context: &RewriteContext,
-    shape: Shape,
-    expr: &ast::Expr,
-    expr_str: &str,
-) -> Option<String> {
-    let attrs = outer_attributes(&expr.attrs);
-    let attr_str = try_opt!(attrs.rewrite(context, shape));
-    let separator = if attr_str.is_empty() {
-        String::new()
-    } else {
-        // Try to recover comments between the attributes and the expression if available.
-        let missing_snippet = context.snippet(mk_sp(attrs[attrs.len() - 1].span.hi, expr.span.lo));
-        let comment_opening_pos = missing_snippet.chars().position(|c| c == '/');
-        let prefer_same_line = if let Some(pos) = comment_opening_pos {
-            !missing_snippet[..pos].contains('\n')
-        } else {
-            !missing_snippet.contains('\n')
-        };
-
-        let trimmed = missing_snippet.trim();
-        let missing_comment = if trimmed.is_empty() {
-            String::new()
-        } else {
-            try_opt!(rewrite_comment(&trimmed, false, shape, context.config))
-        };
-
-        // 2 = ` ` + ` `
-        let one_line_width =
-            attr_str.len() + missing_comment.len() + 2 + first_line_width(expr_str);
-        let attr_expr_separator = if prefer_same_line && !missing_comment.starts_with("//") &&
-            one_line_width <= shape.width
-        {
-            String::from(" ")
-        } else {
-            format!("\n{}", shape.indent.to_string(context.config))
-        };
-
-        if missing_comment.is_empty() {
-            attr_expr_separator
-        } else {
-            // 1 = ` `
-            let one_line_width =
-                last_line_width(&attr_str) + 1 + first_line_width(&missing_comment);
-            let attr_comment_separator = if prefer_same_line && one_line_width <= shape.width {
-                String::from(" ")
-            } else {
-                format!("\n{}", shape.indent.to_string(context.config))
-            };
-            attr_comment_separator + &missing_comment + &attr_expr_separator
-        }
-    };
-    Some(format!("{}{}{}", attr_str, separator, expr_str))
-}
-
 pub fn format_expr(
     expr: &ast::Expr,
     expr_type: ExprType,
@@ -355,7 +301,13 @@ pub fn format_expr(
             recover_comment_removed(expr_str, expr.span, context, shape)
         })
         .and_then(|expr_str| {
-            combine_attr_and_expr(context, shape, expr, &expr_str)
+            let attrs = outer_attributes(&expr.attrs);
+            let attrs_str = try_opt!(attrs.rewrite(context, shape));
+            let span = mk_sp(
+                attrs.last().map_or(expr.span.lo, |attr| attr.span.hi),
+                expr.span.lo,
+            );
+            combine_strs_with_missing_comments(context, &attrs_str, &expr_str, span, shape, false)
         })
 }