Merge pull request #897 from matklad/refactor-run

Refactor run family of functions

Author: nrc

src/bin/rustfmt.rs

@@ -17,7 +17,7 @@ extern crate toml;
 extern crate env_logger;
 extern crate getopts;
 
-use rustfmt::{run, run_from_stdin};
+use rustfmt::{run, Input};
 use rustfmt::config::{Config, WriteMode};
 
 use std::env;
@@ -197,7 +197,7 @@ fn execute() -> i32 {
             // write_mode is always Plain for Stdin.
             config.write_mode = WriteMode::Plain;
 
-            run_from_stdin(input, &config);
+            run(Input::Text(input), &config);
             0
         }
         Operation::Format { files, config_path } => {
@@ -233,7 +233,7 @@ fn execute() -> i32 {
                     print_usage(&opts, &e);
                     return 1;
                 }
-                run(&file, &config);
+                run(Input::File(file), &config);
             }
             0
         }

src/filemap.rs

@@ -31,14 +31,14 @@ pub fn append_newlines(file_map: &mut FileMap) {
     }
 }
 
-pub fn write_all_files<T>(file_map: &FileMap, mut out: T, config: &Config) -> Result<(), io::Error>
+pub fn write_all_files<T>(file_map: &FileMap, out: &mut T, config: &Config) -> Result<(), io::Error>
     where T: Write
 {
-    output_header(&mut out, config.write_mode).ok();
+    output_header(out, config.write_mode).ok();
     for filename in file_map.keys() {
-        try!(write_file(&file_map[filename], filename, &mut out, config));
+        try!(write_file(&file_map[filename], filename, out, config));
     }
-    output_footer(&mut out, config.write_mode).ok();
+    output_footer(out, config.write_mode).ok();
 
     Ok(())
 }
@@ -80,11 +80,11 @@ pub fn write_system_newlines<T>(writer: T,
     }
 }
 
-pub fn write_file<T>(text: &StringBuffer,
-                     filename: &str,
-                     out: &mut T,
-                     config: &Config)
-                     -> Result<Option<String>, io::Error>
+fn write_file<T>(text: &StringBuffer,
+                 filename: &str,
+                 out: &mut T,
+                 config: &Config)
+                 -> Result<Option<String>, io::Error>
     where T: Write
 {
 

src/lib.rs

@@ -33,15 +33,15 @@ use syntax::parse::{self, ParseSess};
 
 use std::io::stdout;
 use std::ops::{Add, Sub};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::collections::HashMap;
 use std::fmt;
 
 use issues::{BadIssueSeeker, Issue};
 use filemap::FileMap;
 use visitor::FmtVisitor;
-use config::Config;
+use config::{Config, WriteMode};
 
 #[macro_use]
 mod utils;
@@ -287,7 +287,7 @@ fn fmt_ast(krate: &ast::Crate,
 // Formatting done on a char by char or line by line basis.
 // 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 {
+fn format_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
     let mut truncate_todo = Vec::new();
     let mut report = FormatReport { file_error_map: HashMap::new() };
 
@@ -367,7 +367,7 @@ pub fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
     report
 }
 
-pub fn format_string(input: String, config: &Config) -> FileMap {
+fn format_string(input: String, config: &Config) -> FileMap {
     let path = "stdin";
     let codemap = Rc::new(CodeMap::new());
 
@@ -403,7 +403,7 @@ pub fn format_string(input: String, config: &Config) -> FileMap {
     file_map
 }
 
-pub fn format(file: &Path, config: &Config) -> FileMap {
+fn format_file(file: &Path, config: &Config) -> FileMap {
     let codemap = Rc::new(CodeMap::new());
 
     let tty_handler = Handler::with_tty_emitter(ColorConfig::Auto,
@@ -428,27 +428,35 @@ pub fn format(file: &Path, config: &Config) -> FileMap {
     file_map
 }
 
-pub fn run(file: &Path, config: &Config) {
-    let mut result = format(file, config);
+pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
+    let mut file_map = match input {
+        Input::File(ref file) => format_file(file, config),
+        Input::Text(text) => format_string(text, config),
+    };
 
-    print!("{}", fmt_lines(&mut result, config));
-    let out = stdout();
-    let write_result = filemap::write_all_files(&result, out, config);
+    let report = format_lines(&mut file_map, config);
+    (file_map, report)
+}
 
-    if let Err(msg) = write_result {
-        println!("Error writing files: {}", msg);
-    }
+pub enum Input {
+    File(PathBuf),
+    Text(String),
 }
 
-// Similar to run, but takes an input String instead of a file to format
-pub fn run_from_stdin(input: String, config: &Config) {
-    let mut result = format_string(input, config);
-    fmt_lines(&mut result, config);
+pub fn run(input: Input, config: &Config) {
+    let (file_map, report) = format_input(input, config);
+
+    let ignore_errors = config.write_mode == WriteMode::Plain;
+    if !ignore_errors {
+        print!("{}", report);
+    }
 
     let mut out = stdout();
-    let write_result = filemap::write_file(&result["stdin"], "stdin", &mut out, config);
+    let write_result = filemap::write_all_files(&file_map, &mut out, config);
 
     if let Err(msg) = write_result {
-        panic!("Error writing to stdout: {}", msg);
+        if !ignore_errors {
+            println!("Error writing files: {}", msg);
+        }
     }
 }

tests/system.rs

@@ -16,7 +16,7 @@ extern crate term;
 use std::collections::HashMap;
 use std::fs;
 use std::io::{self, Read, BufRead, BufReader};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use rustfmt::*;
 use rustfmt::filemap::{write_system_newlines, FileMap};
@@ -74,12 +74,8 @@ fn checkstyle_test() {
 // 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: Option<WriteMode>) {
-    let file_map = run_rustfmt(source.to_string(), write_mode);
-
-    let mut config = read_config(&source);
-    if let Some(write_mode) = write_mode {
-        config.write_mode = write_mode;
-    }
+    let config = read_config(&source, write_mode);
+    let (file_map, _report) = format_file(source, &config);
 
     // Populate output by writing to a vec.
     let mut out = vec![];
@@ -180,7 +176,7 @@ fn print_mismatches(result: HashMap<String, Vec<Mismatch>>) {
     assert!(t.reset().unwrap());
 }
 
-fn read_config(filename: &str) -> Config {
+fn read_config(filename: &str, write_mode: Option<WriteMode>) -> Config {
     let sig_comments = read_significant_comments(&filename);
     let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..]));
 
@@ -192,25 +188,25 @@ fn read_config(filename: &str) -> Config {
 
     // Don't generate warnings for to-do items.
     config.report_todo = ReportTactic::Never;
+
+    if let Some(mode) = write_mode {
+        config.write_mode = mode
+    }
+
     config
 }
 
-// Simulate run()
-fn run_rustfmt(filename: String, write_mode: Option<WriteMode>) -> FileMap {
-    let mut config = read_config(&filename);
-    if let Some(write_mode) = write_mode {
-        config.write_mode = write_mode;
-    }
-    format(Path::new(&filename), &config)
+fn format_file<P: Into<PathBuf>>(filename: P, config: &Config) -> (FileMap, FormatReport) {
+    let input = Input::File(filename.into());
+    format_input(input, &config)
 }
 
 pub fn idempotent_check(filename: String,
                         write_mode: Option<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 config = read_config(&filename, write_mode);
+    let (file_map, format_report) = format_file(filename, &config);
 
     let mut write_result = HashMap::new();
     for (filename, text) in file_map.iter() {

tests/writemode/checkstyle.xml

@@ -1,2 +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>
+<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 {}`" /></file></checkstyle>