diff options
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | cli/Cargo.toml | 2 | ||||
| -rw-r--r-- | cli/lsp/diagnostics.rs | 19 | ||||
| -rw-r--r-- | cli/lsp/documents.rs | 17 | ||||
| -rw-r--r-- | cli/lsp/jsr_resolver.rs | 120 | ||||
| -rw-r--r-- | cli/lsp/language_server.rs | 24 | ||||
| -rw-r--r-- | cli/lsp/mod.rs | 1 | ||||
| -rw-r--r-- | test_util/src/lsp.rs | 2 | ||||
| -rw-r--r-- | tests/integration/lsp_tests.rs | 91 |
9 files changed, 265 insertions, 15 deletions
diff --git a/Cargo.lock b/Cargo.lock index c52c62bf4..972cefbd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1404,9 +1404,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.65.1" +version = "0.65.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06fadc0cd21f54794ab22e852d6c92487a65f0dd70b40eadbf3a8e249c760223" +checksum = "1f5d44da4195e0908b78bbf5db30dbc7defc8173f8d3034462863beccb368df4" dependencies = [ "anyhow", "async-trait", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 6ec311db6..083cb4aa8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -65,7 +65,7 @@ deno_config = "=0.9.2" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_doc = { version = "=0.103.0", features = ["html"] } deno_emit = "=0.36.0" -deno_graph = "=0.65.1" +deno_graph = "=0.65.2" deno_lint = { version = "=0.56.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.16.0" diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 0c193040b..0b3faa551 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -42,6 +42,7 @@ use deno_lint::rules::LintRule; use deno_runtime::deno_fs; use deno_runtime::deno_node; use deno_runtime::tokio_util::create_basic_runtime; +use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use log::error; @@ -329,6 +330,8 @@ impl DiagnosticsState { if diagnostic.code == Some(lsp::NumberOrString::String("no-cache".to_string())) || diagnostic.code + == Some(lsp::NumberOrString::String("no-cache-jsr".to_string())) + || diagnostic.code == Some(lsp::NumberOrString::String("no-cache-npm".to_string())) { no_cache_diagnostics.push(diagnostic.clone()); @@ -968,6 +971,8 @@ pub enum DenoDiagnostic { NoAttributeType, /// A remote module was not found in the cache. NoCache(ModuleSpecifier), + /// A remote jsr package reference was not found in the cache. + NoCacheJsr(PackageReq, ModuleSpecifier), /// A remote npm package reference was not found in the cache. NoCacheNpm(PackageReq, ModuleSpecifier), /// A local module was not found on the local file system. @@ -994,6 +999,7 @@ impl DenoDiagnostic { Self::InvalidAttributeType(_) => "invalid-attribute-type", Self::NoAttributeType => "no-attribute-type", Self::NoCache(_) => "no-cache", + Self::NoCacheJsr(_, _) => "no-cache-jsr", Self::NoCacheNpm(_, _) => "no-cache-npm", Self::NoLocal(_) => "no-local", Self::Redirect { .. } => "redirect", @@ -1072,7 +1078,7 @@ impl DenoDiagnostic { }), ..Default::default() }, - "no-cache" | "no-cache-npm" => { + "no-cache" | "no-cache-jsr" | "no-cache-npm" => { let data = diagnostic .data .clone() @@ -1188,6 +1194,7 @@ impl DenoDiagnostic { match code.as_str() { "import-map-remap" | "no-cache" + | "no-cache-jsr" | "no-cache-npm" | "no-attribute-type" | "redirect" @@ -1226,6 +1233,7 @@ impl DenoDiagnostic { Self::InvalidAttributeType(assert_type) => (lsp::DiagnosticSeverity::ERROR, format!("The module is a JSON module and expected an attribute type of \"json\". Instead got \"{assert_type}\"."), None), Self::NoAttributeType => (lsp::DiagnosticSeverity::ERROR, "The module is a JSON module and not being imported with an import attribute. Consider adding `with { type: \"json\" }` to the import statement.".to_string(), None), Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: {specifier}"), Some(json!({ "specifier": specifier }))), + Self::NoCacheJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing jsr package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoCacheNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoLocal(specifier) => { let sloppy_resolution = SloppyImportsResolver::resolve_with_fs(&deno_fs::RealFs, specifier); @@ -1310,7 +1318,7 @@ fn diagnose_resolution( // If the module was redirected, we want to issue an informational // diagnostic that indicates this. This then allows us to issue a code // action to replace the specifier with the final redirected one. - if doc_specifier != specifier { + if specifier.scheme() != "jsr" && doc_specifier != specifier { diagnostics.push(DenoDiagnostic::Redirect { from: specifier.clone(), to: doc_specifier.clone(), @@ -1332,8 +1340,11 @@ fn diagnose_resolution( None => diagnostics.push(DenoDiagnostic::NoAttributeType), } } - } else if specifier.scheme() == "jsr" { - // TODO(nayeemrmn): Check if jsr specifiers are cached. + } else if let Ok(pkg_ref) = + JsrPackageReqReference::from_specifier(specifier) + { + let req = pkg_ref.into_inner().req; + diagnostics.push(DenoDiagnostic::NoCacheJsr(req, specifier.clone())); } else if let Ok(pkg_ref) = NpmPackageReqReference::from_specifier(specifier) { diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 94d0e979b..97ee91801 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -2,6 +2,7 @@ use super::cache::calculate_fs_version; use super::cache::calculate_fs_version_at_path; +use super::jsr_resolver::JsrResolver; use super::language_server::StateNpmSnapshot; use super::text::LineIndex; use super::tsc; @@ -38,6 +39,7 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::GraphImport; use deno_graph::Resolution; +use deno_lockfile::Lockfile; use deno_runtime::deno_fs::RealFs; use deno_runtime::deno_node; use deno_runtime::deno_node::NodeResolution; @@ -855,6 +857,7 @@ pub struct UpdateDocumentConfigOptions<'a> { pub maybe_import_map: Option<Arc<import_map::ImportMap>>, pub maybe_config_file: Option<&'a ConfigFile>, pub maybe_package_json: Option<&'a PackageJson>, + pub maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, pub node_resolver: Option<Arc<NodeResolver>>, pub npm_resolver: Option<Arc<dyn CliNpmResolver>>, } @@ -893,6 +896,7 @@ pub struct Documents { /// A resolver that takes into account currently loaded import map and JSX /// settings. resolver: Arc<CliGraphResolver>, + jsr_resolver: Arc<JsrResolver>, /// The npm package requirements found in npm specifiers. npm_specifier_reqs: Arc<Vec<PackageReq>>, /// Gets if any document had a node: specifier such that a @types/node package @@ -927,6 +931,7 @@ impl Documents { bare_node_builtins_enabled: false, sloppy_imports_resolver: None, })), + jsr_resolver: Default::default(), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, redirect_resolver: Arc::new(RedirectResolver::new(cache)), @@ -1084,7 +1089,11 @@ impl Documents { .into_owned(), ) } else { - self.redirect_resolver.resolve(specifier) + let specifier = match self.jsr_resolver.jsr_to_registry_url(specifier) { + Some(url) => Cow::Owned(url), + None => Cow::Borrowed(specifier), + }; + self.redirect_resolver.resolve(&specifier) } } @@ -1425,6 +1434,10 @@ impl Documents { // specifier for free. sloppy_imports_resolver: None, })); + self.jsr_resolver = Arc::new(JsrResolver::from_cache_and_lockfile( + self.cache.clone(), + options.maybe_lockfile, + )); self.redirect_resolver = Arc::new(RedirectResolver::new(self.cache.clone())); self.imports = Arc::new( @@ -2252,6 +2265,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, + maybe_lockfile: None, node_resolver: None, npm_resolver: None, }); @@ -2295,6 +2309,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, + maybe_lockfile: None, node_resolver: None, npm_resolver: None, }); diff --git a/cli/lsp/jsr_resolver.rs b/cli/lsp/jsr_resolver.rs new file mode 100644 index 000000000..4ea3a35bb --- /dev/null +++ b/cli/lsp/jsr_resolver.rs @@ -0,0 +1,120 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use crate::args::deno_registry_url; +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::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)] +pub struct JsrResolver { + nv_by_req: HashMap<PackageReq, PackageNv>, + /// 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<PackageNv, JsrPackageVersionInfo>, +} + +impl JsrResolver { + pub fn from_cache_and_lockfile( + cache: Arc<dyn HttpCache>, + lockfile: Option<Arc<Mutex<Lockfile>>>, + ) -> Self { + let mut nv_by_req = HashMap::new(); + let mut info_by_nv = HashMap::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 { + continue; + }; + let Some(nv) = nv_url.strip_prefix("jsr:") else { + continue; + }; + let Ok(req) = PackageReq::from_str(req) else { + continue; + }; + let Ok(nv) = PackageNv::from_str(nv) else { + continue; + }; + nv_by_req.insert(req, nv); + } + } + for nv in nv_by_req.values() { + if info_by_nv.contains_key(nv) { + continue; + } + let Ok(meta_url) = deno_registry_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::<serde_json::Value>(&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, + } + } + + pub fn jsr_to_registry_url( + &self, + specifier: &ModuleSpecifier, + ) -> Option<ModuleSpecifier> { + 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 path = info.export(&normalize_export_name(req_ref.sub_path()))?; + deno_registry_url() + .join(&format!("{}/{}/{}", &nv.name, &nv.version, &path)) + .ok() + } +} + +// 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<str> { + let Some(sub_path) = sub_path else { + return Cow::Borrowed("."); + }; + if sub_path.is_empty() || matches!(sub_path, "/" | ".") { + Cow::Borrowed(".") + } else { + let sub_path = if sub_path.starts_with('/') { + Cow::Owned(format!(".{}", sub_path)) + } else if !sub_path.starts_with("./") { + Cow::Owned(format!("./{}", sub_path)) + } else { + Cow::Borrowed(sub_path) + }; + if let Some(prefix) = sub_path.strip_suffix('/') { + Cow::Owned(prefix.to_string()) + } else { + sub_path + } + } +} diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 573fb1eb4..e775790fe 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -362,9 +362,24 @@ impl LanguageServer { .client .show_message(MessageType::WARNING, err); } - // do npm resolution in a write—we should have everything - // cached by this point anyway - self.0.write().await.refresh_npm_specifiers().await; + 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; + } // now refresh the data in a read self.0.read().await.post_cache(result.mark).await; } @@ -1330,6 +1345,7 @@ impl Inner { maybe_import_map: self.maybe_import_map.clone(), maybe_config_file: self.config.maybe_config_file(), maybe_package_json: self.maybe_package_json.as_ref(), + maybe_lockfile: self.config.maybe_lockfile().cloned(), node_resolver: self.npm.node_resolver.clone(), npm_resolver: self.npm.resolver.clone(), }); @@ -2010,6 +2026,8 @@ impl Inner { if diagnostic.code == Some(NumberOrString::String("no-cache".to_string())) || diagnostic.code + == Some(NumberOrString::String("no-cache-jsr".to_string())) + || diagnostic.code == Some(NumberOrString::String("no-cache-npm".to_string())) { includes_no_cache = true; diff --git a/cli/lsp/mod.rs b/cli/lsp/mod.rs index c941a02a8..ef6462524 100644 --- a/cli/lsp/mod.rs +++ b/cli/lsp/mod.rs @@ -21,6 +21,7 @@ mod completions; mod config; mod diagnostics; mod documents; +mod jsr_resolver; pub mod language_server; mod logging; mod lsp_custom; diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs index 9b63b79cd..532cf1482 100644 --- a/test_util/src/lsp.rs +++ b/test_util/src/lsp.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::deno_exe_path; +use crate::jsr_registry_url; use crate::npm_registry_url; use crate::PathRef; @@ -523,6 +524,7 @@ impl LspClientBuilder { command .env("DENO_DIR", deno_dir.path()) .env("NPM_CONFIG_REGISTRY", npm_registry_url()) + .env("DENO_REGISTRY_URL", jsr_registry_url()) // turn on diagnostic synchronization communication .env( "DENO_DONT_USE_INTERNAL_LSP_DIAGNOSTIC_SYNC_FLAG", diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index c9abae241..7d1022176 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -4658,12 +4658,15 @@ fn lsp_code_actions_deno_cache() { } #[test] -fn lsp_jsr_uncached() { +fn lsp_code_actions_deno_cache_jsr() { let context = TestContextBuilder::new() .use_http_server() .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!({ @@ -4671,11 +4674,91 @@ fn lsp_jsr_uncached() { "uri": temp_dir.uri().join("file.ts").unwrap(), "languageId": "typescript", "version": 1, - "text": r#"import "jsr:@foo/bar";"#, + "text": r#" + import { add } from "jsr:@denotest/add@1"; + console.log(add(1, 2)); + "#, }, })); - // TODO(nayeemrmn): This should check if the jsr dep is cached and give a - // diagnostic. + assert_eq!( + json!(diagnostics.messages_with_source("deno")), + json!({ + "uri": temp_dir.uri().join("file.ts").unwrap(), + "diagnostics": [{ + "range": { + "start": { "line": 1, "character": 28 }, + "end": { "line": 1, "character": 49 }, + }, + "severity": 1, + "code": "no-cache-jsr", + "source": "deno", + "message": "Uncached or missing jsr package: @denotest/add@1", + "data": { "specifier": "jsr:@denotest/add@1" }, + }], + "version": 1, + }) + ); + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { "uri": temp_dir.uri().join("file.ts").unwrap() }, + "range": { + "start": { "line": 1, "character": 28 }, + "end": { "line": 1, "character": 49 }, + }, + "context": { + "diagnostics": [{ + "range": { + "start": { "line": 1, "character": 28 }, + "end": { "line": 1, "character": 49 }, + }, + "severity": 1, + "code": "no-cache-jsr", + "source": "deno", + "message": "Uncached or missing jsr package: @denotest/add@1", + "data": { "specifier": "jsr:@denotest/add@1" }, + }], + "only": ["quickfix"], + } + }), + ); + assert_eq!( + res, + json!([{ + "title": "Cache \"jsr:@denotest/add@1\" and its dependencies.", + "kind": "quickfix", + "diagnostics": [{ + "range": { + "start": { "line": 1, "character": 28 }, + "end": { "line": 1, "character": 49 }, + }, + "severity": 1, + "code": "no-cache-jsr", + "source": "deno", + "message": "Uncached or missing jsr package: @denotest/add@1", + "data": { "specifier": "jsr:@denotest/add@1" }, + }], + "command": { + "title": "", + "command": "deno.cache", + "arguments": [ + ["jsr:@denotest/add@1"], + temp_dir.uri().join("file.ts").unwrap(), + ], + }, + }]) + ); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [ + ["jsr:@denotest/add@1"], + temp_dir.uri().join("file.ts").unwrap(), + ], + }), + ); + let diagnostics = client.read_diagnostics(); assert_eq!(json!(diagnostics.all()), json!([])); client.shutdown(); } |