Skip to content

Commit

Permalink
Do not normalize vertical spaces unless they are between items (#4295)
Browse files Browse the repository at this point in the history
  • Loading branch information
topecongiro authored Jul 3, 2020
2 parents cccf7fe + 146c3e6 commit 7159095
Show file tree
Hide file tree
Showing 16 changed files with 123 additions and 12 deletions.
3 changes: 0 additions & 3 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,11 @@ fn bar() {
#### `1`
```rust
fn foo() {

println!("a");
}

fn bar() {

println!("b");

println!("c");
}
```
Expand Down
4 changes: 2 additions & 2 deletions src/formatting/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 17 additions & 4 deletions src/formatting/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)>(
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion src/formatting/reorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/formatting/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -819,6 +826,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
macro_rewrite_failure: false,
report,
skip_context: Default::default(),
normalize_vertical_spaces: false,
}
}

Expand Down
26 changes: 26 additions & 0 deletions tests/source/configs/blank_lines_lower_bound/2.rs
Original file line number Diff line number Diff line change
@@ -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;
1 change: 1 addition & 0 deletions tests/target/assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ fn main() {
single_line_fit = 5;
single_lit_fit >>= 10;


// #2791
let x = 2;
}
Expand Down
45 changes: 45 additions & 0 deletions tests/target/configs/blank_lines_lower_bound/2.rs
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 4 additions & 0 deletions tests/target/control-brace-style-always-next-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tests/target/control-brace-style-always-same-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions tests/target/else-if-brace-style-always-next-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ fn main() {
let bar = ();
}


let a = if 0 > 1 { unreachable!() } else { 0x0 };


if true
{
let foo = ();
Expand Down
2 changes: 2 additions & 0 deletions tests/target/else-if-brace-style-always-same-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ fn main() {
let bar = ();
}


let a = if 0 > 1 { unreachable!() } else { 0x0 };


if true {
let foo = ();
} else if false {
Expand Down
2 changes: 2 additions & 0 deletions tests/target/else-if-brace-style-closing-next-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ fn main() {
let bar = ();
}


let a = if 0 > 1 { unreachable!() } else { 0x0 };


if true {
let foo = ();
}
Expand Down
1 change: 1 addition & 0 deletions tests/target/extern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tests/target/multiple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ fn main() {
println!("{}", i);
}


while true {
hello();
}
Expand Down
4 changes: 4 additions & 0 deletions tests/target/remove_blank_lines.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
fn main() {
let x = 1;


let y = 2;


println!("x + y = {}", x + y);
}

Expand All @@ -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);
}

0 comments on commit 7159095

Please sign in to comment.