Merge pull request #326 from marcusklaas/interface-refactor

Refactor rustfmt's interface and test code

9 files changed, 134 insertions, 114 deletions

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<S: Into<String>>(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<T> Iterator for CharClasses<T> 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<u8>
             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<String> {
-        // 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<String, String>)),
+    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<String, Vec<FormattingError>>,
 }
 
+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<Box<config::Config>>,
+    config: Rc<Config>,
+    result: Rc<RefCell<Option<FileMap>>>,
 }
 
 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<String>, 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<String>, write_mode: WriteMode, config: Box<Config>) {
-    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<String>, 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<String> {
         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<String> {
-        // 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");