rustfmt fails on an empty file; --check's diff doesn't result in properly formatted file

[roy-work]

Generate an empty file, and run rustfmt --check on it:

$ touch zero_lines.rs
$ rustfmt --check zero_lines.rs
Diff in /Users/roy/code/random/test-crate/src/zero_lines.rs at line 1:
+
+
$

So, this is interesting; to me, an empty file is inherently correctly formatted (what else would you put in the file?). But the diff emitted by rustfmt here indicates that, instead of just an empty file, that it would like a file with two blank lines. But if you give it what it wants:

$ printf '\n\n' > two_lines.rs
$ rustfmt --check two_lines.rs
Diff in /Users/roy/code/random/test-crate/src/two_lines.rs at line 1:

-

$

… the file is still not valid, and … the diff is nonsensical. This diff indicates a 3 line file, where the second line is removed, which doesn't match our input.

What rustfmt --check seems to want, is:

$ printf '\n' > one_line.rs
$ rustfmt --check one_line.rs
$

A file with one line in it. Which is fairly arbitrary (why have that blank line?).

This is in a crate whose lib.rs is empty, as it's just a placeholder for bin/a.rs, bin/b.rs, etc. stuff.

In practice, I'm going to slap a docstring into this particular lib.rs to hack around this.

rustfmt 1.4.37-stable (59eed8a2 2021-11-01)

[calebcartwright]

Yeah that's rather annoying isn't it. Not sure what the root cause is, but probably warrants a special case handling.

Definitely open to alternative suggestions, and particularly proposed fixes, but given the uncommon nature and viable workarounds I don't think this is one we'll get to ourselves any time soon

[carsonRadtke]

https://github.com/rust-lang/rustfmt/blob/master/tests/target/empty_file.rs

It appears the the expected formatting of an "empty" file is a file with a single line. Culprit appears to be src/formatting.rs:221:

// For some reason, the source_map does not include terminating                                                                                                           
// newlines so we must add one on for each file. This is sad.                                                                                                       
source_file::append_newline(&mut visitor.buffer);

A simple fix would be wrap the append in a if visitor.buffer.len() > 0 { ... }. I have fork with this change, but there are many tests that expect an empty file to format into a single lined file. Perhaps this was an intentional design decision once upon a time?

[calebcartwright]

Thanks for looking at this @carsonRadtke! The biggest problem here is the idempotence failure, as subsequent runs shouldn't be complaining about the output from prior runs.

Treating a completely empty file as needing to be changed to have one newline character is longstanding behavior, and I'm sure if we had an operable time machine we could revisit that decision, but it's not something we should, nor frankly can, change at this point.

If you can find a way to resolve the idempotence failure that'd be great though!

[carsonRadtke]

Understood, but if a single line is expected, then there is no idempotence failure. A file containing two lines has a single "\n". A file containing two "\n"s Would look like:

\n
\n
<eof>

And rustfmt formats to

\n
<eof>

So it appears rustfmt is behaving as expected.

[calebcartwright]

Ah perhaps I misunderstood then. I thought we were running rustfmt (in the default file edit mode) and then a subsequent check was failing. If it's just a case of the check output being unclear due to the diff being exclusively whitespace characters then that's something different, and perhaps best addressed via doc updates for increased clarity

[carsonRadtke]

That doesn't appear to be the case for me.

$ touch empty.rs
$ rustfmt --check empty.rs
Diff in empty.rs at line 1:
+
+
$ rustfmt empty.rs
$ rustfmt --check empty.rs
$ echo $?
0
$ rustfmt --version
rustfmt 1.4.38-nightly (fee3a459 2022-06-05)

perhaps best addressed via doc updates for increased clarity

I'd be happy to update the docs. That being said, I don't see anything in docs/index.html that references --check or the diff. What kind of doc update did you have in mind?

[calebcartwright]

That being said, I don't see anything in docs/index.html that references --check or the diff. What kind of doc update did you have in mind?

That site is purely for configuration, but I think we can extend the help text that folks see from rustfmt itself:

rustfmt/src/bin/main.rs

Lines 99 to 103 in 08105e8

	opts.optflag(
	"",
	"check",
	"Run in 'check' mode. Exits with 0 if input is formatted correctly. Exits \
	with 1 and prints a diff if formatting is required.",

and then maybe just even in the brief blurb on --check on the readme in

https://github.com/rust-lang/rustfmt#verifying-code-is-formatted

It may even be worth expanding it a bit to note the limitations of whitespace diffs that can be/are displayed in that output, e.g. #4839

[calebcartwright]

Also just as a heads up @carsonRadtke note that you can claim an issue you'd like to work on by dropping a comment with text @rustbot claim

[carsonRadtke]

Okay, @calebcartwright, thanks for the help.

[carsonRadtke]

@rustbot claim