From 8f1a92f3c39a4db7582824944d2c9319a11efcc0 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Sun, 14 Apr 2024 22:42:58 +0100 Subject: [PATCH] refactor(lsp): use fallback resolution in op_resolve() (#23329) --- cli/lsp/documents.rs | 34 ++++++++++++++++----------- cli/lsp/tsc.rs | 55 +++++++++++++++++--------------------------- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 9ca2db3169..8100b0732b 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1173,15 +1173,11 @@ impl Documents { /// tsc when type checking. pub fn resolve( &self, - specifiers: Vec, - referrer_doc: &AssetOrDocument, + specifiers: &[String], + referrer: &ModuleSpecifier, maybe_npm: Option<&StateNpmSnapshot>, ) -> Vec> { - let referrer = referrer_doc.specifier(); - let dependencies = match referrer_doc { - AssetOrDocument::Asset(_) => None, - AssetOrDocument::Document(doc) => Some(doc.dependencies.clone()), - }; + let dependencies = self.get(referrer).map(|d| d.dependencies.clone()); let mut results = Vec::new(); for specifier in specifiers { if let Some(npm) = maybe_npm { @@ -1191,7 +1187,7 @@ impl Documents { npm .node_resolver .resolve( - &specifier, + specifier, referrer, NodeResolutionMode::Types, &PermissionsContainer::allow_all(), @@ -1203,14 +1199,14 @@ impl Documents { } } if specifier.starts_with("asset:") { - if let Ok(specifier) = ModuleSpecifier::parse(&specifier) { + if let Ok(specifier) = ModuleSpecifier::parse(specifier) { let media_type = MediaType::from_specifier(&specifier); results.push(Some((specifier, media_type))); } else { results.push(None); } } else if let Some(dep) = - dependencies.as_ref().and_then(|d| d.deps.get(&specifier)) + dependencies.as_ref().and_then(|d| d.deps.get(specifier)) { if let Some(specifier) = dep.maybe_type.maybe_specifier() { results.push(self.resolve_dependency(specifier, maybe_npm, referrer)); @@ -1220,18 +1216,28 @@ impl Documents { results.push(None); } } else if let Some(specifier) = self - .resolve_imports_dependency(&specifier) + .resolve_imports_dependency(specifier) .and_then(|r| r.maybe_specifier()) { results.push(self.resolve_dependency(specifier, maybe_npm, referrer)); } else if let Ok(npm_req_ref) = - NpmPackageReqReference::from_str(&specifier) + NpmPackageReqReference::from_str(specifier) { results.push(node_resolve_npm_req_ref( &npm_req_ref, maybe_npm, referrer, )); + } else if let Ok(specifier) = self.get_resolver().resolve( + specifier, + &deno_graph::Range { + specifier: referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, + ResolutionMode::Types, + ) { + results.push(self.resolve_dependency(&specifier, maybe_npm, referrer)); } else { results.push(None); } @@ -1495,7 +1501,9 @@ impl Documents { if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) { return node_resolve_npm_req_ref(&npm_ref, maybe_npm, referrer); } - let doc = self.get(specifier)?; + let Some(doc) = self.get(specifier) else { + return Some((specifier.clone(), MediaType::from_specifier(specifier))); + }; let maybe_module = doc.maybe_js_module().and_then(|r| r.as_ref().ok()); let maybe_types_dependency = maybe_module .and_then(|m| m.maybe_types_dependency.as_ref().map(|d| &d.dependency)); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 5c52a8eb0f..ac4871dfcd 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4043,37 +4043,24 @@ fn op_resolve_inner( let state = state.borrow_mut::(); let mark = state.performance.mark_with_args("tsc.op.op_resolve", &args); let referrer = state.specifier_map.normalize(&args.base)?; - let specifiers = match state.get_asset_or_document(&referrer) { - Some(referrer_doc) => { - let resolved = state.state_snapshot.documents.resolve( - args.specifiers, - &referrer_doc, - state.state_snapshot.npm.as_ref(), - ); - resolved - .into_iter() - // Resolved `node:` specifier means the user doesn't have @types/node, - // resolve to stub. - .map(|o| match o.filter(|(s, _)| s.scheme() != "node") { - Some((s, mt)) => Some(( - state.specifier_map.denormalize(&s), - mt.as_ts_extension().to_string(), - )), - None => Some(( - MISSING_DEPENDENCY_SPECIFIER.to_string(), - MediaType::Dts.as_ts_extension().to_string(), - )), - }) - .collect() - } - None => { - lsp_warn!( - "Error resolving. Referring specifier \"{}\" was not found.", - args.base - ); - vec![None; args.specifiers.len()] - } - }; + let specifiers = state + .state_snapshot + .documents + .resolve( + &args.specifiers, + &referrer, + state.state_snapshot.npm.as_ref(), + ) + .into_iter() + .map(|o| { + o.map(|(s, mt)| { + ( + state.specifier_map.denormalize(&s), + mt.as_ts_extension().to_string(), + ) + }) + }) + .collect(); state.performance.measure(mark); Ok(specifiers) @@ -5572,7 +5559,7 @@ mod tests { } #[tokio::test] - async fn resolve_unknown_dependency_to_stub_module() { + async fn resolve_unknown_dependency() { let temp_dir = TempDir::new(); let (_, snapshot, _) = setup( &temp_dir, @@ -5597,8 +5584,8 @@ mod tests { assert_eq!( resolved, vec![Some(( - MISSING_DEPENDENCY_SPECIFIER.to_string(), - MediaType::Dts.as_ts_extension().to_string() + "file:///b.ts".to_string(), + MediaType::TypeScript.as_ts_extension().to_string() ))] ); }