From 63ba58239c2d21e7bfcae72720eed6b2ac0a44ab Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 29 Sep 2023 20:44:59 +0100 Subject: [PATCH] feat(lsp): jupyter notebook analysis (#20719) --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/lsp/completions.rs | 12 +- cli/lsp/config.rs | 4 + cli/lsp/documents.rs | 63 ++- cli/lsp/tsc.rs | 632 +++++++++++++++++++++++------ cli/tests/integration/lsp_tests.rs | 45 +- 7 files changed, 623 insertions(+), 135 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d26fa833b2..53eed62d90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -980,6 +980,7 @@ dependencies = [ "clap_complete", "clap_complete_fig", "console_static_text", + "dashmap 5.5.3", "data-encoding", "data-url", "deno_ast", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index cb150e5db8..c9cda2a2d3 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -72,6 +72,7 @@ clap = { version = "=4.3.3", features = ["string"] } clap_complete = "=4.3.1" clap_complete_fig = "=4.3.1" console_static_text.workspace = true +dashmap = "5.5.3" data-encoding.workspace = true data-url.workspace = true dissimilar = "=1.0.4" diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index 94111fee81..c6fc6216e8 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -2,6 +2,7 @@ use super::client::Client; use super::config::ConfigSnapshot; +use super::documents::cell_to_file_specifier; use super::documents::Documents; use super::documents::DocumentsFilter; use super::lsp_custom; @@ -364,11 +365,16 @@ fn get_local_completions( current: &str, range: &lsp::Range, ) -> Option> { + let base = match cell_to_file_specifier(base) { + Some(s) => s, + None => base.clone(), + }; + if base.scheme() != "file" { return None; } - let mut base_path = specifier_to_file_path(base).ok()?; + let mut base_path = specifier_to_file_path(&base).ok()?; base_path.pop(); let mut current_path = normalize_path(base_path.join(current)); // if the current text does not end in a `/` then we are still selecting on @@ -388,10 +394,10 @@ fn get_local_completions( let de = de.ok()?; let label = de.path().file_name()?.to_string_lossy().to_string(); let entry_specifier = resolve_path(de.path().to_str()?, &cwd).ok()?; - if &entry_specifier == base { + if entry_specifier == base { return None; } - let full_text = relative_specifier(base, &entry_specifier)?; + let full_text = relative_specifier(&base, &entry_specifier)?; // this weeds out situations where we are browsing in the parent, but // we want to filter out non-matches when the completion is manually // invoked by the user, but still allows for things like `../src/../` diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index d0ef33d405..9af05484c5 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -692,10 +692,14 @@ impl WorkspaceSettings { self.code_lens.implementations || self.code_lens.references } + // TODO(nayeemrmn): Factor in out-of-band media type here. pub fn language_settings_for_specifier( &self, specifier: &ModuleSpecifier, ) -> Option<&LanguageWorkspaceSettings> { + if specifier.scheme() == "deno-notebook-cell" { + return Some(&self.typescript); + } match MediaType::from_specifier(specifier) { MediaType::JavaScript | MediaType::Jsx diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 1c672f6237..14c01d13b7 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -16,7 +16,6 @@ use crate::cache::HttpCache; use crate::file_fetcher::get_source_from_bytes; use crate::file_fetcher::get_source_from_data_url; use crate::file_fetcher::map_content_type; -use crate::file_fetcher::SUPPORTED_SCHEMES; use crate::lsp::logging::lsp_warn; use crate::npm::CliNpmRegistryApi; use crate::npm::NpmResolution; @@ -93,6 +92,15 @@ static TSX_HEADERS: Lazy> = Lazy::new(|| { .collect() }); +pub const DOCUMENT_SCHEMES: [&str; 6] = [ + "data", + "blob", + "file", + "http", + "https", + "deno-notebook-cell", +]; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum LanguageId { JavaScript, @@ -254,6 +262,27 @@ impl AssetOrDocument { } } +/// Convert a `deno-notebook-cell:` specifier to a `file:` specifier. +/// ```rust +/// assert_eq!( +/// cell_to_file_specifier( +/// &Url::parse("deno-notebook-cell:/path/to/file.ipynb#abc").unwrap(), +/// ), +/// Some(Url::parse("file:///path/to/file.ipynb#abc").unwrap()), +/// ); +pub fn cell_to_file_specifier(specifier: &Url) -> Option { + if specifier.scheme() == "deno-notebook-cell" { + if let Ok(specifier) = ModuleSpecifier::parse(&format!( + "file://{}", + &specifier.as_str() + [url::quirks::internal_components(specifier).host_end as usize..], + )) { + return Some(specifier); + } + } + None +} + #[derive(Debug, Default)] struct DocumentDependencies { deps: IndexMap, @@ -270,10 +299,32 @@ impl DocumentDependencies { } pub fn from_module(module: &deno_graph::EsmModule) -> Self { - Self { + let mut deps = Self { deps: module.dependencies.clone(), maybe_types_dependency: module.maybe_types_dependency.clone(), + }; + if module.specifier.scheme() == "deno-notebook-cell" { + for (_, dep) in &mut deps.deps { + if let Resolution::Ok(resolved) = &mut dep.maybe_code { + if let Some(specifier) = cell_to_file_specifier(&resolved.specifier) { + resolved.specifier = specifier; + } + } + if let Resolution::Ok(resolved) = &mut dep.maybe_type { + if let Some(specifier) = cell_to_file_specifier(&resolved.specifier) { + resolved.specifier = specifier; + } + } + } + if let Some(dep) = &mut deps.maybe_types_dependency { + if let Resolution::Ok(resolved) = &mut dep.dependency { + if let Some(specifier) = cell_to_file_specifier(&resolved.specifier) { + resolved.specifier = specifier; + } + } + } } + deps } } @@ -677,13 +728,11 @@ impl SpecifierResolver { specifier: &ModuleSpecifier, ) -> Option { let scheme = specifier.scheme(); - if !SUPPORTED_SCHEMES.contains(&scheme) { + if !DOCUMENT_SCHEMES.contains(&scheme) { return None; } - if scheme == "data" || scheme == "blob" || scheme == "file" { - Some(specifier.clone()) - } else { + if scheme == "http" || scheme == "https" { let mut redirects = self.redirects.lock(); if let Some(specifier) = redirects.get(specifier) { Some(specifier.clone()) @@ -692,6 +741,8 @@ impl SpecifierResolver { redirects.insert(specifier.clone(), redirect.clone()); Some(redirect) } + } else { + Some(specifier.clone()) } } diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 7999cb1df9..e45d6d52b4 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3,6 +3,7 @@ use super::analysis::CodeActionData; use super::code_lens; use super::config; +use super::documents::cell_to_file_specifier; use super::documents::AssetOrDocument; use super::documents::DocumentsFilter; use super::language_server; @@ -31,6 +32,7 @@ use crate::tsc::ResolveArgs; use crate::util::path::relative_specifier; use crate::util::path::specifier_to_file_path; +use dashmap::DashMap; use deno_ast::MediaType; use deno_core::anyhow::anyhow; use deno_core::error::custom_error; @@ -126,14 +128,32 @@ pub enum SemicolonPreference { Remove, } +fn normalize_diagnostic( + diagnostic: &mut crate::tsc::Diagnostic, + specifier_map: &TscSpecifierMap, +) -> Result<(), AnyError> { + if let Some(file_name) = &mut diagnostic.file_name { + *file_name = specifier_map.normalize(&file_name)?.to_string(); + } + for ri in diagnostic.related_information.iter_mut().flatten() { + normalize_diagnostic(ri, specifier_map)?; + } + Ok(()) +} + #[derive(Clone, Debug)] -pub struct TsServer(mpsc::UnboundedSender); +pub struct TsServer { + sender: mpsc::UnboundedSender, + specifier_map: Arc, +} impl TsServer { pub fn new(performance: Arc, cache: Arc) -> Self { + let specifier_map = Arc::new(TscSpecifierMap::default()); + let specifier_map_ = specifier_map.clone(); let (tx, mut rx) = mpsc::unbounded_channel::(); let _join_handle = thread::spawn(move || { - let mut ts_runtime = js_runtime(performance, cache); + let mut ts_runtime = js_runtime(performance, cache, specifier_map_); let runtime = create_basic_runtime(); runtime.block_on(async { @@ -152,7 +172,10 @@ impl TsServer { }) }); - Self(tx) + Self { + sender: tx, + specifier_map, + } } pub async fn get_diagnostics( @@ -162,7 +185,16 @@ impl TsServer { token: CancellationToken, ) -> Result>, AnyError> { let req = RequestMethod::GetDiagnostics(specifiers); - self.request_with_cancellation(snapshot, req, token).await + let diagnostics_map_ = self.request_with_cancellation::>>(snapshot, req, token).await?; + let mut diagnostics_map = HashMap::new(); + for (mut specifier, mut diagnostics) in diagnostics_map_ { + specifier = self.specifier_map.normalize(&specifier)?.to_string(); + for diagnostic in &mut diagnostics { + normalize_diagnostic(diagnostic, &self.specifier_map)?; + } + diagnostics_map.insert(specifier, diagnostics); + } + Ok(diagnostics_map) } pub async fn find_references( @@ -175,10 +207,19 @@ impl TsServer { specifier, position, }; - self.request(snapshot, req).await.map_err(|err| { - log::error!("Unable to get references from TypeScript: {}", err); - LspError::internal_error() - }) + self + .request::>>(snapshot, req) + .await + .and_then(|mut symbols| { + for symbol in symbols.iter_mut().flatten() { + symbol.normalize(&self.specifier_map)?; + } + Ok(symbols) + }) + .map_err(|err| { + log::error!("Unable to get references from TypeScript: {}", err); + LspError::internal_error() + }) } pub async fn get_navigation_tree( @@ -244,7 +285,16 @@ impl TsServer { format_code_settings, preferences, )); - match self.request(snapshot, req).await { + let result = self + .request::>(snapshot, req) + .await + .and_then(|mut actions| { + for action in &mut actions { + action.normalize(&self.specifier_map)?; + } + Ok(actions) + }); + match result { Ok(items) => items, Err(err) => { // sometimes tsc reports errors when retrieving code actions @@ -293,10 +343,17 @@ impl TsServer { format_code_settings, preferences, )); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Unable to get combined fix from TypeScript: {}", err); - LspError::internal_error() - }) + self + .request::(snapshot, req) + .await + .and_then(|mut actions| { + actions.normalize(&self.specifier_map)?; + Ok(actions) + }) + .map_err(|err| { + log::error!("Unable to get combined fix from TypeScript: {}", err); + LspError::internal_error() + }) } #[allow(clippy::too_many_arguments)] @@ -321,10 +378,17 @@ impl TsServer { action_name, preferences, )); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::(snapshot, req) + .await + .and_then(|mut info| { + info.normalize(&self.specifier_map)?; + Ok(info) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn get_edits_for_file_rename( @@ -341,10 +405,19 @@ impl TsServer { format_code_settings, user_preferences, )); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>(snapshot, req) + .await + .and_then(|mut changes| { + for changes in &mut changes { + changes.normalize(&self.specifier_map)?; + } + Ok(changes) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn get_document_highlights( @@ -372,10 +445,19 @@ impl TsServer { position: u32, ) -> Result, LspError> { let req = RequestMethod::GetDefinition((specifier, position)); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Unable to get definition from TypeScript: {}", err); - LspError::internal_error() - }) + self + .request::>(snapshot, req) + .await + .and_then(|mut info| { + if let Some(info) = &mut info { + info.normalize(&self.specifier_map)?; + } + Ok(info) + }) + .map_err(|err| { + log::error!("Unable to get definition from TypeScript: {}", err); + LspError::internal_error() + }) } pub async fn get_type_definition( @@ -388,10 +470,19 @@ impl TsServer { specifier, position, }; - self.request(snapshot, req).await.map_err(|err| { - log::error!("Unable to get type definition from TypeScript: {}", err); - LspError::internal_error() - }) + self + .request::>>(snapshot, req) + .await + .and_then(|mut infos| { + for info in infos.iter_mut().flatten() { + info.normalize(&self.specifier_map)?; + } + Ok(infos) + }) + .map_err(|err| { + log::error!("Unable to get type definition from TypeScript: {}", err); + LspError::internal_error() + }) } pub async fn get_completions( @@ -423,7 +514,15 @@ impl TsServer { args: GetCompletionDetailsArgs, ) -> Result, AnyError> { let req = RequestMethod::GetCompletionDetails(args); - self.request(snapshot, req).await + self + .request::>(snapshot, req) + .await + .and_then(|mut details| { + if let Some(details) = &mut details { + details.normalize(&self.specifier_map)?; + } + Ok(details) + }) } pub async fn get_implementations( @@ -433,10 +532,19 @@ impl TsServer { position: u32, ) -> Result>, LspError> { let req = RequestMethod::GetImplementation((specifier, position)); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>>(snapshot, req) + .await + .and_then(|mut locations| { + for location in locations.iter_mut().flatten() { + location.normalize(&self.specifier_map)?; + } + Ok(locations) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn get_outlining_spans( @@ -459,10 +567,19 @@ impl TsServer { ) -> Result, LspError> { let req = RequestMethod::ProvideCallHierarchyIncomingCalls((specifier, position)); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>(snapshot, req) + .await + .and_then(|mut calls| { + for call in &mut calls { + call.normalize(&self.specifier_map)?; + } + Ok(calls) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn provide_call_hierarchy_outgoing_calls( @@ -473,10 +590,19 @@ impl TsServer { ) -> Result, LspError> { let req = RequestMethod::ProvideCallHierarchyOutgoingCalls((specifier, position)); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>(snapshot, req) + .await + .and_then(|mut calls| { + for call in &mut calls { + call.normalize(&self.specifier_map)?; + } + Ok(calls) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn prepare_call_hierarchy( @@ -486,10 +612,27 @@ impl TsServer { position: u32, ) -> Result>, LspError> { let req = RequestMethod::PrepareCallHierarchy((specifier, position)); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>>(snapshot, req) + .await + .and_then(|mut items| { + match &mut items { + Some(OneOrMany::One(item)) => { + item.normalize(&self.specifier_map)?; + } + Some(OneOrMany::Many(items)) => { + for item in items { + item.normalize(&self.specifier_map)?; + } + } + None => {} + } + Ok(items) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn find_rename_locations( @@ -505,10 +648,19 @@ impl TsServer { find_in_comments: false, provide_prefix_and_suffix_text_for_rename: false, }; - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed to request to tsserver {}", err); - LspError::invalid_request() - }) + self + .request::>>(snapshot, req) + .await + .and_then(|mut locations| { + for location in locations.iter_mut().flatten() { + location.normalize(&self.specifier_map)?; + } + Ok(locations) + }) + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) } pub async fn get_smart_selection_range( @@ -565,10 +717,19 @@ impl TsServer { args: GetNavigateToItemsArgs, ) -> Result, LspError> { let req = RequestMethod::GetNavigateToItems(args); - self.request(snapshot, req).await.map_err(|err| { - log::error!("Failed request to tsserver: {}", err); - LspError::invalid_request() - }) + self + .request::>(snapshot, req) + .await + .and_then(|mut items| { + for items in &mut items { + items.normalize(&self.specifier_map)?; + } + Ok(items) + }) + .map_err(|err| { + log::error!("Failed request to tsserver: {}", err); + LspError::invalid_request() + }) } pub async fn provide_inlay_hints( @@ -619,7 +780,7 @@ impl TsServer { R: de::DeserializeOwned, { let (tx, rx) = oneshot::channel::>(); - if self.0.send((req, snapshot, tx, token)).is_err() { + if self.sender.send((req, snapshot, tx, token)).is_err() { return Err(anyhow!("failed to send request to tsc thread")); } let value = rx.await??; @@ -1280,6 +1441,16 @@ pub struct DocumentSpan { original_context_span: Option, } +impl DocumentSpan { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.file_name = specifier_map.normalize(&self.file_name)?.to_string(); + Ok(()) + } +} + impl DocumentSpan { pub fn to_link( &self, @@ -1377,6 +1548,16 @@ pub struct NavigateToItem { // container_kind: ScriptElementKind, } +impl NavigateToItem { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.file_name = specifier_map.normalize(&self.file_name)?.to_string(); + Ok(()) + } +} + impl NavigateToItem { pub fn to_symbol_information( &self, @@ -1629,6 +1810,14 @@ pub struct ImplementationLocation { } impl ImplementationLocation { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.document_span.normalize(specifier_map)?; + Ok(()) + } + pub fn to_location( &self, line_index: Arc, @@ -1667,6 +1856,16 @@ pub struct RenameLocation { // suffix_text: Option, } +impl RenameLocation { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.document_span.normalize(specifier_map)?; + Ok(()) + } +} + pub struct RenameLocations { pub locations: Vec, } @@ -1744,7 +1943,7 @@ pub struct HighlightSpan { kind: HighlightSpanKind, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct DefinitionInfo { // kind: ScriptElementKind, @@ -1755,6 +1954,16 @@ pub struct DefinitionInfo { pub document_span: DocumentSpan, } +impl DefinitionInfo { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.document_span.normalize(specifier_map)?; + Ok(()) + } +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct DefinitionInfoAndBoundSpan { @@ -1763,6 +1972,16 @@ pub struct DefinitionInfoAndBoundSpan { } impl DefinitionInfoAndBoundSpan { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for definition in self.definitions.iter_mut().flatten() { + definition.normalize(specifier_map)?; + } + Ok(()) + } + pub async fn to_definition( &self, line_index: Arc, @@ -1849,6 +2068,14 @@ pub struct FileTextChanges { } impl FileTextChanges { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.file_name = specifier_map.normalize(&self.file_name)?.to_string(); + Ok(()) + } + pub fn to_text_document_edit( &self, language_server: &language_server::Inner, @@ -2119,7 +2346,7 @@ pub fn file_text_changes_to_workspace_edit( })) } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct RefactorEditInfo { edits: Vec, @@ -2128,6 +2355,16 @@ pub struct RefactorEditInfo { } impl RefactorEditInfo { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for changes in &mut self.edits { + changes.normalize(specifier_map)?; + } + Ok(()) + } + pub async fn to_workspace_edit( &self, language_server: &language_server::Inner, @@ -2145,6 +2382,18 @@ pub struct CodeAction { commands: Option>, } +impl CodeAction { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for changes in &mut self.changes { + changes.normalize(specifier_map)?; + } + Ok(()) + } +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub struct CodeFixAction { @@ -2166,6 +2415,18 @@ pub struct CodeFixAction { pub fix_all_description: Option, } +impl CodeFixAction { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for changes in &mut self.changes { + changes.normalize(specifier_map)?; + } + Ok(()) + } +} + #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CombinedCodeActions { @@ -2173,21 +2434,56 @@ pub struct CombinedCodeActions { pub commands: Option>, } -#[derive(Debug, Deserialize)] +impl CombinedCodeActions { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for changes in &mut self.changes { + changes.normalize(specifier_map)?; + } + Ok(()) + } +} + +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ReferencedSymbol { pub definition: ReferencedSymbolDefinitionInfo, pub references: Vec, } -#[derive(Debug, Deserialize)] +impl ReferencedSymbol { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.definition.normalize(specifier_map)?; + for reference in &mut self.references { + reference.normalize(specifier_map)?; + } + Ok(()) + } +} + +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ReferencedSymbolDefinitionInfo { #[serde(flatten)] pub definition_info: DefinitionInfo, } -#[derive(Debug, Deserialize)] +impl ReferencedSymbolDefinitionInfo { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.definition_info.normalize(specifier_map)?; + Ok(()) + } +} + +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ReferencedSymbolEntry { #[serde(default)] @@ -2196,7 +2492,17 @@ pub struct ReferencedSymbolEntry { pub entry: ReferenceEntry, } -#[derive(Debug, Deserialize)] +impl ReferencedSymbolEntry { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.entry.normalize(specifier_map)?; + Ok(()) + } +} + +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ReferenceEntry { // is_write_access: bool, @@ -2205,6 +2511,16 @@ pub struct ReferenceEntry { pub document_span: DocumentSpan, } +impl ReferenceEntry { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.document_span.normalize(specifier_map)?; + Ok(()) + } +} + impl ReferenceEntry { pub fn to_location( &self, @@ -2238,6 +2554,14 @@ pub struct CallHierarchyItem { } impl CallHierarchyItem { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.file = specifier_map.normalize(&self.file)?.to_string(); + Ok(()) + } + pub fn try_resolve_call_hierarchy_item( &self, language_server: &language_server::Inner, @@ -2337,6 +2661,14 @@ pub struct CallHierarchyIncomingCall { } impl CallHierarchyIncomingCall { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.from.normalize(specifier_map)?; + Ok(()) + } + pub fn try_resolve_call_hierarchy_incoming_call( &self, language_server: &language_server::Inner, @@ -2369,6 +2701,14 @@ pub struct CallHierarchyOutgoingCall { } impl CallHierarchyOutgoingCall { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + self.to.normalize(specifier_map)?; + Ok(()) + } + pub fn try_resolve_call_hierarchy_outgoing_call( &self, line_index: Arc, @@ -2524,6 +2864,16 @@ pub struct CompletionEntryDetails { } impl CompletionEntryDetails { + pub fn normalize( + &mut self, + specifier_map: &TscSpecifierMap, + ) -> Result<(), AnyError> { + for action in self.code_actions.iter_mut().flatten() { + action.normalize(specifier_map)?; + } + Ok(()) + } + pub fn as_completion_item( &self, original_item: &lsp::CompletionItem, @@ -3164,41 +3514,30 @@ struct Response { data: Value, } -struct State { - last_id: usize, - performance: Arc, - response: Option, - state_snapshot: Arc, - normalized_specifiers: HashMap, - denormalized_specifiers: HashMap, - token: CancellationToken, +#[derive(Debug, Default)] +pub struct TscSpecifierMap { + normalized_specifiers: DashMap, + denormalized_specifiers: DashMap, } -impl State { - fn new( - state_snapshot: Arc, - performance: Arc, - ) -> Self { - Self { - last_id: 1, - performance, - response: None, - state_snapshot, - normalized_specifiers: HashMap::default(), - denormalized_specifiers: HashMap::default(), - token: Default::default(), - } - } - +impl TscSpecifierMap { /// Convert the specifier to one compatible with tsc. Cache the resulting /// mapping in case it needs to be reversed. - fn denormalize_specifier(&mut self, specifier: &ModuleSpecifier) -> String { + // TODO(nayeemrmn): Factor in out-of-band media type here. + pub fn denormalize(&self, specifier: &ModuleSpecifier) -> String { let original = specifier; if let Some(specifier) = self.denormalized_specifiers.get(original) { return specifier.to_string(); } let mut specifier = original.to_string(); - let media_type = MediaType::from_specifier(original); + let media_type = if original.scheme() == "deno-notebook-cell" { + if let Some(s) = cell_to_file_specifier(original) { + specifier = s.to_string(); + } + MediaType::TypeScript + } else { + MediaType::from_specifier(original) + }; // If the URL-inferred media type doesn't correspond to tsc's path-inferred // media type, force it to be the same by appending an extension. if MediaType::from_path(Path::new(specifier.as_str())) != media_type { @@ -3214,8 +3553,8 @@ impl State { /// Convert the specifier from one compatible with tsc. Cache the resulting /// mapping in case it needs to be reversed. - fn normalize_specifier>( - &mut self, + pub fn normalize>( + &self, specifier: S, ) -> Result { let original = specifier.as_ref(); @@ -3234,6 +3573,32 @@ impl State { } Ok(specifier) } +} + +struct State { + last_id: usize, + performance: Arc, + response: Option, + state_snapshot: Arc, + specifier_map: Arc, + token: CancellationToken, +} + +impl State { + fn new( + state_snapshot: Arc, + specifier_map: Arc, + performance: Arc, + ) -> Self { + Self { + last_id: 1, + performance, + response: None, + state_snapshot, + specifier_map, + token: Default::default(), + } + } fn get_asset_or_document( &self, @@ -3316,7 +3681,7 @@ fn op_load( ) -> Result, AnyError> { let state = state.borrow_mut::(); let mark = state.performance.mark("op_load", Some(&args)); - let specifier = state.normalize_specifier(args.specifier)?; + let specifier = state.specifier_map.normalize(args.specifier)?; let asset_or_document = state.get_asset_or_document(&specifier); state.performance.measure(mark); Ok(asset_or_document.map(|doc| LoadResponse { @@ -3334,7 +3699,7 @@ fn op_resolve( ) -> Result>, AnyError> { let state = state.borrow_mut::(); let mark = state.performance.mark("op_resolve", Some(&args)); - let referrer = state.normalize_specifier(&args.base)?; + let referrer = state.specifier_map.normalize(&args.base)?; let result = match state.get_asset_or_document(&referrer) { Some(referrer_doc) => { let resolved = state.state_snapshot.documents.resolve( @@ -3348,7 +3713,7 @@ fn op_resolve( .map(|o| { o.map(|(s, mt)| { ( - state.denormalize_specifier(&s), + state.specifier_map.denormalize(&s), mt.as_ts_extension().to_string(), ) }) @@ -3419,6 +3784,12 @@ fn op_script_names(state: &mut OpState) -> Vec { } result + .into_iter() + .map(|s| match ModuleSpecifier::parse(&s) { + Ok(s) => state.specifier_map.denormalize(&s), + Err(_) => s, + }) + .collect() } #[derive(Debug, Deserialize, Serialize)] @@ -3436,7 +3807,7 @@ fn op_script_version( let state = state.borrow_mut::(); // this op is very "noisy" and measuring its performance is not useful, so we // don't measure it uniquely anymore. - let specifier = state.normalize_specifier(args.specifier)?; + let specifier = state.specifier_map.normalize(args.specifier)?; Ok(state.script_version(&specifier)) } @@ -3446,9 +3817,10 @@ fn op_script_version( fn js_runtime( performance: Arc, cache: Arc, + specifier_map: Arc, ) -> JsRuntime { JsRuntime::new(RuntimeOptions { - extensions: vec![deno_tsc::init_ops(performance, cache)], + extensions: vec![deno_tsc::init_ops(performance, cache, specifier_map)], startup_snapshot: Some(tsc::compiler_snapshot()), ..Default::default() }) @@ -3467,6 +3839,7 @@ deno_core::extension!(deno_tsc, options = { performance: Arc, cache: Arc, + specifier_map: Arc, }, state = |state, options| { state.put(State::new( @@ -3478,6 +3851,7 @@ deno_core::extension!(deno_tsc, maybe_import_map: None, npm: None, }), + options.specifier_map, options.performance, )); }, @@ -3969,7 +4343,7 @@ impl RequestMethod { json!({ "id": id, "method": "findRenameLocations", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, "findInStrings": find_in_strings, "findInComments": find_in_comments, @@ -3988,7 +4362,7 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getApplicableRefactors", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "range": { "pos": span.start, "end": span.start + span.length }, "preferences": preferences, "kind": kind, @@ -4003,7 +4377,7 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getEditsForRefactor", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "formatCodeSettings": format_code_settings, "range": { "pos": span.start, "end": span.start + span.length}, "refactorName": refactor_name, @@ -4018,8 +4392,8 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getEditsForFileRename", - "oldSpecifier": state.denormalize_specifier(old_specifier), - "newSpecifier": state.denormalize_specifier(new_specifier), + "oldSpecifier": state.specifier_map.denormalize(old_specifier), + "newSpecifier": state.specifier_map.denormalize(new_specifier), "formatCodeSettings": format_code_settings, "preferences": preferences, }), @@ -4033,7 +4407,7 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getCodeFixes", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "startPosition": start_pos, "endPosition": end_pos, "errorCodes": error_codes, @@ -4048,16 +4422,24 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getCombinedCodeFix", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "fixId": fix_id, "formatCodeSettings": format_code_settings, "preferences": preferences, }), - RequestMethod::GetCompletionDetails(args) => json!({ - "id": id, - "method": "getCompletionDetails", - "args": args - }), + RequestMethod::GetCompletionDetails(args) => { + let mut args = json!(args); + let specifier = + args.as_object_mut().unwrap().get_mut("specifier").unwrap(); + if let Ok(s) = ModuleSpecifier::parse(specifier.as_str().unwrap()) { + *specifier = json!(state.specifier_map.denormalize(&s)); + } + json!({ + "id": id, + "method": "getCompletionDetails", + "args": args + }) + } RequestMethod::GetCompletions(( specifier, position, @@ -4067,7 +4449,7 @@ impl RequestMethod { json!({ "id": id, "method": "getCompletions", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, "preferences": preferences, "formatCodeSettings": format_code_settings, @@ -4076,13 +4458,13 @@ impl RequestMethod { RequestMethod::GetDefinition((specifier, position)) => json!({ "id": id, "method": "getDefinition", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, }), RequestMethod::GetDiagnostics(specifiers) => json!({ "id": id, "method": "getDiagnostics", - "specifiers": specifiers.iter().map(|s| state.denormalize_specifier(s)).collect::>(), + "specifiers": specifiers.iter().map(|s| state.specifier_map.denormalize(s)).collect::>(), }), RequestMethod::GetDocumentHighlights(( specifier, @@ -4091,22 +4473,22 @@ impl RequestMethod { )) => json!({ "id": id, "method": "getDocumentHighlights", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, - "filesToSearch": files_to_search, + "filesToSearch": files_to_search.iter().map(|s| state.specifier_map.denormalize(s)).collect::>(), }), RequestMethod::GetEncodedSemanticClassifications((specifier, span)) => { json!({ "id": id, "method": "getEncodedSemanticClassifications", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "span": span, }) } RequestMethod::GetImplementation((specifier, position)) => json!({ "id": id, "method": "getImplementation", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, }), RequestMethod::GetNavigateToItems(GetNavigateToItemsArgs { @@ -4123,17 +4505,17 @@ impl RequestMethod { RequestMethod::GetNavigationTree(specifier) => json!({ "id": id, "method": "getNavigationTree", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), }), RequestMethod::GetOutliningSpans(specifier) => json!({ "id": id, "method": "getOutliningSpans", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), }), RequestMethod::GetQuickInfo((specifier, position)) => json!({ "id": id, "method": "getQuickInfo", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, }), RequestMethod::FindReferences { @@ -4142,14 +4524,14 @@ impl RequestMethod { } => json!({ "id": id, "method": "findReferences", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, }), RequestMethod::GetSignatureHelpItems((specifier, position, options)) => { json!({ "id": id, "method": "getSignatureHelpItems", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position, "options": options, }) @@ -4158,7 +4540,7 @@ impl RequestMethod { json!({ "id": id, "method": "getSmartSelectionRange", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position }) } @@ -4172,14 +4554,14 @@ impl RequestMethod { } => json!({ "id": id, "method": "getTypeDefinition", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position }), RequestMethod::PrepareCallHierarchy((specifier, position)) => { json!({ "id": id, "method": "prepareCallHierarchy", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position }) } @@ -4190,7 +4572,7 @@ impl RequestMethod { json!({ "id": id, "method": "provideCallHierarchyIncomingCalls", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position }) } @@ -4201,7 +4583,7 @@ impl RequestMethod { json!({ "id": id, "method": "provideCallHierarchyOutgoingCalls", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "position": position }) } @@ -4209,7 +4591,7 @@ impl RequestMethod { json!({ "id": id, "method": "provideInlayHints", - "specifier": state.denormalize_specifier(specifier), + "specifier": state.specifier_map.denormalize(specifier), "span": span, "preferences": preferences, }) @@ -4316,7 +4698,7 @@ mod tests { let cache = Arc::new(GlobalHttpCache::new(location.clone(), RealDenoCacheEnv)); let state_snapshot = Arc::new(mock_state_snapshot(sources, &location)); - let mut runtime = js_runtime(Default::default(), cache); + let mut runtime = js_runtime(Default::default(), cache, Default::default()); start(&mut runtime, debug).unwrap(); let ts_config = TsConfig::new(config); assert_eq!( diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index ca4b77f4d2..11cb099949 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -7734,6 +7734,49 @@ fn lsp_diagnostics_refresh_dependents() { assert_eq!(client.queue_len(), 0); } +#[test] +fn lsp_jupyter_diagnostics() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": "deno-notebook-cell:/a/file.ts#abc", + "languageId": "typescript", + "version": 1, + "text": "Deno.readTextFileSync(1234);", + }, + })); + assert_eq!( + json!(diagnostics.all_messages()), + json!([ + { + "uri": "deno-notebook-cell:/a/file.ts#abc", + "diagnostics": [ + { + "range": { + "start": { + "line": 0, + "character": 22, + }, + "end": { + "line": 0, + "character": 26, + }, + }, + "severity": 1, + "code": 2345, + "source": "deno-ts", + "message": "Argument of type 'number' is not assignable to parameter of type 'string | URL'.", + }, + ], + "version": 1, + }, + ]) + ); + client.shutdown(); +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct PerformanceAverage { @@ -9504,7 +9547,7 @@ fn lsp_data_urls_with_jsx_compiler_option() { "end": { "line": 1, "character": 1 } } }, { - "uri": "deno:/5c42b5916c4a3fb55be33fdb0c3b1f438639420592d150fca1b6dc043c1df3d9/data_url.ts", + "uri": "deno:/ed0224c51f7e2a845dfc0941ed6959675e5e3e3d2a39b127f0ff569c1ffda8d8/data_url.ts", "range": { "start": { "line": 0, "character": 7 }, "end": {"line": 0, "character": 14 },