Merge pull request #245 from nrc/bugs

Fix a bunch of misc. bugs I found

Author: marcusklaas

src/expr.rs

@@ -203,7 +203,7 @@ fn rewrite_closure(capture: ast::CaptureClause,
 
     let fmt = ListFormatting::for_fn(argument_budget, argument_offset);
     let prefix = format!("{}|{}|", mover, write_list(&arg_items.collect::<Vec<_>>(), &fmt));
-    let block_indent = closure_block_indent(context, offset);
+    let closure_indent = closure_indent(context, offset);
 
     // Try to format closure body as a single line expression without braces.
     if body.stmts.is_empty() {
@@ -232,11 +232,11 @@ fn rewrite_closure(capture: ast::CaptureClause,
 
     // We couldn't format the closure body as a single line expression; fall
     // back to block formatting.
-    let inner_context = &RewriteContext { block_indent: block_indent, ..*context };
+    let inner_context = context.overflow_context(closure_indent - context.block_indent);
     let body_rewrite = if let ast::Expr_::ExprBlock(ref inner) = body.expr.as_ref().unwrap().node {
-        inner.rewrite(inner_context, 0, 0)
+        inner.rewrite(&inner_context, 0, 0)
     } else {
-        body.rewrite(inner_context, 0, 0)
+        body.rewrite(&inner_context, 0, 0)
     };
 
     Some(format!("{} {}", prefix, try_opt!(body_rewrite)))
@@ -250,7 +250,7 @@ impl Rewrite for ast::Block {
         }
 
         let mut visitor = FmtVisitor::from_codemap(context.codemap, context.config);
-        visitor.block_indent = context.block_indent;
+        visitor.block_indent = context.block_indent + context.overflow_indent;
 
         let prefix = match self.rules {
             ast::BlockCheckMode::PushUnsafeBlock(..) |
@@ -541,9 +541,9 @@ fn rewrite_match(context: &RewriteContext,
     let cond_str = try_opt!(cond.rewrite(context, cond_budget, offset + 6));
     let mut result = format!("match {} {{", cond_str);
 
-    let block_indent = context.block_indent;
     let nested_context = context.nested_context();
-    let arm_indent_str = make_indent(nested_context.block_indent);
+    let arm_indent = nested_context.block_indent + context.overflow_indent;
+    let arm_indent_str = make_indent(arm_indent);
 
     let open_brace_pos = span_after(mk_sp(cond.span.hi, arm_start_pos(&arms[0])),
                                     "{",
@@ -578,9 +578,8 @@ fn rewrite_match(context: &RewriteContext,
         result.push_str(&arm_indent_str);
 
         let arm_str = arm.rewrite(&nested_context,
-                                  context.config.max_width -
-                                      nested_context.block_indent,
-                                  nested_context.block_indent);
+                                  context.config.max_width - arm_indent,
+                                  arm_indent);
         if let Some(ref arm_str) = arm_str {
             result.push_str(arm_str);
         } else {
@@ -594,7 +593,7 @@ fn rewrite_match(context: &RewriteContext,
     // match expression, but meh.
 
     result.push('\n');
-    result.push_str(&make_indent(block_indent));
+    result.push_str(&make_indent(context.block_indent + context.overflow_indent));
     result.push('}');
     Some(result)
 }
@@ -694,15 +693,14 @@ impl Rewrite for ast::Arm {
         } else {
             ","
         };
-        let nested_indent = context.block_indent + context.config.tab_spaces;
 
         // Let's try and get the arm body on the same line as the condition.
         // 4 = ` => `.len()
         if context.config.max_width > line_start + comma.len() + 4 {
             let budget = context.config.max_width - line_start - comma.len() - 4;
             if let Some(ref body_str) = body.rewrite(context,
                                                      budget,
-                                                     nested_indent) {
+                                                     line_start + 4) {
                 if first_line_width(body_str) <= budget {
                     return Some(format!("{}{} => {}{}",
                                         attr_str.trim_left(),
@@ -720,7 +718,9 @@ impl Rewrite for ast::Arm {
         }
 
         let body_budget = try_opt!(width.checked_sub(context.config.tab_spaces));
-        let body_str = try_opt!(body.rewrite(context, body_budget, nested_indent));
+        let body_str = try_opt!(body.rewrite(context,
+                                             body_budget,
+                                             context.block_indent));
         Some(format!("{}{} =>\n{}{},",
                      attr_str.trim_left(),
                      pats_str,
@@ -868,8 +868,8 @@ fn rewrite_call(context: &RewriteContext,
     // 2 is for parens.
     let remaining_width = try_opt!(width.checked_sub(extra_offset + 2));
     let offset = offset + extra_offset + 1;
-    let block_indent = expr_block_indent(context, offset);
-    let inner_context = &RewriteContext { block_indent: block_indent, ..*context };
+    let inner_indent = expr_indent(context, offset);
+    let inner_context = context.overflow_context(inner_indent - context.block_indent);
 
     let items = itemize_list(context.codemap,
                              args.iter(),
@@ -878,7 +878,7 @@ fn rewrite_call(context: &RewriteContext,
                              |item| item.span.hi,
                              // Take old span when rewrite fails.
                              |item| {
-                                 item.rewrite(inner_context, remaining_width, offset)
+                                 item.rewrite(&inner_context, remaining_width, offset)
                                      .unwrap_or(context.snippet(item.span))
                              },
                              callee.span.hi + BytePos(1),
@@ -901,8 +901,8 @@ macro_rules! block_indent_helper {
     );
 }
 
-block_indent_helper!(expr_block_indent, expr_indent_style);
-block_indent_helper!(closure_block_indent, closure_indent_style);
+block_indent_helper!(expr_indent, expr_indent_style);
+block_indent_helper!(closure_indent, closure_indent_style);
 
 fn rewrite_paren(context: &RewriteContext,
                  subexpr: &ast::Expr,
@@ -1192,7 +1192,9 @@ pub fn rewrite_assign_rhs<S: Into<String>>(context: &RewriteContext,
             result.push_str(&format!("\n{}", make_indent(new_offset)));
 
             let max_width = try_opt!(context.config.max_width.checked_sub(new_offset + 1));
-            let rhs = try_opt!(ex.rewrite(&context, max_width, new_offset));
+            let rhs = try_opt!(ex.rewrite(&context.overflow_context(context.config.tab_spaces),
+                                          max_width,
+                                          new_offset));
 
             result.push_str(&rhs);
         }

src/items.rs

@@ -632,13 +632,11 @@ impl<'a> FmtVisitor<'a> {
         let break_line = !is_tuple || generics_str.contains('\n') ||
                          single_line_cost as usize + used_budget > self.config.max_width;
 
-        if break_line {
+        let tactic = if break_line {
             let indentation = make_indent(offset + self.config.tab_spaces);
             result.push('\n');
             result.push_str(&indentation);
-        }
 
-        let tactic = if break_line {
             ListTactic::Vertical
         } else {
             ListTactic::Horizontal

src/lists.rs

@@ -70,9 +70,11 @@ impl<'a> ListFormatting<'a> {
 
 pub struct ListItem {
     pub pre_comment: Option<String>,
-    // Item should include attributes and doc comments
+    // Item should include attributes and doc comments.
     pub item: String,
     pub post_comment: Option<String>,
+    // Whether there is extra whitespace before this item.
+    pub new_lines: bool,
 }
 
 impl ListItem {
@@ -86,7 +88,7 @@ impl ListItem {
     }
 
     pub fn from_str<S: Into<String>>(s: S) -> ListItem {
-        ListItem { pre_comment: None, item: s.into(), post_comment: None }
+        ListItem { pre_comment: None, item: s.into(), post_comment: None, new_lines: false }
     }
 }
 
@@ -206,10 +208,8 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
 
         // Post-comments
         if tactic != ListTactic::Vertical && item.post_comment.is_some() {
-            let formatted_comment = rewrite_comment(item.post_comment.as_ref().unwrap(),
-                                                    true,
-                                                    formatting.v_width,
-                                                    0);
+            let comment = item.post_comment.as_ref().unwrap();
+            let formatted_comment = rewrite_comment(comment, true, formatting.v_width, 0);
 
             result.push(' ');
             result.push_str(&formatted_comment);
@@ -234,6 +234,10 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
             result.push(' ');
             result.push_str(&formatted_comment);
         }
+
+        if !last && tactic == ListTactic::Vertical && item.new_lines {
+            result.push('\n');
+        }
     }
 
     result
@@ -264,13 +268,14 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
         let white_space: &[_] = &[' ', '\t'];
 
         self.inner.next().map(|item| {
+            let mut new_lines = false;
             // Pre-comment
             let pre_snippet = self.codemap.span_to_snippet(codemap::mk_sp(self.prev_span_end,
                                                                           (self.get_lo)(&item)))
                                           .unwrap();
-            let pre_snippet = pre_snippet.trim();
-            let pre_comment = if !pre_snippet.is_empty() {
-                Some(pre_snippet.to_owned())
+            let trimmed_pre_snippet = pre_snippet.trim();
+            let pre_comment = if !trimmed_pre_snippet.is_empty() {
+                Some(trimmed_pre_snippet.to_owned())
             } else {
                 None
             };
@@ -307,7 +312,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
                                      separator_index + 1)
                         }
                         // Potential *single* line comment.
-                        (_, Some(j)) => { j + 1 }
+                        (_, Some(j)) => j + 1,
                         _ => post_snippet.len()
                     }
                 },
@@ -317,18 +322,40 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
                 }
             };
 
+            if !post_snippet.is_empty() && comment_end > 0 {
+                // Account for extra whitespace between items. This is fiddly
+                // because of the way we divide pre- and post- comments.
+
+                // Everything from the separator to the next item.
+                let test_snippet = &post_snippet[comment_end-1..];
+                let first_newline = test_snippet.find('\n').unwrap_or(test_snippet.len());
+                // From the end of the first line of comments.
+                let test_snippet = &test_snippet[first_newline..];
+                let first = test_snippet.find(|c: char| !c.is_whitespace())
+                                        .unwrap_or(test_snippet.len());
+                // From the end of the first line of comments to the next non-whitespace char.
+                let test_snippet = &test_snippet[..first];
+
+                if test_snippet.chars().filter(|c| c == &'\n').count() > 1 {
+                    // There were multiple line breaks which got trimmed to nothing.
+                    new_lines = true;
+                }
+            }
+
             // Cleanup post-comment: strip separators and whitespace.
             self.prev_span_end = (self.get_hi)(&item) + BytePos(comment_end as u32);
-            let mut post_snippet = post_snippet[..comment_end].trim();
+            let post_snippet = post_snippet[..comment_end].trim();
 
-            if post_snippet.starts_with(',') {
-                post_snippet = post_snippet[1..].trim_matches(white_space);
+            let post_snippet_trimmed = if post_snippet.starts_with(',') {
+                post_snippet[1..].trim_matches(white_space)
             } else if post_snippet.ends_with(",") {
-                post_snippet = post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space);
-            }
+                post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space)
+            } else {
+                post_snippet
+            };
 
-            let post_comment = if !post_snippet.is_empty() {
-                Some(post_snippet.to_owned())
+            let post_comment = if !post_snippet_trimmed.is_empty() {
+                Some(post_snippet_trimmed.to_owned())
             } else {
                 None
             };
@@ -337,6 +364,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
                 pre_comment: pre_comment,
                 item: (self.get_item_string)(&item),
                 post_comment: post_comment,
+                new_lines: new_lines,
             }
         })
     }

src/rewrite.rs

@@ -28,7 +28,14 @@ pub trait Rewrite {
 pub struct RewriteContext<'a> {
     pub codemap: &'a CodeMap,
     pub config: &'a Config,
+
+    // Indentation due to nesting of blocks.
     pub block_indent: usize,
+    // *Extra* indentation due to overflowing to the next line, e.g.,
+    // let foo =
+    //     bar();
+    // The extra 4 spaces when formatting `bar()` is overflow_indent.
+    pub overflow_indent: usize,
 }
 
 impl<'a> RewriteContext<'a> {
@@ -37,6 +44,16 @@ impl<'a> RewriteContext<'a> {
             codemap: self.codemap,
             config: self.config,
             block_indent: self.block_indent + self.config.tab_spaces,
+            overflow_indent: self.overflow_indent,
+        }
+    }
+
+    pub fn overflow_context(&self, overflow: usize) -> RewriteContext<'a> {
+        RewriteContext {
+            codemap: self.codemap,
+            config: self.config,
+            block_indent: self.block_indent,
+            overflow_indent: overflow,
         }
     }
 

src/visitor.rs

@@ -338,6 +338,7 @@ impl<'a> FmtVisitor<'a> {
             codemap: self.codemap,
             config: self.config,
             block_indent: self.block_indent,
+            overflow_indent: 0,
         };
         // 1 = ";"
         match vp.rewrite(&context, self.config.max_width - offset - 1, offset) {
@@ -369,6 +370,7 @@ impl<'a> FmtVisitor<'a> {
             codemap: self.codemap,
             config: self.config,
             block_indent: self.block_indent,
+            overflow_indent: 0,
         }
     }
 }

tests/source/struct_lits.rs

@@ -32,6 +32,13 @@ fn main() {
         second: Item
     };
 
+    Some(Data::MethodCallData(MethodCallData {
+        span: sub_span.unwrap(),
+        scope: self.enclosing_scope(id),
+        ref_id: def_id,
+        decl_id: Some(decl_id),
+    }));
+
     Diagram { /*                 o        This graph demonstrates how                  
                *                / \       significant whitespace is           
                *               o   o      preserved.  

tests/source/structs.rs

@@ -49,15 +49,43 @@ pub struct Foo<'a, Y: Baz>
 }
 
 struct Baz {
+
     a: A,  // Comment A
     b: B, // Comment B
     c: C,   // Comment C
+
+}
+
+struct Baz {
+    a: A,  // Comment A
+
+    b: B, // Comment B
+
+
+
+
+    c: C,   // Comment C
+}
+
+struct Baz {
+
+    a: A,
+
+    b: B,
+    c: C,
+
+
+
+    
+    d: D
+
 }
 
 struct Baz
 {
     // Comment A
     a: A,
+    
     // Comment B
 b: B,
     // Comment C