Fix issue 1124 - detect start of output rather than start of input file when writing output source file

[studoot]

Fix issue 1124 by detecting start of output rather than start of an input file to introduce a newline before a use. Also add in code to detect insertion of a span that either

1. contains whitespace directly before a use, or
2. contains whitespace running on from the last insertion before a use (as a newline will be inserted before the use is rendered, so the whitespace will become trailing whitespace on a line).

[crumblingstatue]

This is great! Thanks for fixing this!

I tried it on one of my projects, and I noticed it left an empty newline at the beginning on one of the source files.

It seems that imports that are put to the beginning of the source after reordering bring their separating newlines along with them.

For example, use d;use c;use b;\n\nuse a; get reformatted to

// Empty newline
// Empty newline
use a;
use b;
use c;
use d;

I don't think the empty newlines are necessary at the beginning of the source/block. If you could also fix this, that would make this even more awesome!

Another idea: Rustfmt could strip all empty newlines between the beginning of a block and the use statements, even if there were empty newlines originally, since they never add any information or make things prettier.

[studoot]

That could be done. I've so far not looked to remove blank lines that were included in the source - just ones that were introduced within the formatting. It's difficult to decide whether blank lines are wanted by the user or not - I know that I like to have vertical space to separate things - this is probably a case where we're straying into finer matters of taste ;-)

[crumblingstatue]

It's difficult to decide whether blank lines are wanted by the user or not - I know that I like to have vertical space to separate things - this is probably a case where we're straying into finer matters of taste ;-)

The problem is that since the ordering of the use statements has been altered, the newline that was meant to logically separate things does not retain its original meaning, rendering that point invalid.

use b1;
use b2;
use b3;
// Logical empty line to separate 2 groups of imports
use a1;
use a2;
use a3;

fn foo() {}

gets reformatted to

// empty line at the beginning that serves no logical purpose
// empty line at the beginning that serves no logical purpose
use a1;
use a2;
use a3;
use b1;
use b2;
use b3;

fn foo() {}

Empty lines at the beginning of a block serve no separation purposes, since they are before the first item on the list.

[nrc]

I'd like to see some tests with blanks lines between imports and trailing whitespace before a newline

[studoot]

The current test has a space at the end of the first line already - I'll add a comment (after the use line - the whole point was putting that line at the start of the file!) to highlight that.

As for a test with blank lines - yes, no problem. That will reflect the current behaviour of the import reordering, which doesn't eliminate blank lines (as @crumblingstatue suggests, and which I think is worth looking at), but keeps them in verbatim.

[nrc]

Looks good, thank you! r+ with an extra test or two.

[studoot]

@crumblingstatue - after doing the extra test I mentioned above, I tend to agree with you about blank lines. However, I'd rather defer looking at the moment, as it'll quickly get messy with the current setup. Could you raise an issue to suggest it as a feature & CC me on it, please?

[nrc]

Thanks!