Make --check work when running from stdin. (#3896)

Author: jugglerchris

Diff for: src/bin/main.rs

@@ -75,14 +75,10 @@ pub enum OperationError {
     /// An io error during reading or writing.
     #[fail(display = "io error: {}", _0)]
     IoError(IoError),
-    /// Attempt to use --check with stdin, which isn't currently
-    /// supported.
-    #[fail(display = "The `--check` option is not supported with standard input.")]
-    CheckWithStdin,
-    /// Attempt to use --emit=json with stdin, which isn't currently
-    /// supported.
-    #[fail(display = "Using `--emit` other than stdout is not supported with standard input.")]
-    EmitWithStdin,
+    /// Attempt to use --emit with a mode which is not currently
+    /// supported with stdandard input.
+    #[fail(display = "Emit mode {} not supported with standard output.", _0)]
+    StdinBadEmit(EmitMode),
 }
 
 impl From<IoError> for OperationError {
@@ -253,15 +249,20 @@ fn format_string(input: String, options: GetOptsOptions) -> Result<i32, FailureE
     let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?;
 
     if options.check {
-        return Err(OperationError::CheckWithStdin.into());
-    }
-    if let Some(emit_mode) = options.emit_mode {
-        if emit_mode != EmitMode::Stdout {
-            return Err(OperationError::EmitWithStdin.into());
+        config.set().emit_mode(EmitMode::Diff);
+    } else {
+        match options.emit_mode {
+            // Emit modes which work with standard input
+            // None means default, which is Stdout.
+            None | Some(EmitMode::Stdout) | Some(EmitMode::Checkstyle) | Some(EmitMode::Json) => {}
+            Some(emit_mode) => {
+                return Err(OperationError::StdinBadEmit(emit_mode).into());
+            }
         }
+        config
+            .set()
+            .emit_mode(options.emit_mode.unwrap_or(EmitMode::Stdout));
     }
-    // emit mode is always Stdout for Stdin.
-    config.set().emit_mode(EmitMode::Stdout);
     config.set().verbose(Verbosity::Quiet);
 
     // parse file_lines

Diff for: src/emitter/checkstyle.rs

@@ -2,7 +2,6 @@ use self::xml::XmlEscaped;
 use super::*;
 use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
 use std::io::{self, Write};
-use std::path::Path;
 
 mod xml;
 
@@ -30,7 +29,6 @@ impl Emitter for CheckstyleEmitter {
         }: FormattedFile<'_>,
     ) -> Result<EmitterResult, io::Error> {
         const CONTEXT_SIZE: usize = 0;
-        let filename = ensure_real_path(filename);
         let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
         output_checkstyle_file(output, filename, diff)?;
         Ok(EmitterResult::default())
@@ -39,13 +37,13 @@ impl Emitter for CheckstyleEmitter {
 
 pub(crate) fn output_checkstyle_file<T>(
     mut writer: T,
-    filename: &Path,
+    filename: &FileName,
     diff: Vec<Mismatch>,
 ) -> Result<(), io::Error>
 where
     T: Write,
 {
-    write!(writer, r#"<file name="{}">"#, filename.display())?;
+    write!(writer, r#"<file name="{}">"#, filename)?;
     for mismatch in diff {
         let begin_line = mismatch.line_number;
         let mut current_line;
@@ -77,7 +75,11 @@ mod tests {
     fn emits_empty_record_on_file_with_no_mismatches() {
         let file_name = "src/well_formatted.rs";
         let mut writer = Vec::new();
-        let _ = output_checkstyle_file(&mut writer, &PathBuf::from(file_name), vec![]);
+        let _ = output_checkstyle_file(
+            &mut writer,
+            &FileName::Real(PathBuf::from(file_name)),
+            vec![],
+        );
         assert_eq!(
             &writer[..],
             format!(r#"<file name="{}"></file>"#, file_name).as_bytes()

Diff for: src/emitter/json.rs

@@ -3,7 +3,6 @@ use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
 use serde::Serialize;
 use serde_json::to_string as to_json_string;
 use std::io::{self, Write};
-use std::path::Path;
 
 #[derive(Debug, Default)]
 pub(crate) struct JsonEmitter {
@@ -47,7 +46,6 @@ impl Emitter for JsonEmitter {
         }: FormattedFile<'_>,
     ) -> Result<EmitterResult, io::Error> {
         const CONTEXT_SIZE: usize = 0;
-        let filename = ensure_real_path(filename);
         let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
         let has_diff = !diff.is_empty();
 
@@ -62,7 +60,7 @@ impl Emitter for JsonEmitter {
 
 fn output_json_file<T>(
     mut writer: T,
-    filename: &Path,
+    filename: &FileName,
     diff: Vec<Mismatch>,
     num_emitted_files: u32,
 ) -> Result<(), io::Error>
@@ -106,7 +104,7 @@ where
         });
     }
     let json = to_json_string(&MismatchedFile {
-        name: String::from(filename.to_str().unwrap()),
+        name: format!("{}", filename),
         mismatches,
     })?;
     let prefix = if num_emitted_files > 0 { "," } else { "" };
@@ -148,7 +146,12 @@ mod tests {
 
         let mut writer = Vec::new();
         let exp_json = to_json_string(&mismatched_file).unwrap();
-        let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
+        let _ = output_json_file(
+            &mut writer,
+            &FileName::Real(PathBuf::from(file)),
+            vec![mismatch],
+            0,
+        );
         assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
     }
 
@@ -188,7 +191,12 @@ mod tests {
 
         let mut writer = Vec::new();
         let exp_json = to_json_string(&mismatched_file).unwrap();
-        let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
+        let _ = output_json_file(
+            &mut writer,
+            &FileName::Real(PathBuf::from(file)),
+            vec![mismatch],
+            0,
+        );
         assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
     }
 

Diff for: src/test/mod.rs

@@ -305,6 +305,52 @@ fn assert_output(source: &Path, expected_filename: &Path) {
     }
 }
 
+// Helper function for comparing the results of rustfmt
+// to a known output generated by one of the write modes.
+fn assert_stdin_output(
+    source: &Path,
+    expected_filename: &Path,
+    emit_mode: EmitMode,
+    has_diff: bool,
+) {
+    let mut config = Config::default();
+    config.set().newline_style(NewlineStyle::Unix);
+    config.set().emit_mode(emit_mode);
+
+    let mut source_file = fs::File::open(&source).expect("couldn't open source");
+    let mut source_text = String::new();
+    source_file
+        .read_to_string(&mut source_text)
+        .expect("Failed reading target");
+    let input = Input::Text(source_text);
+
+    // Populate output by writing to a vec.
+    let mut buf: Vec<u8> = vec![];
+    {
+        let mut session = Session::new(config, Some(&mut buf));
+        session.format(input).unwrap();
+        let errors = ReportedErrors {
+            has_diff: has_diff,
+            ..Default::default()
+        };
+        assert_eq!(session.errors, errors);
+    }
+
+    let mut expected_file = fs::File::open(&expected_filename).expect("couldn't open target");
+    let mut expected_text = String::new();
+    expected_file
+        .read_to_string(&mut expected_text)
+        .expect("Failed reading target");
+
+    let output = String::from_utf8(buf).unwrap();
+    let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE);
+    if !compare.is_empty() {
+        let mut failures = HashMap::new();
+        failures.insert(source.to_owned(), compare);
+        print_mismatches_default_message(failures);
+        panic!("Text does not match expected output");
+    }
+}
 // Idempotence tests. Files in tests/target are checked to be unaltered by
 // rustfmt.
 #[test]
@@ -428,6 +474,30 @@ fn stdin_works_with_modified_lines() {
     assert_eq!(buf, output.as_bytes());
 }
 
+/// Ensures that `EmitMode::Json` works with input from `stdin`.
+#[test]
+fn stdin_works_with_json() {
+    init_log();
+    assert_stdin_output(
+        Path::new("tests/writemode/source/stdin.rs"),
+        Path::new("tests/writemode/target/stdin.json"),
+        EmitMode::Json,
+        true,
+    );
+}
+
+/// Ensures that `EmitMode::Checkstyle` works with input from `stdin`.
+#[test]
+fn stdin_works_with_checkstyle() {
+    init_log();
+    assert_stdin_output(
+        Path::new("tests/writemode/source/stdin.rs"),
+        Path::new("tests/writemode/target/stdin.xml"),
+        EmitMode::Checkstyle,
+        false,
+    );
+}
+
 #[test]
 fn stdin_disable_all_formatting_test() {
     init_log();
@@ -865,3 +935,26 @@ fn verify_check_works() {
         .status()
         .expect("run with check option failed");
 }
+
+#[test]
+fn verify_check_works_with_stdin() {
+    init_log();
+
+    let mut child = Command::new(rustfmt().to_str().unwrap())
+        .arg("--check")
+        .stdin(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .expect("run with check option failed");
+
+    {
+        let stdin = child.stdin.as_mut().expect("Failed to open stdin");
+        stdin
+            .write_all("fn main() {}\n".as_bytes())
+            .expect("Failed to write to rustfmt --check");
+    }
+    let output = child
+        .wait_with_output()
+        .expect("Failed to wait on rustfmt child");
+    assert!(output.status.success());
+}

Diff for: tests/writemode/source/stdin.rs

@@ -0,0 +1,6 @@
+
+fn
+ some( )
+{
+}
+fn main () {}

Diff for: tests/writemode/target/stdin.json

@@ -0,0 +1 @@
+[{"name":"stdin","mismatches":[{"original_begin_line":1,"original_end_line":6,"expected_begin_line":1,"expected_end_line":2,"original":"\nfn\n some( )\n{\n}\nfn main () {}","expected":"fn some() {}\nfn main() {}"}]}]

Diff for: tests/writemode/target/stdin.xml

@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="utf-8"?>
+<checkstyle version="4.3"><file name="stdin"><error line="1" severity="warning" message="Should be `fn some() {}`" /><error line="2" severity="warning" message="Should be `fn main() {}`" /></file></checkstyle>