Merge pull request #1942 from topecongiro/fixes

Some Fixes

Author: nrc

src/expr.rs

@@ -630,26 +630,14 @@ fn rewrite_closure(
             false
         };
         if no_return_type && !needs_block {
-            // lock.stmts.len() == 1
+            // block.stmts.len() == 1
             if let Some(expr) = stmt_expr(&block.stmts[0]) {
                 if let Some(rw) = rewrite_closure_expr(expr, &prefix, context, body_shape) {
                     return Some(rw);
                 }
             }
         }
 
-        if !needs_block {
-            // We need braces, but we might still prefer a one-liner.
-            let stmt = &block.stmts[0];
-            // 4 = braces and spaces.
-            if let Some(body_shape) = body_shape.sub_width(4) {
-                // Checks if rewrite succeeded and fits on a single line.
-                if let Some(rewrite) = and_one_line(stmt.rewrite(context, body_shape)) {
-                    return Some(format!("{} {{ {} }}", prefix, rewrite));
-                }
-            }
-        }
-
         // Either we require a block, or tried without and failed.
         rewrite_closure_block(block, &prefix, context, body_shape)
     } else {
@@ -882,13 +870,8 @@ impl Rewrite for ast::Stmt {
                     ""
                 };
 
-                let expr_type = match self.node {
-                    ast::StmtKind::Expr(_) => ExprType::SubExpression,
-                    ast::StmtKind::Semi(_) => ExprType::Statement,
-                    _ => unreachable!(),
-                };
                 let shape = try_opt!(shape.sub_width(suffix.len()));
-                format_expr(ex, expr_type, context, shape).map(|s| s + suffix)
+                format_expr(ex, ExprType::Statement, context, shape).map(|s| s + suffix)
             }
             ast::StmtKind::Mac(..) | ast::StmtKind::Item(..) => None,
         };

src/visitor.rs

@@ -22,7 +22,6 @@ use comment::{contains_comment, recover_missing_comment_in_span, CodeCharKind, C
               FindUncommented};
 use comment::rewrite_comment;
 use config::{BraceStyle, Config};
-use expr::{format_expr, ExprType};
 use items::{format_impl, format_trait, rewrite_associated_impl_type, rewrite_associated_type,
             rewrite_static, rewrite_type_alias};
 use lists::{itemize_list, write_list, DefinitiveListTactic, ListFormatting, SeparatorPlace,
@@ -77,12 +76,7 @@ impl<'a> FmtVisitor<'a> {
                 let rewrite = stmt.rewrite(&self.get_context(), self.shape());
                 self.push_rewrite(stmt.span(), rewrite);
             }
-            ast::StmtKind::Expr(ref expr) => {
-                let rewrite =
-                    format_expr(expr, ExprType::Statement, &self.get_context(), self.shape());
-                self.push_rewrite(stmt.span(), rewrite)
-            }
-            ast::StmtKind::Semi(..) => {
+            ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => {
                 let rewrite = stmt.rewrite(&self.get_context(), self.shape());
                 self.push_rewrite(stmt.span(), rewrite)
             }
@@ -979,7 +973,7 @@ impl<'a> Rewrite for [ast::Attribute] {
                         Some(&(_, next_attr)) if is_derive(next_attr) => insert_new_line = false,
                         // If not, rewrite the merged derives.
                         _ => {
-                            result.push_str(&format!("#[derive({})]", derive_args.join(", ")));
+                            result.push_str(&try_opt!(format_derive(context, &derive_args, shape)));
                             derive_args.clear();
                         }
                     }
@@ -996,6 +990,38 @@ impl<'a> Rewrite for [ast::Attribute] {
     }
 }
 
+// Format `#[derive(..)]`, using visual indent & mixed style when we need to go multiline.
+fn format_derive(context: &RewriteContext, derive_args: &[String], shape: Shape) -> Option<String> {
+    let mut result = String::with_capacity(128);
+    result.push_str("#[derive(");
+    // 11 = `#[derive()]`
+    let initial_budget = try_opt!(shape.width.checked_sub(11));
+    let mut budget = initial_budget;
+    let num = derive_args.len();
+    for (i, a) in derive_args.iter().enumerate() {
+        // 2 = `, ` or `)]`
+        let width = a.len() + 2;
+        if width > budget {
+            if i > 0 {
+                // Remove trailing whitespace.
+                result.pop();
+            }
+            result.push('\n');
+            // 9 = `#[derive(`
+            result.push_str(&(shape.indent + 9).to_string(context.config));
+            budget = initial_budget;
+        } else {
+            budget = budget.checked_sub(width).unwrap_or(0);
+        }
+        result.push_str(a);
+        if i != num - 1 {
+            result.push_str(", ")
+        }
+    }
+    result.push_str(")]");
+    Some(result)
+}
+
 fn is_derive(attr: &ast::Attribute) -> bool {
     match attr.meta() {
         Some(meta_item) => match meta_item.node {

tests/source/attrib.rs

@@ -146,3 +146,7 @@ fn attributes_on_statements() {
     # [ attr ( on ( mac ) ) ]
     foo!();
 }
+
+// Large derive
+#[derive(Add, Sub, Mul, Div, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Debug, Hash, Serialize, Deserialize)]
+pub struct HP(pub u8);

tests/target/attrib.rs

@@ -146,3 +146,8 @@ fn attributes_on_statements() {
     #[attr(on(mac))]
     foo!();
 }
+
+// Large derive
+#[derive(Add, Sub, Mul, Div, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Debug, Hash, Serialize,
+         Deserialize)]
+pub struct HP(pub u8);

tests/target/chains-visual.rs

@@ -42,7 +42,9 @@ fn main() {
     });
 
     fffffffffffffffffffffffffffffffffff(a, {
-        SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); });
+        SCRIPT_TASK_ROOT.with(|root| {
+            *root.borrow_mut() = Some(&script_task);
+        });
     });
 
     let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum =

tests/target/chains.rs

@@ -45,7 +45,9 @@ fn main() {
         });
 
     fffffffffffffffffffffffffffffffffff(a, {
-        SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); });
+        SCRIPT_TASK_ROOT.with(|root| {
+            *root.borrow_mut() = Some(&script_task);
+        });
     });
 
     let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum =

tests/target/hard-tabs.rs

@@ -80,7 +80,9 @@ fn main() {
 	});
 
 	fffffffffffffffffffffffffffffffffff(a, {
-		SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); });
+		SCRIPT_TASK_ROOT.with(|root| {
+			*root.borrow_mut() = Some(&script_task);
+		});
 	});
 	a.b.c.d();