fix: stdout and stderr encoding on Windows (#14559)

cli/tests/integration/repl_tests.rs

@@ -206,6 +206,20 @@ fn pty_assign_global_this() {
   });
 }
 
+#[test]
+fn pty_emoji() {
+  // windows was having issues displaying this
+  util::with_pty(&["repl"], |mut console| {
+    console.write_line("console.log('🦕');");
+    console.write_line("close();");
+
+    let output = console.read_all_output();
+    // one for input, one for output
+    let emoji_count = output.chars().filter(|c| *c == '🦕').count();
+    assert_eq!(emoji_count, 2);
+  });
+}
+
 #[test]
 fn console_log() {
   let (out, err) = util::run_and_collect_output(

runtime/ops/fs.rs

@@ -322,11 +322,8 @@ fn seek_helper(args: SeekArgs) -> Result<(u32, SeekFrom), AnyError> {
 #[op]
 fn op_seek_sync(state: &mut OpState, args: SeekArgs) -> Result<u64, AnyError> {
   let (rid, seek_from) = seek_helper(args)?;
-  let pos = StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => std_file.seek(seek_from).map_err(AnyError::from),
-    Err(_) => Err(type_error(
-      "cannot seek on this type of resource".to_string(),
-    )),
+  let pos = StdFileResource::with_file(state, rid, |std_file| {
+    std_file.seek(seek_from).map_err(AnyError::from)
   })?;
   Ok(pos)
 }
@@ -343,10 +340,10 @@ async fn op_seek_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || {
-    let mut std_file = std_file.lock().unwrap();
+    let mut std_file = std_file.lock();
     std_file.seek(seek_from)
   })
   .await?
@@ -358,9 +355,8 @@ fn op_fdatasync_sync(
   state: &mut OpState,
   rid: ResourceId,
 ) -> Result<(), AnyError> {
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => std_file.sync_data().map_err(AnyError::from),
-    Err(_) => Err(type_error("cannot sync this type of resource".to_string())),
+  StdFileResource::with_file(state, rid, |std_file| {
+    std_file.sync_data().map_err(AnyError::from)
   })?;
   Ok(())
 }
@@ -375,10 +371,10 @@ async fn op_fdatasync_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     std_file.sync_data()
   })
   .await?
@@ -387,9 +383,8 @@ async fn op_fdatasync_async(
 
 #[op]
 fn op_fsync_sync(state: &mut OpState, rid: ResourceId) -> Result<(), AnyError> {
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => std_file.sync_all().map_err(AnyError::from),
-    Err(_) => Err(type_error("cannot sync this type of resource".to_string())),
+  StdFileResource::with_file(state, rid, |std_file| {
+    std_file.sync_all().map_err(AnyError::from)
   })?;
   Ok(())
 }
@@ -404,10 +399,10 @@ async fn op_fsync_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     std_file.sync_all()
   })
   .await?
@@ -419,9 +414,8 @@ fn op_fstat_sync(
   state: &mut OpState,
   rid: ResourceId,
 ) -> Result<FsStat, AnyError> {
-  let metadata = StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => std_file.metadata().map_err(AnyError::from),
-    Err(_) => Err(type_error("cannot stat this type of resource".to_string())),
+  let metadata = StdFileResource::with_file(state, rid, |std_file| {
+    std_file.metadata().map_err(AnyError::from)
   })?;
   Ok(get_stat(metadata))
 }
@@ -436,10 +430,10 @@ async fn op_fstat_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   let metadata = tokio::task::spawn_blocking(move || {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     std_file.metadata()
   })
   .await?
@@ -456,16 +450,13 @@ fn op_flock_sync(
   use fs3::FileExt;
   super::check_unstable(state, "Deno.flockSync");
 
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => {
+  StdFileResource::with_file(state, rid, |std_file| {
     if exclusive {
       std_file.lock_exclusive()?;
     } else {
       std_file.lock_shared()?;
     }
     Ok(())
-    }
-    Err(_) => Err(type_error("cannot lock this type of resource".to_string())),
   })
 }
 
@@ -483,10 +474,10 @@ async fn op_flock_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || -> Result<(), AnyError> {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     if exclusive {
       std_file.lock_exclusive()?;
     } else {
@@ -505,12 +496,9 @@ fn op_funlock_sync(
   use fs3::FileExt;
   super::check_unstable(state, "Deno.funlockSync");
 
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => {
+  StdFileResource::with_file(state, rid, |std_file| {
     std_file.unlock()?;
     Ok(())
-    }
-    Err(_) => Err(type_error("cannot lock this type of resource".to_string())),
   })
 }
 
@@ -527,10 +515,10 @@ async fn op_funlock_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || -> Result<(), AnyError> {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     std_file.unlock()?;
     Ok(())
   })
@@ -1590,9 +1578,8 @@ fn op_ftruncate_sync(
 ) -> Result<(), AnyError> {
   let rid = args.rid;
   let len = args.len as u64;
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => std_file.set_len(len).map_err(AnyError::from),
-    Err(_) => Err(type_error("cannot truncate this type of resource")),
+  StdFileResource::with_file(state, rid, |std_file| {
+    std_file.set_len(len).map_err(AnyError::from)
   })?;
   Ok(())
 }
@@ -1610,10 +1597,10 @@ async fn op_ftruncate_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
 
   tokio::task::spawn_blocking(move || {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     std_file.set_len(len)
   })
   .await?
@@ -1879,14 +1866,9 @@ fn op_futime_sync(
   let atime = filetime::FileTime::from_unix_time(args.atime.0, args.atime.1);
   let mtime = filetime::FileTime::from_unix_time(args.mtime.0, args.mtime.1);
 
-  StdFileResource::with(state, rid, |r| match r {
-    Ok(std_file) => {
+  StdFileResource::with_file(state, rid, |std_file| {
     filetime::set_file_handle_times(std_file, Some(atime), Some(mtime))
       .map_err(AnyError::from)
-    }
-    Err(_) => Err(type_error(
-      "cannot futime on this type of resource".to_string(),
-    )),
   })?;
 
   Ok(())
@@ -1907,9 +1889,9 @@ async fn op_futime_async(
     .resource_table
     .get::<StdFileResource>(rid)?;
 
-  let std_file = resource.std_file()?;
+  let std_file = resource.std_file();
   tokio::task::spawn_blocking(move || {
-    let std_file = std_file.lock().unwrap();
+    let std_file = std_file.lock();
     filetime::set_file_handle_times(&std_file, Some(atime), Some(mtime))?;
     Ok(())
   })

runtime/ops/io.rs

@@ -1,10 +1,8 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
-use deno_core::error::bad_resource_id;
-use deno_core::error::not_supported;
-use deno_core::error::resource_unavailable;
 use deno_core::error::AnyError;
 use deno_core::op;
+use deno_core::parking_lot::Mutex;
 use deno_core::AsyncMutFuture;
 use deno_core::AsyncRefCell;
 use deno_core::AsyncResult;
@@ -20,11 +18,11 @@ use once_cell::sync::Lazy;
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::fs::File as StdFile;
+use std::io::ErrorKind;
 use std::io::Read;
 use std::io::Write;
 use std::rc::Rc;
 use std::sync::Arc;
-use std::sync::Mutex;
 use tokio::io::AsyncRead;
 use tokio::io::AsyncReadExt;
 use tokio::io::AsyncWrite;
@@ -40,6 +38,9 @@ use {
   winapi::um::{processenv::GetStdHandle, winbase},
 };
 
+// Store the stdio fd/handles in global statics in order to keep them
+// alive for the duration of the application since the last handle/fd
+// being dropped will close the corresponding pipe.
 #[cfg(unix)]
 static STDIN_HANDLE: Lazy<StdFile> =
   Lazy::new(|| unsafe { StdFile::from_raw_fd(0) });
@@ -50,14 +51,6 @@ static STDOUT_HANDLE: Lazy<StdFile> =
 static STDERR_HANDLE: Lazy<StdFile> =
   Lazy::new(|| unsafe { StdFile::from_raw_fd(2) });
 
-/// Due to portability issues on Windows handle to stdout is created from raw
-/// file descriptor.  The caveat of that approach is fact that when this
-/// handle is dropped underlying file descriptor is closed - that is highly
-/// not desirable in case of stdout.  That's why we store this global handle
-/// that is then cloned when obtaining stdio for process. In turn when
-/// resource table is dropped storing reference to that handle, the handle
-/// itself won't be closed (so Deno.core.print) will still work.
-// TODO(ry) It should be possible to close stdout.
 #[cfg(windows)]
 static STDIN_HANDLE: Lazy<StdFile> = Lazy::new(|| unsafe {
   StdFile::from_raw_handle(GetStdHandle(winbase::STD_INPUT_HANDLE))
@@ -122,23 +115,23 @@ pub fn init_stdio(stdio: Stdio) -> Extension {
         .expect("Extension only supports being used once.");
       let t = &mut state.resource_table;
       t.add(StdFileResource::stdio(
-        match &stdio.stdin {
-          StdioPipe::Inherit => &STDIN_HANDLE,
-          StdioPipe::File(pipe) => pipe,
+        match stdio.stdin {
+          StdioPipe::Inherit => StdFileResourceInner::Stdin,
+          StdioPipe::File(pipe) => StdFileResourceInner::file(pipe),
         },
         "stdin",
       ));
       t.add(StdFileResource::stdio(
-        match &stdio.stdout {
-          StdioPipe::Inherit => &STDOUT_HANDLE,
-          StdioPipe::File(pipe) => pipe,
+        match stdio.stdout {
+          StdioPipe::Inherit => StdFileResourceInner::Stdout,
+          StdioPipe::File(pipe) => StdFileResourceInner::file(pipe),
         },
         "stdout",
       ));
       t.add(StdFileResource::stdio(
-        match &stdio.stderr {
-          StdioPipe::Inherit => &STDERR_HANDLE,
-          StdioPipe::File(pipe) => pipe,
+        match stdio.stderr {
+          StdioPipe::Inherit => StdFileResourceInner::Stderr,
+          StdioPipe::File(pipe) => StdFileResourceInner::file(pipe),
         },
         "stderr",
       ));
@@ -301,16 +294,94 @@ impl Resource for ChildStderrResource {
   }
 }
 
+#[derive(Clone)]
+enum StdFileResourceInner {
+  // Ideally we would store stdio as an StdFile, but we get some Windows
+  // specific functionality for free by using Rust std's wrappers. So we
+  // take a bit of a complexity hit here in order to not have to duplicate
+  // the functionality in Rust's std/src/sys/windows/stdio.rs
+  Stdin,
+  Stdout,
+  Stderr,
+  File(Arc<Mutex<StdFile>>),
+}
+
+impl StdFileResourceInner {
+  pub fn file(fs_file: StdFile) -> Self {
+    StdFileResourceInner::File(Arc::new(Mutex::new(fs_file)))
+  }
+
+  pub fn with_file<R>(&self, mut f: impl FnMut(&mut StdFile) -> R) -> R {
+    match self {
+      Self::Stdin => f(&mut STDIN_HANDLE.try_clone().unwrap()),
+      Self::Stdout => f(&mut STDOUT_HANDLE.try_clone().unwrap()),
+      Self::Stderr => f(&mut STDERR_HANDLE.try_clone().unwrap()),
+      Self::File(file) => {
+        let mut file = file.lock();
+        f(&mut file)
+      }
+    }
+  }
+
+  pub fn write_and_maybe_flush(
+    &mut self,
+    buf: &[u8],
+  ) -> Result<usize, AnyError> {
+    let nwritten = self.write(buf)?;
+    if !matches!(self, StdFileResourceInner::File(_)) {
+      // Rust will line buffer and we don't want that behavior
+      // (see https://github.com/denoland/deno/issues/948), so flush.
+      // Although an alternative solution could be to bypass Rust's std by
+      // using the raw fds/handles, it will cause encoding issues on Windows
+      // that we get solved for free by using Rust's stdio wrappers (see
+      // std/src/sys/windows/stdio.rs in Rust's source code).
+      self.flush()?;
+    }
+    Ok(nwritten)
+  }
+}
+
+impl Read for StdFileResourceInner {
+  fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+    match self {
+      Self::Stdout => Err(ErrorKind::Unsupported.into()),
+      Self::Stderr => Err(ErrorKind::Unsupported.into()),
+      Self::Stdin => std::io::stdin().read(buf),
+      Self::File(file) => file.lock().read(buf),
+    }
+  }
+}
+
+impl Write for StdFileResourceInner {
+  fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+    match self {
+      Self::Stdout => std::io::stdout().write(buf),
+      Self::Stderr => std::io::stderr().write(buf),
+      Self::Stdin => Err(ErrorKind::Unsupported.into()),
+      Self::File(file) => file.lock().write(buf),
+    }
+  }
+
+  fn flush(&mut self) -> std::io::Result<()> {
+    match self {
+      Self::Stdout => std::io::stdout().flush(),
+      Self::Stderr => std::io::stderr().flush(),
+      Self::Stdin => Err(ErrorKind::Unsupported.into()),
+      Self::File(file) => file.lock().flush(),
+    }
+  }
+}
+
 pub struct StdFileResource {
-  fs_file: Option<Arc<Mutex<StdFile>>>,
+  inner: StdFileResourceInner,
   metadata: RefCell<FileMetadata>,
   name: String,
 }
 
 impl StdFileResource {
-  pub fn stdio(std_file: &StdFile, name: &str) -> Self {
+  fn stdio(inner: StdFileResourceInner, name: &str) -> Self {
     Self {
-      fs_file: std_file.try_clone().map(|s| Arc::new(Mutex::new(s))).ok(),
+      inner,
       metadata: Default::default(),
       name: name.to_string(),
     }
@@ -318,16 +389,24 @@ impl StdFileResource {
 
   pub fn fs_file(fs_file: StdFile) -> Self {
     Self {
-      fs_file: Some(Arc::new(Mutex::new(fs_file))),
+      inner: StdFileResourceInner::file(fs_file),
       metadata: Default::default(),
       name: "fsFile".to_string(),
     }
   }
 
-  pub fn std_file(&self) -> Result<Arc<Mutex<StdFile>>, AnyError> {
-    match &self.fs_file {
-      Some(fs_file) => Ok(fs_file.clone()),
-      None => Err(bad_resource_id()),
+  pub fn std_file(&self) -> Arc<Mutex<StdFile>> {
+    match &self.inner {
+      StdFileResourceInner::File(fs_file) => fs_file.clone(),
+      StdFileResourceInner::Stdin => {
+        Arc::new(Mutex::new(STDIN_HANDLE.try_clone().unwrap()))
+      }
+      StdFileResourceInner::Stdout => {
+        Arc::new(Mutex::new(STDOUT_HANDLE.try_clone().unwrap()))
+      }
+      StdFileResourceInner::Stderr => {
+        Arc::new(Mutex::new(STDERR_HANDLE.try_clone().unwrap()))
+      }
     }
   }
 
@@ -339,49 +418,65 @@ impl StdFileResource {
     self: Rc<Self>,
     mut buf: ZeroCopyBuf,
   ) -> Result<(usize, ZeroCopyBuf), AnyError> {
-    let std_file = self.fs_file.as_ref().unwrap().clone();
+    let mut inner = self.inner.clone();
     tokio::task::spawn_blocking(
       move || -> Result<(usize, ZeroCopyBuf), AnyError> {
-        let mut std_file = std_file.lock().unwrap();
-        Ok((std_file.read(&mut buf)?, buf))
+        Ok((inner.read(&mut buf)?, buf))
       },
     )
     .await?
   }
 
   async fn write(self: Rc<Self>, buf: ZeroCopyBuf) -> Result<usize, AnyError> {
-    let std_file = self.fs_file.as_ref().unwrap().clone();
-    tokio::task::spawn_blocking(move || {
-      let mut std_file = std_file.lock().unwrap();
-      std_file.write(&buf)
-    })
+    let mut inner = self.inner.clone();
+    tokio::task::spawn_blocking(move || inner.write_and_maybe_flush(&buf))
       .await?
       .map_err(AnyError::from)
   }
 
-  pub fn with<F, R>(
+  fn with_inner<F, R>(
     state: &mut OpState,
     rid: ResourceId,
     mut f: F,
   ) -> Result<R, AnyError>
   where
-    F: FnMut(Result<&mut std::fs::File, ()>) -> Result<R, AnyError>,
+    F: FnMut(StdFileResourceInner) -> Result<R, AnyError>,
   {
     let resource = state.resource_table.get::<StdFileResource>(rid)?;
-
-    match &resource.fs_file {
-      Some(r) => f(Ok(&mut r.as_ref().lock().unwrap())),
-      None => Err(resource_unavailable()),
+    f(resource.inner.clone())
   }
+
+  pub fn with_file<F, R>(
+    state: &mut OpState,
+    rid: ResourceId,
+    f: F,
+  ) -> Result<R, AnyError>
+  where
+    F: FnMut(&mut StdFile) -> Result<R, AnyError>,
+  {
+    let resource = state.resource_table.get::<StdFileResource>(rid)?;
+    resource.inner.with_file(f)
   }
 
   pub fn clone_file(
     state: &mut OpState,
     rid: ResourceId,
-  ) -> Result<std::fs::File, AnyError> {
-    Self::with(state, rid, move |r| match r {
-      Ok(std_file) => std_file.try_clone().map_err(AnyError::from),
-      Err(_) => Err(bad_resource_id()),
+  ) -> Result<StdFile, AnyError> {
+    Self::with_file(state, rid, move |std_file| {
+      std_file.try_clone().map_err(AnyError::from)
+    })
+  }
+
+  pub fn as_stdio(
+    state: &mut OpState,
+    rid: u32,
+  ) -> Result<std::process::Stdio, AnyError> {
+    Self::with_inner(state, rid, |inner| match inner {
+      StdFileResourceInner::File(file) => {
+        let file = file.lock().try_clone()?;
+        Ok(file.into())
+      }
+      _ => Ok(std::process::Stdio::inherit()),
     })
   }
 }
@@ -411,13 +506,10 @@ pub fn op_print(
   is_err: bool,
 ) -> Result<(), AnyError> {
   let rid = if is_err { 2 } else { 1 };
-  StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => {
-      std_file.write_all(msg.as_bytes())?;
-      std_file.flush().unwrap();
+  StdFileResource::with_inner(state, rid, move |mut inner| {
+    inner.write_all(msg.as_bytes())?;
+    inner.flush().unwrap();
     Ok(())
-    }
-    Err(_) => Err(not_supported()),
   })
 }
 
@@ -427,12 +519,11 @@ fn op_read_sync(
   rid: ResourceId,
   mut buf: ZeroCopyBuf,
 ) -> Result<u32, AnyError> {
-  StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => std_file
+  StdFileResource::with_inner(state, rid, move |mut inner| {
+    inner
       .read(&mut buf)
       .map(|n: usize| n as u32)
-      .map_err(AnyError::from),
-    Err(_) => Err(not_supported()),
+      .map_err(AnyError::from)
   })
 }
 
@@ -442,11 +533,10 @@ fn op_write_sync(
   rid: ResourceId,
   buf: ZeroCopyBuf,
 ) -> Result<u32, AnyError> {
-  StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => std_file
-      .write(&buf)
+  StdFileResource::with_inner(state, rid, move |mut inner| {
+    inner
+      .write_and_maybe_flush(&buf)
       .map(|nwritten: usize| nwritten as u32)
-      .map_err(AnyError::from),
-    Err(_) => Err(not_supported()),
+      .map_err(AnyError::from)
   })
 }

runtime/ops/process.rs

@@ -93,10 +93,7 @@ impl StdioOrRid {
   ) -> Result<std::process::Stdio, AnyError> {
     match &self {
       StdioOrRid::Stdio(val) => Ok(val.as_stdio()),
-      StdioOrRid::Rid(rid) => {
-        let file = StdFileResource::clone_file(state, *rid)?;
-        Ok(file.into())
-      }
+      StdioOrRid::Rid(rid) => StdFileResource::as_stdio(state, *rid),
     }
   }
 }

runtime/ops/tty.rs

@@ -1,7 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
 use super::io::StdFileResource;
-use deno_core::error::bad_resource_id;
 use deno_core::error::AnyError;
 use deno_core::op;
 use deno_core::Extension;
@@ -89,8 +88,8 @@ fn op_set_raw(state: &mut OpState, args: SetRawArgs) -> Result<(), AnyError> {
       return Err(deno_core::error::not_supported());
     }
 
-    let std_file = resource.std_file()?;
-    let std_file = std_file.lock().unwrap(); // hold the lock
+    let std_file = resource.std_file();
+    let std_file = std_file.lock(); // hold the lock
     let handle = std_file.as_raw_handle();
 
     if handle == handleapi::INVALID_HANDLE_VALUE {
@@ -120,8 +119,8 @@ fn op_set_raw(state: &mut OpState, args: SetRawArgs) -> Result<(), AnyError> {
     use std::os::unix::io::AsRawFd;
 
     let resource = state.resource_table.get::<StdFileResource>(rid)?;
-    let std_file = resource.std_file()?;
-    let raw_fd = std_file.lock().unwrap().as_raw_fd();
+    let std_file = resource.std_file();
+    let raw_fd = std_file.lock().as_raw_fd();
     let mut meta_data = resource.metadata_mut();
     let maybe_tty_mode = &mut meta_data.tty.mode;
 
@@ -164,16 +163,16 @@ fn op_set_raw(state: &mut OpState, args: SetRawArgs) -> Result<(), AnyError> {
 
 #[op]
 fn op_isatty(state: &mut OpState, rid: ResourceId) -> Result<bool, AnyError> {
-  let isatty: bool = StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => {
+  let isatty: bool = StdFileResource::with_file(state, rid, move |std_file| {
     #[cfg(windows)]
     {
+      use winapi::shared::minwindef::FALSE;
       use winapi::um::consoleapi;
 
       let handle = get_windows_handle(std_file)?;
       let mut test_mode: DWORD = 0;
       // If I cannot get mode out of console, it is not a console.
-        Ok(unsafe { consoleapi::GetConsoleMode(handle, &mut test_mode) != 0 })
+      Ok(unsafe { consoleapi::GetConsoleMode(handle, &mut test_mode) != FALSE })
     }
     #[cfg(unix)]
     {
@@ -181,8 +180,6 @@ fn op_isatty(state: &mut OpState, rid: ResourceId) -> Result<bool, AnyError> {
       let raw_fd = std_file.as_raw_fd();
       Ok(unsafe { libc::isatty(raw_fd as libc::c_int) == 1 })
     }
-    }
-    _ => Ok(false),
   })?;
   Ok(isatty)
 }
@@ -200,8 +197,7 @@ fn op_console_size(
 ) -> Result<ConsoleSize, AnyError> {
   super::check_unstable(state, "Deno.consoleSize");
 
-  let size = StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => {
+  let size = StdFileResource::with_file(state, rid, move |std_file| {
     #[cfg(windows)]
     {
       use std::os::windows::io::AsRawHandle;
@@ -211,10 +207,8 @@ fn op_console_size(
         let mut bufinfo: winapi::um::wincon::CONSOLE_SCREEN_BUFFER_INFO =
           std::mem::zeroed();
 
-          if winapi::um::wincon::GetConsoleScreenBufferInfo(
-            handle,
-            &mut bufinfo,
-          ) == 0
+        if winapi::um::wincon::GetConsoleScreenBufferInfo(handle, &mut bufinfo)
+          == 0
         {
           return Err(Error::last_os_error().into());
         }
@@ -244,8 +238,6 @@ fn op_console_size(
         })
       }
     }
-    }
-    Err(_) => Err(bad_resource_id()),
   })?;
 
   Ok(size)