From d95666cae04376c041f5276774dc5445c6ca2415 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Tue, 9 Feb 2021 20:48:53 +1100 Subject: [PATCH] fix(lsp): handle code lenses for non-documents (#9454) --- cli/lsp/documents.rs | 27 ---- cli/lsp/language_server.rs | 131 ++++++++++++------ cli/lsp/utils.rs | 30 ++++ cli/tests/lsp/code_lens_request_asset.json | 10 ++ .../lsp/code_lens_resolve_request_asset.json | 21 +++ .../lsp/did_open_notification_asset.json | 2 +- 6 files changed, 154 insertions(+), 67 deletions(-) create mode 100644 cli/tests/lsp/code_lens_request_asset.json create mode 100644 cli/tests/lsp/code_lens_resolve_request_asset.json diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 4cda1f0487..955ca1c782 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -2,7 +2,6 @@ use super::analysis; use super::text::LineIndex; -use super::tsc::NavigationTree; use crate::import_map::ImportMap; use crate::media_type::MediaType; @@ -34,7 +33,6 @@ impl IndexValid { pub struct DocumentData { bytes: Option>, line_index: Option, - navigation_tree: Option, dependencies: Option>, version: Option, } @@ -74,7 +72,6 @@ impl DocumentData { } else { Some(LineIndex::new(&content)) }; - self.navigation_tree = None; Ok(()) } @@ -190,14 +187,6 @@ impl DocumentCache { doc.line_index.clone() } - pub fn navigation_tree( - &self, - specifier: &ModuleSpecifier, - ) -> Option { - let doc = self.docs.get(specifier)?; - doc.navigation_tree.clone() - } - pub fn open( &mut self, specifier: ModuleSpecifier, @@ -229,22 +218,6 @@ impl DocumentCache { .collect() } - pub fn set_navigation_tree( - &mut self, - specifier: &ModuleSpecifier, - navigation_tree: NavigationTree, - ) -> Result<(), AnyError> { - if let Some(mut doc) = self.docs.get_mut(specifier) { - doc.navigation_tree = Some(navigation_tree); - Ok(()) - } else { - Err(custom_error( - "NotFound", - "The document \"{}\" was unexpectedly missing.", - )) - } - } - pub fn version(&self, specifier: &ModuleSpecifier) -> Option { self.docs.get(specifier).and_then(|doc| doc.version) } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 91613d21a5..fc2f80aebb 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1,7 +1,6 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. use deno_core::error::anyhow; -use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::serde::Deserialize; use deno_core::serde::Serialize; @@ -86,6 +85,8 @@ pub(crate) struct Inner { maybe_import_map: Option, /// The URL for the import map which is used to determine relative imports. maybe_import_map_uri: Option, + /// A map of all the cached navigation trees. + navigation_trees: HashMap, /// A collection of measurements which instrument that performance of the LSP. performance: Performance, /// Cached sources that are read-only. @@ -119,6 +120,7 @@ impl Inner { maybe_config_uri: Default::default(), maybe_import_map: Default::default(), maybe_import_map_uri: Default::default(), + navigation_trees: Default::default(), performance: Default::default(), sources, ts_fixable_diagnostics: Default::default(), @@ -184,33 +186,24 @@ impl Inner { &mut self, specifier: &ModuleSpecifier, ) -> Result { - if self.documents.contains(specifier) { - let mark = self.performance.mark("get_navigation_tree"); - if let Some(navigation_tree) = self.documents.navigation_tree(specifier) { - self.performance.measure(mark); - Ok(navigation_tree) - } else { - let res = self - .ts_server - .request( - self.snapshot(), - tsc::RequestMethod::GetNavigationTree(specifier.clone()), - ) - .await - .unwrap(); - let navigation_tree: tsc::NavigationTree = - serde_json::from_value(res).unwrap(); - self - .documents - .set_navigation_tree(specifier, navigation_tree.clone())?; - self.performance.measure(mark); - Ok(navigation_tree) - } + let mark = self.performance.mark("get_navigation_tree"); + if let Some(navigation_tree) = self.navigation_trees.get(specifier) { + self.performance.measure(mark); + Ok(navigation_tree.clone()) } else { - Err(custom_error( - "NotFound", - format!("The document \"{}\" was unexpectedly not found.", specifier), - )) + let res = self + .ts_server + .request( + self.snapshot(), + tsc::RequestMethod::GetNavigationTree(specifier.clone()), + ) + .await?; + let navigation_tree: tsc::NavigationTree = serde_json::from_value(res)?; + self + .navigation_trees + .insert(specifier.clone(), navigation_tree.clone()); + self.performance.measure(mark); + Ok(navigation_tree) } } @@ -648,6 +641,9 @@ impl Inner { { error!("{}", err); } + // there are scenarios where local documents with a nav tree are opened in + // the editor + self.navigation_trees.remove(&specifier); self.performance.measure(mark); // TODO(@kitsonk): how to better lazily do this? @@ -672,6 +668,7 @@ impl Inner { { error!("{}", err); } + self.navigation_trees.remove(&specifier); self.performance.measure(mark); // TODO(@kitsonk): how to better lazily do this? @@ -690,12 +687,13 @@ impl Inner { } let specifier = utils::normalize_url(params.text_document.uri); self.documents.close(&specifier); + self.navigation_trees.remove(&specifier); + self.performance.measure(mark); // TODO(@kitsonk): how to better lazily do this? if let Err(err) = self.prepare_diagnostics().await { error!("{}", err); } - self.performance.measure(mark); } async fn did_change_configuration( @@ -713,7 +711,7 @@ impl Inner { .await .map(|vec| vec.get(0).cloned()) .unwrap_or_else(|err| { - error!("failed to fetch the extension settings {:?}", err); + error!("failed to fetch the extension settings {}", err); None }) } else { @@ -1026,7 +1024,7 @@ impl Inner { let line_index = self.get_line_index_sync(&specifier).unwrap(); let navigation_tree = self.get_navigation_tree(&specifier).await.map_err(|err| { - error!("Failed to retrieve nav tree: {:#?}", err); + error!("Failed to retrieve nav tree: {}", err); LspError::invalid_request() })?; @@ -1191,11 +1189,16 @@ impl Inner { } else { "1 implementation".to_string() }; + let url = utils::normalize_specifier(&code_lens_data.specifier) + .map_err(|err| { + error!("{}", err); + LspError::internal_error() + })?; Command { title, command: "deno.showReferences".to_string(), arguments: Some(vec![ - serde_json::to_value(code_lens_data.specifier).unwrap(), + serde_json::to_value(url).unwrap(), serde_json::to_value(params.range.start).unwrap(), serde_json::to_value(locations).unwrap(), ]), @@ -1272,11 +1275,16 @@ impl Inner { } else { "1 reference".to_string() }; + let url = utils::normalize_specifier(&code_lens_data.specifier) + .map_err(|err| { + error!("{}", err); + LspError::internal_error() + })?; Command { title, command: "deno.showReferences".to_string(), arguments: Some(vec![ - serde_json::to_value(code_lens_data.specifier).unwrap(), + serde_json::to_value(url).unwrap(), serde_json::to_value(params.range.start).unwrap(), serde_json::to_value(locations).unwrap(), ]), @@ -1531,13 +1539,13 @@ impl Inner { .request(self.snapshot(), req) .await .map_err(|err| { - error!("Failed to request to tsserver {:#?}", err); + error!("Failed to request to tsserver {}", err); LspError::invalid_request() })?; let maybe_implementations = serde_json::from_value::>>(res) .map_err(|err| { - error!("Failed to deserialized tsserver response to Vec {:#?}", err); + error!("Failed to deserialized tsserver response to Vec {}", err); LspError::internal_error() })?; @@ -1596,7 +1604,7 @@ impl Inner { .request(self.snapshot(), req) .await .map_err(|err| { - error!("Failed to request to tsserver {:#?}", err); + error!("Failed to request to tsserver {}", err); LspError::invalid_request() })?; @@ -1605,7 +1613,7 @@ impl Inner { >(res) .map_err(|err| { error!( - "Failed to deserialize tsserver response to Vec {:#?}", + "Failed to deserialize tsserver response to Vec {}", err ); LspError::internal_error() @@ -1617,7 +1625,7 @@ impl Inner { .into_workspace_edit(¶ms.new_name, self) .await .map_err(|err| { - error!("Failed to get workspace edits: {:#?}", err); + error!("Failed to get workspace edits: {}", err); LspError::internal_error() })?; self.performance.measure(mark); @@ -1637,7 +1645,7 @@ impl Inner { "deno/cache" => match params.map(serde_json::from_value) { Some(Ok(params)) => Ok(Some( serde_json::to_value(self.cache(params).await?).map_err(|err| { - error!("Failed to serialize cache response: {:#?}", err); + error!("Failed to serialize cache response: {}", err); LspError::internal_error() })?, )), @@ -1650,7 +1658,7 @@ impl Inner { serde_json::to_value(self.virtual_text_document(params).await?) .map_err(|err| { error!( - "Failed to serialize virtual_text_document response: {:#?}", + "Failed to serialize virtual_text_document response: {}", err ); LspError::internal_error() @@ -2584,6 +2592,51 @@ mod tests { harness.run().await; } + #[derive(Deserialize)] + struct CodeLensResponse { + pub result: Option>, + } + + #[derive(Deserialize)] + struct CodeLensResolveResponse { + pub result: CodeLens, + } + + #[tokio::test] + async fn test_code_lens_non_doc_nav_tree() { + let mut harness = LspTestHarness::new(vec![ + ("initialize_request.json", LspResponse::RequestAny), + ("initialized_notification.json", LspResponse::None), + ("did_open_notification_asset.json", LspResponse::None), + ( + "virtual_text_document_request.json", + LspResponse::RequestAny, + ), + ( + "code_lens_request_asset.json", + LspResponse::RequestAssert(|value| { + let resp: CodeLensResponse = serde_json::from_value(value).unwrap(); + let lenses = resp.result.unwrap(); + assert!(lenses.len() > 50); + }), + ), + ( + "code_lens_resolve_request_asset.json", + LspResponse::RequestAssert(|value| { + let resp: CodeLensResolveResponse = + serde_json::from_value(value).unwrap(); + assert!(resp.result.command.is_some()); + }), + ), + ( + "shutdown_request.json", + LspResponse::Request(3, json!(null)), + ), + ("exit_notification.json", LspResponse::None), + ]); + harness.run().await; + } + #[tokio::test] async fn test_code_actions() { let mut harness = LspTestHarness::new(vec![ diff --git a/cli/lsp/utils.rs b/cli/lsp/utils.rs index 09657a71d1..8f4de9c05e 100644 --- a/cli/lsp/utils.rs +++ b/cli/lsp/utils.rs @@ -16,6 +16,19 @@ pub fn normalize_file_name(file_name: &str) -> Result { Url::parse(&specifier_str).map_err(|err| err.into()) } +pub fn normalize_specifier( + specifier: &ModuleSpecifier, +) -> Result { + let url = specifier.as_url(); + if url.scheme() == "file" { + Ok(url.clone()) + } else { + let specifier_str = + format!("deno:///{}", url.as_str().replacen("://", "/", 1)); + Url::parse(&specifier_str).map_err(|err| err.into()) + } +} + /// Normalize URLs from the client, where "virtual" `deno:///` URLs are /// converted into proper module specifiers. pub fn normalize_url(url: Url) -> ModuleSpecifier { @@ -40,6 +53,23 @@ pub fn normalize_url(url: Url) -> ModuleSpecifier { mod tests { use super::*; + #[test] + fn test_normalize_file_name() { + let fixture = "https://deno.land/x/mod.ts"; + let actual = normalize_file_name(fixture).unwrap(); + let expected = Url::parse("deno:///https/deno.land/x/mod.ts").unwrap(); + assert_eq!(actual, expected); + } + + #[test] + fn test_normalize_specifier() { + let fixture = + ModuleSpecifier::resolve_url("https://deno.land/x/mod.ts").unwrap(); + let actual = normalize_specifier(&fixture).unwrap(); + let expected = Url::parse("deno:///https/deno.land/x/mod.ts").unwrap(); + assert_eq!(actual, expected); + } + #[test] fn test_normalize_url() { let fixture = Url::parse("deno:///https/deno.land/x/mod.ts").unwrap(); diff --git a/cli/tests/lsp/code_lens_request_asset.json b/cli/tests/lsp/code_lens_request_asset.json new file mode 100644 index 0000000000..b9db86749e --- /dev/null +++ b/cli/tests/lsp/code_lens_request_asset.json @@ -0,0 +1,10 @@ +{ + "jsonrpc": "2.0", + "id": 4, + "method": "textDocument/codeLens", + "params": { + "textDocument": { + "uri": "deno:/asset//lib.es2015.symbol.wellknown.d.ts" + } + } +} diff --git a/cli/tests/lsp/code_lens_resolve_request_asset.json b/cli/tests/lsp/code_lens_resolve_request_asset.json new file mode 100644 index 0000000000..c225a62e2c --- /dev/null +++ b/cli/tests/lsp/code_lens_resolve_request_asset.json @@ -0,0 +1,21 @@ +{ + "jsonrpc": "2.0", + "id": 5, + "method": "codeLens/resolve", + "params": { + "range": { + "start": { + "line": 93, + "character": 10 + }, + "end": { + "line": 93, + "character": 15 + } + }, + "data": { + "specifier": "asset:///lib.es2015.symbol.wellknown.d.ts", + "source": "implementations" + } + } +} diff --git a/cli/tests/lsp/did_open_notification_asset.json b/cli/tests/lsp/did_open_notification_asset.json index 413353f298..6288bbc534 100644 --- a/cli/tests/lsp/did_open_notification_asset.json +++ b/cli/tests/lsp/did_open_notification_asset.json @@ -6,7 +6,7 @@ "uri": "file:///a/file.ts", "languageId": "typescript", "version": 1, - "text": "console.log(Date.now());\n" + "text": "console.log(\"hello deno!\");\n" } } }