refactor(ext/node): combine deno_node::Fs with deno_fs::FileSystem (#18991)

Cargo.lock

@@ -1154,6 +1154,7 @@ dependencies = [
  "cbc",
  "data-encoding",
  "deno_core",
+ "deno_fs",
  "deno_media_type",
  "deno_npm",
  "deno_semver",

cli/build.rs

@@ -326,6 +326,7 @@ deno_core::extension!(
 fn create_cli_snapshot(snapshot_path: PathBuf) {
   // NOTE(bartlomieju): ordering is important here, keep it in sync with
   // `runtime/worker.rs`, `runtime/web_worker.rs` and `runtime/build.rs`!
+  let fs = Arc::new(deno_fs::RealFs);
   let extensions: Vec<Extension> = vec![
     deno_webidl::deno_webidl::init_ops(),
     deno_console::deno_console::init_ops(),
@@ -360,11 +361,8 @@ fn create_cli_snapshot(snapshot_path: PathBuf) {
     deno_napi::deno_napi::init_ops::<PermissionsContainer>(),
     deno_http::deno_http::init_ops(),
     deno_io::deno_io::init_ops(Default::default()),
-    deno_fs::deno_fs::init_ops::<PermissionsContainer>(
+    deno_fs::deno_fs::init_ops::<PermissionsContainer>(false, fs.clone()),
-      false,
+    deno_node::deno_node::init_ops::<PermissionsContainer>(None, fs),
-      Arc::new(deno_fs::RealFs),
-    ),
-    deno_node::deno_node::init_ops::<PermissionsContainer>(None, None),
     cli::init_ops_and_esm(), // NOTE: This needs to be init_ops_and_esm!
   ];
 

cli/factory.rs

@@ -43,7 +43,6 @@ use deno_core::error::AnyError;
 use deno_core::parking_lot::Mutex;
 
 use deno_runtime::deno_fs;
-use deno_runtime::deno_node;
 use deno_runtime::deno_node::analyze::NodeCodeTranslator;
 use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_tls::RootCertStoreProvider;
@@ -133,6 +132,7 @@ struct CliFactoryServices {
   http_client: Deferred<Arc<HttpClient>>,
   emit_cache: Deferred<EmitCache>,
   emitter: Deferred<Arc<Emitter>>,
+  fs: Deferred<Arc<dyn deno_fs::FileSystem>>,
   graph_container: Deferred<Arc<ModuleGraphContainer>>,
   lockfile: Deferred<Option<Arc<Mutex<Lockfile>>>>,
   maybe_import_map: Deferred<Option<Arc<ImportMap>>>,
@@ -146,7 +146,6 @@ struct CliFactoryServices {
   module_graph_builder: Deferred<Arc<ModuleGraphBuilder>>,
   module_load_preparer: Deferred<Arc<ModuleLoadPreparer>>,
   node_code_translator: Deferred<Arc<CliNodeCodeTranslator>>,
-  node_fs: Deferred<Arc<dyn deno_node::NodeFs>>,
   node_resolver: Deferred<Arc<NodeResolver>>,
   npm_api: Deferred<Arc<CliNpmRegistryApi>>,
   npm_cache: Deferred<Arc<NpmCache>>,
@@ -245,6 +244,10 @@ impl CliFactory {
     })
   }
 
+  pub fn fs(&self) -> &Arc<dyn deno_fs::FileSystem> {
+    self.services.fs.get_or_init(|| Arc::new(deno_fs::RealFs))
+  }
+
   pub fn maybe_lockfile(&self) -> &Option<Arc<Mutex<Lockfile>>> {
     self
       .services
@@ -292,13 +295,6 @@ impl CliFactory {
       .await
   }
 
-  pub fn node_fs(&self) -> &Arc<dyn deno_node::NodeFs> {
-    self
-      .services
-      .node_fs
-      .get_or_init(|| Arc::new(deno_node::RealFs))
-  }
-
   pub async fn npm_resolver(&self) -> Result<&Arc<CliNpmResolver>, AnyError> {
     self
       .services
@@ -306,7 +302,7 @@ impl CliFactory {
       .get_or_try_init_async(async {
         let npm_resolution = self.npm_resolution().await?;
         let npm_fs_resolver = create_npm_fs_resolver(
-          self.node_fs().clone(),
+          self.fs().clone(),
           self.npm_cache()?.clone(),
           self.text_only_progress_bar(),
           CliNpmRegistryApi::default_url().to_owned(),
@@ -437,7 +433,7 @@ impl CliFactory {
       .node_resolver
       .get_or_try_init_async(async {
         Ok(Arc::new(NodeResolver::new(
-          self.node_fs().clone(),
+          self.fs().clone(),
           self.npm_resolver().await?.clone(),
         )))
       })
@@ -458,7 +454,7 @@ impl CliFactory {
 
         Ok(Arc::new(NodeCodeTranslator::new(
           cjs_esm_analyzer,
-          self.node_fs().clone(),
+          self.fs().clone(),
           self.node_resolver().await?.clone(),
           self.npm_resolver().await?.clone(),
         )))
@@ -554,8 +550,7 @@ impl CliFactory {
     let node_code_translator = self.node_code_translator().await?.clone();
     let options = self.cli_options().clone();
     let main_worker_options = self.create_cli_main_worker_options()?;
-    let fs = Arc::new(deno_fs::RealFs);
+    let fs = self.fs().clone();
-    let node_fs = self.node_fs().clone();
     let root_cert_store_provider = self.root_cert_store_provider().clone();
     let node_resolver = self.node_resolver().await?.clone();
     let npm_resolver = self.npm_resolver().await?.clone();
@@ -582,7 +577,6 @@ impl CliFactory {
         )),
         root_cert_store_provider.clone(),
         fs.clone(),
-        node_fs.clone(),
         maybe_inspector_server.clone(),
         main_worker_options.clone(),
       )
@@ -613,8 +607,7 @@ impl CliFactory {
         ),
       )),
       self.root_cert_store_provider().clone(),
-      Arc::new(deno_fs::RealFs),
+      self.fs().clone(),
-      self.node_fs().clone(),
       self.maybe_inspector_server().clone(),
       self.create_cli_main_worker_options()?,
     ))

cli/lsp/language_server.rs

@@ -9,7 +9,7 @@ use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::serde_json::Value;
 use deno_core::ModuleSpecifier;
-use deno_runtime::deno_node;
+use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_node::PackageJson;
 use deno_runtime::deno_tls::rustls::RootCertStore;
@@ -458,7 +458,7 @@ fn create_lsp_structs(
   let resolution =
     Arc::new(NpmResolution::from_serialized(api.clone(), None, None));
   let fs_resolver = create_npm_fs_resolver(
-    Arc::new(deno_node::RealFs),
+    Arc::new(deno_fs::RealFs),
     npm_cache.clone(),
     &progress_bar,
     registry_url.clone(),
@@ -709,7 +709,7 @@ impl Inner {
       self.npm_resolution.snapshot(),
       None,
     ));
-    let node_fs = Arc::new(deno_node::RealFs);
+    let node_fs = Arc::new(deno_fs::RealFs);
     let npm_resolver = Arc::new(CliNpmResolver::new(
       npm_resolution.clone(),
       create_npm_fs_resolver(

cli/npm/resolvers/local.rs

@@ -23,7 +23,7 @@ use deno_npm::resolution::NpmResolutionSnapshot;
 use deno_npm::NpmPackageCacheFolderId;
 use deno_npm::NpmPackageId;
 use deno_runtime::deno_core::futures;
-use deno_runtime::deno_node::NodeFs;
+use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::PackageJson;
@@ -44,7 +44,7 @@ use super::common::NpmPackageFsResolver;
 /// and resolves packages from it.
 #[derive(Debug)]
 pub struct LocalNpmPackageResolver {
-  fs: Arc<dyn NodeFs>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   cache: Arc<NpmCache>,
   progress_bar: ProgressBar,
   resolution: Arc<NpmResolution>,
@@ -55,7 +55,7 @@ pub struct LocalNpmPackageResolver {
 
 impl LocalNpmPackageResolver {
   pub fn new(
-    fs: Arc<dyn NodeFs>,
+    fs: Arc<dyn deno_fs::FileSystem>,
     cache: Arc<NpmCache>,
     progress_bar: ProgressBar,
     registry_url: Url,
@@ -94,7 +94,7 @@ impl LocalNpmPackageResolver {
       // Canonicalize the path so it's not pointing to the symlinked directory
       // in `node_modules` directory of the referrer.
       Some(path) => {
-        Ok(deno_core::strip_unc_prefix(self.fs.canonicalize(&path)?))
+        Ok(deno_core::strip_unc_prefix(self.fs.realpath_sync(&path)?))
       }
       None => bail!("could not find npm package for '{}'", specifier),
     }

cli/npm/resolvers/mod.rs

@@ -18,7 +18,7 @@ use deno_npm::resolution::NpmResolutionSnapshot;
 use deno_npm::resolution::PackageReqNotFoundError;
 use deno_npm::resolution::SerializedNpmResolutionSnapshot;
 use deno_npm::NpmPackageId;
-use deno_runtime::deno_node;
+use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NpmResolver;
@@ -270,7 +270,7 @@ impl NpmResolver for CliNpmResolver {
 }
 
 pub fn create_npm_fs_resolver(
-  fs: Arc<dyn deno_node::NodeFs>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   cache: Arc<NpmCache>,
   progress_bar: &ProgressBar,
   registry_url: Url,

cli/standalone/mod.rs

@@ -31,7 +31,6 @@ use deno_core::ModuleType;
 use deno_core::ResolutionKind;
 use deno_graph::source::Resolver;
 use deno_runtime::deno_fs;
-use deno_runtime::deno_node;
 use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_tls::rustls::RootCertStore;
 use deno_runtime::deno_tls::RootCertStoreProvider;
@@ -208,11 +207,11 @@ pub async fn run(
     http_client.clone(),
     progress_bar.clone(),
   ));
-  let node_fs = Arc::new(deno_node::RealFs);
+  let fs = Arc::new(deno_fs::RealFs);
   let npm_resolution =
     Arc::new(NpmResolution::from_serialized(npm_api.clone(), None, None));
   let npm_fs_resolver = create_npm_fs_resolver(
-    node_fs.clone(),
+    fs.clone(),
     npm_cache,
     &progress_bar,
     npm_registry_url,
@@ -225,7 +224,7 @@ pub async fn run(
     None,
   ));
   let node_resolver =
-    Arc::new(NodeResolver::new(node_fs.clone(), npm_resolver.clone()));
+    Arc::new(NodeResolver::new(fs.clone(), npm_resolver.clone()));
   let module_loader_factory = StandaloneModuleLoaderFactory {
     loader: EmbeddedModuleLoader {
       eszip: Arc::new(eszip),
@@ -254,8 +253,7 @@ pub async fn run(
     BlobStore::default(),
     Box::new(module_loader_factory),
     root_cert_store_provider,
-    Arc::new(deno_fs::RealFs),
+    fs,
-    node_fs,
     None,
     CliMainWorkerOptions {
       argv: metadata.argv,

cli/worker.rs

@@ -99,7 +99,6 @@ struct SharedWorkerState {
   module_loader_factory: Box<dyn ModuleLoaderFactory>,
   root_cert_store_provider: Arc<dyn RootCertStoreProvider>,
   fs: Arc<dyn deno_fs::FileSystem>,
-  node_fs: Arc<dyn deno_node::NodeFs>,
   maybe_inspector_server: Option<Arc<InspectorServer>>,
 }
 
@@ -311,7 +310,6 @@ impl CliMainWorkerFactory {
     module_loader_factory: Box<dyn ModuleLoaderFactory>,
     root_cert_store_provider: Arc<dyn RootCertStoreProvider>,
     fs: Arc<dyn deno_fs::FileSystem>,
-    node_fs: Arc<dyn deno_node::NodeFs>,
     maybe_inspector_server: Option<Arc<InspectorServer>>,
     options: CliMainWorkerOptions,
   ) -> Self {
@@ -329,7 +327,6 @@ impl CliMainWorkerFactory {
         module_loader_factory,
         root_cert_store_provider,
         fs,
-        node_fs,
         maybe_inspector_server,
       }),
     }
@@ -450,7 +447,6 @@ impl CliMainWorkerFactory {
       should_wait_for_inspector_session: shared.options.inspect_wait,
       module_loader,
       fs: shared.fs.clone(),
-      node_fs: Some(shared.node_fs.clone()),
       npm_resolver: Some(shared.npm_resolver.clone()),
       get_error_class_fn: Some(&errors::get_error_class_name),
       cache_storage_dir,
@@ -576,7 +572,6 @@ fn create_web_worker_callback(
       source_map_getter: maybe_source_map_getter,
       module_loader,
       fs: shared.fs.clone(),
-      node_fs: Some(shared.node_fs.clone()),
       npm_resolver: Some(shared.npm_resolver.clone()),
       worker_type: args.worker_type,
       maybe_inspector_server,

ext/fs/interface.rs

@@ -73,7 +73,7 @@ pub struct FsDirEntry {
 }
 
 #[async_trait::async_trait(?Send)]
-pub trait FileSystem: Send + Sync {
+pub trait FileSystem: std::fmt::Debug + Send + Sync {
   fn cwd(&self) -> FsResult<PathBuf>;
   fn tmp_dir(&self) -> FsResult<PathBuf>;
   fn chdir(&self, path: &Path) -> FsResult<()>;
@@ -225,4 +225,26 @@ pub trait FileSystem: Send + Sync {
     let buf = file.read_all_async().await?;
     Ok(buf)
   }
+
+  fn is_file(&self, path: &Path) -> bool {
+    self.stat_sync(path).map(|m| m.is_file).unwrap_or(false)
+  }
+
+  fn is_dir(&self, path: &Path) -> bool {
+    self
+      .stat_sync(path)
+      .map(|m| m.is_directory)
+      .unwrap_or(false)
+  }
+
+  fn exists(&self, path: &Path) -> bool {
+    self.stat_sync(path).is_ok()
+  }
+
+  fn read_to_string(&self, path: &Path) -> FsResult<String> {
+    let buf = self.read_file_sync(path)?;
+    String::from_utf8(buf).map_err(|err| {
+      std::io::Error::new(std::io::ErrorKind::InvalidData, err).into()
+    })
+  }
 }

ext/fs/std_fs.rs

@@ -22,7 +22,7 @@ use crate::OpenOptions;
 #[cfg(not(unix))]
 use deno_io::fs::FsError;
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct RealFs;
 
 #[async_trait::async_trait(?Send)]

ext/io/fs.rs

@@ -21,6 +21,16 @@ pub enum FsError {
   NotSupported,
 }
 
+impl FsError {
+  pub fn kind(&self) -> io::ErrorKind {
+    match self {
+      Self::Io(err) => err.kind(),
+      Self::FileBusy => io::ErrorKind::Other,
+      Self::NotSupported => io::ErrorKind::Other,
+    }
+  }
+}
+
 impl From<io::Error> for FsError {
   fn from(err: io::Error) -> Self {
     Self::Io(err)

ext/node/Cargo.toml

@@ -18,6 +18,7 @@ aes.workspace = true
 cbc.workspace = true
 data-encoding = "2.3.3"
 deno_core.workspace = true
+deno_fs.workspace = true
 deno_media_type.workspace = true
 deno_npm.workspace = true
 deno_semver.workspace = true

ext/node/analyze.rs

@@ -13,7 +13,6 @@ use once_cell::sync::Lazy;
 
 use deno_core::error::AnyError;
 
-use crate::NodeFs;
 use crate::NodeModuleKind;
 use crate::NodePermissions;
 use crate::NodeResolutionMode;
@@ -67,7 +66,7 @@ pub trait CjsEsmCodeAnalyzer {
 
 pub struct NodeCodeTranslator<TCjsEsmCodeAnalyzer: CjsEsmCodeAnalyzer> {
   cjs_esm_code_analyzer: TCjsEsmCodeAnalyzer,
-  fs: Arc<dyn NodeFs>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   node_resolver: Arc<NodeResolver>,
   npm_resolver: Arc<dyn NpmResolver>,
 }
@@ -77,7 +76,7 @@ impl<TCjsEsmCodeAnalyzer: CjsEsmCodeAnalyzer>
 {
   pub fn new(
     cjs_esm_code_analyzer: TCjsEsmCodeAnalyzer,
-    fs: Arc<dyn NodeFs>,
+    fs: Arc<dyn deno_fs::FileSystem>,
     node_resolver: Arc<NodeResolver>,
     npm_resolver: Arc<dyn NpmResolver>,
   ) -> Self {
@@ -161,6 +160,7 @@ impl<TCjsEsmCodeAnalyzer: CjsEsmCodeAnalyzer>
       let reexport_file_text = self
         .fs
         .read_to_string(&resolved_reexport)
+        .map_err(AnyError::from)
         .with_context(|| {
           format!(
             "Could not find '{}' ({}) referenced from {}",

ext/node/clippy.toml

@@ -1,40 +1,40 @@
 disallowed-methods = [
-  { path = "std::env::current_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::env::current_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::exists", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::exists", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::canonicalize", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::canonicalize", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::is_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::is_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::is_file", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::is_file", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::is_symlink", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::is_symlink", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::read_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::read_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::read_link", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::read_link", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::symlink_metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::symlink_metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::Path::try_exists", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::Path::try_exists", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::exists", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::exists", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::canonicalize", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::canonicalize", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::is_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::is_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::is_file", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::is_file", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::is_symlink", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::is_symlink", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::read_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::read_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::read_link", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::read_link", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::symlink_metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::symlink_metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::path::PathBuf::try_exists", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::path::PathBuf::try_exists", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::canonicalize", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::canonicalize", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::copy", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::copy", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::create_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::create_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::create_dir_all", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::create_dir_all", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::hard_link", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::hard_link", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::read", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::read", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::read_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::read_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::read_link", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::read_link", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::read_to_string", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::read_to_string", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::remove_dir", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::remove_dir", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::remove_dir_all", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::remove_dir_all", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::remove_file", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::remove_file", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::rename", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::rename", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::set_permissions", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::set_permissions", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::symlink_metadata", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::symlink_metadata", reason = "File system operations should be done using FileSystem trait" },
-  { path = "std::fs::write", reason = "File system operations should be done using NodeFs trait" },
+  { path = "std::fs::write", reason = "File system operations should be done using FileSystem trait" },
 ]

ext/node/lib.rs

@@ -14,7 +14,6 @@ use deno_semver::npm::NpmPackageReq;
 use deno_semver::npm::NpmPackageReqReference;
 use once_cell::sync::Lazy;
 use std::collections::HashSet;
-use std::io;
 use std::path::Path;
 use std::path::PathBuf;
 use std::rc::Rc;
@@ -51,71 +50,6 @@ impl NodePermissions for AllowAllNodePermissions {
   }
 }
 
-#[derive(Default, Clone)]
-pub struct NodeFsMetadata {
-  pub is_file: bool,
-  pub is_dir: bool,
-}
-
-pub trait NodeFs: std::fmt::Debug + Send + Sync {
-  fn current_dir(&self) -> io::Result<PathBuf>;
-  fn metadata(&self, path: &Path) -> io::Result<NodeFsMetadata>;
-  fn is_file(&self, path: &Path) -> bool;
-  fn is_dir(&self, path: &Path) -> bool;
-  fn exists(&self, path: &Path) -> bool;
-  fn read_to_string(&self, path: &Path) -> io::Result<String>;
-  fn canonicalize(&self, path: &Path) -> io::Result<PathBuf>;
-}
-
-#[derive(Debug)]
-pub struct RealFs;
-
-impl NodeFs for RealFs {
-  fn current_dir(&self) -> io::Result<PathBuf> {
-    #[allow(clippy::disallowed_methods)]
-    std::env::current_dir()
-  }
-
-  fn metadata(&self, path: &Path) -> io::Result<NodeFsMetadata> {
-    #[allow(clippy::disallowed_methods)]
-    std::fs::metadata(path).map(|metadata| {
-      // on most systems, calling is_file() and is_dir() is cheap
-      // and returns information already found in the metadata object
-      NodeFsMetadata {
-        is_file: metadata.is_file(),
-        is_dir: metadata.is_dir(),
-      }
-    })
-  }
-
-  fn exists(&self, path: &Path) -> bool {
-    #[allow(clippy::disallowed_methods)]
-    std::fs::metadata(path).is_ok()
-  }
-
-  fn is_file(&self, path: &Path) -> bool {
-    #[allow(clippy::disallowed_methods)]
-    std::fs::metadata(path)
-      .map(|m| m.is_file())
-      .unwrap_or(false)
-  }
-
-  fn is_dir(&self, path: &Path) -> bool {
-    #[allow(clippy::disallowed_methods)]
-    std::fs::metadata(path).map(|m| m.is_dir()).unwrap_or(false)
-  }
-
-  fn read_to_string(&self, path: &Path) -> io::Result<String> {
-    #[allow(clippy::disallowed_methods)]
-    std::fs::read_to_string(path)
-  }
-
-  fn canonicalize(&self, path: &Path) -> io::Result<PathBuf> {
-    #[allow(clippy::disallowed_methods)]
-    std::path::Path::canonicalize(path)
-  }
-}
-
 pub trait NpmResolver: std::fmt::Debug + Send + Sync {
   /// Resolves an npm package folder path from an npm package referrer.
   fn resolve_package_folder_from_package(
@@ -516,10 +450,10 @@ deno_core::extension!(deno_node,
   ],
   options = {
     maybe_npm_resolver: Option<Arc<dyn NpmResolver>>,
-    fs: Option<Arc<dyn NodeFs>>,
+    fs: Arc<dyn deno_fs::FileSystem>,
   },
   state = |state, options| {
-    let fs = options.fs.unwrap_or_else(|| Arc::new(RealFs));
+    let fs = options.fs;
     state.put(fs.clone());
     if let Some(npm_resolver) = options.maybe_npm_resolver {
       state.put(npm_resolver.clone());

ext/node/ops/require.rs

@@ -16,7 +16,6 @@ use std::rc::Rc;
 use std::sync::Arc;
 
 use crate::resolution;
-use crate::NodeFs;
 use crate::NodeModuleKind;
 use crate::NodePermissions;
 use crate::NodeResolutionMode;
@@ -94,11 +93,11 @@ pub fn op_require_node_module_paths<P>(
 where
   P: NodePermissions + 'static,
 {
-  let fs = state.borrow::<Arc<dyn NodeFs>>();
+  let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
   // Guarantee that "from" is absolute.
   let from = deno_core::resolve_path(
     &from,
-    &(fs.current_dir()).context("Unable to get CWD")?,
+    &(fs.cwd().map_err(AnyError::from)).context("Unable to get CWD")?,
   )
   .unwrap()
   .to_file_path()
@@ -263,8 +262,8 @@ where
 {
   let path = PathBuf::from(path);
   ensure_read_permission::<P>(state, &path)?;
-  let fs = state.borrow::<Arc<dyn NodeFs>>();
+  let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
-  if let Ok(metadata) = fs.metadata(&path) {
+  if let Ok(metadata) = fs.stat_sync(&path) {
     if metadata.is_file {
       return Ok(0);
     } else {
@@ -285,8 +284,9 @@ where
 {
   let path = PathBuf::from(request);
   ensure_read_permission::<P>(state, &path)?;
-  let fs = state.borrow::<Arc<dyn NodeFs>>();
+  let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
-  let canonicalized_path = deno_core::strip_unc_prefix(fs.canonicalize(&path)?);
+  let canonicalized_path =
+    deno_core::strip_unc_prefix(fs.realpath_sync(&path)?);
   Ok(canonicalized_path.to_string_lossy().to_string())
 }
 
@@ -346,8 +346,8 @@ where
 
   if let Some(parent_id) = maybe_parent_id {
     if parent_id == "<repl>" || parent_id == "internal/preload" {
-      let fs = state.borrow::<Arc<dyn NodeFs>>();
+      let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
-      if let Ok(cwd) = fs.current_dir() {
+      if let Ok(cwd) = fs.cwd() {
         ensure_read_permission::<P>(state, &cwd)?;
         return Ok(Some(cwd.to_string_lossy().to_string()));
       }
@@ -429,7 +429,7 @@ where
 {
   let file_path = PathBuf::from(file_path);
   ensure_read_permission::<P>(state, &file_path)?;
-  let fs = state.borrow::<Arc<dyn NodeFs>>();
+  let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
   Ok(fs.read_to_string(&file_path)?)
 }
 
@@ -457,7 +457,7 @@ fn op_require_resolve_exports<P>(
 where
   P: NodePermissions + 'static,
 {
-  let fs = state.borrow::<Arc<dyn NodeFs>>();
+  let fs = state.borrow::<Arc<dyn deno_fs::FileSystem>>();
   let npm_resolver = state.borrow::<Arc<dyn NpmResolver>>();
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
   let permissions = state.borrow::<P>();

ext/node/package_json.rs

@@ -1,6 +1,5 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-use crate::NodeFs;
 use crate::NodeModuleKind;
 use crate::NodePermissions;
 
@@ -63,7 +62,7 @@ impl PackageJson {
   }
 
   pub fn load(
-    fs: &dyn NodeFs,
+    fs: &dyn deno_fs::FileSystem,
     resolver: &dyn NpmResolver,
     permissions: &dyn NodePermissions,
     path: PathBuf,
@@ -73,7 +72,7 @@ impl PackageJson {
   }
 
   pub fn load_skip_read_permission(
-    fs: &dyn NodeFs,
+    fs: &dyn deno_fs::FileSystem,
     path: PathBuf,
   ) -> Result<PackageJson, AnyError> {
     assert!(path.is_absolute());
@@ -90,7 +89,7 @@ impl PackageJson {
       Err(err) => bail!(
         "Error loading package.json at {}. {:#}",
         path.display(),
-        err
+        AnyError::from(err),
       ),
     };
 

ext/node/resolution.rs

@@ -19,7 +19,6 @@ use deno_semver::npm::NpmPackageReqReference;
 
 use crate::errors;
 use crate::AllowAllNodePermissions;
-use crate::NodeFs;
 use crate::NodePermissions;
 use crate::NpmResolver;
 use crate::PackageJson;
@@ -107,12 +106,15 @@ impl NodeResolution {
 
 #[derive(Debug)]
 pub struct NodeResolver {
-  fs: Arc<dyn NodeFs>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   npm_resolver: Arc<dyn NpmResolver>,
 }
 
 impl NodeResolver {
-  pub fn new(fs: Arc<dyn NodeFs>, npm_resolver: Arc<dyn NpmResolver>) -> Self {
+  pub fn new(
+    fs: Arc<dyn deno_fs::FileSystem>,
+    npm_resolver: Arc<dyn NpmResolver>,
+  ) -> Self {
     Self { fs, npm_resolver }
   }
 
@@ -280,8 +282,9 @@ impl NodeResolver {
       p_str.to_string()
     };
 
-    let (is_dir, is_file) = if let Ok(stats) = self.fs.metadata(Path::new(&p)) {
+    let (is_dir, is_file) = if let Ok(stats) = self.fs.stat_sync(Path::new(&p))
-      (stats.is_dir, stats.is_file)
+    {
+      (stats.is_directory, stats.is_file)
     } else {
       (false, false)
     };
@@ -491,7 +494,7 @@ impl NodeResolver {
     referrer_kind: NodeModuleKind,
   ) -> Option<PathBuf> {
     fn probe_extensions(
-      fs: &dyn NodeFs,
+      fs: &dyn deno_fs::FileSystem,
       path: &Path,
       referrer_kind: NodeModuleKind,
     ) -> Option<PathBuf> {
@@ -1079,7 +1082,7 @@ impl NodeResolver {
   ) -> Result<PathBuf, AnyError> {
     let file_path = url.to_file_path().unwrap();
     let current_dir = deno_core::strip_unc_prefix(
-      self.fs.canonicalize(file_path.parent().unwrap())?,
+      self.fs.realpath_sync(file_path.parent().unwrap())?,
     );
     let mut current_dir = current_dir.as_path();
     let package_json_path = current_dir.join("package.json");

runtime/build.rs

@@ -273,6 +273,7 @@ mod startup_snapshot {
   pub fn create_runtime_snapshot(snapshot_path: PathBuf) {
     // NOTE(bartlomieju): ordering is important here, keep it in sync with
     // `runtime/worker.rs`, `runtime/web_worker.rs` and `cli/build.rs`!
+    let fs = std::sync::Arc::new(deno_fs::RealFs);
     let extensions: Vec<Extension> = vec![
       deno_webidl::deno_webidl::init_ops_and_esm(),
       deno_console::deno_console::init_ops_and_esm(),
@@ -309,14 +310,11 @@ mod startup_snapshot {
       deno_napi::deno_napi::init_ops_and_esm::<Permissions>(),
       deno_http::deno_http::init_ops_and_esm(),
       deno_io::deno_io::init_ops_and_esm(Default::default()),
-      deno_fs::deno_fs::init_ops_and_esm::<Permissions>(
+      deno_fs::deno_fs::init_ops_and_esm::<Permissions>(false, fs.clone()),
-        false,
-        std::sync::Arc::new(deno_fs::RealFs),
-      ),
       runtime::init_ops_and_esm(),
       // FIXME(bartlomieju): these extensions are specified last, because they
       // depend on `runtime`, even though it should be other way around
-      deno_node::deno_node::init_ops_and_esm::<Permissions>(None, None),
+      deno_node::deno_node::init_ops_and_esm::<Permissions>(None, fs),
       #[cfg(not(feature = "snapshot_from_snapshot"))]
       runtime_main::init_ops_and_esm(),
     ];

runtime/web_worker.rs

@@ -333,7 +333,6 @@ pub struct WebWorkerOptions {
   pub seed: Option<u64>,
   pub fs: Arc<dyn FileSystem>,
   pub module_loader: Rc<dyn ModuleLoader>,
-  pub node_fs: Option<Arc<dyn deno_node::NodeFs>>,
   pub npm_resolver: Option<Arc<dyn deno_node::NpmResolver>>,
   pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
   pub preload_module_cb: Arc<ops::worker_host::WorkerEventCb>,
@@ -442,10 +441,13 @@ impl WebWorker {
       deno_napi::deno_napi::init_ops::<PermissionsContainer>(),
       deno_http::deno_http::init_ops(),
       deno_io::deno_io::init_ops(Some(options.stdio)),
-      deno_fs::deno_fs::init_ops::<PermissionsContainer>(unstable, options.fs),
+      deno_fs::deno_fs::init_ops::<PermissionsContainer>(
+        unstable,
+        options.fs.clone(),
+      ),
       deno_node::deno_node::init_ops::<PermissionsContainer>(
         options.npm_resolver,
-        options.node_fs,
+        options.fs,
       ),
       // Runtime ops that are always initialized for WebWorkers
       ops::web_worker::deno_web_worker::init_ops(),

runtime/worker.rs

@@ -94,7 +94,6 @@ pub struct WorkerOptions {
   /// If not provided runtime will error if code being
   /// executed tries to load modules.
   pub module_loader: Rc<dyn ModuleLoader>,
-  pub node_fs: Option<Arc<dyn deno_node::NodeFs>>,
   pub npm_resolver: Option<Arc<dyn deno_node::NpmResolver>>,
   // Callbacks invoked when creating new instance of WebWorker
   pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
@@ -166,7 +165,6 @@ impl Default for WorkerOptions {
       broadcast_channel: Default::default(),
       source_map_getter: Default::default(),
       root_cert_store_provider: Default::default(),
-      node_fs: Default::default(),
       npm_resolver: Default::default(),
       blob_store: Default::default(),
       extensions: Default::default(),
@@ -268,10 +266,13 @@ impl MainWorker {
       deno_napi::deno_napi::init_ops::<PermissionsContainer>(),
       deno_http::deno_http::init_ops(),
       deno_io::deno_io::init_ops(Some(options.stdio)),
-      deno_fs::deno_fs::init_ops::<PermissionsContainer>(unstable, options.fs),
+      deno_fs::deno_fs::init_ops::<PermissionsContainer>(
+        unstable,
+        options.fs.clone(),
+      ),
       deno_node::deno_node::init_ops::<PermissionsContainer>(
         options.npm_resolver,
-        options.node_fs,
+        options.fs,
       ),
       // Ops from this crate
       ops::runtime::deno_runtime::init_ops(main_module.clone()),