Implemented rough draft of check write mode. (#2539)

* Add rough draft of `check` mode. Not unit tested.

* Added assert-cli; simple test case for `--write-mode=check`

* Lightly documented `check` WriteMode

* wrote clearer phrasing for config::options::WriteMode::Check

* Implemented default for WriteMode where default is Overwrite

* Simplified exit code handling

* updated README.md as per @nrc' comment

* collapsed exit-code handling

* Removed write_mode from Summary, introduced partial option parsing earlier

* Handle write-mode parsing in a slightly better way.

Author: davidbarsky

Cargo.lock

@@ -50,6 +50,7 @@ rustc-ap-syntax = "103.0.0"
 
 [dev-dependencies]
 lazy_static = "1.0.0"
+assert_cli = "0.5"
 
 [target.'cfg(unix)'.dependencies]
 libc = "0.2.11"

Cargo.toml

@@ -106,14 +106,17 @@ read data from stdin. Alternatively, you can use `cargo fmt` to format all
 binary and library targets of your crate.
 
 You'll probably want to specify the write mode. Currently, there are modes for
-`diff`, `replace`, `overwrite`, `display`, `coverage`, `checkstyle`, and `plain`.
+`check`, `diff`, `replace`, `overwrite`, `display`, `coverage`, `checkstyle`, and `plain`.
 
 * `overwrite` Is the default and overwrites the original files _without_ creating backups.
 * `replace` Overwrites the original files after creating backups of the files.
 * `display` Will print the formatted files to stdout.
 * `plain` Also writes to stdout, but with no metadata.
 * `diff` Will print a diff between the original files and formatted files to stdout.
          Will also exit with an error code if there are any differences.
+* `check` Checks if the program's formatting matches what rustfmt would do. Silently exits
+          with code 0 if so, emits a diff and exits with code 1 if not. This option is
+          designed to be run in CI-like where a non-zero exit signifies incorrect formatting.
 * `checkstyle` Will output the lines that need to be corrected as a checkstyle XML file,
   that can be used by tools like Jenkins.
 

README.md

@@ -30,7 +30,7 @@ use rustfmt::{run, FileName, Input, Summary};
 type FmtError = Box<error::Error + Send + Sync>;
 type FmtResult<T> = std::result::Result<T, FmtError>;
 
-const WRITE_MODE_LIST: &str = "[replace|overwrite|display|plain|diff|coverage|checkstyle]";
+const WRITE_MODE_LIST: &str = "[replace|overwrite|display|plain|diff|coverage|checkstyle|check]";
 
 /// Rustfmt operations.
 enum Operation {
@@ -303,6 +303,7 @@ fn execute(opts: &Options) -> FmtResult<Summary> {
             let mut out = &mut stdout();
             checkstyle::output_header(&mut out, config.write_mode())?;
             let mut error_summary = Summary::default();
+
             for file in files {
                 if !file.exists() {
                     eprintln!("Error: file `{}` does not exist", file.to_str().unwrap());
@@ -350,22 +351,28 @@ fn execute(opts: &Options) -> FmtResult<Summary> {
     }
 }
 
+fn determine_write_mode(opts: &Options) -> WriteMode {
+    let matches = opts.parse(env::args().skip(1)).unwrap();
+    let options = CliOptions::from_matches(&matches).unwrap();
+    match options.write_mode {
+        Some(m) => m,
+        None => WriteMode::default(),
+    }
+}
+
 fn main() {
     env_logger::init();
-
     let opts = make_opts();
+    // Only handles arguments passed in through the CLI.
+    let write_mode = determine_write_mode(&opts);
 
     let exit_code = match execute(&opts) {
         Ok(summary) => {
-            if summary.has_operational_errors() {
+            if summary.has_operational_errors()
+                || summary.has_diff && write_mode == WriteMode::Check
+                || summary.has_parsing_errors() || summary.has_formatting_errors()
+            {
                 1
-            } else if summary.has_parsing_errors() {
-                2
-            } else if summary.has_formatting_errors() {
-                3
-            } else if summary.has_diff {
-                // should only happen in diff mode
-                4
             } else {
                 assert!(summary.has_no_errors());
                 0

src/bin/main.rs

@@ -120,7 +120,7 @@ create_config! {
     // Control options (changes the operation of rustfmt, rather than the formatting)
     write_mode: WriteMode, WriteMode::Overwrite, false,
         "What Write Mode to use when none is supplied: \
-         Replace, Overwrite, Display, Plain, Diff, Coverage";
+         Replace, Overwrite, Display, Plain, Diff, Coverage, Check";
     color: Color, Color::Auto, false,
         "What Color option to use when none is supplied: Always, Never, Auto";
     required_version: String, env!("CARGO_PKG_VERSION").to_owned(), false,

src/config/mod.rs

@@ -183,6 +183,9 @@ configuration_option_enum! { WriteMode:
     Checkstyle,
     // Output the changed lines (for internal value only)
     Modified,
+    // Checks if a diff can be generated. If so, rustfmt outputs a diff and quits with exit code 1.
+    // This option is designed to be run in CI where a non-zero exit signifies non-standard code formatting.
+    Check,
 }
 
 configuration_option_enum! { Color:
@@ -250,6 +253,12 @@ impl ::std::str::FromStr for WidthHeuristics {
     }
 }
 
+impl Default for WriteMode {
+    fn default() -> WriteMode {
+        WriteMode::Overwrite
+    }
+}
+
 /// A set of directories, files and modules that rustfmt should ignore.
 #[derive(Default, Deserialize, Serialize, Clone, Debug)]
 pub struct IgnoreList(HashSet<PathBuf>);

src/config/options.rs

@@ -104,10 +104,7 @@ impl Summary {
     pub fn print_exit_codes() {
         let exit_codes = r#"Exit Codes:
     0 = No errors
-    1 = Encountered operational errors e.g. an IO error
-    2 = Failed to reformat code because of parsing errors
-    3 = Code is valid, but it is impossible to format it properly
-    4 = Formatted code differs from existing code (write-mode diff only)"#;
+    1 = Encountered error in formatting code"#;
         println!("{}", exit_codes);
     }
 }

src/config/summary.rs

@@ -178,6 +178,19 @@ where
             let diff = create_diff(filename, text, config)?;
             output_checkstyle_file(out, filename, diff)?;
         }
+        WriteMode::Check => {
+            let filename = filename_to_path();
+            if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) {
+                let mismatch = make_diff(&ori, &fmt, 3);
+                let has_diff = !mismatch.is_empty();
+                print_diff(
+                    mismatch,
+                    |line_num| format!("Diff in {} at line {}:", filename.display(), line_num),
+                    config.color(),
+                );
+                return Ok(has_diff);
+            }
+        }
     }
 
     // when we are not in diff mode, don't indicate differing files

src/filemap.rs

@@ -14,6 +14,7 @@
 #![allow(stable_features)]
 #![feature(match_default_bindings)]
 #![feature(type_ascription)]
+#![feature(unicode_internals)]
 
 #[macro_use]
 extern crate derive_new;

src/lib.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+extern crate assert_cli;
 #[macro_use]
 extern crate lazy_static;
 #[macro_use]
@@ -862,3 +863,29 @@ fn configuration_snippet_tests() {
     println!("Ran {} configurations tests.", blocks.len());
     assert_eq!(failures, 0, "{} configurations tests failed", failures);
 }
+
+#[test]
+fn verify_check_works() {
+    assert_cli::Assert::command(&[
+        "cargo",
+        "run",
+        "--bin=rustfmt",
+        "--",
+        "--write-mode=check",
+        "src/bin/main.rs",
+    ]).succeeds()
+        .unwrap();
+}
+
+#[test]
+fn verify_diff_works() {
+    assert_cli::Assert::command(&[
+        "cargo",
+        "run",
+        "--bin=rustfmt",
+        "--",
+        "--write-mode=diff",
+        "src/bin/main.rs",
+    ]).succeeds()
+        .unwrap();
+}