From 9e25512e482949bed6624fd600de4f051fdfb31f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 13 May 2024 17:55:31 +0100 Subject: [PATCH] refactor(lsp): reuse CliGraphResolverOptions::sloppy_imports_resolver (#23764) --- cli/graph_util.rs | 11 +-- cli/lsp/diagnostics.rs | 6 +- cli/lsp/documents.rs | 84 +++------------- cli/lsp/language_server.rs | 2 +- cli/lsp/resolver.rs | 190 +++++++++---------------------------- cli/lsp/tsc.rs | 40 ++++---- cli/resolver.rs | 106 ++++++--------------- cli/tools/vendor/build.rs | 2 +- 8 files changed, 119 insertions(+), 322 deletions(-) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index ac7f8a3652..375096f988 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -65,7 +65,7 @@ pub struct GraphValidOptions { /// for the CLI. pub fn graph_valid( graph: &ModuleGraph, - fs: &dyn FileSystem, + fs: Arc, roots: &[ModuleSpecifier], options: GraphValidOptions, ) -> Result<(), AnyError> { @@ -99,7 +99,7 @@ pub fn graph_valid( ) } ModuleGraphError::ModuleError(e) => { - enhanced_module_error_message(fs, e) + enhanced_module_error_message(fs.clone(), e) } }; @@ -661,7 +661,7 @@ impl ModuleGraphBuilder { ) -> Result<(), AnyError> { graph_valid( graph, - self.fs.as_ref(), + self.fs.clone(), roots, GraphValidOptions { is_vendoring: false, @@ -705,14 +705,13 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String { } pub fn enhanced_module_error_message( - fs: &dyn FileSystem, + fs: Arc, error: &ModuleError, ) -> String { let additional_message = match error { ModuleError::LoadingErr(specifier, _, _) // ex. "Is a directory" error | ModuleError::Missing(specifier, _) => { - SloppyImportsResolver::resolve_with_fs( - fs, + SloppyImportsResolver::new(fs).resolve( specifier, ResolutionMode::Execution, ) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index edaf30e835..8472ad185c 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -800,7 +800,7 @@ fn generate_lint_diagnostics( break; } // ignore any npm package files - if snapshot.resolver.in_npm_package(specifier) { + if snapshot.resolver.in_node_modules(specifier) { continue; } let version = document.maybe_lsp_version(); @@ -1231,7 +1231,7 @@ impl DenoDiagnostic { Self::NoCacheJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing jsr package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoCacheNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoLocal(specifier) => { - let sloppy_resolution = SloppyImportsResolver::resolve_with_fs(&deno_fs::RealFs, specifier, ResolutionMode::Execution); + let sloppy_resolution = SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)).resolve(specifier, ResolutionMode::Execution); let data = sloppy_resolution.as_lsp_quick_fix_message().map(|message| { json!({ "specifier": specifier, @@ -1434,7 +1434,7 @@ fn diagnose_dependency( dependency_key: &str, dependency: &deno_graph::Dependency, ) { - if snapshot.resolver.in_npm_package(referrer) { + if snapshot.resolver.in_node_modules(referrer) { return; // ignore, surface typescript errors instead } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 6c7f8433fd..5ae15b362e 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -12,8 +12,6 @@ use super::tsc; use super::tsc::AssetDocument; use crate::graph_util::CliJsrUrlProvider; -use crate::lsp::logging::lsp_warn; -use deno_graph::source::Resolver; use deno_runtime::fs_util::specifier_to_file_path; use dashmap::DashMap; @@ -31,7 +29,6 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::Resolution; use deno_runtime::deno_node; -use deno_runtime::deno_node::NodeResolution; use deno_runtime::deno_node::NodeResolutionMode; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; @@ -388,7 +385,7 @@ impl Document { d.with_new_resolver( s, &CliJsrUrlProvider, - Some(&graph_resolver), + Some(graph_resolver), Some(npm_resolver), ), ) @@ -398,7 +395,7 @@ impl Document { maybe_types_dependency = self.maybe_types_dependency.as_ref().map(|d| { Arc::new(d.with_new_resolver( &CliJsrUrlProvider, - Some(&graph_resolver), + Some(graph_resolver), Some(npm_resolver), )) }); @@ -663,17 +660,9 @@ fn resolve_media_type( maybe_language_id: Option, resolver: &LspResolver, ) -> MediaType { - if resolver.in_npm_package(specifier) { - match resolver.url_to_node_resolution(specifier.clone()) { - Ok(Some(resolution)) => { - let (_, media_type) = - NodeResolution::into_specifier_and_media_type(Some(resolution)); - return media_type; - } - Err(err) => { - lsp_warn!("Node resolution failed for '{}': {}", specifier, err); - } - _ => {} + if resolver.in_node_modules(specifier) { + if let Some(media_type) = resolver.node_media_type(specifier) { + return media_type; } } @@ -979,7 +968,7 @@ impl Documents { } } - pub fn resolve_specifier( + pub fn resolve_document_specifier( &self, specifier: &ModuleSpecifier, ) -> Option { @@ -998,7 +987,7 @@ impl Documents { /// Return `true` if the specifier can be resolved to a document. pub fn exists(&self, specifier: &ModuleSpecifier) -> bool { - let specifier = self.resolve_specifier(specifier); + let specifier = self.resolve_document_specifier(specifier); if let Some(specifier) = specifier { if self.open_docs.contains_key(&specifier) { return true; @@ -1035,7 +1024,7 @@ impl Documents { &self, original_specifier: &ModuleSpecifier, ) -> Option> { - let specifier = self.resolve_specifier(original_specifier)?; + let specifier = self.resolve_document_specifier(original_specifier)?; if let Some(document) = self.open_docs.get(&specifier) { Some(document.clone()) } else { @@ -1048,13 +1037,6 @@ impl Documents { } } - pub fn is_open(&self, specifier: &ModuleSpecifier) -> bool { - let Some(specifier) = self.resolve_specifier(specifier) else { - return false; - }; - self.open_docs.contains_key(&specifier) - } - /// Return a collection of documents that are contained in the document store /// based on the provided filter. pub fn documents(&self, filter: DocumentsFilter) -> Vec> { @@ -1108,17 +1090,6 @@ impl Documents { let dependencies = document.as_ref().map(|d| d.dependencies()); let mut results = Vec::new(); for specifier in specifiers { - if self.resolver.in_npm_package(referrer) { - // we're in an npm package, so use node resolution - results.push(Some(NodeResolution::into_specifier_and_media_type( - self - .resolver - .node_resolve(specifier, referrer, NodeResolutionMode::Types) - .ok() - .flatten(), - ))); - continue; - } if specifier.starts_with("asset:") { if let Ok(specifier) = ModuleSpecifier::parse(specifier) { let media_type = MediaType::from_specifier(&specifier); @@ -1136,20 +1107,6 @@ impl Documents { } else { results.push(None); } - } else if let Some(specifier) = self - .resolver - .resolve_graph_import(specifier) - .and_then(|r| r.maybe_specifier()) - { - results.push(self.resolve_dependency(specifier, referrer)); - } else if let Ok(npm_req_ref) = - NpmPackageReqReference::from_str(specifier) - { - results.push(node_resolve_npm_req_ref( - &npm_req_ref, - referrer, - &self.resolver, - )); } else if let Ok(specifier) = self.resolver.as_graph_resolver().resolve( specifier, &deno_graph::Range { @@ -1314,7 +1271,11 @@ impl Documents { } if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) { - return node_resolve_npm_req_ref(&npm_ref, referrer, &self.resolver); + return self.resolver.npm_to_file_url( + &npm_ref, + referrer, + NodeResolutionMode::Types, + ); } let Some(doc) = self.get(specifier) else { return Some((specifier.clone(), MediaType::from_specifier(specifier))); @@ -1328,23 +1289,6 @@ impl Documents { } } -fn node_resolve_npm_req_ref( - npm_req_ref: &NpmPackageReqReference, - referrer: &ModuleSpecifier, - resolver: &LspResolver, -) -> Option<(ModuleSpecifier, MediaType)> { - Some(NodeResolution::into_specifier_and_media_type( - resolver - .resolve_npm_req_reference( - npm_req_ref, - referrer, - NodeResolutionMode::Types, - ) - .ok() - .flatten(), - )) -} - /// Loader that will look at the open documents. pub struct OpenDocumentsGraphLoader<'a> { pub inner_loader: &'a mut dyn deno_graph::source::Loader, @@ -1441,7 +1385,7 @@ fn analyze_module( // dynamic imports like import(`./dir/${something}`) in the LSP file_system: &deno_graph::source::NullFileSystem, jsr_url_provider: &CliJsrUrlProvider, - maybe_resolver: Some(&resolver.as_graph_resolver()), + maybe_resolver: Some(resolver.as_graph_resolver()), maybe_npm_resolver: Some(resolver.as_graph_npm_resolver()), }, )), diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 0c327929b3..377dd7a08e 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -243,7 +243,7 @@ impl LanguageServer { .await?; graph_util::graph_valid( &graph, - factory.fs().as_ref(), + factory.fs().clone(), &roots, graph_util::GraphValidOptions { is_vendoring: false, diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index b42f253c4c..0f515060a0 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -19,11 +19,11 @@ use crate::npm::ManagedCliNpmResolver; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; use crate::resolver::CliNodeResolver; -use crate::resolver::SloppyImportsFsEntry; use crate::resolver::SloppyImportsResolver; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; use dashmap::DashMap; +use deno_ast::MediaType; use deno_cache_dir::HttpCache; use deno_core::error::AnyError; use deno_core::url::Url; @@ -31,7 +31,6 @@ use deno_graph::source::NpmResolver; use deno_graph::source::Resolver; use deno_graph::GraphImport; use deno_graph::ModuleSpecifier; -use deno_graph::Resolution; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolution; @@ -64,7 +63,6 @@ pub struct LspResolver { redirect_resolver: Option>, graph_imports: Arc>, config: Arc, - unstable_sloppy_imports: bool, } impl Default for LspResolver { @@ -78,7 +76,6 @@ impl Default for LspResolver { redirect_resolver: None, graph_imports: Default::default(), config: Default::default(), - unstable_sloppy_imports: false, } } } @@ -146,10 +143,6 @@ impl LspResolver { npm_config_hash, redirect_resolver, graph_imports, - unstable_sloppy_imports: config_data - .and_then(|d| d.config_file.as_ref()) - .map(|cf| cf.has_unstable("sloppy-imports")) - .unwrap_or(false), config: Arc::new(config.clone()), }) } @@ -172,7 +165,6 @@ impl LspResolver { redirect_resolver: self.redirect_resolver.clone(), graph_imports: self.graph_imports.clone(), config: self.config.clone(), - unstable_sloppy_imports: self.unstable_sloppy_imports, }) } @@ -192,17 +184,28 @@ impl LspResolver { Ok(()) } - pub fn as_graph_resolver(&self) -> LspGraphResolver { - LspGraphResolver { - inner: &self.graph_resolver, - unstable_sloppy_imports: self.unstable_sloppy_imports, - } + pub fn as_graph_resolver(&self) -> &dyn Resolver { + self.graph_resolver.as_ref() } pub fn as_graph_npm_resolver(&self) -> &dyn NpmResolver { self.graph_resolver.as_ref() } + pub fn maybe_managed_npm_resolver(&self) -> Option<&ManagedCliNpmResolver> { + self.npm_resolver.as_ref().and_then(|r| r.as_managed()) + } + + pub fn graph_import_specifiers( + &self, + ) -> impl Iterator { + self + .graph_imports + .values() + .flat_map(|i| i.dependencies.values()) + .flat_map(|value| value.get_type().or_else(|| value.get_code())) + } + pub fn jsr_to_registry_url( &self, req_ref: &JsrPackageReqReference, @@ -222,81 +225,41 @@ impl LspResolver { self.jsr_resolver.as_ref()?.lookup_req_for_nv(nv) } - pub fn maybe_managed_npm_resolver(&self) -> Option<&ManagedCliNpmResolver> { - self.npm_resolver.as_ref().and_then(|r| r.as_managed()) - } - - pub fn graph_import_specifiers( + pub fn npm_to_file_url( &self, - ) -> impl Iterator { - self - .graph_imports - .values() - .flat_map(|i| i.dependencies.values()) - .flat_map(|value| value.get_type().or_else(|| value.get_code())) + req_ref: &NpmPackageReqReference, + referrer: &ModuleSpecifier, + mode: NodeResolutionMode, + ) -> Option<(ModuleSpecifier, MediaType)> { + let node_resolver = self.node_resolver.as_ref()?; + Some(NodeResolution::into_specifier_and_media_type( + node_resolver + .resolve_req_reference( + req_ref, + &PermissionsContainer::allow_all(), + referrer, + mode, + ) + .ok(), + )) } - pub fn resolve_graph_import(&self, specifier: &str) -> Option<&Resolution> { - for graph_imports in self.graph_imports.values() { - let maybe_dep = graph_imports.dependencies.get(specifier); - if maybe_dep.is_some() { - return maybe_dep.map(|d| &d.maybe_type); - } - } - None - } - - pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { + pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool { if let Some(npm_resolver) = &self.npm_resolver { return npm_resolver.in_npm_package(specifier); } false } - pub fn node_resolve( + pub fn node_media_type( &self, - specifier: &str, - referrer: &ModuleSpecifier, - mode: NodeResolutionMode, - ) -> Result, AnyError> { - let Some(node_resolver) = self.node_resolver.as_ref() else { - return Ok(None); - }; - node_resolver.resolve( - specifier, - referrer, - mode, - &PermissionsContainer::allow_all(), - ) - } - - pub fn resolve_npm_req_reference( - &self, - req_ref: &NpmPackageReqReference, - referrer: &ModuleSpecifier, - mode: NodeResolutionMode, - ) -> Result, AnyError> { - let Some(node_resolver) = self.node_resolver.as_ref() else { - return Ok(None); - }; - node_resolver - .resolve_req_reference( - req_ref, - &PermissionsContainer::allow_all(), - referrer, - mode, - ) - .map(Some) - } - - pub fn url_to_node_resolution( - &self, - specifier: ModuleSpecifier, - ) -> Result, AnyError> { - let Some(node_resolver) = self.node_resolver.as_ref() else { - return Ok(None); - }; - node_resolver.url_to_node_resolution(specifier).map(Some) + specifier: &ModuleSpecifier, + ) -> Option { + let node_resolver = self.node_resolver.as_ref()?; + let resolution = node_resolver + .url_to_node_resolution(specifier.clone()) + .ok()?; + Some(NodeResolution::into_specifier_and_media_type(Some(resolution)).1) } pub fn get_closest_package_json( @@ -335,68 +298,6 @@ impl LspResolver { } } -#[derive(Debug)] -pub struct LspGraphResolver<'a> { - inner: &'a CliGraphResolver, - unstable_sloppy_imports: bool, -} - -impl<'a> Resolver for LspGraphResolver<'a> { - fn default_jsx_import_source(&self) -> Option { - self.inner.default_jsx_import_source() - } - - fn default_jsx_import_source_types(&self) -> Option { - self.inner.default_jsx_import_source_types() - } - - fn jsx_import_source_module(&self) -> &str { - self.inner.jsx_import_source_module() - } - - fn resolve( - &self, - specifier_text: &str, - referrer_range: &deno_graph::Range, - mode: deno_graph::source::ResolutionMode, - ) -> Result { - let specifier = self.inner.resolve(specifier_text, referrer_range, mode)?; - if self.unstable_sloppy_imports && specifier.scheme() == "file" { - Ok( - SloppyImportsResolver::resolve_with_stat_sync( - &specifier, - mode, - |path| { - path.metadata().ok().and_then(|m| { - if m.is_file() { - Some(SloppyImportsFsEntry::File) - } else if m.is_dir() { - Some(SloppyImportsFsEntry::Dir) - } else { - None - } - }) - }, - ) - .into_specifier() - .into_owned(), - ) - } else { - Ok(specifier) - } - } - - fn resolve_types( - &self, - specifier: &deno_ast::ModuleSpecifier, - ) -> Result< - Option<(deno_ast::ModuleSpecifier, Option)>, - deno_graph::source::ResolveError, - > { - self.inner.resolve_types(specifier) - } -} - async fn create_npm_resolver( config_data: &ConfigData, cache: &LspCache, @@ -467,6 +368,8 @@ fn create_graph_resolver( node_resolver: Option<&Arc>, ) -> Arc { let config_file = config_data.and_then(|d| d.config_file.as_deref()); + let unstable_sloppy_imports = + config_file.is_some_and(|cf| cf.has_unstable("sloppy-imports")); Arc::new(CliGraphResolver::new(CliGraphResolverOptions { node_resolver: node_resolver.cloned(), npm_resolver: npm_resolver.cloned(), @@ -484,8 +387,9 @@ fn create_graph_resolver( bare_node_builtins_enabled: config_file .map(|cf| cf.has_unstable("bare-node-builtins")) .unwrap_or(false), - // not used in the LSP as the LspGraphResolver handles this - sloppy_imports_resolver: None, + sloppy_imports_resolver: unstable_sloppy_imports.then(|| { + SloppyImportsResolver::new_without_stat_cache(Arc::new(deno_fs::RealFs)) + }), })) } diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index a950ea0bb9..6999b40eff 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4053,7 +4053,7 @@ fn op_is_node_file(state: &mut OpState, #[string] path: String) -> bool { let state = state.borrow::(); let mark = state.performance.mark("tsc.op.op_is_node_file"); let r = match ModuleSpecifier::parse(&path) { - Ok(specifier) => state.state_snapshot.resolver.in_npm_package(&specifier), + Ok(specifier) => state.state_snapshot.resolver.in_node_modules(&specifier), Err(_) => false, }; state.performance.measure(mark); @@ -4250,12 +4250,14 @@ fn op_respond( fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let mark = state.performance.mark("tsc.op.op_script_names"); - let documents = &state.state_snapshot.documents; - let all_docs = documents.documents(DocumentsFilter::AllDiagnosable); let mut seen = HashSet::new(); let mut result = Vec::new(); - if documents.has_injected_types_node_package() { + if state + .state_snapshot + .documents + .has_injected_types_node_package() + { // ensure this is first so it resolves the node types first let specifier = "asset:///node_types.d.ts"; result.push(specifier.to_string()); @@ -4269,25 +4271,17 @@ fn op_script_names(state: &mut OpState) -> Vec { } } - // finally include the documents and all their dependencies - for doc in &all_docs { - let specifiers = std::iter::once(doc.specifier()).chain( - doc - .dependencies() - .values() - .filter_map(|dep| dep.get_type().or_else(|| dep.get_code())), - ); - for specifier in specifiers { - if seen.insert(specifier.as_str()) { - if let Some(specifier) = documents.resolve_specifier(specifier) { - // only include dependencies we know to exist otherwise typescript will error - if documents.exists(&specifier) - && (specifier.scheme() == "file" || documents.is_open(&specifier)) - { - result.push(specifier.to_string()); - } - } - } + // finally include the documents + let docs = state + .state_snapshot + .documents + .documents(DocumentsFilter::AllDiagnosable); + for doc in &docs { + let specifier = doc.specifier(); + if seen.insert(specifier.as_str()) + && (doc.is_open() || specifier.scheme() == "file") + { + result.push(specifier.to_string()); } } diff --git a/cli/resolver.rs b/cli/resolver.rs index 32233e961f..a11a12b5d7 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use dashmap::DashMap; use deno_ast::MediaType; use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; @@ -33,7 +34,6 @@ use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use import_map::ImportMap; use std::borrow::Cow; -use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; @@ -858,38 +858,6 @@ impl NpmResolver for CliGraphResolver { } } -#[derive(Debug)] -struct SloppyImportsStatCache { - fs: Arc, - cache: Mutex>>, -} - -impl SloppyImportsStatCache { - pub fn new(fs: Arc) -> Self { - Self { - fs, - cache: Default::default(), - } - } - - pub fn stat_sync(&self, path: &Path) -> Option { - // there will only ever be one thread in here at a - // time, so it's ok to hold the lock for so long - let mut cache = self.cache.lock(); - if let Some(entry) = cache.get(path) { - return *entry; - } - - let entry = self - .fs - .stat_sync(path) - .ok() - .and_then(|stat| SloppyImportsFsEntry::from_fs_stat(&stat)); - cache.insert(path.to_owned(), entry); - entry - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SloppyImportsFsEntry { File, @@ -989,33 +957,27 @@ impl<'a> SloppyImportsResolution<'a> { #[derive(Debug)] pub struct SloppyImportsResolver { - stat_cache: SloppyImportsStatCache, + fs: Arc, + cache: Option>>, } impl SloppyImportsResolver { pub fn new(fs: Arc) -> Self { Self { - stat_cache: SloppyImportsStatCache::new(fs), + fs, + cache: Some(Default::default()), } } - pub fn resolve_with_fs<'a>( - fs: &dyn FileSystem, + pub fn new_without_stat_cache(fs: Arc) -> Self { + Self { fs, cache: None } + } + + pub fn resolve<'a>( + &self, specifier: &'a ModuleSpecifier, mode: ResolutionMode, ) -> SloppyImportsResolution<'a> { - Self::resolve_with_stat_sync(specifier, mode, |path| { - fs.stat_sync(path) - .ok() - .and_then(|stat| SloppyImportsFsEntry::from_fs_stat(&stat)) - }) - } - - pub fn resolve_with_stat_sync( - specifier: &ModuleSpecifier, - mode: ResolutionMode, - stat_sync: impl Fn(&Path) -> Option, - ) -> SloppyImportsResolution { fn path_without_ext( path: &Path, media_type: MediaType, @@ -1065,7 +1027,7 @@ impl SloppyImportsResolver { } let probe_paths: Vec<(PathBuf, SloppyImportsResolutionReason)> = - match (stat_sync)(&path) { + match self.stat_sync(&path) { Some(SloppyImportsFsEntry::File) => { if mode.is_types() { let media_type = MediaType::from_specifier(specifier); @@ -1243,7 +1205,7 @@ impl SloppyImportsResolver { }; for (probe_path, reason) in probe_paths { - if (stat_sync)(&probe_path) == Some(SloppyImportsFsEntry::File) { + if self.stat_sync(&probe_path) == Some(SloppyImportsFsEntry::File) { if let Ok(specifier) = ModuleSpecifier::from_file_path(probe_path) { match reason { SloppyImportsResolutionReason::JsToTs => { @@ -1263,14 +1225,22 @@ impl SloppyImportsResolver { SloppyImportsResolution::None(specifier) } - pub fn resolve<'a>( - &self, - specifier: &'a ModuleSpecifier, - mode: ResolutionMode, - ) -> SloppyImportsResolution<'a> { - Self::resolve_with_stat_sync(specifier, mode, |path| { - self.stat_cache.stat_sync(path) - }) + fn stat_sync(&self, path: &Path) -> Option { + if let Some(cache) = &self.cache { + if let Some(entry) = cache.get(path) { + return *entry; + } + } + + let entry = self + .fs + .stat_sync(path) + .ok() + .and_then(|stat| SloppyImportsFsEntry::from_fs_stat(&stat)); + if let Some(cache) = &self.cache { + cache.insert(path.to_owned(), entry); + } + entry } } @@ -1278,7 +1248,6 @@ impl SloppyImportsResolver { mod test { use std::collections::BTreeMap; - use deno_runtime::deno_fs::RealFs; use test_util::TestContext; use super::*; @@ -1346,21 +1315,8 @@ mod test { #[test] fn test_unstable_sloppy_imports() { fn resolve(specifier: &ModuleSpecifier) -> SloppyImportsResolution { - SloppyImportsResolver::resolve_with_stat_sync( - specifier, - ResolutionMode::Execution, - |path| { - RealFs.stat_sync(path).ok().and_then(|stat| { - if stat.is_file { - Some(SloppyImportsFsEntry::File) - } else if stat.is_directory { - Some(SloppyImportsFsEntry::Dir) - } else { - None - } - }) - }, - ) + SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)) + .resolve(specifier, ResolutionMode::Execution) } let context = TestContext::default(); diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 5ff986f0cb..5435a0035e 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -126,7 +126,7 @@ pub async fn build< // surface any errors graph_util::graph_valid( &graph, - &deno_fs::RealFs, + Arc::new(deno_fs::RealFs), &graph.roots, graph_util::GraphValidOptions { is_vendoring: true,