Merge pull request #1495 from topecongiro/poor-formatting/closure

Fix weird indentation inside closures

src/chains.rs

@@ -176,9 +176,15 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
         .fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len();
     let one_line_len = rewrites.iter().fold(0, |a, r| a + r.len()) + parent_rewrite.len();
 
-    let veto_single_line = if one_line_len > context.config.chain_one_line_max - 1 &&
-                              rewrites.len() > 1 {
+    let veto_single_line = if one_line_len > context.config.chain_one_line_max {
+        if rewrites.len() > 1 {
             true
+        } else if rewrites.len() == 1 {
+            let one_line_len = parent_rewrite.len() + first_line_width(&rewrites[0]);
+            one_line_len > shape.width
+        } else {
+            false
+        }
     } else if context.config.take_source_hints && subexpr_list.len() > 1 {
         // Look at the source code. Unless all chain elements start on the same
         // line, we won't consider putting them on a single line either.

src/expr.rs

@@ -577,22 +577,25 @@ fn rewrite_closure(capture: ast::CaptureBy,
                              -> Option<String> {
         // Start with visual indent, then fall back to block indent if the
         // closure is large.
-        let rewrite = try_opt!(block.rewrite(&context, shape));
-
+        if let Some(block_str) = block.rewrite(&context, shape) {
             let block_threshold = context.config.closure_block_indent_threshold;
-        if block_threshold < 0 || rewrite.matches('\n').count() <= block_threshold as usize {
-            if let Some(rewrite) = wrap_str(rewrite, context.config.max_width, shape) {
-                return Some(format!("{} {}", prefix, rewrite));
+            if block_threshold < 0 || block_str.matches('\n').count() <= block_threshold as usize {
+                if let Some(block_str) = block_str.rewrite(context, shape) {
+                    return Some(format!("{} {}", prefix, block_str));
+                }
             }
         }
 
         // The body of the closure is big enough to be block indented, that
         // means we must re-format.
-        let block_shape = shape.block();
-        let rewrite = try_opt!(block.rewrite(&context, block_shape));
+        let block_shape = Shape {
+            width: context.config.max_width - shape.block().indent.width(),
+            ..shape.block()
+        };
+        let block_str = try_opt!(block.rewrite(&context, block_shape));
         Some(format!("{} {}",
                      prefix,
-                     try_opt!(wrap_str(rewrite, block_shape.width, block_shape))))
+                     try_opt!(block_str.rewrite(context, block_shape))))
     }
 }
 
@@ -679,10 +682,13 @@ impl Rewrite for ast::Block {
         };
 
         visitor.visit_block(self);
-
+        if visitor.failed {
+            None
+        } else {
             Some(format!("{}{}", prefix, visitor.buffer))
         }
     }
+}
 
 impl Rewrite for ast::Stmt {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {

src/file_lines.rs

@@ -123,10 +123,8 @@ impl FileLines {
             Some(ref map) => map,
         };
 
-        match canonicalize_path_string(range.file_name()).and_then(|canonical| {
-                                                                       map.get_vec(&canonical)
-                                                                           .ok_or(())
-                                                                   }) {
+        match canonicalize_path_string(range.file_name())
+                  .and_then(|canonical| map.get_vec(&canonical).ok_or(())) {
             Ok(ranges) => ranges.iter().any(|r| r.contains(Range::from(range))),
             Err(_) => false,
         }
@@ -140,10 +138,8 @@ impl FileLines {
             Some(ref map) => map,
         };
 
-        match canonicalize_path_string(range.file_name()).and_then(|canonical| {
-                                                                       map.get_vec(&canonical)
-                                                                           .ok_or(())
-                                                                   }) {
+        match canonicalize_path_string(range.file_name())
+                  .and_then(|canonical| map.get_vec(&canonical).ok_or(())) {
             Ok(ranges) => ranges.iter().any(|r| r.intersects(Range::from(range))),
             Err(_) => false,
         }

src/items.rs

@@ -278,8 +278,7 @@ impl<'a> FmtVisitor<'a> {
             result.push(' ');
         }
 
-        self.single_line_fn(&result, block)
-            .or_else(|| Some(result))
+        self.single_line_fn(&result, block).or_else(|| Some(result))
     }
 
     pub fn rewrite_required_fn(&mut self,
@@ -912,9 +911,7 @@ fn format_struct_struct(context: &RewriteContext,
         let snippet = context.snippet(mk_sp(body_lo, span.hi - BytePos(1)));
         if snippet.trim().is_empty() {
             // `struct S {}`
-        } else if snippet
-                      .trim_right_matches(&[' ', '\t'][..])
-                      .ends_with('\n') {
+        } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') {
             // fix indent
             result.push_str(&snippet.trim_right());
             result.push('\n');
@@ -1030,9 +1027,7 @@ fn format_tuple_struct(context: &RewriteContext,
         let snippet = context.snippet(mk_sp(body_lo, context.codemap.span_before(span, ")")));
         if snippet.is_empty() {
             // `struct S ()`
-        } else if snippet
-                      .trim_right_matches(&[' ', '\t'][..])
-                      .ends_with('\n') {
+        } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') {
             result.push_str(&snippet.trim_right());
             result.push('\n');
             result.push_str(&offset.to_string(context.config));
@@ -1229,8 +1224,7 @@ impl Rewrite for ast::StructField {
         let type_offset = shape.indent.block_indent(context.config);
         let rewrite_type_in_next_line = || {
             let budget = try_opt!(context.config.max_width.checked_sub(type_offset.width()));
-            self.ty
-                .rewrite(context, Shape::legacy(budget, type_offset))
+            self.ty.rewrite(context, Shape::legacy(budget, type_offset))
         };
 
         let last_line_width = last_line_width(&result) + type_annotation_spacing.1.len();

src/string.rs

@@ -51,10 +51,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
     let ender_length = fmt.line_end.len();
     // If we cannot put at least a single character per line, the rewrite won't
     // succeed.
-    let max_chars = try_opt!(shape
-                                 .width
-                                 .checked_sub(fmt.opener.len() + ender_length + 1)) +
-                    1;
+    let max_chars = try_opt!(shape.width.checked_sub(fmt.opener.len() + ender_length + 1)) + 1;
 
     // Snip a line at a time from `orig` until it is used up. Push the snippet
     // onto result.

src/visitor.rs

@@ -39,6 +39,7 @@ pub struct FmtVisitor<'a> {
     // FIXME: use an RAII util or closure for indenting
     pub block_indent: Indent,
     pub config: &'a Config,
+    pub failed: bool,
 }
 
 impl<'a> FmtVisitor<'a> {
@@ -65,6 +66,9 @@ impl<'a> FmtVisitor<'a> {
                                            Shape::legacy(self.config.max_width -
                                                          self.block_indent.width(),
                                                          self.block_indent));
+                if rewrite.is_none() {
+                    self.failed = true;
+                }
                 self.push_rewrite(stmt.span, rewrite);
             }
             ast::StmtKind::Mac(ref mac) => {
@@ -457,6 +461,7 @@ impl<'a> FmtVisitor<'a> {
                 alignment: 0,
             },
             config: config,
+            failed: false,
         }
     }
 

tests/source/chains.rs

@@ -13,6 +13,8 @@ fn main() {
 
     bbbbbbbbbbbbbbbbbbb.ccccccccccccccccccccccccccccccccccccc.ddddddddddddddddddddddddddd.eeeeeeee();
 
+    let f = fooooooooooooooooooooooooooooooooooooooooooooooooooo.baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaar;
+
     // Test case where first chain element isn't a path, but is shorter than
     // the size of a tab.
     x()

tests/source/closure.rs

@@ -89,3 +89,18 @@ fn foo() {
         };
     });
 }
+
+fn issue1405() {
+    open_raw_fd(fd, b'r')
+        .and_then(|file| Capture::new_raw(None, |_, err| unsafe {
+            raw::pcap_fopen_offline(file, err)
+        }));
+}
+
+fn issue1466() {
+    let vertex_buffer = frame.scope(|ctx| {
+        let buffer =
+            ctx.create_host_visible_buffer::<VertexBuffer<Vertex>>(&vertices);
+        ctx.create_device_local_buffer(buffer)
+    });
+}

tests/source/configs-chain_indent-block.rs

@@ -2,5 +2,5 @@
 // Chain indent
 
 fn main() {
-    let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elit();
+    let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elite();
 }

tests/source/configs-chain_indent-visual.rs

@@ -2,5 +2,5 @@
 // Chain indent
 
 fn main() {
-    let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elit();
+    let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elite();
 }

tests/target/chains.rs

@@ -13,6 +13,9 @@ fn main() {
         .ddddddddddddddddddddddddddd
         .eeeeeeee();
 
+    let f = fooooooooooooooooooooooooooooooooooooooooooooooooooo
+        .baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaar;
+
     // Test case where first chain element isn't a path, but is shorter than
     // the size of a tab.
     x().y(|| match cond() {

tests/target/closure.rs

@@ -106,3 +106,16 @@ fn foo() {
                              };
                          });
 }
+
+fn issue1405() {
+    open_raw_fd(fd, b'r').and_then(|file| {
+        Capture::new_raw(None, |_, err| unsafe { raw::pcap_fopen_offline(file, err) })
+    });
+}
+
+fn issue1466() {
+    let vertex_buffer = frame.scope(|ctx| {
+        let buffer = ctx.create_host_visible_buffer::<VertexBuffer<Vertex>>(&vertices);
+        ctx.create_device_local_buffer(buffer)
+    });
+}

tests/target/configs-chain_indent-block.rs

@@ -8,5 +8,5 @@ fn main() {
         .amet()
         .consectetur()
         .adipiscing()
-        .elit();
+        .elite();
 }

tests/target/configs-chain_indent-visual.rs

@@ -7,5 +7,5 @@ fn main() {
                      .amet()
                      .consectetur()
                      .adipiscing()
-                     .elit();
+                     .elite();
 }