file-lines changes whitespace outside of given range

[not-my-profile]

$ printf '\n\n\nmod test;' | rustfmt +nightly --unstable-features --file-lines []
mod test;
$ printf '    mod test;' | rustfmt +nightly --unstable-features --file-lines []
mod test;
$ printf '//! ' | rustfmt +nightly --unstable-features --file-lines [] | tr ' ' .
//!
$ printf '' | rustfmt +nightly --unstable-features --file-lines [] | tr $'\n' X
X
$ printf 'fn f(){} // what' | rustfmt +nightly --unstable-features --file-lines []
fn f(){}// what

Note that:

• intial newlines are removed
• initial whitespace is removed
• the space after the doc comment is removed
• a newline at the end of the file is added
• a space before a comment is lost in certain cases

Why this is a problem
file-lines is meant to restrict the reformatting to specific sets of lines. Whitespace changes elsewhere are therefore undesirable.

#3397

[not-my-profile]

@rustbot label: +bug +only-with-option

[rustbot]

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please let @rust-lang/release know if you're having trouble with this bot.

[ytmimi]

@not-my-profile thank you for submitting the issue. I've done a little work to look into this. if you run rustfmt --help=file-lines it shows the expected syntax for specifying files:

rustfmt --file-lines '[
    {"file":"src/lib.rs","range":[7,13]},
    {"file":"src/lib.rs","range":[21,29]},
    {"file":"src/foo.rs","range":[10,11]},
    {"file":"src/foo.rs","range":[15,15]}]'

In the example you provided I see that you're using stdin as your input source for rustfmt. I believe you can specify that your input is stdin like this:

rustfmt --file-lines '[{"file":"stdin","range":[n,m]}]'  // actual line ranges not included in example

Can you give that a shot and see if it helps? Also, is this an issue when you're not using stdin?

Without looking too deeply into the code I'm assuming that by not including any files in the file-lines list rustfmt just assumes that it needs to reformat everything.

[calebcartwright]

Without looking too deeply into the code I'm assuming that by not including any files in the file-lines list rustfmt just assumes that it needs to reformat everything

I actually thought I had a vague memory of that being the case, but the help text also directly contradicts that fact. That needs to be rectified one way or the other, and probably worth checking to see if that was something that was already sorted on the other branch sometime last year

[not-my-profile]

It has nothing to do with stdin. All of these issues can be observed when specifying files with the described syntax and providing files via paths.

[ytmimi]

@not-my-profile, I finally had some time to dig into this. I don't have answers for each of the points you brought up, but I can at least explain a few of them:

• intial newlines are removed
• initial whitespace is removed

The FmtVisitor, which walks the ast, keeps track of all the edits that it makes in an internal buffer. There's no code that pushes leading whitespace at the beginning of the file to this buffer so that's why we loose all leading whitespace. Here's the code that kicks off formatting a file (aptly named format_file):

rustfmt/src/formatting.rs

Lines 180 to 198 in 8984438

	// Formats a single file/module.
	fn format_file(
	&mut self,
	path: FileName,
	module: &Module<'_>,
	is_macro_def: bool,
	) -> Result<(), ErrorKind> {
	let snippet_provider = self.parse_session.snippet_provider(module.span);
	let mut visitor = FmtVisitor::from_parse_sess(
	&self.parse_session,
	self.config,
	&snippet_provider,
	self.report.clone(),
	);
	visitor.skip_context.update_with_attrs(&self.krate.attrs);
	visitor.is_macro_def = is_macro_def;
	visitor.last_pos = snippet_provider.start_pos();
	visitor.skip_empty_lines(snippet_provider.end_pos());
	visitor.format_separate_mod(module, snippet_provider.end_pos());

I was initially tempted to look into FmtVisitor::skip_empty_lines, but all that's happening there is we're moving the visitor's last_pos pointer to point to the first line that contains some text.

As an aside, from my initial investigation FmtVisitor::skip_empty_lines seems like a decent place where we could add logic to preserve leading whitespace, however if we do that I feel like we should rename it to FmtVisitor::maybe_skip_empty_lines

---

• a newline at the end of the file is added

That's because we're always appending a newline line (also in format_file)

rustfmt/src/formatting.rs

Lines 207 to 209 in 8984438

	// 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);

---

• the space after the doc comment is removed

I haven't looked at this at all, but I assume this could be fixed by having ast::Attribute::rewrite pass span information to rewrite_doc_comment, so that we could use out_of_file_lines_range! or something similar to check if we can edit that line.

---

• a space before a comment is lost in certain cases

I'm haven't looked into this one either. It might have something to do with the comment being placed after the last ast node, but it's totally possible that it's an issue that would affect all comments. You mentioned that it only happened in certain situations. Do you have any examples where leading whitespace before a comment is preserved?

[not-my-profile]

Thanks for looking into this!

Do you have any examples where leading whitespace before a comment is preserved?

Yes, interestingly adding a newline results in the leading whitespace to be preserved:

$ printf 'fn f(){} // what' | rustfmt +nightly --unstable-features --file-lines []
fn f(){}// what
$ printf 'fn f(){} // what\n' | rustfmt +nightly --unstable-features --file-lines []
fn f(){} // what