Merge branch 'master' of github.com:nrc/rustfmt into fn_arg_one_line

src/bin/rustfmt.rs

@@ -10,14 +10,14 @@
 
 #![cfg(not(test))]
 
-#[macro_use]
+
 extern crate log;
 extern crate rustfmt;
 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;
@@ -28,14 +28,6 @@ use std::str::FromStr;
 
 use getopts::{Matches, Options};
 
-macro_rules! msg {
-    ($($arg:tt)*) => (
-        match writeln!(&mut ::std::io::stderr(), $($arg)* ) {
-            Ok(_) => {},
-            Err(x) => panic!("Unable to write to stderr: {}", x),
-        }
-    )
-}
 
 /// Rustfmt operations.
 enum Operation {
@@ -197,7 +189,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 } => {
@@ -212,7 +204,7 @@ fn execute() -> i32 {
                 path = path_tmp;
             };
             if let Some(path) = path.as_ref() {
-                msg!("Using rustfmt config file {}", path.display());
+                println!("Using rustfmt config file {}", path.display());
             }
             for file in files {
                 // Check the file directory if the config-path could not be read or not provided
@@ -222,9 +214,9 @@ fn execute() -> i32 {
                                                                        for {}",
                                                                       file.display()));
                     if let Some(path) = path_tmp.as_ref() {
-                        msg!("Using rustfmt config file {} for {}",
-                             path.display(),
-                             file.display());
+                        println!("Using rustfmt config file {} for {}",
+                                 path.display(),
+                                 file.display());
                     }
                     config = config_tmp;
                 }
@@ -233,7 +225,7 @@ fn execute() -> i32 {
                     print_usage(&opts, &e);
                     return 1;
                 }
-                run(&file, &config);
+                run(Input::File(file), &config);
             }
             0
         }

src/config.rs

@@ -11,6 +11,7 @@
 extern crate toml;
 
 use lists::{SeparatorTactic, ListTactic};
+use std::io::Write;
 
 macro_rules! configuration_option_enum{
     ($e:ident: $( $x:ident ),+ $(,)*) => {
@@ -45,7 +46,7 @@ configuration_option_enum! { ReturnIndent:
     WithWhereClause,
 }
 
-// How to stle a struct literal.
+// How to style a struct literal.
 configuration_option_enum! { StructLitStyle:
     // First line on the same line as the opening brace, all lines aligned with
     // the first line.
@@ -222,9 +223,9 @@ macro_rules! create_config {
                 let parsed_config:ParsedConfig = match toml::decode(parsed) {
                     Some(decoded) => decoded,
                     None => {
-                        println!("Decoding config file failed. Config:\n{}", toml);
+                        msg!("Decoding config file failed. Config:\n{}", toml);
                         let parsed: toml::Value = toml.parse().expect("Could not parse TOML");
-                        println!("\n\nParsed:\n{:?}", parsed);
+                        msg!("\n\nParsed:\n{:?}", parsed);
                         panic!();
                     }
                 };

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

@@ -31,9 +31,9 @@ use syntax::errors::Handler;
 use syntax::errors::emitter::{ColorConfig, EmitterWriter};
 use syntax::parse::{self, ParseSess};
 
-use std::io::stdout;
+use std::io::{stdout, Write};
 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;
@@ -41,7 +41,7 @@ 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);
-
-    if let Err(msg) = write_result {
-        println!("Error writing files: {}", msg);
-    }
+    let report = format_lines(&mut file_map, config);
+    (file_map, report)
 }
 
-// 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 enum Input {
+    File(PathBuf),
+    Text(String),
+}
+
+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 {
+            msg!("Error writing files: {}", msg);
+        }
     }
 }

src/utils.rs

@@ -232,6 +232,16 @@ macro_rules! try_opt {
     })
 }
 
+macro_rules! msg {
+    ($($arg:tt)*) => (
+        match writeln!(&mut ::std::io::stderr(), $($arg)* ) {
+            Ok(_) => {},
+            Err(x) => panic!("Unable to write to stderr: {}", x),
+        }
+    )
+}
+
+
 // Wraps string-like values in an Option. Returns Some when the string adheres
 // to the Rewrite constraints defined for the Rewrite trait and else otherwise.
 pub fn wrap_str<S: AsRef<str>>(s: S, max_width: usize, width: usize, offset: Indent) -> Option<S> {

tests/coverage/source/comments.rs

@@ -1,3 +1,4 @@
+// rustfmt-write_mode: coverage
 /// Here's a doc comment!
 fn main() {
     // foo is bar

tests/coverage/target/comments.rs

@@ -1,3 +1,4 @@
+XX XXXXXXXXXXXXXXXXXXX XXXXXXXX
 /// Here's a doc comment!
 fn main() {
     XX XXX XX XXX

tests/system.rs

@@ -16,11 +16,11 @@ 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};
-use rustfmt::config::{Config, ReportTactic, WriteMode};
+use rustfmt::config::{Config, ReportTactic};
 use rustfmt::rustfmt_diff::*;
 
 const DIFF_CONTEXT_SIZE: usize = 3;
@@ -44,7 +44,7 @@ fn system_tests() {
     // 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, None);
+    let (_reports, count, fails) = check_files(files);
 
     // Display results.
     println!("Ran {} system tests.", count);
@@ -55,9 +55,9 @@ fn system_tests() {
 // the only difference is the coverage mode
 #[test]
 fn coverage_tests() {
-    let files = fs::read_dir("tests/coverage-source").expect("Couldn't read source dir");
+    let files = fs::read_dir("tests/coverage/source").expect("Couldn't read source dir");
     let files = files.map(get_path_string);
-    let (_reports, count, fails) = check_files(files, Some(WriteMode::Coverage));
+    let (_reports, count, fails) = check_files(files);
 
     println!("Ran {} tests in coverage mode.", count);
     assert!(fails == 0, "{} tests failed", fails);
@@ -65,21 +65,17 @@ fn coverage_tests() {
 
 #[test]
 fn checkstyle_test() {
-    let filename = "tests/source/fn-single-line.rs";
-    let expected_filename = "tests/writemode/checkstyle.xml";
-    assert_output(filename, expected_filename, Some(WriteMode::Checkstyle));
+    let filename = "tests/writemode/source/fn-single-line.rs";
+    let expected_filename = "tests/writemode/target/checkstyle.xml";
+    assert_output(filename, expected_filename);
 }
 
 
 // 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;
-    }
+fn assert_output(source: &str, expected_filename: &str) {
+    let config = read_config(&source);
+    let (file_map, _report) = format_file(source, &config);
 
     // Populate output by writing to a vec.
     let mut out = vec![];
@@ -108,7 +104,7 @@ fn idempotence_tests() {
     let files = fs::read_dir("tests/target")
                     .expect("Couldn't read target dir")
                     .map(get_path_string);
-    let (_reports, count, fails) = check_files(files, None);
+    let (_reports, count, fails) = check_files(files);
 
     // Display results.
     println!("Ran {} idempotent tests.", count);
@@ -126,7 +122,7 @@ fn self_tests() {
     // 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, None);
+    let (reports, count, fails) = check_files(files);
     let mut warnings = 0;
 
     // Display results.
@@ -145,7 +141,7 @@ fn self_tests() {
 
 // For each file, run rustfmt and collect the output.
 // Returns the number of files checked and the number of failures.
-fn check_files<I>(files: I, write_mode: Option<WriteMode>) -> (Vec<FormatReport>, u32, u32)
+fn check_files<I>(files: I) -> (Vec<FormatReport>, u32, u32)
     where I: Iterator<Item = String>
 {
     let mut count = 0;
@@ -155,7 +151,7 @@ fn check_files<I>(files: I, write_mode: Option<WriteMode>) -> (Vec<FormatReport>
     for file_name in files.filter(|f| f.ends_with(".rs")) {
         println!("Testing '{}'...", file_name);
 
-        match idempotent_check(file_name, write_mode) {
+        match idempotent_check(file_name) {
             Ok(report) => reports.push(report),
             Err(msg) => {
                 print_mismatches(msg);
@@ -192,25 +188,19 @@ fn read_config(filename: &str) -> Config {
 
     // Don't generate warnings for to-do items.
     config.report_todo = ReportTactic::Never;
+
     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>>> {
+pub fn idempotent_check(filename: String) -> 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 (file_map, format_report) = format_file(filename, &config);
 
     let mut write_result = HashMap::new();
     for (filename, text) in file_map.iter() {
@@ -224,7 +214,7 @@ pub fn idempotent_check(filename: String,
 
     let target = sig_comments.get("target").map(|x| &(*x)[..]);
 
-    handle_result(write_result, target, write_mode).map(|_| format_report)
+    handle_result(write_result, target).map(|_| format_report)
 }
 
 // Reads test config file from comments and reads its contents.
@@ -272,14 +262,13 @@ fn read_significant_comments(file_name: &str) -> HashMap<String, String> {
 // Compare output to input.
 // TODO: needs a better name, more explanation.
 fn handle_result(result: HashMap<String, String>,
-                 target: Option<&str>,
-                 write_mode: Option<WriteMode>)
+                 target: Option<&str>)
                  -> Result<(), HashMap<String, Vec<Mismatch>>> {
     let mut failures = HashMap::new();
 
     for (file_name, fmt_text) in result {
         // If file is in tests/source, compare to file with same name in tests/target.
-        let target = get_target(&file_name, target, write_mode);
+        let target = get_target(&file_name, target);
         let mut f = fs::File::open(&target).expect("Couldn't open target");
 
         let mut text = String::new();
@@ -301,29 +290,20 @@ fn handle_result(result: HashMap<String, String>,
 }
 
 // Map source file paths to their target paths.
-fn get_target(file_name: &str, target: Option<&str>, write_mode: Option<WriteMode>) -> String {
-    let file_path = Path::new(file_name);
-    let (source_path_prefix, target_path_prefix) = match write_mode {
-        Some(WriteMode::Coverage) => {
-            (Path::new("tests/coverage-source/"), "tests/coverage-target/")
+fn get_target(file_name: &str, target: Option<&str>) -> String {
+    if file_name.contains("source") {
+        let target_file_name = file_name.replace("source", "target");
+        if let Some(replace_name) = target {
+            Path::new(&target_file_name)
+                .with_file_name(replace_name)
+                .into_os_string()
+                .into_string()
+                .unwrap()
+        } else {
+            target_file_name
         }
-        _ => (Path::new("tests/source/"), "tests/target/"),
-    };
-
-    if file_path.starts_with(source_path_prefix) {
-        let mut components = file_path.components();
-        // Can't skip(2) as the resulting iterator can't as_path()
-        components.next();
-        components.next();
-
-        let new_target = match components.as_path().to_str() {
-            Some(string) => string,
-            None => file_name,
-        };
-        let base = target.unwrap_or(new_target);
-
-        format!("{}{}", target_path_prefix, base)
     } else {
+        // This is either and idempotence check or a self check
         file_name.to_owned()
     }
 }

tests/writemode/checkstyle.xml

@@ -1,2 +0,0 @@
-<?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>

tests/writemode/source/fn-single-line.rs

@@ -0,0 +1,80 @@
+// rustfmt-fn_single_line: true
+// rustfmt-write_mode: checkstyle
+// Test single-line functions.
+
+fn foo_expr() {
+    1
+}
+
+fn foo_stmt() {
+    foo();
+}
+
+fn foo_decl_local()  {
+    let z = 5;
+   }
+
+fn    foo_decl_item(x: &mut i32) {
+    x = 3;
+}
+
+   fn empty()     {
+
+}
+
+fn foo_return() -> String {
+    "yay"
+}
+
+fn foo_where() -> T where T: Sync {
+    let x = 2;
+}
+
+fn fooblock() {
+    {
+        "inner-block"
+    }
+}
+
+fn fooblock2(x: i32) {
+    let z = match x {
+        _ => 2,
+    };
+}
+
+fn comment() {
+    // this is a test comment
+    1
+}
+
+fn comment2() {
+    // multi-line comment
+    let z = 2;
+    1
+}
+
+fn only_comment() {
+    // Keep this here
+}
+
+fn aaaaaaaaaaaaaaaaa_looooooooooooooooooooooong_name() {
+    let z = "aaaaaaawwwwwwwwwwwwwwwwwwwwwwwwwwww";
+}
+
+fn lots_of_space                      ()                                                           {
+                           1                 
+}
+
+fn mac() -> Vec<i32> { vec![] }
+
+trait CoolTypes {
+    fn dummy(&self) {
+    }
+}
+
+trait CoolerTypes { fn dummy(&self) { 
+}
+}
+
+fn Foo<T>() where T: Bar {
+}

tests/writemode/target/checkstyle.xml

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