Skip to content

Commit

Permalink
Fix issue 1124 - detect start of output rather than start of input fi…
Browse files Browse the repository at this point in the history
…le when writing output source file (#1133)

* Change required to prevent a trailing space at the end of a separate module being propagated

* Detect the start of the output file rather than the start of the input file when deciding whether to output preceding snippets - this stops unnecessary whitespace and blank lines from being inserted when spans and statements are output in an order other than that from the input file.

* Add code to prevent space from being added with the prefix snippet if a) the snippet is entirely horizontal whitespace, or b) the snippet contains whitespace  followed by a newline. This prevents trailing spaces at the end of a line from being added.

* Tests for this issue

* Tidy up `match` statements

* Add test with blank lines between `use` statements
  • Loading branch information
studoot authored and nrc committed Aug 24, 2016
1 parent d022f05 commit 61042e6
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 11 deletions.
23 changes: 15 additions & 8 deletions src/imports.rs
Expand Up @@ -178,19 +178,26 @@ impl<'a> FmtVisitor<'a> {
// Order the imports by view-path & other import path properties
ordered_use_items.sort_by(|a, b| compare_use_items(a.0, b.0).unwrap());
// First, output the span before the first import
let prev_span_str = self.snippet(codemap::mk_sp(self.last_pos, pos_before_first_use_item));
// Look for purely trailing space at the start of the prefix snippet before a linefeed, or
// a prefix that's entirely horizontal whitespace.
let prefix_span_start = match prev_span_str.find('\n') {
Some(offset) if prev_span_str[..offset].trim().is_empty() => {
self.last_pos + BytePos(offset as u32)
}
None if prev_span_str.trim().is_empty() => pos_before_first_use_item,
_ => self.last_pos,
};
// Look for indent (the line part preceding the use is all whitespace) and excise that
// from the prefix
let prev_span_str = self.snippet(codemap::mk_sp(self.last_pos, pos_before_first_use_item));
let span_end = match prev_span_str.rfind('\n') {
Some(offset) => {
if prev_span_str[offset..].trim().is_empty() {
self.last_pos + BytePos(offset as u32)
} else {
pos_before_first_use_item
}
Some(offset) if prev_span_str[offset..].trim().is_empty() => {
self.last_pos + BytePos(offset as u32)
}
None => pos_before_first_use_item,
_ => pos_before_first_use_item,
};

self.last_pos = prefix_span_start;
self.format_missing(span_end);
for ordered in ordered_use_items {
// Fake out the formatter by setting `self.last_pos` to the appropriate location before
Expand Down
8 changes: 6 additions & 2 deletions src/missed_spans.rs
Expand Up @@ -14,6 +14,10 @@ use syntax::codemap::{self, BytePos, Span, Pos};
use comment::{CodeCharKind, CommentCodeSlices, rewrite_comment};

impl<'a> FmtVisitor<'a> {
fn output_at_start(&self) -> bool {
self.buffer.len == 0
}

// TODO these format_missing methods are ugly. Refactor and add unit tests
// for the central whitespace stripping loop.
pub fn format_missing(&mut self, end: BytePos) {
Expand All @@ -25,7 +29,7 @@ impl<'a> FmtVisitor<'a> {
let config = self.config;
self.format_missing_inner(end, |this, last_snippet, snippet| {
this.buffer.push_str(last_snippet.trim_right());
if last_snippet == snippet {
if last_snippet == snippet && !this.output_at_start() {
// No new lines in the snippet.
this.buffer.push_str("\n");
}
Expand All @@ -41,7 +45,7 @@ impl<'a> FmtVisitor<'a> {

if start == end {
// Do nothing if this is the beginning of the file.
if start != self.codemap.lookup_char_pos(start).file.start_pos {
if !self.output_at_start() {
process_last_snippet(self, "", "");
}
return;
Expand Down
2 changes: 1 addition & 1 deletion src/visitor.rs
Expand Up @@ -559,7 +559,7 @@ impl<'a> FmtVisitor<'a> {
self.last_pos = filemap.start_pos;
self.block_indent = Indent::empty();
self.walk_mod_items(m);
self.format_missing(filemap.end_pos);
self.format_missing_with_indent(filemap.end_pos);
}

pub fn get_context(&self) -> RewriteContext {
Expand Down
1 change: 1 addition & 0 deletions tests/config/issue-1124.toml
@@ -0,0 +1 @@
reorder_imports = true
13 changes: 13 additions & 0 deletions tests/source/issue-1124.rs
@@ -0,0 +1,13 @@
use d; use c; use b; use a;
// The previous line has a space after the `use a;`

mod a { use d; use c; use b; use a; }

use z;

use y;



use x;
use a;
21 changes: 21 additions & 0 deletions tests/target/issue-1124.rs
@@ -0,0 +1,21 @@
use a;
use b;
use c;
use d;
// The previous line has a space after the `use a;`

mod a {
use a;
use b;
use c;
use d;
}

use a;



use x;

use y;
use z;

0 comments on commit 61042e6

Please sign in to comment.