Do not normalize vertical spaces unless they are between items (#4295)

Author: topecongiro

Diff for: Configurations.md

@@ -164,14 +164,11 @@ fn bar() {
 #### `1`
 ```rust
 fn foo() {
-
     println!("a");
 }
 
 fn bar() {
-
     println!("b");
-
     println!("c");
 }
 ```

Diff for: src/formatting/macros.rs

@@ -75,7 +75,7 @@ impl Rewrite for ast::Item {
         let mut visitor = FmtVisitor::from_context(context);
         visitor.block_indent = shape.indent;
         visitor.last_pos = self.span().lo();
-        visitor.visit_item(self);
+        visitor.visit_item(self, false);
         Some(visitor.buffer.to_owned())
     }
 }
@@ -1562,7 +1562,7 @@ fn rewrite_macro_with_items(
             MacroArg::Item(item) => item,
             _ => return None,
         };
-        visitor.visit_item(&item);
+        visitor.visit_item(&item, false);
     }
 
     let mut result = String::with_capacity(256);

Diff for: src/formatting/missed_spans.rs

@@ -50,7 +50,8 @@ impl<'a> FmtVisitor<'a> {
             self.last_pos = end;
             return;
         }
-        self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet))
+        self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet));
+        self.normalize_vertical_spaces = false;
     }
 
     pub(crate) fn format_missing_with_indent(&mut self, end: BytePos) {
@@ -63,13 +64,15 @@ impl<'a> FmtVisitor<'a> {
             }
             let indent = this.block_indent.to_string(config);
             this.push_str(&indent);
-        })
+        });
+        self.normalize_vertical_spaces = false;
     }
 
     pub(crate) fn format_missing_no_indent(&mut self, end: BytePos) {
         self.format_missing_inner(end, |this, last_snippet, _| {
             this.push_str(last_snippet.trim_end());
-        })
+        });
+        self.normalize_vertical_spaces = false;
     }
 
     fn format_missing_inner<F: Fn(&mut FmtVisitor<'_>, &str, &str)>(
@@ -113,7 +116,7 @@ impl<'a> FmtVisitor<'a> {
         }
     }
 
-    fn push_vertical_spaces(&mut self, mut newline_count: usize) {
+    fn normalize_newline_count(&self, mut newline_count: usize) -> usize {
         let offset = self.buffer.chars().rev().take_while(|c| *c == '\n').count();
         let newline_upper_bound = self.config.blank_lines_upper_bound() + 1;
         let newline_lower_bound = self.config.blank_lines_lower_bound() + 1;
@@ -132,6 +135,16 @@ impl<'a> FmtVisitor<'a> {
             }
         }
 
+        newline_count
+    }
+
+    fn push_vertical_spaces(&mut self, mut newline_count: usize) {
+        if self.normalize_vertical_spaces {
+            newline_count = self.normalize_newline_count(newline_count);
+        } else if newline_count < 1 {
+            newline_count = 1;
+        }
+
         let blank_lines = "\n".repeat(newline_count);
         self.push_str(&blank_lines);
     }

Diff for: src/formatting/reorder.rs

@@ -351,6 +351,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             .any(|item| !out_of_file_lines_range!(self, item.span));
 
         if at_least_one_in_file_lines && !items.is_empty() {
+            self.normalize_vertical_spaces = true;
             let lo = items.first().unwrap().span().lo();
             let hi = items.last().unwrap().span().hi();
             let span = mk_sp(lo, hi);
@@ -382,7 +383,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 // Reaching here means items were not reordered. There must be at least
                 // one item left in `items`, so calling `unwrap()` here is safe.
                 let (item, rest) = items.split_first().unwrap();
-                self.visit_item(item);
+                self.visit_item(item, true);
                 items = rest;
             }
         }

Diff for: src/formatting/visitor.rs

@@ -88,6 +88,8 @@ pub(crate) struct FmtVisitor<'a> {
     pub(crate) macro_rewrite_failure: bool,
     pub(crate) report: FormatReport,
     pub(crate) skip_context: SkipContext,
+    /// If set to `true`, normalize number of vertical spaces on formatting missing snippets.
+    pub(crate) normalize_vertical_spaces: bool,
 }
 
 impl<'a> Drop for FmtVisitor<'a> {
@@ -141,7 +143,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
         match stmt.as_ast_node().kind {
             ast::StmtKind::Item(ref item) => {
-                self.visit_item(item);
+                self.visit_item(item, true);
                 // If the item requires a trailing ";" (like `struct Foo;`), we should have already
                 // handled it. Otherwise there still may be a trailing ";", but it is unnecessary.
                 // Drop it by fast-forwarding the visitor to the end of the item.
@@ -408,7 +410,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         self.visit_block(block, inner_attrs, true)
     }
 
-    pub(crate) fn visit_item(&mut self, item: &ast::Item) {
+    pub(crate) fn visit_item(&mut self, item: &ast::Item, normalize_spaces: bool) {
+        self.normalize_vertical_spaces = normalize_spaces;
+        self.visit_item_inner(item);
+    }
+
+    fn visit_item_inner(&mut self, item: &ast::Item) {
         skip_out_of_file_lines_range_visitor!(self, item.span);
 
         // This is where we bail out if there is a skip attribute. This is only
@@ -819,6 +826,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             macro_rewrite_failure: false,
             report,
             skip_context: Default::default(),
+            normalize_vertical_spaces: false,
         }
     }
 

Diff for: tests/source/configs/blank_lines_lower_bound/2.rs

@@ -0,0 +1,26 @@
+// rustfmt-blank_lines_lower_bound: 2
+// rustfmt-blank_lines_upper_bound: 3
+
+#[foo]
+fn foo() {
+    println!("a");
+}
+#[bar]
+#[barbar]
+fn bar() {
+    println!("b");
+    println!("c");
+}
+struct Foo {}
+enum Bar {}
+use std::io;
+extern crate foobar;
+extern crate foo;
+extern crate bar;
+trait Foo = Bar;
+impl Foo {}
+mac!();
+#[temp]
+use std::fs;
+use std::alloc;
+use std::ascii;

Diff for: tests/target/assignment.rs

@@ -16,6 +16,7 @@ fn main() {
     single_line_fit = 5;
     single_lit_fit >>= 10;
 
+
     // #2791
     let x = 2;
 }

Diff for: tests/target/configs/blank_lines_lower_bound/2.rs

@@ -0,0 +1,45 @@
+// rustfmt-blank_lines_lower_bound: 2
+// rustfmt-blank_lines_upper_bound: 3
+
+
+#[foo]
+fn foo() {
+    println!("a");
+}
+
+
+#[bar]
+#[barbar]
+fn bar() {
+    println!("b");
+    println!("c");
+}
+
+
+struct Foo {}
+
+
+enum Bar {}
+
+
+use std::io;
+
+
+extern crate bar;
+extern crate foo;
+extern crate foobar;
+
+
+trait Foo = Bar;
+
+
+impl Foo {}
+
+
+mac!();
+
+
+use std::alloc;
+use std::ascii;
+#[temp]
+use std::fs;

Diff for: tests/target/control-brace-style-always-next-line.rs

@@ -7,24 +7,28 @@ fn main() {
         let bar = ();
     }
 
+
     'label: loop
     // loop comment
     {
         let foo = ();
     }
 
+
     cond = true;
     while cond
     {
         let foo = ();
     }
 
+
     'while_label: while cond
     {
         // while comment
         let foo = ();
     }
 
+
     for obj in iter
     {
         for sub_obj in obj

Diff for: tests/target/control-brace-style-always-same-line.rs

@@ -4,22 +4,26 @@ fn main() {
         let bar = ();
     }
 
+
     'label: loop
     // loop comment
     {
         let foo = ();
     }
 
+
     cond = true;
     while cond {
         let foo = ();
     }
 
+
     'while_label: while cond {
         // while comment
         let foo = ();
     }
 
+
     for obj in iter {
         for sub_obj in obj {
             'nested_while_label: while cond {

Diff for: tests/target/else-if-brace-style-always-next-line.rs

@@ -14,8 +14,10 @@ fn main() {
         let bar = ();
     }
 
+
     let a = if 0 > 1 { unreachable!() } else { 0x0 };
 
+
     if true
     {
         let foo = ();

Diff for: tests/target/else-if-brace-style-always-same-line.rs

@@ -11,8 +11,10 @@ fn main() {
         let bar = ();
     }
 
+
     let a = if 0 > 1 { unreachable!() } else { 0x0 };
 
+
     if true {
         let foo = ();
     } else if false {

Diff for: tests/target/else-if-brace-style-closing-next-line.rs

@@ -13,8 +13,10 @@ fn main() {
         let bar = ();
     }
 
+
     let a = if 0 > 1 { unreachable!() } else { 0x0 };
 
+
     if true {
         let foo = ();
     }

Diff for: tests/target/extern.rs

@@ -74,6 +74,7 @@ extern "C" {
         file: *mut FILE,
     ) -> *mut FILE;
 
+
     async fn foo() -> *mut Bar;
     const fn foo() -> *mut Bar;
     unsafe fn foo() -> *mut Bar;

Diff for: tests/target/multiple.rs

@@ -129,6 +129,7 @@ fn main() {
         println!("{}", i);
     }
 
+
     while true {
         hello();
     }

Diff for: tests/target/remove_blank_lines.rs

@@ -1,8 +1,10 @@
 fn main() {
     let x = 1;
 
+
     let y = 2;
 
+
     println!("x + y = {}", x + y);
 }
 
@@ -20,9 +22,11 @@ fn bar() {
     let x = 1;
     // comment after statement
 
+
     // comment before statement
     let y = 2;
     let z = 3;
 
+
     println!("x + y + z = {}", x + y + z);
 }