Skip to content

Commit 9792ff0

Browse files
calebcartwrighttopecongiro
authored andcommitted
Fix line numbers in checkstyle output (#3694)
1 parent 62432fe commit 9792ff0

File tree

2 files changed

+87
-3
lines changed

2 files changed

+87
-3
lines changed

src/emitter/checkstyle.rs

+86-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ impl Emitter for CheckstyleEmitter {
2929
formatted_text,
3030
}: FormattedFile<'_>,
3131
) -> Result<EmitterResult, io::Error> {
32-
const CONTEXT_SIZE: usize = 3;
32+
const CONTEXT_SIZE: usize = 0;
3333
let filename = ensure_real_path(filename);
3434
let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
3535
output_checkstyle_file(output, filename, diff)?;
@@ -47,13 +47,18 @@ where
4747
{
4848
write!(writer, r#"<file name="{}">"#, filename.display())?;
4949
for mismatch in diff {
50+
let begin_line = mismatch.line_number;
51+
let mut current_line;
52+
let mut line_counter = 0;
5053
for line in mismatch.lines {
5154
// Do nothing with `DiffLine::Context` and `DiffLine::Resulting`.
5255
if let DiffLine::Expected(message) = line {
56+
current_line = begin_line + line_counter;
57+
line_counter += 1;
5358
write!(
5459
writer,
5560
r#"<error line="{}" severity="warning" message="Should be `{}`" />"#,
56-
mismatch.line_number,
61+
current_line,
5762
XmlEscaped(&message)
5863
)?;
5964
}
@@ -62,3 +67,82 @@ where
6267
write!(writer, "</file>")?;
6368
Ok(())
6469
}
70+
71+
#[cfg(test)]
72+
mod tests {
73+
use super::*;
74+
use std::path::PathBuf;
75+
76+
#[test]
77+
fn emits_empty_record_on_file_with_no_mismatches() {
78+
let file_name = "src/well_formatted.rs";
79+
let mut writer = Vec::new();
80+
let _ = output_checkstyle_file(&mut writer, &PathBuf::from(file_name), vec![]);
81+
assert_eq!(
82+
&writer[..],
83+
format!(r#"<file name="{}"></file>"#, file_name).as_bytes()
84+
);
85+
}
86+
87+
// https://github.com/rust-lang/rustfmt/issues/1636
88+
#[test]
89+
fn emits_single_xml_tree_containing_all_files() {
90+
let bin_file = "src/bin.rs";
91+
let bin_original = vec!["fn main() {", "println!(\"Hello, world!\");", "}"];
92+
let bin_formatted = vec!["fn main() {", " println!(\"Hello, world!\");", "}"];
93+
let lib_file = "src/lib.rs";
94+
let lib_original = vec!["fn greet() {", "println!(\"Greetings!\");", "}"];
95+
let lib_formatted = vec!["fn greet() {", " println!(\"Greetings!\");", "}"];
96+
let mut writer = Vec::new();
97+
let mut emitter = CheckstyleEmitter::default();
98+
let _ = emitter.emit_header(&mut writer);
99+
let _ = emitter
100+
.emit_formatted_file(
101+
&mut writer,
102+
FormattedFile {
103+
filename: &FileName::Real(PathBuf::from(bin_file)),
104+
original_text: &bin_original.join("\n"),
105+
formatted_text: &bin_formatted.join("\n"),
106+
},
107+
)
108+
.unwrap();
109+
let _ = emitter
110+
.emit_formatted_file(
111+
&mut writer,
112+
FormattedFile {
113+
filename: &FileName::Real(PathBuf::from(lib_file)),
114+
original_text: &lib_original.join("\n"),
115+
formatted_text: &lib_formatted.join("\n"),
116+
},
117+
)
118+
.unwrap();
119+
let _ = emitter.emit_footer(&mut writer);
120+
let exp_bin_xml = vec![
121+
format!(r#"<file name="{}">"#, bin_file),
122+
format!(
123+
r#"<error line="2" severity="warning" message="Should be `{}`" />"#,
124+
XmlEscaped(&r#" println!("Hello, world!");"#),
125+
),
126+
String::from("</file>"),
127+
];
128+
let exp_lib_xml = vec![
129+
format!(r#"<file name="{}">"#, lib_file),
130+
format!(
131+
r#"<error line="2" severity="warning" message="Should be `{}`" />"#,
132+
XmlEscaped(&r#" println!("Greetings!");"#),
133+
),
134+
String::from("</file>"),
135+
];
136+
assert_eq!(
137+
String::from_utf8(writer).unwrap(),
138+
vec![
139+
r#"<?xml version="1.0" encoding="utf-8"?>"#,
140+
"\n",
141+
r#"<checkstyle version="4.3">"#,
142+
&format!("{}{}", exp_bin_xml.join(""), exp_lib_xml.join("")),
143+
"</checkstyle>\n",
144+
]
145+
.join(""),
146+
);
147+
}
148+
}

tests/writemode/target/checkstyle.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
<?xml version="1.0" encoding="utf-8"?>
2-
<checkstyle version="4.3"><file name="tests/writemode/source/fn-single-line.rs"><error line="2" severity="warning" message="Should be `fn foo_expr() { 1 }`" /><error line="2" severity="warning" message="Should be `fn foo_stmt() { foo(); }`" /><error line="2" severity="warning" message="Should be `fn foo_decl_local() { let z = 5; }`" /><error line="2" severity="warning" message="Should be `fn foo_decl_item(x: &amp;mut i32) { x = 3; }`" /><error line="2" severity="warning" message="Should be `fn empty() {}`" /><error line="2" severity="warning" message="Should be `fn foo_return() -&gt; String { &quot;yay&quot; }`" /><error line="2" severity="warning" message="Should be `fn foo_where() -&gt; T`" /><error line="2" severity="warning" message="Should be `where`" /><error line="2" severity="warning" message="Should be ` T: Sync,`" /><error line="2" severity="warning" message="Should be `{`" /><error line="52" severity="warning" message="Should be `fn lots_of_space() { 1 }`" /><error line="59" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="59" severity="warning" message="Should be `trait CoolerTypes {`" /><error line="59" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="59" severity="warning" message="Should be `fn Foo&lt;T&gt;()`" /><error line="59" severity="warning" message="Should be `where`" /><error line="59" severity="warning" message="Should be ` T: Bar,`" /><error line="59" severity="warning" message="Should be `{`" /></file></checkstyle>
2+
<checkstyle version="4.3"><file name="tests/writemode/source/fn-single-line.rs"><error line="5" severity="warning" message="Should be `fn foo_expr() { 1 }`" /><error line="7" severity="warning" message="Should be `fn foo_stmt() { foo(); }`" /><error line="9" severity="warning" message="Should be `fn foo_decl_local() { let z = 5; }`" /><error line="11" severity="warning" message="Should be `fn foo_decl_item(x: &amp;mut i32) { x = 3; }`" /><error line="13" severity="warning" message="Should be `fn empty() {}`" /><error line="15" severity="warning" message="Should be `fn foo_return() -&gt; String { &quot;yay&quot; }`" /><error line="17" severity="warning" message="Should be `fn foo_where() -&gt; T`" /><error line="18" severity="warning" message="Should be `where`" /><error line="19" severity="warning" message="Should be ` T: Sync,`" /><error line="20" severity="warning" message="Should be `{`" /><error line="55" severity="warning" message="Should be `fn lots_of_space() { 1 }`" /><error line="60" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="63" severity="warning" message="Should be `trait CoolerTypes {`" /><error line="64" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="67" severity="warning" message="Should be `fn Foo&lt;T&gt;()`" /><error line="68" severity="warning" message="Should be `where`" /><error line="69" severity="warning" message="Should be ` T: Bar,`" /><error line="70" severity="warning" message="Should be `{`" /></file></checkstyle>

0 commit comments

Comments
 (0)