Return non-zero exit code on malformed stdin fmt (#4163)

cli/fmt.rs

@@ -8,6 +8,7 @@
 //! the same functions as ops available in JS runtime.
 
 use crate::fs::files_in_subtree;
+use crate::op_error::OpError;
 use deno_core::ErrBox;
 use dprint_plugin_typescript as dprint;
 use std::fs;
@@ -81,7 +82,7 @@ fn check_source_files(
       "files"
     };
     Err(
-      crate::op_error::OpError::other(format!(
+      OpError::other(format!(
         "Found {} not formatted {}",
         not_formatted_files.len(),
         f,
@@ -142,8 +143,7 @@ fn format_source_files(
 /// then the current directory is recursively walked.
 pub fn format(args: Vec<String>, check: bool) -> Result<(), ErrBox> {
   if args.len() == 1 && args[0] == "-" {
-    format_stdin(check);
-    return Ok(());
+    return format_stdin(check);
   }
 
   let mut target_files: Vec<PathBuf> = vec![];
@@ -175,10 +175,10 @@ pub fn format(args: Vec<String>, check: bool) -> Result<(), ErrBox> {
 /// Format stdin and write result to stdout.
 /// Treats input as TypeScript.
 /// Compatible with `--check` flag.
-fn format_stdin(check: bool) {
+fn format_stdin(check: bool) -> Result<(), ErrBox> {
   let mut source = String::new();
   if stdin().read_to_string(&mut source).is_err() {
-    eprintln!("Failed to read from stdin");
+    return Err(OpError::other("Failed to read from stdin".to_string()).into());
   }
   let config = get_config();
 
@@ -190,15 +190,14 @@ fn format_stdin(check: bool) {
           println!("Not formatted stdin");
         }
       } else {
-        let _r = stdout().write_all(formatted_text.as_bytes());
-        // TODO(ry) Only ignore SIGPIPE. Currently ignoring all errors.
+        stdout().write_all(formatted_text.as_bytes())?;
       }
     }
     Err(e) => {
-      eprintln!("Error formatting from stdin");
-      eprintln!("   {}", e);
+      return Err(OpError::other(e).into());
     }
   }
+  Ok(())
 }
 
 #[test]

cli/tests/integration_tests.rs

@@ -96,10 +96,8 @@ fn fetch_test() {
     .output()
     .expect("Failed to spawn script");
 
-  let code = output.status.code();
+  assert!(output.status.success());
   let out = std::str::from_utf8(&output.stdout).unwrap();
-
-  assert_eq!(Some(0), code);
   assert_eq!(out, "");
 
   let expected_path = deno_dir
@@ -114,7 +112,6 @@ fn fetch_test() {
 #[test]
 fn fmt_test() {
   use tempfile::TempDir;
-
   let t = TempDir::new().expect("tempdir fail");
   let fixed = util::root_path().join("cli/tests/badly_formatted_fixed.js");
   let badly_formatted_original =
@@ -123,7 +120,6 @@ fn fmt_test() {
   let badly_formatted_str = badly_formatted.to_str().unwrap();
   std::fs::copy(&badly_formatted_original, &badly_formatted)
     .expect("Failed to copy file");
-
   let status = util::deno_cmd()
     .current_dir(util::root_path())
     .arg("fmt")
@@ -133,9 +129,7 @@ fn fmt_test() {
     .expect("Failed to spawn script")
     .wait()
     .expect("Failed to wait for child process");
-
-  assert_eq!(Some(1), status.code());
-
+  assert!(!status.success());
   let status = util::deno_cmd()
     .current_dir(util::root_path())
     .arg("fmt")
@@ -144,13 +138,34 @@ fn fmt_test() {
     .expect("Failed to spawn script")
     .wait()
     .expect("Failed to wait for child process");
-
-  assert_eq!(Some(0), status.code());
+  assert!(status.success());
   let expected = std::fs::read_to_string(fixed).unwrap();
   let actual = std::fs::read_to_string(badly_formatted).unwrap();
   assert_eq!(expected, actual);
 }
 
+#[test]
+fn fmt_stdin_error() {
+  use std::io::Write;
+  let mut deno = util::deno_cmd()
+    .current_dir(util::root_path())
+    .arg("fmt")
+    .arg("-")
+    .stdin(std::process::Stdio::piped())
+    .stdout(std::process::Stdio::piped())
+    .stderr(std::process::Stdio::piped())
+    .spawn()
+    .unwrap();
+  let stdin = deno.stdin.as_mut().unwrap();
+  let invalid_js = b"import { example }";
+  stdin.write_all(invalid_js).unwrap();
+  let output = deno.wait_with_output().unwrap();
+  // Error message might change. Just check stdout empty, stderr not.
+  assert!(output.stdout.is_empty());
+  assert!(!output.stderr.is_empty());
+  assert!(!output.status.success());
+}
+
 #[test]
 fn installer_test_local_module_run() {
   use deno::flags::Flags;