Merge pull request #745 from markstory/checkstyle-output

RFC implementation of checkstyle output

src/bin/rustfmt.rs

@@ -90,7 +90,7 @@ fn execute() -> i32 {
     opts.optopt("",
                 "write-mode",
                 "mode to write in (not usable when piping from stdin)",
-                "[replace|overwrite|display|diff|coverage]");
+                "[replace|overwrite|display|diff|coverage|checkstyle]");
     opts.optflag("", "skip-children", "don't reformat child modules");
 
     opts.optflag("",

src/checkstyle.rs

@@ -0,0 +1,82 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+use rustfmt_diff::{Mismatch, DiffLine};
+use std::io::{self, Write, Read};
+use config::WriteMode;
+
+
+pub fn output_header<T>(out: &mut T, mode: WriteMode) -> Result<(), io::Error>
+    where T: Write
+{
+    if mode == WriteMode::Checkstyle {
+        let mut xml_heading = String::new();
+        xml_heading.push_str("<?xml version=\"1.0\" encoding=\"utf-8\"?>");
+        xml_heading.push_str("\n");
+        xml_heading.push_str("<checkstyle version=\"4.3\">");
+        try!(write!(out, "{}", xml_heading));
+    }
+    Ok(())
+}
+
+pub fn output_footer<T>(out: &mut T, mode: WriteMode) -> Result<(), io::Error>
+    where T: Write
+{
+    if mode == WriteMode::Checkstyle {
+        let mut xml_tail = String::new();
+        xml_tail.push_str("</checkstyle>");
+        try!(write!(out, "{}", xml_tail));
+    }
+    Ok(())
+}
+
+pub fn output_checkstyle_file<T>(mut writer: T,
+                                 filename: &str,
+                                 diff: Vec<Mismatch>)
+                                 -> Result<(), io::Error>
+    where T: Write
+{
+    try!(write!(writer, "<file name=\"{}\">", filename));
+    for mismatch in diff {
+        for line in mismatch.lines {
+            match line {
+                DiffLine::Expected(ref str) => {
+                    let message = xml_escape_str(&str);
+                    try!(write!(writer,
+                                "<error line=\"{}\" severity=\"warning\" message=\"Should be \
+                                 `{}`\" />",
+                                mismatch.line_number,
+                                message));
+                }
+                _ => {
+                    // Do nothing with context and expected.
+                }
+            }
+        }
+    }
+    try!(write!(writer, "</file>"));
+    Ok(())
+}
+
+// Convert special characters into XML entities.
+// This is needed for checkstyle output.
+fn xml_escape_str(string: &str) -> String {
+    let mut out = String::new();
+    for c in string.chars() {
+        match c {
+            '<' => out.push_str("&lt;"),
+            '>' => out.push_str("&gt;"),
+            '"' => out.push_str("&quot;"),
+            '\'' => out.push_str("&apos;"),
+            '&' => out.push_str("&amp;"),
+            _ => out.push(c),
+        }
+    }
+    out
+}

src/config.rs

@@ -136,6 +136,8 @@ configuration_option_enum! { WriteMode:
     Coverage,
     // Unfancy stdout
     Plain,
+    // Output a checkstyle XML file.
+    Checkstyle,
 }
 
 // This trait and the following impl blocks are there so that we an use

src/filemap.rs

@@ -18,7 +18,8 @@ use std::fs::{self, File};
 use std::io::{self, Write, Read, stdout, BufWriter};
 
 use config::{NewlineStyle, Config, WriteMode};
-use rustfmt_diff::{make_diff, print_diff};
+use rustfmt_diff::{make_diff, print_diff, Mismatch};
+use checkstyle::{output_header, output_footer, output_checkstyle_file};
 
 // A map of the files of a crate, with their new content
 pub type FileMap = HashMap<String, StringBuffer>;
@@ -30,17 +31,23 @@ pub fn append_newlines(file_map: &mut FileMap) {
     }
 }
 
-pub fn write_all_files(file_map: &FileMap,
-                       mode: WriteMode,
-                       config: &Config)
-                       -> Result<(), io::Error> {
+pub fn write_all_files<T>(file_map: &FileMap,
+                          mut out: T,
+                          mode: WriteMode,
+                          config: &Config)
+                          -> Result<(), io::Error>
+    where T: Write
+{
+    output_header(&mut out, mode).ok();
     for filename in file_map.keys() {
-        try!(write_file(&file_map[filename], filename, mode, config));
+        try!(write_file(&file_map[filename], filename, &mut out, mode, config));
     }
+    output_footer(&mut out, mode).ok();
 
     Ok(())
 }
 
+
 // Prints all newlines either as `\n` or as `\r\n`.
 pub fn write_system_newlines<T>(writer: T,
                                 text: &StringBuffer,
@@ -77,11 +84,14 @@ pub fn write_system_newlines<T>(writer: T,
     }
 }
 
-pub fn write_file(text: &StringBuffer,
-                  filename: &str,
-                  mode: WriteMode,
-                  config: &Config)
-                  -> Result<Option<String>, io::Error> {
+pub fn write_file<T>(text: &StringBuffer,
+                     filename: &str,
+                     out: &mut T,
+                     mode: WriteMode,
+                     config: &Config)
+                     -> Result<Option<String>, io::Error>
+    where T: Write
+{
 
     fn source_and_formatted_text(text: &StringBuffer,
                                  filename: &str,
@@ -96,6 +106,14 @@ pub fn write_file(text: &StringBuffer,
         Ok((ori_text, fmt_text))
     }
 
+    fn create_diff(filename: &str,
+                   text: &StringBuffer,
+                   config: &Config)
+                   -> Result<Vec<Mismatch>, io::Error> {
+        let (ori, fmt) = try!(source_and_formatted_text(text, filename, config));
+        Ok(make_diff(&ori, &fmt, 3))
+    }
+
     match mode {
         WriteMode::Replace => {
             if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) {
@@ -142,6 +160,10 @@ pub fn write_file(text: &StringBuffer,
         WriteMode::Default => {
             unreachable!("The WriteMode should NEVER Be default at this point!");
         }
+        WriteMode::Checkstyle => {
+            let diff = try!(create_diff(filename, text, config));
+            try!(output_checkstyle_file(out, filename, diff));
+        }
     }
 
     Ok(None)

src/lib.rs

@@ -30,6 +30,7 @@ use syntax::codemap::{mk_sp, Span};
 use syntax::diagnostic::{EmitterWriter, Handler};
 use syntax::parse::{self, ParseSess};
 
+use std::io::stdout;
 use std::ops::{Add, Sub};
 use std::path::Path;
 use std::collections::HashMap;
@@ -45,6 +46,7 @@ mod utils;
 pub mod config;
 pub mod filemap;
 mod visitor;
+mod checkstyle;
 mod items;
 mod missed_spans;
 mod lists;
@@ -427,8 +429,8 @@ pub fn run(file: &Path, write_mode: WriteMode, config: &Config) {
     let mut result = format(file, config, mode);
 
     print!("{}", fmt_lines(&mut result, config));
-
-    let write_result = filemap::write_all_files(&result, mode, config);
+    let out = stdout();
+    let write_result = filemap::write_all_files(&result, out, mode, config);
 
     if let Err(msg) = write_result {
         println!("Error writing files: {}", msg);
@@ -441,7 +443,8 @@ pub fn run_from_stdin(input: String, write_mode: WriteMode, config: &Config) {
     let mut result = format_string(input, config, mode);
     fmt_lines(&mut result, config);
 
-    let write_result = filemap::write_file(&result["stdin"], "stdin", mode, config);
+    let mut out = stdout();
+    let write_result = filemap::write_file(&result["stdin"], "stdin", &mut out, mode, config);
 
     if let Err(msg) = write_result {
         panic!("Error writing to stdout: {}", msg);

tests/system.rs

@@ -19,7 +19,7 @@ use std::io::{self, Read, BufRead, BufReader};
 use std::path::Path;
 
 use rustfmt::*;
-use rustfmt::filemap::write_system_newlines;
+use rustfmt::filemap::{write_system_newlines, FileMap};
 use rustfmt::config::{Config, ReportTactic, WriteMode};
 use rustfmt::rustfmt_diff::*;
 
@@ -63,6 +63,42 @@ fn coverage_tests() {
     assert!(fails == 0, "{} tests failed", fails);
 }
 
+#[test]
+fn checkstyle_test() {
+    let filename = "tests/source/fn-single-line.rs";
+    let expected_filename = "tests/writemode/checkstyle.xml";
+    assert_output(filename, expected_filename, WriteMode::Checkstyle);
+}
+
+
+// Helper function for comparing the results of rustfmt
+// to a known output file generated by one of the write modes.
+fn assert_output(source: &str, expected_filename: &str, write_mode: WriteMode) {
+    let config = read_config(&source);
+    let file_map = run_rustfmt(source.to_string(), write_mode);
+
+    // Populate output by writing to a vec.
+    let mut out = vec![];
+    let _ = filemap::write_all_files(&file_map, &mut out, write_mode, &config);
+    let output = String::from_utf8(out).unwrap();
+
+    let mut expected_file = fs::File::open(&expected_filename)
+                                .ok()
+                                .expect("Couldn't open target.");
+    let mut expected_text = String::new();
+    expected_file.read_to_string(&mut expected_text)
+                 .ok()
+                 .expect("Failed reading target.");
+
+    let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE);
+    if compare.len() > 0 {
+        let mut failures = HashMap::new();
+        failures.insert(source.to_string(), compare);
+        print_mismatches(failures);
+        assert!(false, "Text does not match expected output");
+    }
+}
+
 // Idempotence tests. Files in tests/target are checked to be unaltered by
 // rustfmt.
 #[test]
@@ -145,9 +181,7 @@ fn print_mismatches(result: HashMap<String, Vec<Mismatch>>) {
     assert!(t.reset().unwrap());
 }
 
-pub fn idempotent_check(filename: String,
-                        write_mode: WriteMode)
-                        -> Result<FormatReport, HashMap<String, Vec<Mismatch>>> {
+fn read_config(filename: &str) -> Config {
     let sig_comments = read_significant_comments(&filename);
     let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..]));
 
@@ -159,8 +193,21 @@ pub fn idempotent_check(filename: String,
 
     // Don't generate warnings for to-do items.
     config.report_todo = ReportTactic::Never;
+    config
+}
 
-    let mut file_map = format(Path::new(&filename), &config, write_mode);
+// Simulate run()
+fn run_rustfmt(filename: String, write_mode: WriteMode) -> FileMap {
+    let config = read_config(&filename);
+    format(Path::new(&filename), &config, write_mode)
+}
+
+pub fn idempotent_check(filename: String,
+                        write_mode: WriteMode)
+                        -> Result<FormatReport, HashMap<String, Vec<Mismatch>>> {
+    let sig_comments = read_significant_comments(&filename);
+    let config = read_config(&filename);
+    let mut file_map = run_rustfmt(filename, write_mode);
     let format_report = fmt_lines(&mut file_map, &config);
 
     let mut write_result = HashMap::new();

tests/writemode/checkstyle.xml

@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="utf-8"?>
+<checkstyle version="4.3"><file name="tests/source/fn-single-line.rs"><error line="1" severity="warning" message="Should be `fn foo_expr() { 1 }`" /><error line="1" severity="warning" message="Should be `fn foo_stmt() { foo(); }`" /><error line="1" severity="warning" message="Should be `fn foo_decl_local() { let z = 5; }`" /><error line="1" severity="warning" message="Should be `fn foo_decl_item(x: &amp;mut i32) { x = 3; }`" /><error line="1" severity="warning" message="Should be `fn empty() {}`" /><error line="1" severity="warning" message="Should be `fn foo_return() -&gt; String { &quot;yay&quot; }`" /><error line="1" severity="warning" message="Should be `fn foo_where() -&gt; T`" /><error line="1" severity="warning" message="Should be `    where T: Sync`" /><error line="1" severity="warning" message="Should be `{`" /><error line="50" severity="warning" message="Should be `fn lots_of_space() { 1 }`" /><error line="57" severity="warning" message="Should be `    fn dummy(&amp;self) {}`" /><error line="57" severity="warning" message="Should be `trait CoolerTypes {`" /><error line="57" severity="warning" message="Should be `    fn dummy(&amp;self) {}`" /><error line="57" severity="warning" message="Should be `fn Foo&lt;T&gt;() where T: Bar {}`" /><error line="57" severity="warning" message="Should be ``" /></file></checkstyle>