return non-zero exit code if there are errors

Author: matklad

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, ErrorSummary};
 use rustfmt::config::{Config, WriteMode};
 
 use std::env;
@@ -189,8 +189,11 @@ fn execute() -> i32 {
             // write_mode is always Plain for Stdin.
             config.write_mode = WriteMode::Plain;
 
-            run(Input::Text(input), &config);
-            0
+            if run(Input::Text(input), &config).has_errors() {
+                1
+            } else {
+                0
+            }
         }
         Operation::Format { files, config_path } => {
             let mut config = Config::default();
@@ -206,6 +209,8 @@ fn execute() -> i32 {
             if let Some(path) = path.as_ref() {
                 println!("Using rustfmt config file {}", path.display());
             }
+
+            let mut error_summary = ErrorSummary::new();
             for file in files {
                 // Check the file directory if the config-path could not be read or not provided
                 if path.is_none() {
@@ -225,9 +230,14 @@ fn execute() -> i32 {
                     print_usage(&opts, &e);
                     return 1;
                 }
-                run(Input::File(file), &config);
+                error_summary.add(run(Input::File(file), &config));
+            }
+
+            if error_summary.has_errors() {
+                1
+            } else {
+                0
             }
-            0
         }
     }
 }

src/lib.rs

@@ -190,6 +190,30 @@ impl Sub<usize> for Indent {
     }
 }
 
+
+#[must_use]
+pub struct ErrorSummary {
+    has_errors: bool,
+}
+
+impl ErrorSummary {
+    pub fn new() -> ErrorSummary {
+        ErrorSummary { has_errors: false }
+    }
+
+    pub fn add_error(&mut self) {
+        self.has_errors = true;
+    }
+
+    pub fn has_errors(&self) -> bool {
+        self.has_errors
+    }
+
+    pub fn add(&mut self, other: ErrorSummary) {
+        self.has_errors |= other.has_errors;
+    }
+}
+
 pub enum ErrorKind {
     // Line has exceeded character limit
     LineOverflow,
@@ -239,9 +263,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 +321,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() {
@@ -378,7 +410,8 @@ fn parse_input(input: Input, parse_session: &ParseSess) -> Result<ast::Crate, Di
     krate
 }
 
-pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
+pub fn format_input(input: Input, config: &Config) -> (ErrorSummary, FileMap, FormatReport) {
+    let mut error_summary = ErrorSummary::new();
     let codemap = Rc::new(CodeMap::new());
 
     let tty_handler = Handler::with_tty_emitter(ColorConfig::Auto,
@@ -397,10 +430,15 @@ pub fn format_input(input: Input, config: &Config) -> (FileMap, FormatReport) {
         Ok(krate) => krate,
         Err(mut diagnostic) => {
             diagnostic.emit();
-            panic!("Unrecoverable parse error");
+            error_summary.add_error();
+            return (error_summary, FileMap::new(), FormatReport::new());
         }
     };
 
+    if parse_session.span_diagnostic.has_errors() {
+        error_summary.add_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,16 +450,19 @@ 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() {
+        error_summary.add_error();
+    }
+    (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) -> ErrorSummary {
+    let (mut error_summary, file_map, report) = format_input(input, config);
 
     let ignore_errors = config.write_mode == WriteMode::Plain;
     if !ignore_errors {
@@ -433,5 +474,8 @@ pub fn run(input: Input, config: &Config) {
 
     if let Err(msg) = write_result {
         msg!("Error writing files: {}", msg);
+        error_summary.add_error();
     }
+
+    error_summary
 }

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_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_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>>> {