refactor(lsp): store the LspUrlMap's state inside a mutex (#13416)

This commit is contained in:
David Sherret 2022-01-19 11:38:40 -05:00 committed by GitHub
parent 82b086752a
commit 1cece36fa5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 55 additions and 45 deletions

View file

@ -341,7 +341,7 @@ fn is_preferred(
/// for an LSP CodeAction. /// for an LSP CodeAction.
pub(crate) async fn ts_changes_to_edit( pub(crate) async fn ts_changes_to_edit(
changes: &[tsc::FileTextChanges], changes: &[tsc::FileTextChanges],
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<Option<lsp::WorkspaceEdit>, AnyError> { ) -> Result<Option<lsp::WorkspaceEdit>, AnyError> {
let mut text_document_edits = Vec::new(); let mut text_document_edits = Vec::new();
for change in changes { for change in changes {
@ -607,7 +607,7 @@ impl CodeActionCollection {
specifier: &ModuleSpecifier, specifier: &ModuleSpecifier,
action: &tsc::CodeFixAction, action: &tsc::CodeFixAction,
diagnostic: &lsp::Diagnostic, diagnostic: &lsp::Diagnostic,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<(), AnyError> { ) -> Result<(), AnyError> {
if action.commands.is_some() { if action.commands.is_some() {
// In theory, tsc can return actions that require "commands" to be applied // In theory, tsc can return actions that require "commands" to be applied

View file

@ -239,7 +239,7 @@ impl Visit for DenoTestCollector {
async fn resolve_implementation_code_lens( async fn resolve_implementation_code_lens(
code_lens: lsp::CodeLens, code_lens: lsp::CodeLens,
data: CodeLensData, data: CodeLensData,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<lsp::CodeLens, AnyError> { ) -> Result<lsp::CodeLens, AnyError> {
let asset_or_doc = let asset_or_doc =
language_server.get_cached_asset_or_document(&data.specifier)?; language_server.get_cached_asset_or_document(&data.specifier)?;
@ -308,7 +308,7 @@ async fn resolve_implementation_code_lens(
async fn resolve_references_code_lens( async fn resolve_references_code_lens(
code_lens: lsp::CodeLens, code_lens: lsp::CodeLens,
data: CodeLensData, data: CodeLensData,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<lsp::CodeLens, AnyError> { ) -> Result<lsp::CodeLens, AnyError> {
let asset_or_document = let asset_or_document =
language_server.get_cached_asset_or_document(&data.specifier)?; language_server.get_cached_asset_or_document(&data.specifier)?;
@ -378,7 +378,7 @@ async fn resolve_references_code_lens(
pub(crate) async fn resolve_code_lens( pub(crate) async fn resolve_code_lens(
code_lens: lsp::CodeLens, code_lens: lsp::CodeLens,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<lsp::CodeLens, AnyError> { ) -> Result<lsp::CodeLens, AnyError> {
let data: CodeLensData = let data: CodeLensData =
serde_json::from_value(code_lens.data.clone().unwrap())?; serde_json::from_value(code_lens.data.clone().unwrap())?;

View file

@ -398,6 +398,8 @@ impl Inner {
maybe_lint_config: self.maybe_lint_config.clone(), maybe_lint_config: self.maybe_lint_config.clone(),
maybe_fmt_config: self.maybe_fmt_config.clone(), maybe_fmt_config: self.maybe_fmt_config.clone(),
module_registries: self.module_registries.clone(), module_registries: self.module_registries.clone(),
// it's ok to get an Arc to the url map because the url_map is a cache
// that does not change once the mapping for a specifier is set
url_map: self.url_map.clone(), url_map: self.url_map.clone(),
})) }))
} }
@ -1147,7 +1149,7 @@ impl Inner {
} }
async fn code_action( async fn code_action(
&mut self, &self,
params: CodeActionParams, params: CodeActionParams,
) -> LspResult<Option<CodeActionResponse>> { ) -> LspResult<Option<CodeActionResponse>> {
let specifier = self.url_map.normalize_url(&params.text_document.uri); let specifier = self.url_map.normalize_url(&params.text_document.uri);
@ -1321,7 +1323,7 @@ impl Inner {
} }
async fn code_action_resolve( async fn code_action_resolve(
&mut self, &self,
params: CodeAction, params: CodeAction,
) -> LspResult<CodeAction> { ) -> LspResult<CodeAction> {
if params.kind.is_none() || params.data.is_none() { if params.kind.is_none() || params.data.is_none() {
@ -1460,7 +1462,7 @@ impl Inner {
} }
async fn code_lens_resolve( async fn code_lens_resolve(
&mut self, &self,
code_lens: CodeLens, code_lens: CodeLens,
) -> LspResult<CodeLens> { ) -> LspResult<CodeLens> {
let mark = self.performance.mark("code_lens_resolve", Some(&code_lens)); let mark = self.performance.mark("code_lens_resolve", Some(&code_lens));
@ -1481,7 +1483,7 @@ impl Inner {
} }
async fn document_highlight( async fn document_highlight(
&mut self, &self,
params: DocumentHighlightParams, params: DocumentHighlightParams,
) -> LspResult<Option<Vec<DocumentHighlight>>> { ) -> LspResult<Option<Vec<DocumentHighlight>>> {
let specifier = self let specifier = self
@ -1526,7 +1528,7 @@ impl Inner {
} }
async fn references( async fn references(
&mut self, &self,
params: ReferenceParams, params: ReferenceParams,
) -> LspResult<Option<Vec<Location>>> { ) -> LspResult<Option<Vec<Location>>> {
let specifier = self let specifier = self
@ -1581,7 +1583,7 @@ impl Inner {
} }
async fn goto_definition( async fn goto_definition(
&mut self, &self,
params: GotoDefinitionParams, params: GotoDefinitionParams,
) -> LspResult<Option<GotoDefinitionResponse>> { ) -> LspResult<Option<GotoDefinitionResponse>> {
let specifier = self let specifier = self
@ -1620,7 +1622,7 @@ impl Inner {
} }
async fn goto_type_definition( async fn goto_type_definition(
&mut self, &self,
params: GotoTypeDefinitionParams, params: GotoTypeDefinitionParams,
) -> LspResult<Option<GotoTypeDefinitionResponse>> { ) -> LspResult<Option<GotoTypeDefinitionResponse>> {
let specifier = self let specifier = self
@ -1668,7 +1670,7 @@ impl Inner {
} }
async fn completion( async fn completion(
&mut self, &self,
params: CompletionParams, params: CompletionParams,
) -> LspResult<Option<CompletionResponse>> { ) -> LspResult<Option<CompletionResponse>> {
let specifier = self let specifier = self
@ -1746,7 +1748,7 @@ impl Inner {
} }
async fn completion_resolve( async fn completion_resolve(
&mut self, &self,
params: CompletionItem, params: CompletionItem,
) -> LspResult<CompletionItem> { ) -> LspResult<CompletionItem> {
let mark = self.performance.mark("completion_resolve", Some(&params)); let mark = self.performance.mark("completion_resolve", Some(&params));
@ -1793,7 +1795,7 @@ impl Inner {
} }
async fn goto_implementation( async fn goto_implementation(
&mut self, &self,
params: GotoImplementationParams, params: GotoImplementationParams,
) -> LspResult<Option<GotoImplementationResponse>> { ) -> LspResult<Option<GotoImplementationResponse>> {
let specifier = self let specifier = self
@ -1841,7 +1843,7 @@ impl Inner {
} }
async fn folding_range( async fn folding_range(
&mut self, &self,
params: FoldingRangeParams, params: FoldingRangeParams,
) -> LspResult<Option<Vec<FoldingRange>>> { ) -> LspResult<Option<Vec<FoldingRange>>> {
let specifier = self.url_map.normalize_url(&params.text_document.uri); let specifier = self.url_map.normalize_url(&params.text_document.uri);
@ -1885,7 +1887,7 @@ impl Inner {
} }
async fn incoming_calls( async fn incoming_calls(
&mut self, &self,
params: CallHierarchyIncomingCallsParams, params: CallHierarchyIncomingCallsParams,
) -> LspResult<Option<Vec<CallHierarchyIncomingCall>>> { ) -> LspResult<Option<Vec<CallHierarchyIncomingCall>>> {
let specifier = self.url_map.normalize_url(&params.item.uri); let specifier = self.url_map.normalize_url(&params.item.uri);
@ -1934,7 +1936,7 @@ impl Inner {
} }
async fn outgoing_calls( async fn outgoing_calls(
&mut self, &self,
params: CallHierarchyOutgoingCallsParams, params: CallHierarchyOutgoingCallsParams,
) -> LspResult<Option<Vec<CallHierarchyOutgoingCall>>> { ) -> LspResult<Option<Vec<CallHierarchyOutgoingCall>>> {
let specifier = self.url_map.normalize_url(&params.item.uri); let specifier = self.url_map.normalize_url(&params.item.uri);
@ -1984,7 +1986,7 @@ impl Inner {
} }
async fn prepare_call_hierarchy( async fn prepare_call_hierarchy(
&mut self, &self,
params: CallHierarchyPrepareParams, params: CallHierarchyPrepareParams,
) -> LspResult<Option<Vec<CallHierarchyItem>>> { ) -> LspResult<Option<Vec<CallHierarchyItem>>> {
let specifier = self let specifier = self
@ -2058,7 +2060,7 @@ impl Inner {
} }
async fn rename( async fn rename(
&mut self, &self,
params: RenameParams, params: RenameParams,
) -> LspResult<Option<WorkspaceEdit>> { ) -> LspResult<Option<WorkspaceEdit>> {
let specifier = self let specifier = self

View file

@ -714,7 +714,7 @@ impl DocumentSpan {
pub(crate) async fn to_link( pub(crate) async fn to_link(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Option<lsp::LocationLink> { ) -> Option<lsp::LocationLink> {
let target_specifier = normalize_specifier(&self.file_name).ok()?; let target_specifier = normalize_specifier(&self.file_name).ok()?;
let target_asset_or_doc = language_server let target_asset_or_doc = language_server
@ -991,7 +991,7 @@ impl ImplementationLocation {
pub(crate) fn to_location( pub(crate) fn to_location(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> lsp::Location { ) -> lsp::Location {
let specifier = normalize_specifier(&self.document_span.file_name) let specifier = normalize_specifier(&self.document_span.file_name)
.unwrap_or_else(|_| ModuleSpecifier::parse("deno://invalid").unwrap()); .unwrap_or_else(|_| ModuleSpecifier::parse("deno://invalid").unwrap());
@ -1008,7 +1008,7 @@ impl ImplementationLocation {
pub(crate) async fn to_link( pub(crate) async fn to_link(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Option<lsp::LocationLink> { ) -> Option<lsp::LocationLink> {
self self
.document_span .document_span
@ -1035,7 +1035,7 @@ impl RenameLocations {
pub(crate) async fn into_workspace_edit( pub(crate) async fn into_workspace_edit(
self, self,
new_name: &str, new_name: &str,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Result<lsp::WorkspaceEdit, AnyError> { ) -> Result<lsp::WorkspaceEdit, AnyError> {
let mut text_document_edit_map: HashMap<Url, lsp::TextDocumentEdit> = let mut text_document_edit_map: HashMap<Url, lsp::TextDocumentEdit> =
HashMap::new(); HashMap::new();
@ -1126,7 +1126,7 @@ impl DefinitionInfoAndBoundSpan {
pub(crate) async fn to_definition( pub(crate) async fn to_definition(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> Option<lsp::GotoDefinitionResponse> { ) -> Option<lsp::GotoDefinitionResponse> {
if let Some(definitions) = &self.definitions { if let Some(definitions) = &self.definitions {
let mut location_links = Vec::<lsp::LocationLink>::new(); let mut location_links = Vec::<lsp::LocationLink>::new();
@ -1528,7 +1528,7 @@ impl ReferenceEntry {
pub(crate) fn to_location( pub(crate) fn to_location(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
) -> lsp::Location { ) -> lsp::Location {
let specifier = normalize_specifier(&self.document_span.file_name) let specifier = normalize_specifier(&self.document_span.file_name)
.unwrap_or_else(|_| INVALID_SPECIFIER.clone()); .unwrap_or_else(|_| INVALID_SPECIFIER.clone());
@ -1560,7 +1560,7 @@ pub struct CallHierarchyItem {
impl CallHierarchyItem { impl CallHierarchyItem {
pub(crate) async fn try_resolve_call_hierarchy_item( pub(crate) async fn try_resolve_call_hierarchy_item(
&self, &self,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
maybe_root_path: Option<&Path>, maybe_root_path: Option<&Path>,
) -> Option<lsp::CallHierarchyItem> { ) -> Option<lsp::CallHierarchyItem> {
let target_specifier = normalize_specifier(&self.file).ok()?; let target_specifier = normalize_specifier(&self.file).ok()?;
@ -1579,7 +1579,7 @@ impl CallHierarchyItem {
pub(crate) fn to_call_hierarchy_item( pub(crate) fn to_call_hierarchy_item(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
maybe_root_path: Option<&Path>, maybe_root_path: Option<&Path>,
) -> lsp::CallHierarchyItem { ) -> lsp::CallHierarchyItem {
let target_specifier = normalize_specifier(&self.file) let target_specifier = normalize_specifier(&self.file)
@ -1661,7 +1661,7 @@ pub struct CallHierarchyIncomingCall {
impl CallHierarchyIncomingCall { impl CallHierarchyIncomingCall {
pub(crate) async fn try_resolve_call_hierarchy_incoming_call( pub(crate) async fn try_resolve_call_hierarchy_incoming_call(
&self, &self,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
maybe_root_path: Option<&Path>, maybe_root_path: Option<&Path>,
) -> Option<lsp::CallHierarchyIncomingCall> { ) -> Option<lsp::CallHierarchyIncomingCall> {
let target_specifier = normalize_specifier(&self.from.file).ok()?; let target_specifier = normalize_specifier(&self.from.file).ok()?;
@ -1696,7 +1696,7 @@ impl CallHierarchyOutgoingCall {
pub(crate) async fn try_resolve_call_hierarchy_outgoing_call( pub(crate) async fn try_resolve_call_hierarchy_outgoing_call(
&self, &self,
line_index: Arc<LineIndex>, line_index: Arc<LineIndex>,
language_server: &mut language_server::Inner, language_server: &language_server::Inner,
maybe_root_path: Option<&Path>, maybe_root_path: Option<&Path>,
) -> Option<lsp::CallHierarchyOutgoingCall> { ) -> Option<lsp::CallHierarchyOutgoingCall> {
let target_specifier = normalize_specifier(&self.to.file).ok()?; let target_specifier = normalize_specifier(&self.to.file).ok()?;

View file

@ -6,11 +6,13 @@ use data_url::DataUrl;
use deno_ast::MediaType; use deno_ast::MediaType;
use deno_core::error::uri_error; use deno_core::error::uri_error;
use deno_core::error::AnyError; use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::url::Position; use deno_core::url::Position;
use deno_core::url::Url; use deno_core::url::Url;
use deno_core::ModuleSpecifier; use deno_core::ModuleSpecifier;
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc;
/// Used in situations where a default URL needs to be used where otherwise a /// Used in situations where a default URL needs to be used where otherwise a
/// panic is undesired. /// panic is undesired.
@ -57,16 +59,13 @@ fn hash_data_specifier(specifier: &ModuleSpecifier) -> String {
crate::checksum::gen(&[file_name_str.as_bytes()]) crate::checksum::gen(&[file_name_str.as_bytes()])
} }
/// A bi-directional map of URLs sent to the LSP client and internal module #[derive(Debug, Default)]
/// specifiers. We need to map internal specifiers into `deno:` schema URLs struct LspUrlMapInner {
/// to allow the Deno language server to manage these as virtual documents.
#[derive(Debug, Default, Clone)]
pub struct LspUrlMap {
specifier_to_url: HashMap<ModuleSpecifier, Url>, specifier_to_url: HashMap<ModuleSpecifier, Url>,
url_to_specifier: HashMap<Url, ModuleSpecifier>, url_to_specifier: HashMap<Url, ModuleSpecifier>,
} }
impl LspUrlMap { impl LspUrlMapInner {
fn put(&mut self, specifier: ModuleSpecifier, url: Url) { fn put(&mut self, specifier: ModuleSpecifier, url: Url) {
self.specifier_to_url.insert(specifier.clone(), url.clone()); self.specifier_to_url.insert(specifier.clone(), url.clone());
self.url_to_specifier.insert(url, specifier); self.url_to_specifier.insert(url, specifier);
@ -79,15 +78,24 @@ impl LspUrlMap {
fn get_specifier(&self, url: &Url) -> Option<&ModuleSpecifier> { fn get_specifier(&self, url: &Url) -> Option<&ModuleSpecifier> {
self.url_to_specifier.get(url) self.url_to_specifier.get(url)
} }
}
/// A bi-directional map of URLs sent to the LSP client and internal module
/// specifiers. We need to map internal specifiers into `deno:` schema URLs
/// to allow the Deno language server to manage these as virtual documents.
#[derive(Debug, Default, Clone)]
pub struct LspUrlMap(Arc<Mutex<LspUrlMapInner>>);
impl LspUrlMap {
/// Normalize a specifier that is used internally within Deno (or tsc) to a /// Normalize a specifier that is used internally within Deno (or tsc) to a
/// URL that can be handled as a "virtual" document by an LSP client. /// URL that can be handled as a "virtual" document by an LSP client.
pub fn normalize_specifier( pub fn normalize_specifier(
&mut self, &self,
specifier: &ModuleSpecifier, specifier: &ModuleSpecifier,
) -> Result<Url, AnyError> { ) -> Result<Url, AnyError> {
if let Some(url) = self.get_url(specifier) { let mut inner = self.0.lock();
Ok(url.clone()) if let Some(url) = inner.get_url(specifier).cloned() {
Ok(url)
} else { } else {
let url = if specifier.scheme() == "file" { let url = if specifier.scheme() == "file" {
specifier.clone() specifier.clone()
@ -123,7 +131,7 @@ impl LspUrlMap {
format!("deno:/{}", path) format!("deno:/{}", path)
}; };
let url = Url::parse(&specifier_str)?; let url = Url::parse(&specifier_str)?;
self.put(specifier.clone(), url.clone()); inner.put(specifier.clone(), url.clone());
url url
}; };
Ok(url) Ok(url)
@ -135,8 +143,8 @@ impl LspUrlMap {
/// where the client encodes a file URL differently than Rust does by default /// where the client encodes a file URL differently than Rust does by default
/// causing issues with string matching of URLs. /// causing issues with string matching of URLs.
pub fn normalize_url(&self, url: &Url) -> ModuleSpecifier { pub fn normalize_url(&self, url: &Url) -> ModuleSpecifier {
if let Some(specifier) = self.get_specifier(url) { if let Some(specifier) = self.0.lock().get_specifier(url).cloned() {
return specifier.clone(); return specifier;
} }
if url.scheme() == "file" { if url.scheme() == "file" {
if let Ok(path) = url.to_file_path() { if let Ok(path) = url.to_file_path() {
@ -164,7 +172,7 @@ mod tests {
#[test] #[test]
fn test_lsp_url_map() { fn test_lsp_url_map() {
let mut map = LspUrlMap::default(); let map = LspUrlMap::default();
let fixture = resolve_url("https://deno.land/x/pkg@1.0.0/mod.ts").unwrap(); let fixture = resolve_url("https://deno.land/x/pkg@1.0.0/mod.ts").unwrap();
let actual_url = map let actual_url = map
.normalize_specifier(&fixture) .normalize_specifier(&fixture)
@ -180,7 +188,7 @@ mod tests {
#[test] #[test]
fn test_lsp_url_map_complex_encoding() { fn test_lsp_url_map_complex_encoding() {
// Test fix for #9741 - not properly encoding certain URLs // Test fix for #9741 - not properly encoding certain URLs
let mut map = LspUrlMap::default(); let map = LspUrlMap::default();
let fixture = resolve_url("https://cdn.skypack.dev/-/postcss@v8.2.9-E4SktPp9c0AtxrJHp8iV/dist=es2020,mode=types/lib/postcss.d.ts").unwrap(); let fixture = resolve_url("https://cdn.skypack.dev/-/postcss@v8.2.9-E4SktPp9c0AtxrJHp8iV/dist=es2020,mode=types/lib/postcss.d.ts").unwrap();
let actual_url = map let actual_url = map
.normalize_specifier(&fixture) .normalize_specifier(&fixture)
@ -194,7 +202,7 @@ mod tests {
#[test] #[test]
fn test_lsp_url_map_data() { fn test_lsp_url_map_data() {
let mut map = LspUrlMap::default(); let map = LspUrlMap::default();
let fixture = resolve_url("data:application/typescript;base64,ZXhwb3J0IGNvbnN0IGEgPSAiYSI7CgpleHBvcnQgZW51bSBBIHsKICBBLAogIEIsCiAgQywKfQo=").unwrap(); let fixture = resolve_url("data:application/typescript;base64,ZXhwb3J0IGNvbnN0IGEgPSAiYSI7CgpleHBvcnQgZW51bSBBIHsKICBBLAogIEIsCiAgQywKfQo=").unwrap();
let actual_url = map let actual_url = map
.normalize_specifier(&fixture) .normalize_specifier(&fixture)