From 0c9f27fe5ec8d80bbbd939a3fb30070f993d7044 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 27 Dec 2015 23:13:32 -0500 Subject: [PATCH 01/13] Start hacking checkstyle output in. checkstyle now shows up on the option parser, and the code still compiles/passes tests. Next up will be outputing the XML to stdout. --- src/bin/rustfmt.rs | 2 +- src/config.rs | 2 ++ src/filemap.rs | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index c795d5294090..a8a19d96bc77 100644 --- a/src/bin/rustfmt.rs +++ b/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("", diff --git a/src/config.rs b/src/config.rs index 4576ecc5d2ab..5b6a5e05cf59 100644 --- a/src/config.rs +++ b/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 diff --git a/src/filemap.rs b/src/filemap.rs index b518eaaa3440..b22ca7b9b806 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -38,6 +38,7 @@ pub fn write_all_files(file_map: &FileMap, try!(write_file(&file_map[filename], filename, mode, config)); } + // Output trailers for write mode. Ok(()) } @@ -142,6 +143,19 @@ pub fn write_file(text: &StringBuffer, WriteMode::Default => { unreachable!("The WriteMode should NEVER Be default at this point!"); } + WriteMode::Checkstyle => { + // Generate the diff for the current file. + // Output the XML tags for the lines that are different. + // Use the new text as 'should be X'. + } + WriteMode::Return => { + // io::Write is not implemented for String, working around with + // Vec + let mut v = Vec::new(); + try!(write_system_newlines(&mut v, text, config)); + // won't panic, we are writing correct utf8 + return Ok(Some(String::from_utf8(v).unwrap())); + } } Ok(None) From d188854e133ab704b4c122d43c4c3621cf12caf5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 29 Dec 2015 23:54:35 -0500 Subject: [PATCH 02/13] Hack together checkstyle output that compiles. I'm not sure it does something useful yet though. --- src/filemap.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/src/filemap.rs b/src/filemap.rs index b22ca7b9b806..cecad3f84773 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -18,7 +18,7 @@ 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, DiffLine}; // A map of the files of a crate, with their new content pub type FileMap = HashMap; @@ -34,11 +34,70 @@ pub fn write_all_files(file_map: &FileMap, mode: WriteMode, config: &Config) -> Result<(), io::Error> { + output_heading(&file_map, mode).ok(); for filename in file_map.keys() { try!(write_file(&file_map[filename], filename, mode, config)); } + output_trailing(&file_map, mode).ok(); - // Output trailers for write mode. + Ok(()) +} + +pub fn output_heading(file_map: &FileMap, + mode: WriteMode) -> Result<(), io::Error> { + let stdout = stdout(); + let mut stdout = stdout.lock(); + match mode { + WriteMode::Checkstyle => { + let mut xml_heading = String::new(); + xml_heading.push_str(""); + xml_heading.push_str("\n"); + xml_heading.push_str(""); + try!(write!(stdout, "{}", xml_heading)); + Ok(()) + } + _ => { + Ok(()) + } + } +} + +pub fn output_trailing(file_map: &FileMap, + mode: WriteMode) -> Result<(), io::Error> { + let stdout = stdout(); + let mut stdout = stdout.lock(); + match mode { + WriteMode::Checkstyle => { + let mut xml_tail = String::new(); + xml_tail.push_str(""); + try!(write!(stdout, "{}", xml_tail)); + Ok(()) + } + _ => { + Ok(()) + } + } +} + +pub fn output_checkstyle_file(mut writer: T, + filename: &str, + diff: Vec) -> Result<(), io::Error> + where T: Write +{ + try!(write!(writer, "", filename)); + for mismatch in diff { + for line in mismatch.lines { + match line { + DiffLine::Expected(ref str) => { + try!(write!(writer, "", mismatch.line_number, str)); + } + _ => { + // Do nothing with context and expected. + } + } + } + } + try!(write!(writer, "")); Ok(()) } @@ -144,9 +203,18 @@ pub fn write_file(text: &StringBuffer, unreachable!("The WriteMode should NEVER Be default at this point!"); } WriteMode::Checkstyle => { + let stdout = stdout(); + let stdout = stdout.lock(); // Generate the diff for the current file. + let mut f = try!(File::open(filename)); + let mut ori_text = String::new(); + try!(f.read_to_string(&mut ori_text)); + let mut v = Vec::new(); + try!(write_system_newlines(&mut v, text, config)); + let fmt_text = String::from_utf8(v).unwrap(); + let diff = make_diff(&ori_text, &fmt_text, 3); // Output the XML tags for the lines that are different. - // Use the new text as 'should be X'. + output_checkstyle_file(stdout, filename, diff).unwrap(); } WriteMode::Return => { // io::Write is not implemented for String, working around with From 309f284dfbd15d3d6a019e8c81b74ac8b3a5c862 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 31 Dec 2015 00:33:07 -0500 Subject: [PATCH 03/13] Fix formatting errors. --- src/filemap.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/filemap.rs b/src/filemap.rs index cecad3f84773..56e40e73c785 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -43,8 +43,7 @@ pub fn write_all_files(file_map: &FileMap, Ok(()) } -pub fn output_heading(file_map: &FileMap, - mode: WriteMode) -> Result<(), io::Error> { +pub fn output_heading(file_map: &FileMap, mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); match mode { @@ -56,14 +55,11 @@ pub fn output_heading(file_map: &FileMap, try!(write!(stdout, "{}", xml_heading)); Ok(()) } - _ => { - Ok(()) - } + _ => Ok(()), } } -pub fn output_trailing(file_map: &FileMap, - mode: WriteMode) -> Result<(), io::Error> { +pub fn output_trailing(file_map: &FileMap, mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); match mode { @@ -73,15 +69,14 @@ pub fn output_trailing(file_map: &FileMap, try!(write!(stdout, "{}", xml_tail)); Ok(()) } - _ => { - Ok(()) - } + _ => Ok(()), } } pub fn output_checkstyle_file(mut writer: T, filename: &str, - diff: Vec) -> Result<(), io::Error> + diff: Vec) + -> Result<(), io::Error> where T: Write { try!(write!(writer, "", filename)); @@ -89,7 +84,11 @@ pub fn output_checkstyle_file(mut writer: T, for line in mismatch.lines { match line { DiffLine::Expected(ref str) => { - try!(write!(writer, "", mismatch.line_number, str)); + try!(write!(writer, + "", + mismatch.line_number, + str)); } _ => { // Do nothing with context and expected. From c3632befb5f733a1a0cd8f42c6d0b8dbca100680 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 31 Dec 2015 00:33:51 -0500 Subject: [PATCH 04/13] Remove unused argument. --- src/filemap.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/filemap.rs b/src/filemap.rs index 56e40e73c785..f9e927f03b29 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -34,16 +34,16 @@ pub fn write_all_files(file_map: &FileMap, mode: WriteMode, config: &Config) -> Result<(), io::Error> { - output_heading(&file_map, mode).ok(); + output_heading(mode).ok(); for filename in file_map.keys() { try!(write_file(&file_map[filename], filename, mode, config)); } - output_trailing(&file_map, mode).ok(); + output_trailing(mode).ok(); Ok(()) } -pub fn output_heading(file_map: &FileMap, mode: WriteMode) -> Result<(), io::Error> { +pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); match mode { @@ -59,7 +59,7 @@ pub fn output_heading(file_map: &FileMap, mode: WriteMode) -> Result<(), io::Err } } -pub fn output_trailing(file_map: &FileMap, mode: WriteMode) -> Result<(), io::Error> { +pub fn output_trailing(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); match mode { From de10545906899084162b6511f15771db491551cc Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 3 Jan 2016 23:17:49 -0500 Subject: [PATCH 05/13] Encode XML entities. --- src/filemap.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/filemap.rs b/src/filemap.rs index f9e927f03b29..212b95dd25b4 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -84,11 +84,13 @@ pub fn output_checkstyle_file(mut writer: T, for line in mismatch.lines { match line { DiffLine::Expected(ref str) => { + let message = xml_escape_str(&str); + // TODO XML encode str here. try!(write!(writer, "", mismatch.line_number, - str)); + message)); } _ => { // Do nothing with context and expected. @@ -100,6 +102,23 @@ pub fn output_checkstyle_file(mut writer: T, 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("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + '\'' => out.push_str("'"), + '&' => out.push_str("&"), + _ => out.push(c), + } + } + out +} + // Prints all newlines either as `\n` or as `\r\n`. pub fn write_system_newlines(writer: T, text: &StringBuffer, From 3c968e1e38781001b2023ea6921287a16a3e7b02 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 8 Jan 2016 22:38:27 -0500 Subject: [PATCH 06/13] Update based on pull request feedback. * Extract duplicated logic. * Make checkstyle errors into warnings. --- src/filemap.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/filemap.rs b/src/filemap.rs index 212b95dd25b4..436f5501882c 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -87,7 +87,7 @@ pub fn output_checkstyle_file(mut writer: T, let message = xml_escape_str(&str); // TODO XML encode str here. try!(write!(writer, - "", mismatch.line_number, message)); @@ -174,6 +174,14 @@ pub fn write_file(text: &StringBuffer, Ok((ori_text, fmt_text)) } + fn create_diff(filename: &str, + text: &StringBuffer, + config: &Config) + -> Result, io::Error> { + let ori_text, fmt_text = try!(source_and_formatted_text(text, filename, config)); + Ok(make_diff(&ori_text, &fmt_text, 3)) + } + match mode { WriteMode::Replace => { if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) { @@ -223,16 +231,9 @@ pub fn write_file(text: &StringBuffer, WriteMode::Checkstyle => { let stdout = stdout(); let stdout = stdout.lock(); - // Generate the diff for the current file. - let mut f = try!(File::open(filename)); - let mut ori_text = String::new(); - try!(f.read_to_string(&mut ori_text)); - let mut v = Vec::new(); - try!(write_system_newlines(&mut v, text, config)); - let fmt_text = String::from_utf8(v).unwrap(); - let diff = make_diff(&ori_text, &fmt_text, 3); + let diff = try!(create_diff(filename, text, config)); // Output the XML tags for the lines that are different. - output_checkstyle_file(stdout, filename, diff).unwrap(); + try!(output_checkstyle_file(stdout, filename, diff)); } WriteMode::Return => { // io::Write is not implemented for String, working around with From 58410ddd9385b9570ba65744096ea6bedbb1e040 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 13 Jan 2016 00:22:30 -0500 Subject: [PATCH 07/13] Extract checkstyle output into a separate module. Rename functions a bit now that they are specific to checkstyle output. --- src/checkstyle.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++ src/filemap.rs | 84 +++----------------------------------------- src/lib.rs | 1 + 3 files changed, 94 insertions(+), 79 deletions(-) create mode 100644 src/checkstyle.rs diff --git a/src/checkstyle.rs b/src/checkstyle.rs new file mode 100644 index 000000000000..1eb74a99bbf8 --- /dev/null +++ b/src/checkstyle.rs @@ -0,0 +1,88 @@ +// 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 or the MIT license +// , 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, stdout}; +use WriteMode; + + +pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { + let stdout = stdout(); + let mut stdout = stdout.lock(); + match mode { + WriteMode::Checkstyle => { + let mut xml_heading = String::new(); + xml_heading.push_str(""); + xml_heading.push_str("\n"); + xml_heading.push_str(""); + try!(write!(stdout, "{}", xml_heading)); + Ok(()) + } + _ => Ok(()), + } +} + +pub fn output_footing(mode: WriteMode) -> Result<(), io::Error> { + let stdout = stdout(); + let mut stdout = stdout.lock(); + match mode { + WriteMode::Checkstyle => { + let mut xml_tail = String::new(); + xml_tail.push_str(""); + try!(write!(stdout, "{}", xml_tail)); + Ok(()) + } + _ => Ok(()), + } +} + +pub fn output_checkstyle_file(mut writer: T, + filename: &str, + diff: Vec) + -> Result<(), io::Error> + where T: Write +{ + try!(write!(writer, "", 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, + "", + mismatch.line_number, + message)); + } + _ => { + // Do nothing with context and expected. + } + } + } + } + try!(write!(writer, "")); + 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("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + '\'' => out.push_str("'"), + '&' => out.push_str("&"), + _ => out.push(c), + } + } + out +} diff --git a/src/filemap.rs b/src/filemap.rs index 436f5501882c..92d4267f6e1f 100644 --- a/src/filemap.rs +++ b/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, Mismatch, DiffLine}; +use rustfmt_diff::{make_diff, print_diff, Mismatch}; +use checkstyle::{output_heading, output_footing, output_checkstyle_file}; // A map of the files of a crate, with their new content pub type FileMap = HashMap; @@ -38,86 +39,11 @@ pub fn write_all_files(file_map: &FileMap, for filename in file_map.keys() { try!(write_file(&file_map[filename], filename, mode, config)); } - output_trailing(mode).ok(); + output_footing(mode).ok(); Ok(()) } -pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { - let stdout = stdout(); - let mut stdout = stdout.lock(); - match mode { - WriteMode::Checkstyle => { - let mut xml_heading = String::new(); - xml_heading.push_str(""); - xml_heading.push_str("\n"); - xml_heading.push_str(""); - try!(write!(stdout, "{}", xml_heading)); - Ok(()) - } - _ => Ok(()), - } -} - -pub fn output_trailing(mode: WriteMode) -> Result<(), io::Error> { - let stdout = stdout(); - let mut stdout = stdout.lock(); - match mode { - WriteMode::Checkstyle => { - let mut xml_tail = String::new(); - xml_tail.push_str(""); - try!(write!(stdout, "{}", xml_tail)); - Ok(()) - } - _ => Ok(()), - } -} - -pub fn output_checkstyle_file(mut writer: T, - filename: &str, - diff: Vec) - -> Result<(), io::Error> - where T: Write -{ - try!(write!(writer, "", filename)); - for mismatch in diff { - for line in mismatch.lines { - match line { - DiffLine::Expected(ref str) => { - let message = xml_escape_str(&str); - // TODO XML encode str here. - try!(write!(writer, - "", - mismatch.line_number, - message)); - } - _ => { - // Do nothing with context and expected. - } - } - } - } - try!(write!(writer, "")); - 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("<"), - '>' => out.push_str(">"), - '"' => out.push_str("""), - '\'' => out.push_str("'"), - '&' => out.push_str("&"), - _ => out.push(c), - } - } - out -} // Prints all newlines either as `\n` or as `\r\n`. pub fn write_system_newlines(writer: T, @@ -178,8 +104,8 @@ pub fn write_file(text: &StringBuffer, text: &StringBuffer, config: &Config) -> Result, io::Error> { - let ori_text, fmt_text = try!(source_and_formatted_text(text, filename, config)); - Ok(make_diff(&ori_text, &fmt_text, 3)) + let (ori, fmt) = try!(source_and_formatted_text(text, filename, config)); + Ok(make_diff(&ori, &fmt, 3)) } match mode { diff --git a/src/lib.rs b/src/lib.rs index fccc0f0934c9..6d60c0f278df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,7 @@ mod utils; pub mod config; pub mod filemap; mod visitor; +mod checkstyle; mod items; mod missed_spans; mod lists; From add80569ff1a701304bf3b3ef58feb2d76976113 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 13 Jan 2016 20:56:35 -0500 Subject: [PATCH 08/13] Use PartialEq/Eq implementation instead of match. Also cleanup from rebasing onto master. --- src/checkstyle.rs | 32 +++++++++++++------------------- src/filemap.rs | 8 -------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/checkstyle.rs b/src/checkstyle.rs index 1eb74a99bbf8..2a276dce120d 100644 --- a/src/checkstyle.rs +++ b/src/checkstyle.rs @@ -9,37 +9,31 @@ // except according to those terms. use rustfmt_diff::{Mismatch, DiffLine}; use std::io::{self, Write, Read, stdout}; -use WriteMode; +use config::WriteMode; pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); - match mode { - WriteMode::Checkstyle => { - let mut xml_heading = String::new(); - xml_heading.push_str(""); - xml_heading.push_str("\n"); - xml_heading.push_str(""); - try!(write!(stdout, "{}", xml_heading)); - Ok(()) - } - _ => Ok(()), + if mode == WriteMode::Checkstyle { + let mut xml_heading = String::new(); + xml_heading.push_str(""); + xml_heading.push_str("\n"); + xml_heading.push_str(""); + try!(write!(stdout, "{}", xml_heading)); } + Ok(()) } pub fn output_footing(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); - match mode { - WriteMode::Checkstyle => { - let mut xml_tail = String::new(); - xml_tail.push_str(""); - try!(write!(stdout, "{}", xml_tail)); - Ok(()) - } - _ => Ok(()), + if mode == WriteMode::Checkstyle { + let mut xml_tail = String::new(); + xml_tail.push_str(""); + try!(write!(stdout, "{}", xml_tail)); } + Ok(()) } pub fn output_checkstyle_file(mut writer: T, diff --git a/src/filemap.rs b/src/filemap.rs index 92d4267f6e1f..a2cba78ae96b 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -161,14 +161,6 @@ pub fn write_file(text: &StringBuffer, // Output the XML tags for the lines that are different. try!(output_checkstyle_file(stdout, filename, diff)); } - WriteMode::Return => { - // io::Write is not implemented for String, working around with - // Vec - let mut v = Vec::new(); - try!(write_system_newlines(&mut v, text, config)); - // won't panic, we are writing correct utf8 - return Ok(Some(String::from_utf8(v).unwrap())); - } } Ok(None) From 5632831bd17ca662a61af090a12f17461ca5fe05 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 14 Jan 2016 22:04:24 -0500 Subject: [PATCH 09/13] Clean up function names for checkstyle output. --- src/checkstyle.rs | 4 ++-- src/filemap.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/checkstyle.rs b/src/checkstyle.rs index 2a276dce120d..e25bfc03ba44 100644 --- a/src/checkstyle.rs +++ b/src/checkstyle.rs @@ -12,7 +12,7 @@ use std::io::{self, Write, Read, stdout}; use config::WriteMode; -pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { +pub fn output_header(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); if mode == WriteMode::Checkstyle { @@ -25,7 +25,7 @@ pub fn output_heading(mode: WriteMode) -> Result<(), io::Error> { Ok(()) } -pub fn output_footing(mode: WriteMode) -> Result<(), io::Error> { +pub fn output_footer(mode: WriteMode) -> Result<(), io::Error> { let stdout = stdout(); let mut stdout = stdout.lock(); if mode == WriteMode::Checkstyle { diff --git a/src/filemap.rs b/src/filemap.rs index a2cba78ae96b..dabf7ade3e15 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -19,7 +19,7 @@ use std::io::{self, Write, Read, stdout, BufWriter}; use config::{NewlineStyle, Config, WriteMode}; use rustfmt_diff::{make_diff, print_diff, Mismatch}; -use checkstyle::{output_heading, output_footing, output_checkstyle_file}; +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; @@ -35,11 +35,11 @@ pub fn write_all_files(file_map: &FileMap, mode: WriteMode, config: &Config) -> Result<(), io::Error> { - output_heading(mode).ok(); + output_header(mode).ok(); for filename in file_map.keys() { try!(write_file(&file_map[filename], filename, mode, config)); } - output_footing(mode).ok(); + output_footer(mode).ok(); Ok(()) } From 66d4faf53f59011a57991ed2b21bc198b414b588 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 17 Jan 2016 23:06:00 -0500 Subject: [PATCH 10/13] Start tests for checkstyle. They don't yet pass, I've clearly misunderstood the existing tests. --- tests/system.rs | 55 ++++++++++++++++++++++++++++++++++ tests/writemode/checkstyle.xml | 2 ++ 2 files changed, 57 insertions(+) create mode 100644 tests/writemode/checkstyle.xml diff --git a/tests/system.rs b/tests/system.rs index 378998e30a91..f7224790fd11 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -63,6 +63,30 @@ fn coverage_tests() { assert!(fails == 0, "{} tests failed", fails); } +#[test] +fn checkstyle_test() { + let filename = "tests/target/fn-single-line.rs".to_string(); + let expected = "tests/writemode/checkstyle.xml"; + + let output = run_rustfmt(filename.clone(), WriteMode::Checkstyle); + + let mut expected_file = fs::File::open(&expected) + .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 mut failures = HashMap::new(); + if expected_text != output { + let diff = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE); + failures.insert(filename, diff); + // 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,6 +169,37 @@ fn print_mismatches(result: HashMap>) { assert!(t.reset().unwrap()); } +pub fn run_rustfmt(filename: String, write_mode: WriteMode) -> String { + let sig_comments = read_significant_comments(&filename); + let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..])); + + for (key, val) in &sig_comments { + if key != "target" && key != "config" { + config.override_value(key, val); + } + } + + // Don't generate warnings for to-do items. + config.report_todo = ReportTactic::Never; + + // Simulate run() + let mut file_map = format(Path::new(&filename), &config, write_mode); + // TODO this writes directly to stdout making it impossible to test :( + let write_result = filemap::write_all_files(&file_map, write_mode, &config); + let res = write_result.unwrap(); + String::new() + + // for (filename, text) in file_map.iter() { + // let mut v = Vec::new(); + // // Won't panic, as we're not doing any IO. + // write_system_newlines(&mut v, text, &config).unwrap(); + // // Won't panic, we are writing correct utf8. + // let one_result = String::from_utf8(v).unwrap(); + // write_result.insert(filename, one_result); + // } + // write_result.remove(&filename).unwrap().to_owned() +} + pub fn idempotent_check(filename: String, write_mode: WriteMode) -> Result>> { diff --git a/tests/writemode/checkstyle.xml b/tests/writemode/checkstyle.xml new file mode 100644 index 000000000000..9e536b01a1dd --- /dev/null +++ b/tests/writemode/checkstyle.xml @@ -0,0 +1,2 @@ + + From d8c6f5954ace9890bcdbe46bb618c9465dd1fd71 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 19 Jan 2016 00:02:21 -0500 Subject: [PATCH 11/13] Update checkstyle write mode to take Write arguments. By accepting Write traits we can write tests using StringBuffer. --- src/checkstyle.rs | 18 +++++++++--------- src/filemap.rs | 35 +++++++++++++++++++---------------- src/lib.rs | 8 +++++--- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/checkstyle.rs b/src/checkstyle.rs index e25bfc03ba44..02e214864fad 100644 --- a/src/checkstyle.rs +++ b/src/checkstyle.rs @@ -8,30 +8,30 @@ // 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, stdout}; +use std::io::{self, Write, Read}; use config::WriteMode; -pub fn output_header(mode: WriteMode) -> Result<(), io::Error> { - let stdout = stdout(); - let mut stdout = stdout.lock(); +pub fn output_header(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_heading.push_str("\n"); xml_heading.push_str(""); - try!(write!(stdout, "{}", xml_heading)); + try!(write!(out, "{}", xml_heading)); } Ok(()) } -pub fn output_footer(mode: WriteMode) -> Result<(), io::Error> { - let stdout = stdout(); - let mut stdout = stdout.lock(); +pub fn output_footer(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(""); - try!(write!(stdout, "{}", xml_tail)); + try!(write!(out, "{}", xml_tail)); } Ok(()) } diff --git a/src/filemap.rs b/src/filemap.rs index dabf7ade3e15..61ad573f7027 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -31,15 +31,18 @@ pub fn append_newlines(file_map: &mut FileMap) { } } -pub fn write_all_files(file_map: &FileMap, - mode: WriteMode, - config: &Config) - -> Result<(), io::Error> { - output_header(mode).ok(); +pub fn write_all_files(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(mode).ok(); + output_footer(&mut out, mode).ok(); Ok(()) } @@ -81,11 +84,14 @@ pub fn write_system_newlines(writer: T, } } -pub fn write_file(text: &StringBuffer, - filename: &str, - mode: WriteMode, - config: &Config) - -> Result, io::Error> { +pub fn write_file(text: &StringBuffer, + filename: &str, + out: &mut T, + mode: WriteMode, + config: &Config) + -> Result, io::Error> + where T: Write +{ fn source_and_formatted_text(text: &StringBuffer, filename: &str, @@ -155,11 +161,8 @@ pub fn write_file(text: &StringBuffer, unreachable!("The WriteMode should NEVER Be default at this point!"); } WriteMode::Checkstyle => { - let stdout = stdout(); - let stdout = stdout.lock(); let diff = try!(create_diff(filename, text, config)); - // Output the XML tags for the lines that are different. - try!(output_checkstyle_file(stdout, filename, diff)); + try!(output_checkstyle_file(out, filename, diff)); } } diff --git a/src/lib.rs b/src/lib.rs index 6d60c0f278df..ad9322ab649b 100644 --- a/src/lib.rs +++ b/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; @@ -428,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); @@ -442,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); From 9c275833fcfa488775f6a137282003956d1a807b Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 20 Jan 2016 00:07:01 -0500 Subject: [PATCH 12/13] Get checkstyle tests passing. Fix up the checkstyle test so they pass. There is still an issue with the content, but I think that is caused by a problem with how diffs are being calculated presently. --- tests/system.rs | 33 +++++++++++---------------------- tests/writemode/checkstyle.xml | 2 +- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/tests/system.rs b/tests/system.rs index f7224790fd11..387148136c9b 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -65,7 +65,7 @@ fn coverage_tests() { #[test] fn checkstyle_test() { - let filename = "tests/target/fn-single-line.rs".to_string(); + let filename = "tests/source/fn-single-line.rs".to_string(); let expected = "tests/writemode/checkstyle.xml"; let output = run_rustfmt(filename.clone(), WriteMode::Checkstyle); @@ -78,12 +78,12 @@ fn checkstyle_test() { .ok() .expect("Failed reading target."); - let mut failures = HashMap::new(); - if expected_text != output { - let diff = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE); - failures.insert(filename, diff); - // print_mismatches(failures); - // assert!(false, "Text does not match expected output"); + let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE); + if compare.len() > 0 { + let mut failures = HashMap::new(); + failures.insert(filename, compare); + print_mismatches(failures); + assert!(false, "Text does not match expected output"); } } @@ -183,21 +183,10 @@ pub fn run_rustfmt(filename: String, write_mode: WriteMode) -> String { config.report_todo = ReportTactic::Never; // Simulate run() - let mut file_map = format(Path::new(&filename), &config, write_mode); - // TODO this writes directly to stdout making it impossible to test :( - let write_result = filemap::write_all_files(&file_map, write_mode, &config); - let res = write_result.unwrap(); - String::new() - - // for (filename, text) in file_map.iter() { - // let mut v = Vec::new(); - // // Won't panic, as we're not doing any IO. - // write_system_newlines(&mut v, text, &config).unwrap(); - // // Won't panic, we are writing correct utf8. - // let one_result = String::from_utf8(v).unwrap(); - // write_result.insert(filename, one_result); - // } - // write_result.remove(&filename).unwrap().to_owned() + let mut out = Vec::new(); + let file_map = format(Path::new(&filename), &config, write_mode); + let _ = filemap::write_all_files(&file_map, &mut out, write_mode, &config); + String::from_utf8(out).unwrap() } pub fn idempotent_check(filename: String, diff --git a/tests/writemode/checkstyle.xml b/tests/writemode/checkstyle.xml index 9e536b01a1dd..f655cfb3b6b5 100644 --- a/tests/writemode/checkstyle.xml +++ b/tests/writemode/checkstyle.xml @@ -1,2 +1,2 @@ - + From e9e5621307ee8d6c5b720e426a50f6f56b9216f8 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 21 Jan 2016 22:09:01 -0500 Subject: [PATCH 13/13] Extract helper functions for testing. These functions help reduce duplication in the test harness and make it easier to add tests for other write-modes in the future. --- tests/system.rs | 51 ++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/tests/system.rs b/tests/system.rs index 387148136c9b..84dbf2e1abc1 100644 --- a/tests/system.rs +++ b/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::*; @@ -65,12 +65,24 @@ fn coverage_tests() { #[test] fn checkstyle_test() { - let filename = "tests/source/fn-single-line.rs".to_string(); - let expected = "tests/writemode/checkstyle.xml"; + let filename = "tests/source/fn-single-line.rs"; + let expected_filename = "tests/writemode/checkstyle.xml"; + assert_output(filename, expected_filename, WriteMode::Checkstyle); +} - let output = run_rustfmt(filename.clone(), WriteMode::Checkstyle); - let mut expected_file = fs::File::open(&expected) +// 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(); @@ -81,7 +93,7 @@ fn checkstyle_test() { let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE); if compare.len() > 0 { let mut failures = HashMap::new(); - failures.insert(filename, compare); + failures.insert(source.to_string(), compare); print_mismatches(failures); assert!(false, "Text does not match expected output"); } @@ -169,7 +181,7 @@ fn print_mismatches(result: HashMap>) { assert!(t.reset().unwrap()); } -pub fn run_rustfmt(filename: String, write_mode: WriteMode) -> String { +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)[..])); @@ -181,30 +193,21 @@ pub fn run_rustfmt(filename: String, write_mode: WriteMode) -> String { // Don't generate warnings for to-do items. config.report_todo = ReportTactic::Never; + config +} - // Simulate run() - let mut out = Vec::new(); - let file_map = format(Path::new(&filename), &config, write_mode); - let _ = filemap::write_all_files(&file_map, &mut out, write_mode, &config); - String::from_utf8(out).unwrap() +// 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>> { let sig_comments = read_significant_comments(&filename); - let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..])); - - for (key, val) in &sig_comments { - if key != "target" && key != "config" { - config.override_value(key, val); - } - } - - // Don't generate warnings for to-do items. - config.report_todo = ReportTactic::Never; - - let mut file_map = format(Path::new(&filename), &config, write_mode); + 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();