From 9bdab6fb6b93eb43b1930f40987fa4997287f9c8 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 12 Aug 2024 15:45:27 +0100 Subject: [PATCH] fix(lsp): directly use file referrer when loading document (#24997) --- cli/lsp/completions.rs | 4 ++-- cli/lsp/diagnostics.rs | 13 ++++++------- cli/lsp/documents.rs | 9 ++++----- cli/lsp/language_server.rs | 7 ++++--- cli/lsp/tsc.rs | 28 +++++++++++----------------- 5 files changed, 27 insertions(+), 34 deletions(-) diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index 88900ceefc..7e0705e99d 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -858,8 +858,8 @@ mod tests { .global() .set(&specifier, HashMap::default(), source.as_bytes()) .expect("could not cache file"); - let document = - documents.get_or_load(&specifier, &temp_dir.uri().join("$").unwrap()); + let document = documents + .get_or_load(&specifier, Some(&temp_dir.uri().join("$").unwrap())); assert!(document.is_some(), "source could not be setup"); } documents diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 69f3b11455..a8faee1a85 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1367,21 +1367,20 @@ fn diagnose_resolution( let mut diagnostics = vec![]; match resolution { Resolution::Ok(resolved) => { + let file_referrer = referrer_doc.file_referrer(); let specifier = &resolved.specifier; - let managed_npm_resolver = snapshot - .resolver - .maybe_managed_npm_resolver(referrer_doc.file_referrer()); + let managed_npm_resolver = + snapshot.resolver.maybe_managed_npm_resolver(file_referrer); for (_, headers) in snapshot .resolver - .redirect_chain_headers(specifier, referrer_doc.file_referrer()) + .redirect_chain_headers(specifier, file_referrer) { if let Some(message) = headers.get("x-deno-warning") { diagnostics.push(DenoDiagnostic::DenoWarn(message.clone())); } } - if let Some(doc) = snapshot - .documents - .get_or_load(specifier, referrer_doc.specifier()) + if let Some(doc) = + snapshot.documents.get_or_load(specifier, file_referrer) { if let Some(headers) = doc.maybe_headers() { if let Some(message) = headers.get("x-deno-warning") { diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 57e48fbc3a..e91cfe0ac2 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1180,11 +1180,10 @@ impl Documents { pub fn get_or_load( &self, specifier: &ModuleSpecifier, - referrer: &ModuleSpecifier, + file_referrer: Option<&ModuleSpecifier>, ) -> Option> { - let file_referrer = self.get_file_referrer(referrer); let specifier = - self.resolve_document_specifier(specifier, file_referrer.as_deref())?; + self.resolve_document_specifier(specifier, file_referrer)?; if let Some(document) = self.open_docs.get(&specifier) { Some(document.clone()) } else { @@ -1193,7 +1192,7 @@ impl Documents { &self.resolver, &self.config, &self.cache, - file_referrer.as_deref(), + file_referrer, ) } } @@ -1464,7 +1463,7 @@ impl Documents { specifier = s; media_type = Some(mt); } - let Some(doc) = self.get_or_load(&specifier, referrer) else { + let Some(doc) = self.get_or_load(&specifier, file_referrer) else { let media_type = media_type.unwrap_or_else(|| MediaType::from_specifier(&specifier)); return Some((specifier, media_type)); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 186c941985..4bbc7036fd 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1333,8 +1333,9 @@ impl Inner { { return Ok(None); } - let document = - file_referrer.and_then(|r| self.documents.get_or_load(&specifier, &r)); + let document = self + .documents + .get_or_load(&specifier, file_referrer.as_ref()); let Some(document) = document else { return Ok(None); }; @@ -1448,7 +1449,7 @@ impl Inner { { let dep_doc = dep .get_code() - .and_then(|s| self.documents.get_or_load(s, &specifier)); + .and_then(|s| self.documents.get_or_load(s, file_referrer)); let dep_maybe_types_dependency = dep_doc.as_ref().map(|d| d.maybe_types_dependency()); let value = match (dep.maybe_code.is_none(), dep.maybe_type.is_none(), &dep_maybe_types_dependency) { diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 7dfb68f530..3ac25507cd 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2043,12 +2043,10 @@ impl DocumentSpan { let target_asset_or_doc = language_server.get_maybe_asset_or_document(&target_specifier)?; let target_line_index = target_asset_or_doc.line_index(); - let file_referrer = language_server - .documents - .get_file_referrer(&target_specifier); + let file_referrer = target_asset_or_doc.file_referrer(); let target_uri = language_server .url_map - .normalize_specifier(&target_specifier, file_referrer.as_deref()) + .normalize_specifier(&target_specifier, file_referrer) .ok()?; let (target_range, target_selection_range) = if let Some(context_span) = &self.context_span { @@ -2092,10 +2090,10 @@ impl DocumentSpan { language_server.get_maybe_asset_or_document(&specifier)?; let line_index = asset_or_doc.line_index(); let range = self.text_span.to_range(line_index); - let file_referrer = language_server.documents.get_file_referrer(&specifier); + let file_referrer = asset_or_doc.file_referrer(); let mut target = language_server .url_map - .normalize_specifier(&specifier, file_referrer.as_deref()) + .normalize_specifier(&specifier, file_referrer) .ok()? .into_url(); target.set_fragment(Some(&format!( @@ -2153,10 +2151,10 @@ impl NavigateToItem { let asset_or_doc = language_server.get_asset_or_document(&specifier).ok()?; let line_index = asset_or_doc.line_index(); - let file_referrer = language_server.documents.get_file_referrer(&specifier); + let file_referrer = asset_or_doc.file_referrer(); let uri = language_server .url_map - .normalize_specifier(&specifier, file_referrer.as_deref()) + .normalize_specifier(&specifier, file_referrer) .ok()?; let range = self.text_span.to_range(line_index); let location = lsp::Location { @@ -4232,14 +4230,10 @@ impl State { } fn get_document(&self, specifier: &ModuleSpecifier) -> Option> { - if let Some(scope) = &self.last_scope { - self.state_snapshot.documents.get_or_load(specifier, scope) - } else { - self - .state_snapshot - .documents - .get_or_load(specifier, &ModuleSpecifier::parse("file:///").unwrap()) - } + self + .state_snapshot + .documents + .get_or_load(specifier, self.last_scope.as_ref()) } fn get_asset_or_document( @@ -4561,7 +4555,7 @@ fn op_script_names(state: &mut OpState) -> ScriptNames { specifier, doc.file_referrer(), )?; - let types_doc = documents.get_or_load(&types, specifier)?; + let types_doc = documents.get_or_load(&types, doc.file_referrer())?; Some(types_doc.specifier().clone()) })(); // If there is a types dep, use that as the root instead. But if the doc