diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index 2bc6e255ac0..af83cf13f85 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -65,7 +65,7 @@ fn execute() -> i32 { Err(_) => Default::default(), }; - run(args, write_mode, Box::new(config)); + run(args, write_mode, &config); 0 } @@ -87,7 +87,10 @@ fn print_usage>(reason: S) { reason.into()); for option in Config::get_docs() { - println!("{}, {}, Possible values: {}", option.option_name(), option.doc_string(), option.variant_names()); + println!("{}, {}, Possible values: {}", + option.option_name(), + option.doc_string(), + option.variant_names()); } } diff --git a/src/comment.rs b/src/comment.rs index 42aaaa4f8c9..05117a7cb76 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -288,8 +288,9 @@ impl Iterator for CharClasses where T: Iterator, T::Item: RichChar { mod test { use super::{CharClasses, CodeCharKind, contains_comment, rewrite_comment, FindUncommented}; - // TODO(#217): prevent string literal from going over the limit. + // FIXME(#217): prevent string literal from going over the limit. #[test] + #[rustfmt_skip] fn format_comments() { assert_eq!("/* test */", rewrite_comment(" //test", true, 100, 100)); assert_eq!("// comment\n// on a", rewrite_comment("// comment on a", false, 10, 0)); @@ -301,9 +302,10 @@ mod test { 12)); let input = "// comment"; - let expected = "/* com\n \ - * men\n \ - * t */"; + let expected = + "/* com\n \ + * men\n \ + * t */"; assert_eq!(expected, rewrite_comment(input, true, 9, 69)); assert_eq!("/* trimmed */", rewrite_comment("/* trimmed */", true, 100, 100)); diff --git a/src/config.rs b/src/config.rs index 7ee7d200b8b..2ecee9f98fe 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,9 +10,51 @@ extern crate toml; -use {NewlineStyle, BraceStyle, ReturnIndent, StructLitStyle}; use lists::{SeparatorTactic, ListTactic}; -use issues::ReportTactic; +pub use issues::ReportTactic; + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum NewlineStyle { + Windows, // \r\n + Unix, // \n +} + +impl_enum_decodable!(NewlineStyle, Windows, Unix); + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum BraceStyle { + AlwaysNextLine, + PreferSameLine, + // Prefer same line except where there is a where clause, in which case force + // the brace to the next line. + SameLineWhere, +} + +impl_enum_decodable!(BraceStyle, AlwaysNextLine, PreferSameLine, SameLineWhere); + +// How to indent a function's return type. +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum ReturnIndent { + // Aligned with the arguments + WithArgs, + // Aligned with the where clause + WithWhereClause, +} + +impl_enum_decodable!(ReturnIndent, WithArgs, WithWhereClause); + +// How to stle a struct literal. +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum StructLitStyle { + // First line on the same line as the opening brace, all lines aligned with + // the first line. + Visual, + // First line is on a new line and all lines align with block indent. + Block, + // FIXME Maybe we should also have an option to align types. +} + +impl_enum_decodable!(StructLitStyle, Visual, Block); #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum BlockIndentStyle { @@ -183,9 +225,11 @@ create_config! { fn_args_density: Density, "Argument density in functions", fn_args_layout: StructLitStyle, "Layout of function arguments", fn_arg_indent: BlockIndentStyle, "Indent on function arguments", - where_density: Density, "Density of a where clause", // Should we at least try to put the where clause on the same line as - // the rest of the function decl? - where_indent: BlockIndentStyle, "Indentation of a where clause", // Visual will be treated like Tabbed + // Should we at least try to put the where clause on the same line as the rest of the + // function decl? + where_density: Density, "Density of a where clause", + // Visual will be treated like Tabbed + where_indent: BlockIndentStyle, "Indentation of a where clause", where_layout: ListTactic, "Element layout inside a where clause", where_pred_indent: BlockIndentStyle, "Indentation style of a where predicate", generics_indent: BlockIndentStyle, "Indentation of generics", @@ -196,7 +240,8 @@ create_config! { enum_trailing_comma: bool, "Put a trailing comma on enum declarations", report_todo: ReportTactic, "Report all occurences of TODO in source file comments", report_fixme: ReportTactic, "Report all occurences of FIXME in source file comments", - reorder_imports: bool, "Reorder import statements alphabetically", // Alphabetically, case sensitive. + // Alphabetically, case sensitive. + reorder_imports: bool, "Reorder import statements alphabetically", single_line_if_else: bool, "Put else on same line as closing brace for if statements", format_strings: bool, "Format string literals, or leave as is", chains_overflow_last: bool, "Allow last call in method chain to break the line", diff --git a/src/expr.rs b/src/expr.rs index e0ad66ba34f..a91c6c38572 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -14,11 +14,10 @@ use std::borrow::Borrow; use rewrite::{Rewrite, RewriteContext}; use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic}; use string::{StringFormat, rewrite_string}; -use StructLitStyle; use utils::{span_after, make_indent, extra_offset, first_line_width, last_line_width, wrap_str, binary_search}; use visitor::FmtVisitor; -use config::MultilineStyle; +use config::{StructLitStyle, MultilineStyle}; use comment::{FindUncommented, rewrite_comment, contains_comment}; use types::rewrite_path; use items::{span_lo_for_arg, span_hi_for_arg}; diff --git a/src/filemap.rs b/src/filemap.rs index 91aaece6558..d94c1501734 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -9,15 +9,16 @@ // except according to those terms. -// TODO tests +// TODO: add tests use strings::string_buffer::StringBuffer; + use std::collections::HashMap; use std::fs::{self, File}; use std::io::{self, Write, Read, stdout}; + use WriteMode; -use NewlineStyle; -use config::Config; +use config::{NewlineStyle, Config}; use rustfmt_diff::{make_diff, print_diff}; // A map of the files of a crate, with their new content @@ -116,7 +117,7 @@ fn write_file(text: &StringBuffer, let diff = make_diff(&ori_text, &fmt_text, 3); print_diff(diff, |line_num| format!("\nDiff at line {}:", line_num)); } - WriteMode::Return(_) => { + WriteMode::Return => { // io::Write is not implemented for String, working around with // Vec let mut v = Vec::new(); diff --git a/src/items.rs b/src/items.rs index 6351813f996..2b470e03f95 100644 --- a/src/items.rs +++ b/src/items.rs @@ -10,7 +10,6 @@ // Formatting top-level items - functions, structs, enums, traits, impls. -use {ReturnIndent, BraceStyle, StructLitStyle}; use utils::{format_mutability, format_visibility, make_indent, contains_skip, span_after, end_typaram, wrap_str}; use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic}; @@ -18,7 +17,7 @@ use expr::rewrite_assign_rhs; use comment::FindUncommented; use visitor::FmtVisitor; use rewrite::{Rewrite, RewriteContext}; -use config::{Config, BlockIndentStyle, Density}; +use config::{Config, BlockIndentStyle, Density, ReturnIndent, BraceStyle, StructLitStyle}; use syntax::{ast, abi}; use syntax::codemap::{self, Span, BytePos}; @@ -35,7 +34,7 @@ impl<'a> FmtVisitor<'a> { if let Some(ref ty) = local.ty { infix.push_str(": "); - // FIXME silly width, indent + // FIXME: silly width, indent infix.push_str(&ty.rewrite(&self.get_context(), 1000, 0).unwrap()); } @@ -271,9 +270,9 @@ impl<'a> FmtVisitor<'a> { self.config.fn_args_layout != StructLitStyle::Block { let indent = match self.config.fn_return_indent { ReturnIndent::WithWhereClause => indent + 4, - // TODO we might want to check that using the arg indent doesn't - // blow our budget, and if it does, then fallback to the where - // clause indent. + // TODO: we might want to check that using the arg indent + // doesn't blow our budget, and if it does, then fallback to + // the where clause indent. _ => arg_indent, }; @@ -356,9 +355,10 @@ impl<'a> FmtVisitor<'a> { arg_items.push(ListItem::from_str("")); } - // TODO if there are no args, there might still be a comment, but without - // spans for the comment or parens, there is no chance of getting it right. - // You also don't get to put a comment on self, unless it is explicit. + // TODO(#21): if there are no args, there might still be a comment, but + // without spans for the comment or parens, there is no chance of + // getting it right. You also don't get to put a comment on self, unless + // it is explicit. if args.len() >= min_args { let comment_span_start = if min_args == 2 { span_after(span, ",", self.codemap) @@ -444,7 +444,7 @@ impl<'a> FmtVisitor<'a> { let max_space = self.config.ideal_width + self.config.leeway; if used_space > max_space { // Whoops! bankrupt. - // TODO take evasive action, perhaps kill the indent or something. + // TODO: take evasive action, perhaps kill the indent or something. } else { budgets = Some((0, max_space - used_space, new_indent)); } @@ -574,7 +574,7 @@ impl<'a> FmtVisitor<'a> { result } ast::VariantKind::StructVariantKind(ref struct_def) => { - // TODO Should limit the width, as we have a trailing comma + // TODO: Should limit the width, as we have a trailing comma let struct_rewrite = self.format_struct("", field.node.name, field.node.vis, @@ -795,7 +795,7 @@ impl<'a> FmtVisitor<'a> { generics_offset: usize, span: Span) -> Option { - // FIXME convert bounds to where clauses where they get too big or if + // FIXME: convert bounds to where clauses where they get too big or if // there is a where clause at all. let lifetimes: &[_] = &generics.lifetimes; let tys: &[_] = &generics.ty_params; @@ -811,7 +811,7 @@ impl<'a> FmtVisitor<'a> { }; let h_budget = self.config.max_width - generics_offset - 2; - // TODO might need to insert a newline if the generics are really long + // TODO: might need to insert a newline if the generics are really long. // Strings for the generics. let context = self.get_context(); diff --git a/src/lib.rs b/src/lib.rs index 4ab1782700d..7b6adeba2c8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,8 +47,9 @@ use syntax::diagnostics; use std::path::PathBuf; use std::collections::HashMap; use std::fmt; -use std::mem::swap; use std::str::FromStr; +use std::rc::Rc; +use std::cell::RefCell; use issues::{BadIssueSeeker, Issue}; use filemap::FileMap; @@ -58,7 +59,7 @@ use config::Config; #[macro_use] mod utils; pub mod config; -mod filemap; +pub mod filemap; mod visitor; mod items; mod missed_spans; @@ -92,7 +93,7 @@ pub enum WriteMode { // Write the diff to stdout. Diff, // Return the result as a mapping from filenames to Strings. - Return(&'static Fn(HashMap)), + Return, } impl FromStr for WriteMode { @@ -109,50 +110,7 @@ impl FromStr for WriteMode { } } -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum NewlineStyle { - Windows, // \r\n - Unix, // \n -} - -impl_enum_decodable!(NewlineStyle, Windows, Unix); - -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum BraceStyle { - AlwaysNextLine, - PreferSameLine, - // Prefer same line except where there is a where clause, in which case force - // the brace to the next line. - SameLineWhere, -} - -impl_enum_decodable!(BraceStyle, AlwaysNextLine, PreferSameLine, SameLineWhere); - -// How to indent a function's return type. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum ReturnIndent { - // Aligned with the arguments - WithArgs, - // Aligned with the where clause - WithWhereClause, -} - -impl_enum_decodable!(ReturnIndent, WithArgs, WithWhereClause); - -// How to stle a struct literal. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum StructLitStyle { - // First line on the same line as the opening brace, all lines aligned with - // the first line. - Visual, - // First line is on a new line and all lines align with block indent. - Block, - // FIXME Maybe we should also have an option to align types. -} - -impl_enum_decodable!(StructLitStyle, Visual, Block); - -enum ErrorKind { +pub enum ErrorKind { // Line has exceeded character limit LineOverflow, // Line ends in whitespace @@ -177,8 +135,8 @@ impl fmt::Display for ErrorKind { } } -// Formatting errors that are identified *after* rustfmt has run -struct FormattingError { +// Formatting errors that are identified *after* rustfmt has run. +pub struct FormattingError { line: u32, kind: ErrorKind, } @@ -201,11 +159,17 @@ impl FormattingError { } } -struct FormatReport { - // Maps stringified file paths to their associated formatting errors +pub struct FormatReport { + // Maps stringified file paths to their associated formatting errors. file_error_map: HashMap>, } +impl FormatReport { + pub fn warning_count(&self) -> usize { + self.file_error_map.iter().map(|(_, ref errors)| errors.len()).fold(0, |acc, x| acc + x) + } +} + impl fmt::Display for FormatReport { // Prints all the formatting errors. fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { @@ -237,9 +201,9 @@ fn fmt_ast(krate: &ast::Crate, codemap: &CodeMap, config: &Config) -> FileMap { } // Formatting done on a char by char or line by line basis. -// TODO warn on bad license -// TODO other stuff for parity with make tidy -fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport { +// TODO(#209) warn on bad license +// TODO(#20) other stuff for parity with make tidy +pub fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport { let mut truncate_todo = Vec::new(); let mut report = FormatReport { file_error_map: HashMap::new() }; @@ -310,8 +274,8 @@ fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport { } struct RustFmtCalls { - write_mode: WriteMode, - config: Option>, + config: Rc, + result: Rc>>, } impl<'a> CompilerCalls<'a> for RustFmtCalls { @@ -326,11 +290,8 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls { } fn build_controller(&mut self, _: &Session) -> driver::CompileController<'a> { - let write_mode = self.write_mode; - - let mut config_option = None; - swap(&mut self.config, &mut config_option); - let config = config_option.unwrap(); + let result = self.result.clone(); + let config = self.config.clone(); let mut control = driver::CompileController::basic(); control.after_parse.stop = Compilation::Stop; @@ -341,29 +302,39 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls { // For some reason, the codemap does not include terminating // newlines so we must add one on for each file. This is sad. filemap::append_newlines(&mut file_map); - println!("{}", fmt_lines(&mut file_map, &*config)); - let result = filemap::write_all_files(&file_map, write_mode, &*config); - - match result { - Err(msg) => println!("Error writing files: {}", msg), - Ok(result) => { - if let WriteMode::Return(callback) = write_mode { - callback(result); - } - } - } + *result.borrow_mut() = Some(file_map); }); control } } +pub fn format(args: Vec, config: &Config) -> FileMap { + let result = Rc::new(RefCell::new(None)); + + { + let config = Rc::new(config.clone()); + let mut call_ctxt = RustFmtCalls { config: config, result: result.clone() }; + rustc_driver::run_compiler(&args, &mut call_ctxt); + } + + // Peel the union. + Rc::try_unwrap(result).ok().unwrap().into_inner().unwrap() +} + // args are the arguments passed on the command line, generally passed through // to the compiler. // write_mode determines what happens to the result of running rustfmt, see // WriteMode. -pub fn run(args: Vec, write_mode: WriteMode, config: Box) { - let mut call_ctxt = RustFmtCalls { write_mode: write_mode, config: Some(config) }; - rustc_driver::run_compiler(&args, &mut call_ctxt); +pub fn run(args: Vec, write_mode: WriteMode, config: &Config) { + let mut result = format(args, config); + + println!("{}", fmt_lines(&mut result, config)); + + let write_result = filemap::write_all_files(&result, write_mode, config); + + if let Err(msg) = write_result { + println!("Error writing files: {}", msg); + } } diff --git a/src/types.rs b/src/types.rs index 1f5e667c6fa..e52909b0d5c 100644 --- a/src/types.rs +++ b/src/types.rs @@ -127,7 +127,7 @@ impl<'a> SegmentParam<'a> { } impl<'a> Rewrite for SegmentParam<'a> { - // FIXME doesn't always use width, offset + // FIXME: doesn't always use width, offset. fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option { Some(match *self { SegmentParam::LifeTime(ref lt) => { @@ -270,8 +270,8 @@ fn rewrite_segment(segment: &ast::PathSegment, impl Rewrite for ast::WherePredicate { fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option { - // TODO dead spans? - // TODO assumes we'll always fit on one line... + // TODO: dead spans? + // TODO: don't assume we'll always fit on one line... Some(match *self { ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { ref bound_lifetimes, ref bounded_ty, diff --git a/src/visitor.rs b/src/visitor.rs index f09c198ca33..1e3ebf71cef 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -24,7 +24,7 @@ pub struct FmtVisitor<'a> { pub codemap: &'a CodeMap, pub buffer: StringBuffer, pub last_pos: BytePos, - // TODO RAII util for indenting + // TODO: RAII util for indenting pub block_indent: usize, pub config: &'a Config, } @@ -112,7 +112,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { } self.block_indent -= self.config.tab_spaces; - // TODO we should compress any newlines here to just one + // TODO: we should compress any newlines here to just one. self.format_missing_with_indent(b.span.hi - brace_compensation); self.buffer.push_str("}"); self.last_pos = b.span.hi; @@ -237,13 +237,12 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { let indent = self.block_indent; let new_fn = self.rewrite_required_fn(indent, ti.ident, sig, ti.span); - if let Some(fn_str) = new_fn { self.buffer.push_str(&fn_str); self.last_pos = ti.span.hi; } } - // TODO format trait types + // TODO: format trait types. visit::walk_trait_item(self, ti) } @@ -320,7 +319,7 @@ impl<'a> FmtVisitor<'a> { let local_file_name = self.codemap.span_to_filename(s); let is_internal = local_file_name == self.codemap.span_to_filename(m.inner); - // TODO Should rewrite properly `mod X;` + // TODO: Should rewrite properly `mod X;` if is_internal { debug!("FmtVisitor::format_mod: internal mod"); diff --git a/tests/system.rs b/tests/system.rs index 9086697978e..4e7ec958c18 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(catch_panic)] - extern crate rustfmt; extern crate diff; extern crate regex; @@ -18,9 +16,9 @@ extern crate term; use std::collections::HashMap; use std::fs; use std::io::{self, Read, BufRead, BufReader}; -use std::thread; + use rustfmt::*; -use rustfmt::config::Config; +use rustfmt::config::{Config, ReportTactic}; use rustfmt::rustfmt_diff::*; static DIFF_CONTEXT_SIZE: usize = 3; @@ -39,14 +37,14 @@ fn get_path_string(dir_entry: io::Result) -> String { // least report. #[test] fn system_tests() { - // Get all files in the tests/source directory + // Get all files in the tests/source directory. let files = fs::read_dir("tests/source").ok().expect("Couldn't read source dir."); - // turn a DirEntry into a String that represents the relative path to the file + // Turn a DirEntry into a String that represents the relative path to the + // file. let files = files.map(get_path_string); + let (_reports, count, fails) = check_files(files); - let (count, fails) = check_files(files); - - // Display results + // Display results. println!("Ran {} system tests.", count); assert!(fails == 0, "{} system tests failed", fails); } @@ -55,40 +53,69 @@ fn system_tests() { // rustfmt. #[test] fn idempotence_tests() { - // Get all files in the tests/target directory - let files = fs::read_dir("tests/target").ok().expect("Couldn't read target dir."); - let files = files.chain(fs::read_dir("tests").ok().expect("Couldn't read tests dir.")); - let files = files.chain(fs::read_dir("src/bin").ok().expect("Couldn't read src dir.")); - // turn a DirEntry into a String that represents the relative path to the file - let files = files.map(get_path_string); - // hack because there's no `IntoIterator` impl for `[T; N]` - let files = files.chain(Some("src/lib.rs".to_owned()).into_iter()); + // Get all files in the tests/target directory. + let files = fs::read_dir("tests/target") + .ok() + .expect("Couldn't read target dir.") + .map(get_path_string); + let (_reports, count, fails) = check_files(files); - let (count, fails) = check_files(files); - - // Display results + // Display results. println!("Ran {} idempotent tests.", count); assert!(fails == 0, "{} idempotent tests failed", fails); } +// Run rustfmt on itself. This operation must be idempotent. We also check that +// no warnings are emitted. +#[test] +fn self_tests() { + let files = fs::read_dir("src/bin") + .ok() + .expect("Couldn't read src dir.") + .chain(fs::read_dir("tests").ok().expect("Couldn't read tests dir.")) + .map(get_path_string); + // Hack because there's no `IntoIterator` impl for `[T; N]`. + let files = files.chain(Some("src/lib.rs".to_owned()).into_iter()); + + let (reports, count, fails) = check_files(files); + let mut warnings = 0; + + // Display results. + println!("Ran {} self tests.", count); + assert!(fails == 0, "{} self tests failed", fails); + + for format_report in reports { + println!("{}", format_report); + warnings += format_report.warning_count(); + } + + assert!(warnings == 0, "Rustfmt's code generated {} warnings", warnings); +} + // For each file, run rustfmt and collect the output. // Returns the number of files checked and the number of failures. -fn check_files(files: I) -> (u32, u32) +fn check_files(files: I) -> (Vec, u32, u32) where I: Iterator { let mut count = 0; let mut fails = 0; + let mut reports = vec![]; for file_name in files.filter(|f| f.ends_with(".rs")) { println!("Testing '{}'...", file_name); - if let Err(msg) = idempotent_check(file_name) { - print_mismatches(msg); - fails += 1; + + match idempotent_check(file_name) { + Ok(report) => reports.push(report), + Err(msg) => { + print_mismatches(msg); + fails += 1; + } } + count += 1; } - (count, fails) + (reports, count, fails) } fn print_mismatches(result: HashMap>) { @@ -101,35 +128,34 @@ fn print_mismatches(result: HashMap>) { assert!(t.reset().unwrap()); } -// Ick, just needed to get a &'static to handle_result. -static HANDLE_RESULT: &'static Fn(HashMap) = &handle_result; - -pub fn idempotent_check(filename: String) -> Result<(), HashMap>> { +pub fn idempotent_check(filename: String) -> Result>> { let sig_comments = read_significant_comments(&filename); let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..])); let args = vec!["rustfmt".to_owned(), filename]; - for (key, val) in sig_comments { + for (key, val) in &sig_comments { if key != "target" && key != "config" { - config.override_value(&key, &val); + config.override_value(key, val); } } - // this thread is not used for concurrency, but rather to workaround the issue that the passed - // function handle needs to have static lifetime. Instead of using a global RefCell, we use - // panic to return a result in case of failure. This has the advantage of smoothing the road to - // multithreaded rustfmt - thread::catch_panic(move || { - run(args, WriteMode::Return(HANDLE_RESULT), config); - }) - .map_err(|any| *any.downcast().ok().expect("Downcast failed.")) + // Don't generate warnings for to-do items. + config.report_todo = ReportTactic::Never; + + let mut file_map = format(args, &config); + let format_report = fmt_lines(&mut file_map, &config); + + // Won't panic, as we're not doing any IO. + let write_result = filemap::write_all_files(&file_map, WriteMode::Return, &config).unwrap(); + let target = sig_comments.get("target").map(|x| &(*x)[..]); + + handle_result(write_result, target).map(|_| format_report) } - // Reads test config file from comments and reads its contents. -fn get_config(config_file: Option<&str>) -> Box { +fn get_config(config_file: Option<&str>) -> Config { let config_file_name = match config_file { - None => return Box::new(Default::default()), + None => return Default::default(), Some(file_name) => { let mut full_path = "tests/config/".to_owned(); full_path.push_str(&file_name); @@ -143,7 +169,7 @@ fn get_config(config_file: Option<&str>) -> Box { let mut def_config = String::new(); def_config_file.read_to_string(&mut def_config).ok().expect("Couldn't read config."); - Box::new(Config::from_toml(&def_config)) + Config::from_toml(&def_config) } // Reads significant comments of the form: // rustfmt-key: value @@ -175,29 +201,29 @@ fn read_significant_comments(file_name: &str) -> HashMap { // Compare output to input. // TODO: needs a better name, more explanation. -fn handle_result(result: HashMap) { +fn handle_result(result: HashMap, + target: Option<&str>) + -> Result<(), HashMap>> { let mut failures = HashMap::new(); for (file_name, fmt_text) in result { - // FIXME: reading significant comments again. Is there a way we can just - // pass the target to this function? - let sig_comments = read_significant_comments(&file_name); - // If file is in tests/source, compare to file with same name in tests/target. - let target = get_target(&file_name, sig_comments.get("target").map(|x| &(*x)[..])); + let target = get_target(&file_name, target); let mut f = fs::File::open(&target).ok().expect("Couldn't open target."); let mut text = String::new(); - // TODO: speedup by running through bytes iterator f.read_to_string(&mut text).ok().expect("Failed reading target."); + if fmt_text != text { let diff = make_diff(&text, &fmt_text, DIFF_CONTEXT_SIZE); failures.insert(file_name, diff); } } - if !failures.is_empty() { - panic!(failures); + if failures.is_empty() { + Ok(()) + } else { + Err(failures) } }