Merge pull request #1729 from topecongiro/single-line-block

Allow single line block in expression context

src/bin/rustfmt.rs

@@ -79,7 +79,10 @@ impl CliOptions {
                 ));
             }
         } else {
-            println!("Warning: the default write-mode for Rustfmt will soon change to overwrite - this will not leave backups of changed files.");
+            println!(
+                "Warning: the default write-mode for Rustfmt will soon change to overwrite \
+                 - this will not leave backups of changed files."
+            );
         }
 
         if let Some(ref file_lines) = matches.opt_str("file-lines") {

src/expr.rs

@@ -42,7 +42,7 @@ impl Rewrite for ast::Expr {
 }
 
 #[derive(PartialEq)]
-enum ExprType {
+pub enum ExprType {
     Statement,
     SubExpression,
 }
@@ -67,7 +67,7 @@ fn combine_attr_and_expr(
     format!("{}{}{}", attr_str, separator, expr_str)
 }
 
-fn format_expr(
+pub fn format_expr(
     expr: &ast::Expr,
     expr_type: ExprType,
     context: &RewriteContext,
@@ -160,7 +160,23 @@ fn format_expr(
             to_control_flow(expr, expr_type)
                 .and_then(|control_flow| control_flow.rewrite(context, shape))
         }
-        ast::ExprKind::Block(ref block) => block.rewrite(context, shape),
+        ast::ExprKind::Block(ref block) => {
+            match expr_type {
+                ExprType::Statement => {
+                    if is_unsafe_block(block) {
+                        block.rewrite(context, shape)
+                    } else {
+                        // Rewrite block without trying to put it in a single line.
+                        if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
+                            return rw;
+                        }
+                        let prefix = try_opt!(block_prefix(context, block, shape));
+                        rewrite_block_with_visitor(context, &prefix, block, shape)
+                    }
+                }
+                ExprType::SubExpression => block.rewrite(context, shape),
+            }
+        }
         ast::ExprKind::Match(ref cond, ref arms) => {
             rewrite_match(context, cond, arms, shape, expr.span)
         }
@@ -290,7 +306,9 @@ fn format_expr(
             )
         }
         ast::ExprKind::Catch(ref block) => {
-            if let rewrite @ Some(_) = try_one_line_block(context, shape, "do catch ", block) {
+            if let rewrite @ Some(_) =
+                rewrite_single_line_block(context, "do catch ", block, shape)
+            {
                 return rewrite;
             }
             // 9 = `do catch `
@@ -315,23 +333,6 @@ fn format_expr(
     }
 }
 
-fn try_one_line_block(
-    context: &RewriteContext,
-    shape: Shape,
-    prefix: &str,
-    block: &ast::Block,
-) -> Option<String> {
-    if is_simple_block(block, context.codemap) {
-        let expr_shape = Shape::legacy(shape.width - prefix.len(), shape.indent);
-        let expr_str = try_opt!(block.stmts[0].rewrite(context, expr_shape));
-        let result = format!("{}{{ {} }}", prefix, expr_str);
-        if result.len() <= shape.width && !result.contains('\n') {
-            return Some(result);
-        }
-    }
-    None
-}
-
 pub fn rewrite_pair<LHS, RHS>(
     lhs: &LHS,
     rhs: &RHS,
@@ -763,78 +764,124 @@ fn nop_block_collapse(block_str: Option<String>, budget: usize) -> Option<String
     })
 }
 
-impl Rewrite for ast::Block {
-    fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
-        // shape.width is used only for the single line case: either the empty block `{}`,
-        // or an unsafe expression `unsafe { e }`.
+fn rewrite_empty_block(
+    context: &RewriteContext,
+    block: &ast::Block,
+    shape: Shape,
+) -> Option<String> {
+    if block.stmts.is_empty() && !block_contains_comment(block, context.codemap) &&
+        shape.width >= 2
+    {
+        return Some("{}".to_owned());
+    }
 
-        if self.stmts.is_empty() && !block_contains_comment(self, context.codemap) &&
-            shape.width >= 2
+    // If a block contains only a single-line comment, then leave it on one line.
+    let user_str = context.snippet(block.span);
+    let user_str = user_str.trim();
+    if user_str.starts_with('{') && user_str.ends_with('}') {
+        let comment_str = user_str[1..user_str.len() - 1].trim();
+        if block.stmts.is_empty() && !comment_str.contains('\n') &&
+            !comment_str.starts_with("//") && comment_str.len() + 4 <= shape.width
         {
-            return Some("{}".to_owned());
+            return Some(format!("{{ {} }}", comment_str));
         }
+    }
 
-        // If a block contains only a single-line comment, then leave it on one line.
-        let user_str = context.snippet(self.span);
-        let user_str = user_str.trim();
-        if user_str.starts_with('{') && user_str.ends_with('}') {
-            let comment_str = user_str[1..user_str.len() - 1].trim();
-            if self.stmts.is_empty() && !comment_str.contains('\n') &&
-                !comment_str.starts_with("//") &&
-                comment_str.len() + 4 <= shape.width
-            {
-                return Some(format!("{{ {} }}", comment_str));
+    None
+}
+
+fn block_prefix(context: &RewriteContext, block: &ast::Block, shape: Shape) -> Option<String> {
+    Some(match block.rules {
+        ast::BlockCheckMode::Unsafe(..) => {
+            let snippet = context.snippet(block.span);
+            let open_pos = try_opt!(snippet.find_uncommented("{"));
+            // Extract comment between unsafe and block start.
+            let trimmed = &snippet[6..open_pos].trim();
+
+            if !trimmed.is_empty() {
+                // 9 = "unsafe  {".len(), 7 = "unsafe ".len()
+                let budget = try_opt!(shape.width.checked_sub(9));
+                format!(
+                    "unsafe {} ",
+                    try_opt!(rewrite_comment(
+                        trimmed,
+                        true,
+                        Shape::legacy(budget, shape.indent + 7),
+                        context.config,
+                    ))
+                )
+            } else {
+                "unsafe ".to_owned()
             }
         }
+        ast::BlockCheckMode::Default => String::new(),
+    })
+}
 
-        let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config);
-        visitor.block_indent = shape.indent;
-        visitor.is_if_else_block = context.is_if_else_block;
+fn rewrite_single_line_block(
+    context: &RewriteContext,
+    prefix: &str,
+    block: &ast::Block,
+    shape: Shape,
+) -> Option<String> {
+    if is_simple_block(block, context.codemap) {
+        let expr_shape = Shape::legacy(shape.width - prefix.len(), shape.indent);
+        let expr_str = try_opt!(block.stmts[0].rewrite(context, expr_shape));
+        let result = format!("{}{{ {} }}", prefix, expr_str);
+        if result.len() <= shape.width && !result.contains('\n') {
+            return Some(result);
+        }
+    }
+    None
+}
 
-        let prefix = match self.rules {
-            ast::BlockCheckMode::Unsafe(..) => {
-                let snippet = context.snippet(self.span);
-                let open_pos = try_opt!(snippet.find_uncommented("{"));
-                visitor.last_pos = self.span.lo + BytePos(open_pos as u32);
+fn rewrite_block_with_visitor(
+    context: &RewriteContext,
+    prefix: &str,
+    block: &ast::Block,
+    shape: Shape,
+) -> Option<String> {
+    if let rw @ Some(_) = rewrite_empty_block(context, block, shape) {
+        return rw;
+    }
 
-                // Extract comment between unsafe and block start.
-                let trimmed = &snippet[6..open_pos].trim();
+    let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config);
+    visitor.block_indent = shape.indent;
+    visitor.is_if_else_block = context.is_if_else_block;
+    match block.rules {
+        ast::BlockCheckMode::Unsafe(..) => {
+            let snippet = context.snippet(block.span);
+            let open_pos = try_opt!(snippet.find_uncommented("{"));
+            visitor.last_pos = block.span.lo + BytePos(open_pos as u32)
+        }
+        ast::BlockCheckMode::Default => visitor.last_pos = block.span.lo,
+    }
 
-                let prefix = if !trimmed.is_empty() {
-                    // 9 = "unsafe  {".len(), 7 = "unsafe ".len()
-                    let budget = try_opt!(shape.width.checked_sub(9));
-                    format!(
-                        "unsafe {} ",
-                        try_opt!(rewrite_comment(
-                            trimmed,
-                            true,
-                            Shape::legacy(budget, shape.indent + 7),
-                            context.config,
-                        ))
-                    )
-                } else {
-                    "unsafe ".to_owned()
-                };
-                if let result @ Some(_) = try_one_line_block(context, shape, &prefix, self) {
-                    return result;
-                }
-                prefix
-            }
-            ast::BlockCheckMode::Default => {
-                visitor.last_pos = self.span.lo;
-                String::new()
-            }
-        };
+    visitor.visit_block(block);
+    if visitor.failed && shape.indent.alignment != 0 {
+        block.rewrite(
+            context,
+            Shape::indented(shape.indent.block_only(), context.config),
+        )
+    } else {
+        Some(format!("{}{}", prefix, visitor.buffer))
+    }
+}
 
-        visitor.visit_block(self);
-        if visitor.failed && shape.indent.alignment != 0 {
-            self.rewrite(
-                context,
-                Shape::indented(shape.indent.block_only(), context.config),
-            )
-        } else {
-            Some(format!("{}{}", prefix, visitor.buffer))
+impl Rewrite for ast::Block {
+    fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
+        // shape.width is used only for the single line case: either the empty block `{}`,
+        // or an unsafe expression `unsafe { e }`.
+        if let rw @ Some(_) = rewrite_empty_block(context, self, shape) {
+            return rw;
         }
+
+        let prefix = try_opt!(block_prefix(context, self, shape));
+        if let rw @ Some(_) = rewrite_single_line_block(context, &prefix, self, shape) {
+            return rw;
+        }
+
+        rewrite_block_with_visitor(context, &prefix, self, shape)
     }
 }
 
@@ -1249,7 +1296,12 @@ impl<'a> Rewrite for ControlFlow<'a> {
         };
         let mut block_context = context.clone();
         block_context.is_if_else_block = self.else_block.is_some();
-        let block_str = try_opt!(self.block.rewrite(&block_context, block_shape));
+        let block_str = try_opt!(rewrite_block_with_visitor(
+            &block_context,
+            "",
+            self.block,
+            block_shape,
+        ));
 
         let mut result = format!("{}{}", cond_str, block_str);
 
@@ -1291,7 +1343,7 @@ impl<'a> Rewrite for ControlFlow<'a> {
                         width: min(1, shape.width),
                         ..shape
                     };
-                    else_block.rewrite(context, else_shape)
+                    format_expr(else_block, ExprType::Statement, context, else_shape)
                 }
             };
 
@@ -1658,7 +1710,10 @@ impl Rewrite for ast::Arm {
                 .unwrap()
                 .sub_width(comma.len())
                 .unwrap();
-            let rewrite = nop_block_collapse(body.rewrite(context, arm_shape), arm_shape.width);
+            let rewrite = nop_block_collapse(
+                format_expr(body, ExprType::Statement, context, arm_shape),
+                arm_shape.width,
+            );
             let is_block = if let ast::ExprKind::Block(..) = body.node {
                 true
             } else {
@@ -1693,7 +1748,7 @@ impl Rewrite for ast::Arm {
         // necessary.
         let body_shape = try_opt!(shape.block_left(context.config.tab_spaces()));
         let next_line_body = try_opt!(nop_block_collapse(
-            body.rewrite(context, body_shape),
+            format_expr(body, ExprType::Statement, context, body_shape),
             body_shape.width,
         ));
         let indent_str = shape

src/items.rs

@@ -17,7 +17,7 @@ use utils::{format_mutability, format_visibility, contains_skip, end_typaram, wr
             trimmed_last_line_width, colon_spaces, mk_sp};
 use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, list_helper,
             DefinitiveListTactic, ListTactic, definitive_tactic};
-use expr::{is_empty_block, is_simple_block_stmt, rewrite_assign_rhs};
+use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs, ExprType};
 use comment::{FindUncommented, contains_comment, rewrite_comment, recover_comment_removed};
 use visitor::FmtVisitor;
 use rewrite::{Rewrite, RewriteContext};
@@ -352,7 +352,9 @@ impl<'a> FmtVisitor<'a> {
                         Some(e) => {
                             let suffix = if semicolon_for_expr(e) { ";" } else { "" };
 
-                            e.rewrite(
+                            format_expr(
+                                &e,
+                                ExprType::Statement,
                                 &self.get_context(),
                                 Shape::indented(self.block_indent, self.config),
                             ).map(|s| s + suffix)

src/visitor.rs

@@ -17,6 +17,7 @@ use syntax::parse::ParseSess;
 use strings::string_buffer::StringBuffer;
 
 use {Indent, Shape};
+use expr::{format_expr, ExprType};
 use utils::{self, mk_sp};
 use codemap::{LineRangeUtils, SpanUtils};
 use comment::{contains_comment, FindUncommented};
@@ -87,7 +88,20 @@ impl<'a> FmtVisitor<'a> {
                 );
                 self.push_rewrite(stmt.span, rewrite);
             }
-            ast::StmtKind::Expr(ref expr) |
+            ast::StmtKind::Expr(ref expr) => {
+                let rewrite = format_expr(
+                    expr,
+                    ExprType::Statement,
+                    &self.get_context(),
+                    Shape::indented(self.block_indent, self.config),
+                );
+                let span = if expr.attrs.is_empty() {
+                    stmt.span
+                } else {
+                    mk_sp(expr.attrs[0].span.lo, stmt.span.hi)
+                };
+                self.push_rewrite(span, rewrite)
+            }
             ast::StmtKind::Semi(ref expr) => {
                 let rewrite = stmt.rewrite(
                     &self.get_context(),

tests/source/expr.rs

@@ -299,3 +299,12 @@ fn issue1106() {
         .filter_entry(|entry| exclusions.filter_entry(entry)) {
     }
 }
+
+fn issue1570() {
+    a_very_long_function_name({some_func(1, {1})})
+}
+
+fn issue1714() {
+    v = &mut {v}[mid..];
+    let (left, right) = {v}.split_at_mut(mid);
+}

tests/target/expr.rs

@@ -358,3 +358,12 @@ fn issue1106() {
         .filter_entry(|entry| exclusions.filter_entry(entry))
     {}
 }
+
+fn issue1570() {
+    a_very_long_function_name({ some_func(1, { 1 }) })
+}
+
+fn issue1714() {
+    v = &mut { v }[mid..];
+    let (left, right) = { v }.split_at_mut(mid);
+}