return non-zero exit code if there were errors

Author: matklad

README.md

@@ -63,10 +63,18 @@ diff, replace, overwrite, display, coverage, and checkstyle.
 The write mode can be set by passing the `--write-mode` flag on
 the command line. For example `rustfmt --write-mode=display src/filename.rs`
 
-You can run `rustfmt --help` for more information.
-
 `cargo fmt` uses `--write-mode=replace` by default.
 
+If `rustfmt` successfully reformatted the code it will exit with `0` exit
+status. Exit status `1` signals some unexpected error, like an unknown option or
+a failure to read a file. Exit status `2` is returned if there are syntax errors
+in the input files. `rustfmt` can't format syntatically invalid code. Finally,
+exit status `3` is returned if there are some issues which can't be resolved
+automatically. For example, if you have a very long comment line `rustfmt`
+doesn't split it. Instead it prints a warning and exits with `3`.
+
+You can run `rustfmt --help` for more information.
+
 
 ## Running Rustfmt from your editor
 

src/bin/rustfmt.rs

@@ -17,7 +17,7 @@ extern crate toml;
 extern crate env_logger;
 extern crate getopts;
 
-use rustfmt::{run, Input};
+use rustfmt::{run, Input, Summary};
 use rustfmt::config::{Config, WriteMode};
 
 use std::{env, error};
@@ -156,18 +156,21 @@ fn make_opts() -> Options {
     opts
 }
 
-fn execute(opts: &Options) -> FmtResult<()> {
+fn execute(opts: &Options) -> FmtResult<Summary> {
     let matches = try!(opts.parse(env::args().skip(1)));
 
     match try!(determine_operation(&matches)) {
         Operation::Help => {
             print_usage(&opts, "");
+            Ok(Summary::new())
         }
         Operation::Version => {
             print_version();
+            Ok(Summary::new())
         }
         Operation::ConfigHelp => {
             Config::print_docs();
+            Ok(Summary::new())
         }
         Operation::Stdin { input, config_path } => {
             // try to read config from local directory
@@ -177,7 +180,7 @@ fn execute(opts: &Options) -> FmtResult<()> {
             // write_mode is always Plain for Stdin.
             config.write_mode = WriteMode::Plain;
 
-            run(Input::Text(input), &config);
+            Ok(run(Input::Text(input), &config))
         }
         Operation::Format { files, config_path } => {
             let mut config = Config::default();
@@ -193,6 +196,8 @@ fn execute(opts: &Options) -> FmtResult<()> {
             if let Some(path) = path.as_ref() {
                 println!("Using rustfmt config file {}", path.display());
             }
+
+            let mut error_summary = Summary::new();
             for file in files {
                 // Check the file directory if the config-path could not be read or not provided
                 if path.is_none() {
@@ -209,11 +214,11 @@ fn execute(opts: &Options) -> FmtResult<()> {
                 }
 
                 try!(update_config(&mut config, &matches));
-                run(Input::File(file), &config);
+                error_summary.add(run(Input::File(file), &config));
             }
+            Ok(error_summary)
         }
     }
-    Ok(())
 }
 
 fn main() {
@@ -222,7 +227,18 @@ fn main() {
     let opts = make_opts();
 
     let exit_code = match execute(&opts) {
-        Ok(..) => 0,
+        Ok(summary) => {
+            if summary.has_operational_errors() {
+                1
+            } else if summary.has_parsing_errors() {
+                2
+            } else if summary.has_formatting_errors() {
+                3
+            } else {
+                assert!(summary.has_no_errors());
+                0
+            }
+        }
         Err(e) => {
             print_usage(&opts, &e.to_string());
             1

src/lib.rs

@@ -41,7 +41,9 @@ use std::fmt;
 use issues::{BadIssueSeeker, Issue};
 use filemap::FileMap;
 use visitor::FmtVisitor;
-use config::{Config, WriteMode};
+use config::Config;
+
+pub use self::summary::Summary;
 
 #[macro_use]
 mod utils;
@@ -64,6 +66,7 @@ pub mod rustfmt_diff;
 mod chains;
 mod macros;
 mod patterns;
+mod summary;
 
 const MIN_STRING: usize = 10;
 // When we get scoped annotations, we should have rustfmt::skip.
@@ -239,9 +242,17 @@ pub struct FormatReport {
 }
 
 impl FormatReport {
+    fn new() -> FormatReport {
+        FormatReport { file_error_map: HashMap::new() }
+    }
+
     pub fn warning_count(&self) -> usize {
         self.file_error_map.iter().map(|(_, ref errors)| errors.len()).fold(0, |acc, x| acc + x)
     }
+
+    pub fn has_warnings(&self) -> bool {
+        self.warning_count() > 0
+    }
 }
 
 impl fmt::Display for FormatReport {
@@ -289,7 +300,7 @@ fn format_ast(krate: &ast::Crate,
 // TODO(#20) other stuff for parity with make tidy
 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() };
+    let mut report = FormatReport::new();
 
     // Iterate over the chars in the file map.
     for (f, text) in file_map.iter() {
@@ -368,17 +379,16 @@ fn format_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
 }
 
 fn parse_input(input: Input, parse_session: &ParseSess) -> Result<ast::Crate, DiagnosticBuilder> {
-    let krate = match input {
+    match input {
         Input::File(file) => parse::parse_crate_from_file(&file, Vec::new(), &parse_session),
         Input::Text(text) => {
             parse::parse_crate_from_source_str("stdin".to_owned(), text, Vec::new(), &parse_session)
         }
-    };
-
-    krate
+    }
 }
 
-pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
+pub fn format_input(input: Input, config: &Config) -> (Summary, FileMap, FormatReport) {
+    let mut summary = Summary::new();
     let codemap = Rc::new(CodeMap::new());
 
     let tty_handler = Handler::with_tty_emitter(ColorConfig::Auto,
@@ -397,10 +407,15 @@ pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
         Ok(krate) => krate,
         Err(mut diagnostic) => {
             diagnostic.emit();
-            panic!("Unrecoverable parse error");
+            summary.add_parsing_error();
+            return (summary, FileMap::new(), FormatReport::new());
         }
     };
 
+    if parse_session.span_diagnostic.has_errors() {
+        summary.add_parsing_error();
+    }
+
     // Suppress error output after parsing.
     let silent_emitter = Box::new(EmitterWriter::new(Box::new(Vec::new()), None, codemap.clone()));
     parse_session.span_diagnostic = Handler::with_emitter(true, false, silent_emitter);
@@ -412,22 +427,28 @@ pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
     filemap::append_newlines(&mut file_map);
 
     let report = format_lines(&mut file_map, config);
-    (file_map, report)
+    if report.has_warnings() {
+        summary.add_formatting_error();
+    }
+    (summary, file_map, report)
 }
 
 pub enum Input {
     File(PathBuf),
     Text(String),
 }
 
-pub fn run(input: Input, config: &Config) {
-    let (file_map, report) = format_input(input, config);
+pub fn run(input: Input, config: &Config) -> Summary {
+    let (mut summary, file_map, report) = format_input(input, config);
     msg!("{}", report);
 
     let mut out = stdout();
     let write_result = filemap::write_all_files(&file_map, &mut out, config);
 
     if let Err(msg) = write_result {
         msg!("Error writing files: {}", msg);
+        summary.add_operational_error();
     }
+
+    summary
 }

src/summary.rs

@@ -0,0 +1,55 @@
+#[must_use]
+pub struct Summary {
+    // Encountered e.g. an IO error.
+    has_operational_errors: bool,
+
+    // Failed to reformat code because of parsing errors.
+    has_parsing_errors: bool,
+
+    // Code is valid, but it is impossible to format it properly.
+    has_formatting_errors: bool,
+}
+
+impl Summary {
+    pub fn new() -> Summary {
+        Summary {
+            has_operational_errors: false,
+            has_parsing_errors: false,
+            has_formatting_errors: false,
+        }
+    }
+
+    pub fn has_operational_errors(&self) -> bool {
+        self.has_operational_errors
+    }
+
+    pub fn has_parsing_errors(&self) -> bool {
+        self.has_parsing_errors
+    }
+
+    pub fn has_formatting_errors(&self) -> bool {
+        self.has_formatting_errors
+    }
+
+    pub fn add_operational_error(&mut self) {
+        self.has_operational_errors = true;
+    }
+
+    pub fn add_parsing_error(&mut self) {
+        self.has_parsing_errors = true;
+    }
+
+    pub fn add_formatting_error(&mut self) {
+        self.has_formatting_errors = true;
+    }
+
+    pub fn has_no_errors(&self) -> bool {
+        !(self.has_operational_errors || self.has_parsing_errors || self.has_formatting_errors)
+    }
+
+    pub fn add(&mut self, other: Summary) {
+        self.has_operational_errors |= other.has_operational_errors;
+        self.has_formatting_errors |= other.has_formatting_errors;
+        self.has_parsing_errors |= other.has_parsing_errors;
+    }
+}

tests/system.rs

@@ -143,10 +143,20 @@ fn self_tests() {
 fn stdin_formatting_smoke_test() {
     let input = Input::Text("fn main () {}".to_owned());
     let config = Config::default();
-    let (file_map, _report) = format_input(input, &config);
+    let (error_summary, file_map, _report) = format_input(input, &config);
+    assert!(error_summary.has_no_errors());
     assert_eq!(file_map["stdin"].to_string(), "fn main() {}\n")
 }
 
+#[test]
+fn format_lines_errors_are_reported() {
+    let long_identifier = String::from_utf8(vec![b'a'; 239]).unwrap();
+    let input = Input::Text(format!("fn {}() {{}}", long_identifier));
+    let config = Config::default();
+    let (error_summary, _file_map, _report) = format_input(input, &config);
+    assert!(error_summary.has_formatting_errors());
+}
+
 // 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) -> (Vec<FormatReport>, u32, u32)
@@ -202,7 +212,8 @@ fn read_config(filename: &str) -> Config {
 
 fn format_file<P: Into<PathBuf>>(filename: P, config: &Config) -> (FileMap, FormatReport) {
     let input = Input::File(filename.into());
-    format_input(input, &config)
+    let (_error_summary, file_map, report) = format_input(input, &config);
+    return (file_map, report);
 }
 
 pub fn idempotent_check(filename: String) -> Result<FormatReport, HashMap<String, Vec<Mismatch>>> {