JSON output for removing blank lines is wrong

[Timmmm]

Rustfmt version: rustfmt 2.0.0-rc.2-nightly (e9b039ea 2020-06-03)

If Rustfmt decides to remove a blank line, the JSON output says that actually nothing needs to change. Trivial repro:

 ~ cat foo.rs
fn foo() {

    let i = 0;
}
 ~ rustfmt --emit json foo.rs | jq
[
  {
    "name": "/Users/me/foo.rs",
    "mismatches": [
      {
        "original_begin_line": 2,
        "original_end_line": 2,
        "expected_begin_line": 2,
        "expected_end_line": 2,
        "original": "",
        "expected": ""
      }
    ]
  }
]

It seems fine at removing newlines in other cases:

 ~ cat foo.rs
fn foo() {
    let i =
    0;
}
 ~ rustfmt --emit json foo.rs | jq
[
  {
    "name": "/Users/timh/foo.rs",
    "mismatches": [
      {
        "original_begin_line": 2,
        "original_end_line": 3,
        "expected_begin_line": 2,
        "expected_end_line": 2,
        "original": "    let i =\n    0;",
        "expected": "    let i = 0;"
      }
    ]
  }
]

So I think the output should be something like this:

[
  {
    "name": "/Users/me/foo.rs",
    "mismatches": [
      {
        "original_begin_line": 2,
        "original_end_line": 3,
        "expected_begin_line": 2,
        "expected_end_line": 2,
        "original": "\n",
        "expected": ""
      }
    ]
  }
]

[Timmmm]

Here's another case that causes issues. It seems like there should be newlines that get stripped.

 ~ cat foo.rs
mod a;
mod c;
mod b;
mod d;
 ~ rustfmt --emit json foo.rs | jq
[
  {
    "name": "/Users/timh/foo.rs",
    "mismatches": [
      {
        "original_begin_line": 2,
        "original_end_line": 2,
        "expected_begin_line": 2,
        "expected_end_line": 2,
        "original": "mod c;",
        "expected": ""
      },
      {
        "original_begin_line": 4,
        "original_end_line": 4,
        "expected_begin_line": 3,
        "expected_end_line": 3,
        "original": "",
        "expected": "mod c;"
      }
    ]
  }
]

I think it's trying to reformat it alphabetically, like

mod a;
mod b;
mod c;
mod d;

But what you actually get is

mod a;
mod b;
mod c;mod d;

I think both instances of "mod c;" should actually be "mod c;\n".

[calebcartwright]

Thanks for the report @Timmmm, and for giving the rustfmt 2.0-rc a test drive!

I think it's trying to reformat it alphabetically, like

Yup, the default behavior is to reorder mods alphabetically, and this can controlled by the reorder_modules configuration option.

If Rustfmt decides to remove a blank line, the JSON output says that actually nothing needs to change.

Not exactly. The fact that there's mismatches in the output is indicative that something does need to be changed, it's just ambiguous what those changes are. I feel the question is really about how newline changes should be represented in the output, whether implicit/assumed given the line numbers, or explicitly.

It's clearly not explicit right now, and the scenarios you've shared here raise some good points about why IMO something should change. In addition to the removal of newline chars, it'd also be tricky to know from the json output when a newline is inserted, especially since rustfmt could be emitting different newline styles depending on the config.

The json emit mode (which is also used with --message-format json from cargo fmt) is still unstable, so we'll want to get this sorted before stabilizing the json emit mode. Within the rustfmt maintainer team we've got a lot of higher priority items on our plates right now so it'd probably be a while before we can get around to this.

If anyone's interested and willing to try updating this that would be fantastic and we'd be happy to offer assistance!

The mismatch specification logic for the json emit mode can be found here:

rustfmt/src/emitter/json.rs

Lines 57 to 102 in cf1f0cf

	fn add_misformatted_file(
	&mut self,
	filename: &FileName,
	diff: Vec<Mismatch>,
	) -> Result<(), EmitterError> {
	let mut mismatches = vec![];
	for mismatch in diff {
	let original_begin_line = mismatch.line_number_orig;
	let expected_begin_line = mismatch.line_number;
	let mut original_end_line = original_begin_line;
	let mut expected_end_line = expected_begin_line;
	let mut original_line_counter = 0;
	let mut expected_line_counter = 0;
	let mut original_lines = vec![];
	let mut expected_lines = vec![];
	for line in mismatch.lines {
	match line {
	DiffLine::Expected(msg) => {
	expected_end_line = expected_begin_line + expected_line_counter;
	expected_line_counter += 1;
	expected_lines.push(msg)
	}
	DiffLine::Resulting(msg) => {
	original_end_line = original_begin_line + original_line_counter;
	original_line_counter += 1;
	original_lines.push(msg)
	}
	DiffLine::Context(_) => continue,
	}
	}
	mismatches.push(MismatchedBlock {
	original_begin_line,
	original_end_line,
	expected_begin_line,
	expected_end_line,
	original: original_lines.join("\n"),
	expected: expected_lines.join("\n"),
	});
	}
	self.mismatched_files.push(MismatchedFile {
	name: format!("{}", filename),
	mismatches,
	});
	Ok(())

It may also be helpful to cross reference the function that generates the visual output found with the --check mode.

[Timmmm]

Thanks for the hint! Turns out the fix was quite easy. :-)