Skip to content

Commit

Permalink
Fix line numbers in checkstyle output (rust-lang#3694)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebcartwright authored and topecongiro committed Aug 19, 2019
1 parent 62432fe commit 9792ff0
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 3 deletions.
88 changes: 86 additions & 2 deletions src/emitter/checkstyle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl Emitter for CheckstyleEmitter {
formatted_text,
}: FormattedFile<'_>,
) -> Result<EmitterResult, io::Error> {
const CONTEXT_SIZE: usize = 3;
const CONTEXT_SIZE: usize = 0;
let filename = ensure_real_path(filename);
let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
output_checkstyle_file(output, filename, diff)?;
Expand All @@ -47,13 +47,18 @@ where
{
write!(writer, r#"<file name="{}">"#, filename.display())?;
for mismatch in diff {
let begin_line = mismatch.line_number;
let mut current_line;
let mut line_counter = 0;
for line in mismatch.lines {
// Do nothing with `DiffLine::Context` and `DiffLine::Resulting`.
if let DiffLine::Expected(message) = line {
current_line = begin_line + line_counter;
line_counter += 1;
write!(
writer,
r#"<error line="{}" severity="warning" message="Should be `{}`" />"#,
mismatch.line_number,
current_line,
XmlEscaped(&message)
)?;
}
Expand All @@ -62,3 +67,82 @@ where
write!(writer, "</file>")?;
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use std::path::PathBuf;

#[test]
fn emits_empty_record_on_file_with_no_mismatches() {
let file_name = "src/well_formatted.rs";
let mut writer = Vec::new();
let _ = output_checkstyle_file(&mut writer, &PathBuf::from(file_name), vec![]);
assert_eq!(
&writer[..],
format!(r#"<file name="{}"></file>"#, file_name).as_bytes()
);
}

// https://github.com/rust-lang/rustfmt/issues/1636
#[test]
fn emits_single_xml_tree_containing_all_files() {
let bin_file = "src/bin.rs";
let bin_original = vec!["fn main() {", "println!(\"Hello, world!\");", "}"];
let bin_formatted = vec!["fn main() {", " println!(\"Hello, world!\");", "}"];
let lib_file = "src/lib.rs";
let lib_original = vec!["fn greet() {", "println!(\"Greetings!\");", "}"];
let lib_formatted = vec!["fn greet() {", " println!(\"Greetings!\");", "}"];
let mut writer = Vec::new();
let mut emitter = CheckstyleEmitter::default();
let _ = emitter.emit_header(&mut writer);
let _ = emitter
.emit_formatted_file(
&mut writer,
FormattedFile {
filename: &FileName::Real(PathBuf::from(bin_file)),
original_text: &bin_original.join("\n"),
formatted_text: &bin_formatted.join("\n"),
},
)
.unwrap();
let _ = emitter
.emit_formatted_file(
&mut writer,
FormattedFile {
filename: &FileName::Real(PathBuf::from(lib_file)),
original_text: &lib_original.join("\n"),
formatted_text: &lib_formatted.join("\n"),
},
)
.unwrap();
let _ = emitter.emit_footer(&mut writer);
let exp_bin_xml = vec![
format!(r#"<file name="{}">"#, bin_file),
format!(
r#"<error line="2" severity="warning" message="Should be `{}`" />"#,
XmlEscaped(&r#" println!("Hello, world!");"#),
),
String::from("</file>"),
];
let exp_lib_xml = vec![
format!(r#"<file name="{}">"#, lib_file),
format!(
r#"<error line="2" severity="warning" message="Should be `{}`" />"#,
XmlEscaped(&r#" println!("Greetings!");"#),
),
String::from("</file>"),
];
assert_eq!(
String::from_utf8(writer).unwrap(),
vec![
r#"<?xml version="1.0" encoding="utf-8"?>"#,
"\n",
r#"<checkstyle version="4.3">"#,
&format!("{}{}", exp_bin_xml.join(""), exp_lib_xml.join("")),
"</checkstyle>\n",
]
.join(""),
);
}
}
2 changes: 1 addition & 1 deletion tests/writemode/target/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<?xml version="1.0" encoding="utf-8"?>
<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>
<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 comments on commit 9792ff0

Please sign in to comment.