From da906de18437318527ae368932c2c8663db44f76 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 20 Oct 2022 13:23:21 -0400 Subject: [PATCH] fix(lsp): allow caching deps in non-saved files (#16353) --- cli/lsp/documents.rs | 28 ++++++++++++++++++++++++++++ cli/lsp/language_server.rs | 20 ++++++++++++++++---- cli/proc_state.rs | 27 ++++++++++++++++++++------- cli/tests/integration/lsp_tests.rs | 30 +++++++++++++++--------------- 4 files changed, 79 insertions(+), 26 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index e395b565d2..61db899e59 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -21,6 +21,7 @@ use deno_ast::ParsedSource; use deno_ast::SourceTextInfo; use deno_core::error::custom_error; use deno_core::error::AnyError; +use deno_core::futures::future; use deno_core::parking_lot::Mutex; use deno_core::url; use deno_core::ModuleSpecifier; @@ -1111,6 +1112,33 @@ impl Documents { } } +/// Loader that will look at the open documents. +pub struct DocumentsDenoGraphLoader<'a> { + pub inner_loader: &'a mut dyn deno_graph::source::Loader, + pub open_docs: &'a HashMap, +} + +impl<'a> deno_graph::source::Loader for DocumentsDenoGraphLoader<'a> { + fn load( + &mut self, + specifier: &ModuleSpecifier, + is_dynamic: bool, + ) -> deno_graph::source::LoadFuture { + if specifier.scheme() == "file" { + if let Some(doc) = self.open_docs.get(specifier) { + return Box::pin(future::ready(Ok(Some( + deno_graph::source::LoadResponse::Module { + content: doc.content(), + specifier: doc.specifier().clone(), + maybe_headers: None, + }, + )))); + } + } + self.inner_loader.load(specifier, is_dynamic) + } +} + /// The default parser from `deno_graph` does not include the configuration /// options we require for the lsp. #[derive(Debug, Default)] diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 9141f72886..27d69127c4 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -14,6 +14,7 @@ use import_map::ImportMap; use log::error; use log::warn; use serde_json::from_value; +use std::collections::HashMap; use std::env; use std::fmt::Write as _; use std::path::PathBuf; @@ -2825,9 +2826,19 @@ impl Inner { async fn create_graph_for_caching( cli_options: CliOptions, roots: Vec<(ModuleSpecifier, ModuleKind)>, + open_docs: Vec, ) -> Result<(), AnyError> { + let open_docs = open_docs + .into_iter() + .map(|d| (d.specifier().clone(), d)) + .collect::>(); let ps = ProcState::from_options(Arc::new(cli_options)).await?; - let graph = ps.create_graph(roots).await?; + let mut inner_loader = ps.create_graph_loader(); + let mut loader = crate::lsp::documents::DocumentsDenoGraphLoader { + inner_loader: &mut inner_loader, + open_docs: &open_docs, + }; + let graph = ps.create_graph_with_loader(roots, &mut loader).await?; graph_valid(&graph, true, false)?; Ok(()) } @@ -2867,10 +2878,11 @@ impl Inner { // todo(dsherret): why is running this on a new thread necessary? It does // a compile error otherwise. + let open_docs = self.documents.documents(true, true); let handle = tokio::task::spawn_blocking(|| { - run_local( - async move { create_graph_for_caching(cli_options, roots).await }, - ) + run_local(async move { + create_graph_for_caching(cli_options, roots, open_docs).await + }) }); if let Err(err) = handle.await.unwrap() { self.client.show_message(MessageType::WARNING, err).await; diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 5a95d85b7c..07a7c1f8a6 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -589,16 +589,29 @@ impl ProcState { Ok(()) } - pub async fn create_graph( - &self, - roots: Vec<(ModuleSpecifier, ModuleKind)>, - ) -> Result { - let mut cache = cache::FetchCacher::new( + /// Creates the default loader used for creating a graph. + pub fn create_graph_loader(&self) -> cache::FetchCacher { + cache::FetchCacher::new( self.emit_cache.clone(), self.file_fetcher.clone(), Permissions::allow_all(), Permissions::allow_all(), - ); + ) + } + + pub async fn create_graph( + &self, + roots: Vec<(ModuleSpecifier, ModuleKind)>, + ) -> Result { + let mut cache = self.create_graph_loader(); + self.create_graph_with_loader(roots, &mut cache).await + } + + pub async fn create_graph_with_loader( + &self, + roots: Vec<(ModuleSpecifier, ModuleKind)>, + loader: &mut dyn Loader, + ) -> Result { let maybe_locker = as_maybe_locker(self.lockfile.clone()); let maybe_import_map_resolver = self.maybe_import_map.clone().map(ImportMapResolver::new); @@ -620,7 +633,7 @@ impl ProcState { roots, false, maybe_imports, - &mut cache, + loader, maybe_resolver, maybe_locker, Some(&*analyzer), diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 7dc5ff02da..8390b0f6f4 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -1084,21 +1084,21 @@ fn lsp_inlay_hints() { "text": r#"function a(b: string) { return b; } - + a("foo"); - + enum C { A, } - + parseInt("123", 8); - + const d = Date.now(); - + class E { f = Date.now(); } - + ["a"].map((v) => v + v); "# } @@ -1234,21 +1234,21 @@ fn lsp_inlay_hints_not_enabled() { "text": r#"function a(b: string) { return b; } - + a("foo"); - + enum C { A, } - + parseInt("123", 8); - + const d = Date.now(); - + class E { f = Date.now(); } - + ["a"].map((v) => v + v); "# } @@ -1871,7 +1871,7 @@ fn lsp_hover_dependency() { Some(json!({ "contents": { "kind": "markdown", - "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/xTypeScriptTypes.js\n" + "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/xTypeScriptTypes.js\n\n**Types**: http​://127.0.0.1:4545/xTypeScriptTypes.d.ts\n" }, "range": { "start": { @@ -1905,7 +1905,7 @@ fn lsp_hover_dependency() { Some(json!({ "contents": { "kind": "markdown", - "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/subdir/type_reference.js\n" + "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/subdir/type_reference.js\n\n**Types**: http​://127.0.0.1:4545/subdir/type_reference.d.ts\n" }, "range": { "start": { @@ -4256,7 +4256,7 @@ fn lsp_cache_location() { Some(json!({ "contents": { "kind": "markdown", - "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/xTypeScriptTypes.js\n" + "value": "**Resolved Dependency**\n\n**Code**: http​://127.0.0.1:4545/xTypeScriptTypes.js\n\n**Types**: http​://127.0.0.1:4545/xTypeScriptTypes.d.ts\n" }, "range": { "start": {