From 1ad754b4123009e01dbecb3b880e7f0545e46c2f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 14 Feb 2024 22:48:39 +0000 Subject: [PATCH] feat(lsp): jsr support with cache probing (#22418) --- cli/lsp/documents.rs | 15 ++- cli/lsp/jsr_resolver.rs | 117 ++++++++++++------ cli/lsp/language_server.rs | 22 +--- tests/integration/lsp_tests.rs | 53 +++++++- .../jsr/registry/@denotest/add/0.2.0/mod.ts | 4 + .../registry/@denotest/add/0.2.0_meta.json | 8 ++ .../jsr/registry/@denotest/add/meta.json | 3 +- 7 files changed, 161 insertions(+), 61 deletions(-) create mode 100644 tests/testdata/jsr/registry/@denotest/add/0.2.0/mod.ts create mode 100644 tests/testdata/jsr/registry/@denotest/add/0.2.0_meta.json diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 97ee91801b..c58a392d53 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -931,7 +931,10 @@ impl Documents { bare_node_builtins_enabled: false, sloppy_imports_resolver: None, })), - jsr_resolver: Default::default(), + jsr_resolver: Arc::new(JsrResolver::from_cache_and_lockfile( + cache.clone(), + None, + )), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, redirect_resolver: Arc::new(RedirectResolver::new(cache)), @@ -1332,6 +1335,16 @@ impl Documents { Ok(()) } + pub fn refresh_jsr_resolver( + &mut self, + lockfile: Option>>, + ) { + self.jsr_resolver = Arc::new(JsrResolver::from_cache_and_lockfile( + self.cache.clone(), + lockfile, + )); + } + pub fn update_config(&mut self, options: UpdateDocumentConfigOptions) { #[allow(clippy::too_many_arguments)] fn calculate_resolver_config_hash( diff --git a/cli/lsp/jsr_resolver.rs b/cli/lsp/jsr_resolver.rs index 207f681de4..8243bb0f23 100644 --- a/cli/lsp/jsr_resolver.rs +++ b/cli/lsp/jsr_resolver.rs @@ -1,26 +1,28 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::args::jsr_url; +use dashmap::DashMap; use deno_cache_dir::HttpCache; use deno_core::parking_lot::Mutex; use deno_core::serde_json; -use deno_core::serde_json::json; use deno_core::ModuleSpecifier; +use deno_graph::packages::JsrPackageInfo; use deno_graph::packages::JsrPackageVersionInfo; use deno_lockfile::Lockfile; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use std::borrow::Cow; -use std::collections::HashMap; use std::sync::Arc; -#[derive(Debug, Default)] +#[derive(Debug)] pub struct JsrResolver { - nv_by_req: HashMap, + nv_by_req: DashMap>, /// The `module_graph` field of the version infos should be forcibly absent. /// It can be large and we don't want to store it. - info_by_nv: HashMap, + info_by_nv: DashMap>, + info_by_name: DashMap>, + cache: Arc, } impl JsrResolver { @@ -28,8 +30,7 @@ impl JsrResolver { cache: Arc, lockfile: Option>>, ) -> Self { - let mut nv_by_req = HashMap::new(); - let mut info_by_nv = HashMap::new(); + let nv_by_req = DashMap::new(); if let Some(lockfile) = lockfile { for (req_url, nv_url) in &lockfile.lock().content.packages.specifiers { let Some(req) = req_url.strip_prefix("jsr:") else { @@ -44,40 +45,14 @@ impl JsrResolver { let Ok(nv) = PackageNv::from_str(nv) else { continue; }; - nv_by_req.insert(req, nv); + nv_by_req.insert(req, Some(nv)); } } - for nv in nv_by_req.values() { - if info_by_nv.contains_key(nv) { - continue; - } - let Ok(meta_url) = - jsr_url().join(&format!("{}/{}_meta.json", &nv.name, &nv.version)) - else { - continue; - }; - let Ok(meta_cache_item_key) = cache.cache_item_key(&meta_url) else { - continue; - }; - let Ok(Some(meta_bytes)) = cache.read_file_bytes(&meta_cache_item_key) - else { - continue; - }; - // This is a roundabout way of deserializing `JsrPackageVersionInfo`, - // because we only want the `exports` field and `module_graph` is large. - let Ok(info) = serde_json::from_slice::(&meta_bytes) - else { - continue; - }; - let info = JsrPackageVersionInfo { - exports: json!(info.as_object().and_then(|o| o.get("exports"))), - module_graph: None, - }; - info_by_nv.insert(nv.clone(), info); - } Self { nv_by_req, - info_by_nv, + info_by_nv: Default::default(), + info_by_name: Default::default(), + cache: cache.clone(), } } @@ -86,8 +61,43 @@ impl JsrResolver { specifier: &ModuleSpecifier, ) -> Option { let req_ref = JsrPackageReqReference::from_str(specifier.as_str()).ok()?; - let nv = self.nv_by_req.get(req_ref.req())?; - let info = self.info_by_nv.get(nv)?; + let req = req_ref.req().clone(); + let maybe_nv = self.nv_by_req.entry(req.clone()).or_insert_with(|| { + let name = req.name.clone(); + let maybe_package_info = self + .info_by_name + .entry(name.clone()) + .or_insert_with(|| read_cached_package_info(&name, &self.cache)); + let package_info = maybe_package_info.as_ref()?; + // Find the first matching version of the package which is cached. + let version = package_info + .versions + .keys() + .find(|v| { + if req.version_req.tag().is_some() || !req.version_req.matches(v) { + return false; + } + let nv = PackageNv { + name: name.clone(), + version: (*v).clone(), + }; + self + .info_by_nv + .entry(nv.clone()) + .or_insert_with(|| { + read_cached_package_version_info(&nv, &self.cache) + }) + .is_some() + }) + .cloned()?; + Some(PackageNv { name, version }) + }); + let nv = maybe_nv.as_ref()?; + let maybe_info = self + .info_by_nv + .entry(nv.clone()) + .or_insert_with(|| read_cached_package_version_info(nv, &self.cache)); + let info = maybe_info.as_ref()?; let path = info.export(&normalize_export_name(req_ref.sub_path()))?; jsr_url() .join(&format!("{}/{}/{}", &nv.name, &nv.version, &path)) @@ -95,6 +105,35 @@ impl JsrResolver { } } +fn read_cached_package_info( + name: &str, + cache: &Arc, +) -> Option { + let meta_url = jsr_url().join(&format!("{}/meta.json", name)).ok()?; + let meta_cache_item_key = cache.cache_item_key(&meta_url).ok()?; + let meta_bytes = cache.read_file_bytes(&meta_cache_item_key).ok()??; + serde_json::from_slice::(&meta_bytes).ok() +} + +fn read_cached_package_version_info( + nv: &PackageNv, + cache: &Arc, +) -> Option { + let meta_url = jsr_url() + .join(&format!("{}/{}_meta.json", &nv.name, &nv.version)) + .ok()?; + let meta_cache_item_key = cache.cache_item_key(&meta_url).ok()?; + let meta_bytes = cache.read_file_bytes(&meta_cache_item_key).ok()??; + // This is a roundabout way of deserializing `JsrPackageVersionInfo`, + // because we only want the `exports` field and `module_graph` is large. + let mut info = + serde_json::from_slice::(&meta_bytes).ok()?; + Some(JsrPackageVersionInfo { + exports: info.as_object_mut()?.remove("exports")?, + module_graph: None, + }) +} + // TODO(nayeemrmn): This is duplicated from a private function in deno_graph // 0.65.1. Make it public or cleanup otherwise. fn normalize_export_name(sub_path: Option<&str>) -> Cow { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index e775790fec..7aa4fdc993 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -362,23 +362,11 @@ impl LanguageServer { .client .show_message(MessageType::WARNING, err); } - let mut lockfile_content_changed = false; - if let Some(lockfile) = self.0.read().await.config.maybe_lockfile() { - let lockfile = lockfile.lock(); - let path = lockfile.filename.clone(); - if let Ok(new_lockfile) = Lockfile::new(path, false) { - lockfile_content_changed = FastInsecureHasher::hash(&*lockfile) - != FastInsecureHasher::hash(new_lockfile); - } else { - lockfile_content_changed = true; - } - } - if lockfile_content_changed { - // TODO(nayeemrmn): Remove this branch when the documents config no - // longer depends on the lockfile for JSR resolution. - self.0.write().await.refresh_documents_config().await; - } else { - self.0.write().await.refresh_npm_specifiers().await; + { + let mut inner = self.0.write().await; + let lockfile = inner.config.maybe_lockfile().cloned(); + inner.documents.refresh_jsr_resolver(lockfile); + inner.refresh_npm_specifiers().await; } // now refresh the data in a read self.0.read().await.post_cache(result.mark).await; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 7d1022176e..749af95c49 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -4664,9 +4664,6 @@ fn lsp_code_actions_deno_cache_jsr() { .use_temp_cwd() .build(); let temp_dir = context.temp_dir(); - // TODO(nayeemrmn): JSR resolution currently depends on a lockfile being - // created on cache. Remove this when that's not the case. - temp_dir.write("deno.json", "{}"); let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client.did_open(json!({ @@ -4763,6 +4760,56 @@ fn lsp_code_actions_deno_cache_jsr() { client.shutdown(); } +#[test] +fn lsp_jsr_lockfile() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write("./deno.json", json!({}).to_string()); + temp_dir.write( + "./deno.lock", + json!({ + "version": "3", + "packages": { + "specifiers": { + // This is an old version of the package which exports `sum()` instead + // of `add()`. + "jsr:@denotest/add": "jsr:@denotest/add@0.2.0", + }, + }, + }) + .to_string(), + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": r#" + import { add } from "jsr:@denotest/add"; + console.log(add(1, 2)); + "#, + }, + })); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [ + [], + temp_dir.uri().join("file.ts").unwrap(), + ], + }), + ); + let diagnostics = client.read_diagnostics(); + assert_eq!(json!(diagnostics.all()), json!([])); + client.shutdown(); +} + #[test] fn lsp_code_actions_deno_cache_npm() { let context = TestContextBuilder::new().use_temp_cwd().build(); diff --git a/tests/testdata/jsr/registry/@denotest/add/0.2.0/mod.ts b/tests/testdata/jsr/registry/@denotest/add/0.2.0/mod.ts new file mode 100644 index 0000000000..864e8dd321 --- /dev/null +++ b/tests/testdata/jsr/registry/@denotest/add/0.2.0/mod.ts @@ -0,0 +1,4 @@ +// This is renamed to `add()` in 1.0.0. +export function sum(a: number, b: number): number { + return a + b; +} diff --git a/tests/testdata/jsr/registry/@denotest/add/0.2.0_meta.json b/tests/testdata/jsr/registry/@denotest/add/0.2.0_meta.json new file mode 100644 index 0000000000..6eebe21985 --- /dev/null +++ b/tests/testdata/jsr/registry/@denotest/add/0.2.0_meta.json @@ -0,0 +1,8 @@ +{ + "exports": { + ".": "./mod.ts" + }, + "moduleGraph1": { + "/mod.ts": {} + } +} diff --git a/tests/testdata/jsr/registry/@denotest/add/meta.json b/tests/testdata/jsr/registry/@denotest/add/meta.json index 02601e4d0d..2f4daa8441 100644 --- a/tests/testdata/jsr/registry/@denotest/add/meta.json +++ b/tests/testdata/jsr/registry/@denotest/add/meta.json @@ -1,5 +1,6 @@ { "versions": { - "1.0.0": {} + "1.0.0": {}, + "0.2.0": {} } }