checkstyle line numbers are offset

[aep]

in src/rustfmt_diff.rs line 41 line number is offset by context_queue to start the line at the start of the patch

  mismatch = Mismatch::new(line_number - context_queue.len() as u32);

but will make the output of checkstyle incorrect.
this affects PR1384

would adding be another member to Mismatch with the real line number be ok?

[nrc]

would adding be another member to Mismatch with the real line number be ok?

sgtm

[aep]

ideally we'd have a begin and end of the mismatch for #1384

[TimDiekmann]

Is it possible to raise the priority or give me mentoring on how to correct this bug?

[calebcartwright]

What exactly is the ask, to have the error nodes include both of the Mismatch members (the line number and the original line number)?

If so, I can submit a PR with a fix

[TimDiekmann]

I think this would be a good solution, thanks!

[calebcartwright]

Well this is turning out to be more complicated than I'd anticipated 😄 The biggest question I have is around what the expected or "correct" checkstyle xml content should be in this context as it's not clear to me what folks are using the checkstyle output for.

For a contrived example, take this source file:

 1|fn add(x: i32,
 2|y: i32) -> i32 {
 3|            x + y
 4|}
 5|
 6|
 7|fn multiply(a: i32,
 8|b: i32) -> i32 {
 9|         a * b
10| }

With standard config options, this would be formatted to:

 1|fn add(x: i32, y: i32) -> i32 {
 2|   x + y
 3|}
 4|
 5|fn multiply(a: i32, b: i32) -> i32 {
 6|    a * b
 7|}

If the checkstyle line numbers correspond to the formatted file, then we could reflect the mismatched lines with something like this:

<error line="1" severity="warning" message="Should be `fn add(x: i32, y: i32) -&gt; i32 {`" />
<error line="2" severity="warning" message="Should be `    x + y`" />
<error line="5" severity="warning" message="Should be `fn multiply(a: i32, b: i32) -&gt; i32 {`" />
<error line="6" severity="warning" message="Should be `    a * b`" />
<error line="7" severity="warning" message="Should be `}`" />

Things get a little tricky though when lines are removed as part of formatting. Take that closing } for the add function for example. It was on line 4 but is now on line 3, line 4 is now blank, and lines 8, 9, and 10 are now gone.

Would folks expect any/all of those other lines (3, 4, and/or 8-10) to be included in the checkstyle xml as well?