From 182de1452b7e03f1dd7cd85ac80ca432ef4a621c Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Mon, 8 Nov 2021 11:50:48 +1100 Subject: [PATCH] fix(lsp): display module types only dependencies on hover (#12683) Fixes: #12675 --- cli/lsp/documents.rs | 27 ++++++++++++ cli/lsp/language_server.rs | 28 +++++++----- cli/tests/integration/lsp_tests.rs | 70 ++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 81f978be114796..dc5b0f00443ccd 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -263,6 +263,13 @@ impl Document { self.maybe_lsp_version.is_some() } + fn maybe_types_dependency(&self) -> deno_graph::Resolved { + let module_result = self.maybe_module.as_ref()?; + let module = module_result.as_ref().ok()?; + let (_, maybe_dep) = module.maybe_types_dependency.as_ref()?; + maybe_dep.clone() + } + fn media_type(&self) -> MediaType { if let Some(Ok(module)) = &self.maybe_module { module.media_type @@ -597,6 +604,16 @@ impl Inner { }) } + fn get_maybe_types_for_dependency( + &mut self, + dependency: &deno_graph::Dependency, + ) -> deno_graph::Resolved { + let code_dep = dependency.maybe_code.as_ref()?; + let (specifier, _) = code_dep.as_ref().ok()?; + let doc = self.get(specifier)?; + doc.maybe_types_dependency() + } + fn get_navigation_tree( &mut self, specifier: &ModuleSpecifier, @@ -952,6 +969,16 @@ impl Documents { self.0.lock().get_maybe_dependency(specifier, position) } + /// For a given dependency, try to resolve the maybe_types_dependency for the + /// dependency. This covers modules that assert their own types, like via the + /// triple-slash reference, or the `X-TypeScript-Types` header. + pub fn get_maybe_types_for_dependency( + &self, + dependency: &deno_graph::Dependency, + ) -> deno_graph::Resolved { + self.0.lock().get_maybe_types_for_dependency(dependency) + } + /// Get a reference to the navigation tree stored for a given specifier, if /// any. pub fn get_navigation_tree( diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 656bff1f4dbedf..89e286718c3f26 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1087,21 +1087,34 @@ impl Inner { &specifier, ¶ms.text_document_position_params.position, ) { - let value = match (&dep.maybe_code, &dep.maybe_type) { - (Some(code_dep), Some(type_dep)) => format!( + let maybe_types_dependency = + self.documents.get_maybe_types_for_dependency(&dep); + let value = match (&dep.maybe_code, &dep.maybe_type, &maybe_types_dependency) { + (Some(code_dep), Some(type_dep), None) => format!( "**Resolved Dependency**\n\n**Code**: {}\n\n**Types**: {}\n", to_hover_text(code_dep), to_hover_text(type_dep) ), - (Some(code_dep), None) => format!( + (Some(code_dep), Some(type_dep), Some(types_dep)) => format!( + "**Resolved Dependency**\n\n**Code**: {}\n**Types**: {}\n**Import Types**: {}\n", + to_hover_text(code_dep), + to_hover_text(types_dep), + to_hover_text(type_dep) + ), + (Some(code_dep), None, None) => format!( "**Resolved Dependency**\n\n**Code**: {}\n", to_hover_text(code_dep) ), - (None, Some(type_dep)) => format!( + (Some(code_dep), None, Some(types_dep)) => format!( + "**Resolved Dependency**\n\n**Code**: {}\n\n**Types**: {}\n", + to_hover_text(code_dep), + to_hover_text(types_dep) + ), + (None, Some(type_dep), _) => format!( "**Resolved Dependency**\n\n**Types**: {}\n", to_hover_text(type_dep) ), - (None, None) => unreachable!("{}", json!(params)), + (None, None, _) => unreachable!("{}", json!(params)), }; Some(Hover { contents: HoverContents::Markup(MarkupContent { @@ -2661,11 +2674,6 @@ impl Inner { .performance .mark("virtual_text_document", Some(¶ms)); let specifier = self.url_map.normalize_url(¶ms.text_document.uri); - info!( - "virtual_text_document\n{}\nspecifier: {}", - json!(params), - specifier - ); let contents = if specifier.as_str() == "deno:/status.md" { let mut contents = String::new(); let mut documents_specifiers = self.documents.specifiers(false, false); diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 6fbeec3442b9c0..6e009bd20ef667 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -1066,6 +1066,76 @@ fn lsp_hover_dependency() { ); } +#[test] +fn lsp_hover_typescript_types() { + let _g = http_server(); + let mut client = init("initialize_params.json"); + did_open( + &mut client, + json!({ + "textDocument": { + "uri": "file:///a/file.ts", + "languageId": "typescript", + "version": 1, + "text": "import * as a from \"http://127.0.0.1:4545/xTypeScriptTypes.js\";\n\nconsole.log(a.foo);\n", + } + }), + ); + let (maybe_res, maybe_err) = client + .write_request::<_, _, Value>( + "deno/cache", + json!({ + "referrer": { + "uri": "file:///a/file.ts", + }, + "uris": [ + { + "uri": "http://127.0.0.1:4545/xTypeScriptTypes.js", + } + ], + }), + ) + .unwrap(); + assert!(maybe_err.is_none()); + assert!(maybe_res.is_some()); + let (maybe_res, maybe_err) = client + .write_request::<_, _, Value>( + "textDocument/hover", + json!({ + "textDocument": { + "uri": "file:///a/file.ts" + }, + "position": { + "line": 0, + "character": 24 + } + }), + ) + .unwrap(); + assert!(maybe_res.is_some()); + assert!(maybe_err.is_none()); + assert_eq!( + json!(maybe_res.unwrap()), + json!({ + "contents": { + "kind": "markdown", + "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": { + "line": 0, + "character": 19 + }, + "end": { + "line": 0, + "character": 62 + } + } + }) + ); + shutdown(&mut client); +} + #[test] fn lsp_call_hierarchy() { let mut client = init("initialize_params.json");