From 93b7de5b0110a1ab79714c3dfd009a4aae3e34b1 Mon Sep 17 00:00:00 2001 From: Chris Emerson Date: Wed, 6 Nov 2019 10:15:34 +0000 Subject: [PATCH] Make `--check` work when running from stdin. (#3896) # Conflicts: # src/bin/main.rs --- src/bin/main.rs | 31 ++++++----- src/emitter/checkstyle.rs | 12 ++-- src/emitter/json.rs | 20 +++++-- src/test/mod.rs | 93 +++++++++++++++++++++++++++++++ tests/writemode/source/stdin.rs | 6 ++ tests/writemode/target/stdin.json | 1 + tests/writemode/target/stdin.xml | 2 + 7 files changed, 139 insertions(+), 26 deletions(-) create mode 100644 tests/writemode/source/stdin.rs create mode 100644 tests/writemode/target/stdin.json create mode 100644 tests/writemode/target/stdin.xml diff --git a/src/bin/main.rs b/src/bin/main.rs index 4d845547cdf..6f5b09fc86a 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -74,14 +74,10 @@ pub enum OperationError { /// An io error during reading or writing. #[error("{0}")] IoError(IoError), - /// Attempt to use --check with stdin, which isn't currently - /// supported. - #[error("The `--check` option is not supported with standard input.")] - CheckWithStdin, - /// Attempt to use --emit=json with stdin, which isn't currently - /// supported. - #[error("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. + #[error("Emit mode {0} not supported with standard output.")] + StdinBadEmit(EmitMode), } impl From for OperationError { @@ -255,15 +251,20 @@ fn format_string(input: String, options: GetOptsOptions) -> Result { 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 --git a/src/emitter/checkstyle.rs b/src/emitter/checkstyle.rs index 76f2527db3d..545b259979d 100644 --- a/src/emitter/checkstyle.rs +++ b/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 { 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( mut writer: T, - filename: &Path, + filename: &FileName, diff: Vec, ) -> Result<(), io::Error> where T: Write, { - write!(writer, r#""#, filename.display())?; + write!(writer, r#""#, 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).as_bytes() diff --git a/src/emitter/json.rs b/src/emitter/json.rs index 269dd2d4daf..4d6f972c5e3 100644 --- a/src/emitter/json.rs +++ b/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 { 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( mut writer: T, - filename: &Path, + filename: &FileName, diff: Vec, 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 --git a/src/test/mod.rs b/src/test/mod.rs index cceb28dfea6..e1a7972ec82 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -307,6 +307,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 = 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. #[nightly_only_test] @@ -463,6 +509,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(); @@ -896,3 +966,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 --git a/tests/writemode/source/stdin.rs b/tests/writemode/source/stdin.rs new file mode 100644 index 00000000000..06f8a0c288d --- /dev/null +++ b/tests/writemode/source/stdin.rs @@ -0,0 +1,6 @@ + +fn + some( ) +{ +} +fn main () {} diff --git a/tests/writemode/target/stdin.json b/tests/writemode/target/stdin.json new file mode 100644 index 00000000000..20e38f57f4a --- /dev/null +++ b/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() {}"}]}] \ No newline at end of file diff --git a/tests/writemode/target/stdin.xml b/tests/writemode/target/stdin.xml new file mode 100644 index 00000000000..e70708338f5 --- /dev/null +++ b/tests/writemode/target/stdin.xml @@ -0,0 +1,2 @@ + +