From bb37dfb5b79c0e5ae18d34f01313cb1f39d7a2dd Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 22 May 2023 21:28:36 -0400 Subject: [PATCH] feat(lsp): support lockfile and node_modules directory (#19203) This adds support for the lockfile and node_modules directory to the lsp. In the case of the node_modules directory, it is only enabled when explicitly opted into via `"nodeModulesDir": true` in the configuration file. This is to reduce the language server automatically modifying the node_modules directory when the user doesn't want it to. Closes #16510 Closes #16373 --- cli/args/config_file.rs | 61 ++-- cli/args/lockfile.rs | 20 +- cli/args/mod.rs | 22 +- cli/lsp/diagnostics.rs | 10 +- cli/lsp/documents.rs | 6 +- cli/lsp/language_server.rs | 444 +++++++++++++++++------ cli/npm/installer.rs | 6 + cli/npm/resolvers/mod.rs | 10 + cli/tests/integration/inspector_tests.rs | 10 +- cli/tests/integration/lsp_tests.rs | 144 ++++++++ test_util/src/assertions.rs | 10 + 11 files changed, 567 insertions(+), 176 deletions(-) diff --git a/cli/args/config_file.rs b/cli/args/config_file.rs index ed31a47a643f19..c2f02e5c17fc0a 100644 --- a/cli/args/config_file.rs +++ b/cli/args/config_file.rs @@ -790,11 +790,11 @@ impl ConfigFile { config_path.display() ) })?; - Self::from_specifier(&config_specifier) + Self::from_specifier(config_specifier) } - pub fn from_specifier(specifier: &ModuleSpecifier) -> Result { - let config_path = specifier_to_file_path(specifier)?; + pub fn from_specifier(specifier: ModuleSpecifier) -> Result { + let config_path = specifier_to_file_path(&specifier)?; let config_text = match std::fs::read_to_string(config_path) { Ok(text) => text, Err(err) => bail!( @@ -806,10 +806,7 @@ impl ConfigFile { Self::new(&config_text, specifier) } - pub fn new( - text: &str, - specifier: &ModuleSpecifier, - ) -> Result { + pub fn new(text: &str, specifier: ModuleSpecifier) -> Result { let jsonc = match jsonc_parser::parse_to_serde_value(text, &Default::default()) { Ok(None) => json!({}), @@ -830,10 +827,7 @@ impl ConfigFile { }; let json: ConfigFileJson = serde_json::from_value(jsonc)?; - Ok(Self { - specifier: specifier.to_owned(), - json, - }) + Ok(Self { specifier, json }) } /// Returns true if the configuration indicates that JavaScript should be @@ -1089,6 +1083,26 @@ impl ConfigFile { Ok(None) } } + + pub fn resolve_lockfile_path(&self) -> Result, AnyError> { + match self.to_lock_config()? { + Some(LockConfig::Bool(lock)) if !lock => Ok(None), + Some(LockConfig::PathBuf(lock)) => Ok(Some( + self + .specifier + .to_file_path() + .unwrap() + .parent() + .unwrap() + .join(lock), + )), + _ => { + let mut path = self.specifier.to_file_path().unwrap(); + path.set_file_name("deno.lock"); + Ok(Some(path)) + } + } + } } /// Represents the "default" type library that should be used when type @@ -1324,7 +1338,8 @@ mod tests { }"#; let config_dir = ModuleSpecifier::parse("file:///deno/").unwrap(); let config_specifier = config_dir.join("tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = + ConfigFile::new(config_text, config_specifier.clone()).unwrap(); let (options_value, ignored) = config_file.to_compiler_options().unwrap(); assert!(options_value.is_object()); let options = options_value.as_object().unwrap(); @@ -1405,7 +1420,7 @@ mod tests { }"#; let config_dir = ModuleSpecifier::parse("file:///deno/").unwrap(); let config_specifier = config_dir.join("tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let lint_files = unpack_object(config_file.to_lint_config(), "lint").files; assert_eq!( @@ -1446,7 +1461,7 @@ mod tests { }"#; let config_dir = ModuleSpecifier::parse("file:///deno/").unwrap(); let config_specifier = config_dir.join("tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let lint_include = unpack_object(config_file.to_lint_config(), "lint") .files @@ -1489,9 +1504,9 @@ mod tests { let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); let config_file_both = - ConfigFile::new(config_text_both, &config_specifier).unwrap(); + ConfigFile::new(config_text_both, config_specifier.clone()).unwrap(); let config_file_deprecated = - ConfigFile::new(config_text_deprecated, &config_specifier).unwrap(); + ConfigFile::new(config_text_deprecated, config_specifier).unwrap(); fn unpack_options(config_file: ConfigFile) -> FmtOptionsConfig { unpack_object(config_file.to_fmt_config(), "fmt").options @@ -1509,7 +1524,7 @@ mod tests { let config_text = ""; let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let (options_value, _) = config_file.to_compiler_options().unwrap(); assert!(options_value.is_object()); } @@ -1519,7 +1534,7 @@ mod tests { let config_text = r#"//{"foo":"bar"}"#; let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let (options_value, _) = config_file.to_compiler_options().unwrap(); assert!(options_value.is_object()); } @@ -1535,7 +1550,7 @@ mod tests { }"#; let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let (options_value, _) = config_file.to_compiler_options().unwrap(); assert!(options_value.is_object()); @@ -1561,7 +1576,7 @@ mod tests { }"#; let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let (options_value, _) = config_file.to_compiler_options().unwrap(); assert!(options_value.is_object()); @@ -1587,7 +1602,7 @@ mod tests { let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); // Emit error: Unable to parse config file JSON "" because of Unexpected token on line 1 column 6. - assert!(ConfigFile::new(config_text, &config_specifier).is_err()); + assert!(ConfigFile::new(config_text, config_specifier).is_err()); } #[test] @@ -1596,7 +1611,7 @@ mod tests { let config_specifier = ModuleSpecifier::parse("file:///deno/tsconfig.json").unwrap(); // Emit error: config file JSON "" should be an object - assert!(ConfigFile::new(config_text, &config_specifier).is_err()); + assert!(ConfigFile::new(config_text, config_specifier).is_err()); } #[test] @@ -1708,7 +1723,7 @@ mod tests { fn run_task_error_test(config_text: &str, expected_error: &str) { let config_dir = ModuleSpecifier::parse("file:///deno/").unwrap(); let config_specifier = config_dir.join("tsconfig.json").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); assert_eq!( config_file .resolve_tasks_config() diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index aa7e51fa1c314c..c169c7b002c152 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -17,7 +17,6 @@ use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; use deno_npm::NpmPackageId; use deno_semver::npm::NpmPackageReq; -use crate::args::config_file::LockConfig; use crate::args::ConfigFile; use crate::npm::CliNpmRegistryApi; use crate::Flags; @@ -45,22 +44,9 @@ pub fn discover( None => match maybe_config_file { Some(config_file) => { if config_file.specifier.scheme() == "file" { - match config_file.to_lock_config()? { - Some(LockConfig::Bool(lock)) if !lock => { - return Ok(None); - } - Some(LockConfig::PathBuf(lock)) => config_file - .specifier - .to_file_path() - .unwrap() - .parent() - .unwrap() - .join(lock), - _ => { - let mut path = config_file.specifier.to_file_path().unwrap(); - path.set_file_name("deno.lock"); - path - } + match config_file.resolve_lockfile_path()? { + Some(path) => path, + None => return Ok(None), } } else { return Ok(None); diff --git a/cli/args/mod.rs b/cli/args/mod.rs index fdef509839c060..5dd723eafe9a58 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -8,7 +8,7 @@ mod lockfile; pub mod package_json; pub use self::import_map::resolve_import_map_from_specifier; -use self::lockfile::snapshot_from_lockfile; +pub use self::lockfile::snapshot_from_lockfile; use self::package_json::PackageJsonDeps; use ::import_map::ImportMap; use deno_core::resolve_url_or_path; @@ -577,7 +577,7 @@ impl CliOptions { flags: Flags, initial_cwd: PathBuf, maybe_config_file: Option, - maybe_lockfile: Option, + maybe_lockfile: Option>>, maybe_package_json: Option, ) -> Result { if let Some(insecure_allowlist) = @@ -594,7 +594,6 @@ impl CliOptions { eprintln!("{}", colors::yellow(msg)); } - let maybe_lockfile = maybe_lockfile.map(|l| Arc::new(Mutex::new(l))); let maybe_node_modules_folder = resolve_local_node_modules_folder( &initial_cwd, &flags, @@ -647,7 +646,7 @@ impl CliOptions { flags, initial_cwd, maybe_config_file, - maybe_lock_file, + maybe_lock_file.map(|l| Arc::new(Mutex::new(l))), maybe_package_json, ) } @@ -1348,7 +1347,7 @@ pub fn resolve_no_prompt(flags: &Flags) -> bool { flags.no_prompt || has_flag_env_var("DENO_NO_PROMPT") } -fn has_flag_env_var(name: &str) -> bool { +pub fn has_flag_env_var(name: &str) -> bool { let value = env::var(name); matches!(value.as_ref().map(|s| s.as_str()), Ok("1")) } @@ -1375,7 +1374,7 @@ mod test { }"#; let config_specifier = ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let actual = resolve_import_map_specifier( None, Some(&config_file), @@ -1396,7 +1395,7 @@ mod test { }"#; let config_specifier = ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let actual = resolve_import_map_specifier( None, Some(&config_file), @@ -1419,7 +1418,7 @@ mod test { }"#; let config_specifier = ModuleSpecifier::parse("https://example.com/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let actual = resolve_import_map_specifier( None, Some(&config_file), @@ -1443,7 +1442,7 @@ mod test { let cwd = &std::env::current_dir().unwrap(); let config_specifier = ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let actual = resolve_import_map_specifier( Some("import-map.json"), Some(&config_file), @@ -1465,7 +1464,8 @@ mod test { }"#; let config_specifier = ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = + ConfigFile::new(config_text, config_specifier.clone()).unwrap(); let actual = resolve_import_map_specifier( None, Some(&config_file), @@ -1481,7 +1481,7 @@ mod test { let config_text = r#"{}"#; let config_specifier = ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap(); - let config_file = ConfigFile::new(config_text, &config_specifier).unwrap(); + let config_file = ConfigFile::new(config_text, config_specifier).unwrap(); let actual = resolve_import_map_specifier( None, Some(&config_file), diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 7b5a30a0eaa0c8..1a57ad03b88edd 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -908,10 +908,7 @@ fn diagnose_resolution( { if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { // show diagnostics for npm package references that aren't cached - if npm_resolver - .resolve_pkg_id_from_pkg_req(&pkg_ref.req) - .is_err() - { + if !npm_resolver.is_pkg_req_folder_cached(&pkg_ref.req) { diagnostics .push(DenoDiagnostic::NoCacheNpm(pkg_ref, specifier.clone())); } @@ -925,10 +922,7 @@ fn diagnose_resolution( // check that a @types/node package exists in the resolver let types_node_ref = NpmPackageReqReference::from_str("npm:@types/node").unwrap(); - if npm_resolver - .resolve_pkg_id_from_pkg_req(&types_node_ref.req) - .is_err() - { + if !npm_resolver.is_pkg_req_folder_cached(&types_node_ref.req) { diagnostics.push(DenoDiagnostic::NoCacheNpm( types_node_ref, ModuleSpecifier::parse("npm:@types/node").unwrap(), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 8fd9cdbb23a4bd..4bfb9342a1f78e 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1224,11 +1224,7 @@ impl Documents { ); let deps_provider = Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); - let deps_installer = Arc::new(PackageJsonDepsInstaller::new( - deps_provider.clone(), - options.npm_registry_api.clone(), - options.npm_resolution.clone(), - )); + let deps_installer = Arc::new(PackageJsonDepsInstaller::no_op()); self.resolver = Arc::new(CliGraphResolver::new( maybe_jsx_config, options.maybe_import_map, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 25d908a522f174..594c4adbc39c17 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -4,12 +4,15 @@ use deno_ast::MediaType; use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_core::resolve_url; use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_core::task::spawn; use deno_core::ModuleSpecifier; +use deno_lockfile::Lockfile; +use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; @@ -71,6 +74,7 @@ use super::urls::LspClientUrl; use crate::args::get_root_cert_store; use crate::args::package_json; use crate::args::resolve_import_map_from_specifier; +use crate::args::snapshot_from_lockfile; use crate::args::CaData; use crate::args::CacheSetting; use crate::args::CliOptions; @@ -80,6 +84,7 @@ use crate::args::FmtOptions; use crate::args::LintOptions; use crate::args::TsConfig; use crate::cache::DenoDir; +use crate::cache::FastInsecureHasher; use crate::cache::HttpCache; use crate::factory::CliFactory; use crate::file_fetcher::FileFetcher; @@ -93,6 +98,7 @@ use crate::npm::NpmCache; use crate::npm::NpmResolution; use crate::tools::fmt::format_file; use crate::tools::fmt::format_parsed_source; +use crate::util::fs::canonicalize_path_maybe_not_exists; use crate::util::fs::remove_dir_all_if_exists; use crate::util::path::specifier_to_file_path; use crate::util::progress_bar::ProgressBar; @@ -106,6 +112,71 @@ impl RootCertStoreProvider for LspRootCertStoreProvider { } } +#[derive(Debug)] +struct LspNpmServices { + /// When this hash changes, the services need updating + config_hash: LspNpmConfigHash, + /// Npm's registry api. + api: Arc, + /// Npm cache + cache: Arc, + /// Npm resolution that is stored in memory. + resolution: Arc, + /// Resolver for npm packages. + resolver: Arc, +} + +#[derive(Debug, PartialEq, Eq)] +struct LspNpmConfigHash(u64); + +impl LspNpmConfigHash { + pub fn from_inner(inner: &Inner) -> Self { + let mut hasher = FastInsecureHasher::new(); + hasher.write_hashable(&inner.maybe_node_modules_dir_path()); + hasher.write_hashable(&inner.maybe_cache_path); + hash_lockfile_with_hasher(&mut hasher, inner.maybe_lockfile()); + Self(hasher.finish()) + } +} + +fn hash_lockfile(maybe_lockfile: Option<&Arc>>) -> u64 { + let mut hasher = FastInsecureHasher::new(); + hash_lockfile_with_hasher(&mut hasher, maybe_lockfile); + hasher.finish() +} + +fn hash_lockfile_with_hasher( + hasher: &mut FastInsecureHasher, + maybe_lockfile: Option<&Arc>>, +) { + if let Some(lockfile) = &maybe_lockfile { + let lockfile = lockfile.lock(); + hasher.write_hashable(&*lockfile); + } +} + +fn resolve_lockfile_from_path( + lockfile_path: PathBuf, +) -> Option>> { + match Lockfile::new(lockfile_path, false) { + Ok(value) => Some(Arc::new(Mutex::new(value))), + Err(err) => { + lsp_warn!("Error loading lockfile: {:#}", err); + None + } + } +} + +/// Contains the config file and dependent information. +#[derive(Debug)] +struct LspConfigFileInfo { + config_file: ConfigFile, + /// An optional deno.lock file, which is resolved relative to the config file. + maybe_lockfile: Option>>, + /// The canonicalized node_modules directory, which is found relative to the config file. + maybe_node_modules_dir: Option, +} + #[derive(Debug, Clone)] pub struct LanguageServer(Arc>); @@ -146,8 +217,8 @@ pub struct Inner { /// options. maybe_cache_path: Option, /// An optional configuration file which has been specified in the client - /// options. - maybe_config_file: Option, + /// options along with some data that is computed after the config file is set. + maybe_config_file_info: Option, /// An optional import map which is used to resolve modules. maybe_import_map: Option>, /// The URL for the import map which is used to determine relative imports. @@ -160,14 +231,8 @@ pub struct Inner { lint_options: LintOptions, /// A lazily create "server" for handling test run requests. maybe_testing_server: Option, - /// Npm's registry api. - npm_api: Arc, - /// Npm cache - npm_cache: Arc, - /// Npm resolution that is stored in memory. - npm_resolution: Arc, - /// Resolver for npm packages. - npm_resolver: Arc, + /// Services used for dealing with npm related functionality. + npm: LspNpmServices, /// A collection of measurements which instrument that performance of the LSP. performance: Arc, /// A memoized version of fixable diagnostic codes retrieved from TypeScript. @@ -198,7 +263,8 @@ impl LanguageServer { .into_iter() .map(|d| (d.specifier().clone(), d)) .collect::>(); - let factory = CliFactory::from_cli_options(Arc::new(cli_options)); + let cli_options = Arc::new(cli_options); + let factory = CliFactory::from_cli_options(cli_options.clone()); let module_graph_builder = factory.module_graph_builder().await?; let mut inner_loader = module_graph_builder.create_graph_loader(); let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader { @@ -217,13 +283,23 @@ impl LanguageServer { check_js: false, }, )?; + + // Update the lockfile on the file system with anything new + // found after caching + if let Some(lockfile) = cli_options.maybe_lockfile() { + let lockfile = lockfile.lock(); + if let Err(err) = lockfile.write() { + lsp_warn!("Error writing lockfile: {}", err); + } + } + Ok(()) } match params.map(serde_json::from_value) { Some(Ok(params)) => { // do as much as possible in a read, then do a write outside - let maybe_cache_result = { + let maybe_prepare_cache_result = { let inner = self.0.read().await; // ensure dropped match inner.prepare_cache(params) { Ok(maybe_cache_result) => maybe_cache_result, @@ -238,7 +314,7 @@ impl LanguageServer { } } }; - if let Some(result) = maybe_cache_result { + if let Some(result) = maybe_prepare_cache_result { let cli_options = result.cli_options; let roots = result.roots; let open_docs = result.open_docs; @@ -420,28 +496,33 @@ impl LanguageServer { if ls.config.update_enabled_paths() { touched = true; } - - if touched { - ls.refresh_documents_config(); - ls.diagnostics_server.invalidate_all(); - ls.send_diagnostics_update(); - } } touched } } -fn create_lsp_structs( +fn resolve_node_modules_dir(config_file: &ConfigFile) -> Option { + // For the language server, require an explicit opt-in via the + // `nodeModulesDir: true` setting in the deno.json file. This is to + // reduce the chance of modifying someone's node_modules directory + // without them having asked us to do so. + if config_file.node_modules_dir() != Some(true) { + return None; + } + if config_file.specifier.scheme() != "file" { + return None; + } + let file_path = config_file.specifier.to_file_path().ok()?; + let node_modules_dir = file_path.parent()?.join("node_modules"); + canonicalize_path_maybe_not_exists(&node_modules_dir).ok() +} + +fn create_npm_api_and_cache( dir: &DenoDir, http_client: Arc, -) -> ( - Arc, - Arc, - Arc, - Arc, -) { - let registry_url = CliNpmRegistryApi::default_url(); - let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly); + registry_url: &ModuleSpecifier, + progress_bar: &ProgressBar, +) -> (Arc, Arc) { let npm_cache = Arc::new(NpmCache::new( dir.npm_folder_path(), // Use an "only" cache setting in order to make the @@ -458,25 +539,41 @@ fn create_lsp_structs( http_client, progress_bar.clone(), )); - let resolution = - Arc::new(NpmResolution::from_serialized(api.clone(), None, None)); + (api, npm_cache) +} + +fn create_npm_resolver_and_resolution( + registry_url: &ModuleSpecifier, + progress_bar: ProgressBar, + api: Arc, + npm_cache: Arc, + node_modules_dir_path: Option, + maybe_snapshot: Option, +) -> (Arc, Arc) { + let resolution = Arc::new(NpmResolution::from_serialized( + api, + maybe_snapshot, + // Don't provide the lockfile. We don't want these resolvers + // updating it. Only the cache request should update the lockfile. + None, + )); let fs = Arc::new(deno_fs::RealFs); let fs_resolver = create_npm_fs_resolver( fs.clone(), - npm_cache.clone(), + npm_cache, &progress_bar, registry_url.clone(), resolution.clone(), - None, + node_modules_dir_path, NpmSystemInfo::default(), ); ( - api, - npm_cache, Arc::new(CliNpmResolver::new( fs, resolution.clone(), fs_resolver, + // Don't provide the lockfile. We don't want these resolvers + // updating it. Only the cache request should update the lockfile. None, )), resolution, @@ -505,8 +602,23 @@ impl Inner { ts_server.clone(), ); let assets = Assets::new(ts_server.clone()); - let (npm_api, npm_cache, npm_resolver, npm_resolution) = - create_lsp_structs(&dir, http_client.clone()); + let registry_url = CliNpmRegistryApi::default_url(); + let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly); + + let (npm_api, npm_cache) = create_npm_api_and_cache( + &dir, + http_client.clone(), + registry_url, + &progress_bar, + ); + let (npm_resolver, npm_resolution) = create_npm_resolver_and_resolution( + registry_url, + progress_bar, + npm_api.clone(), + npm_cache.clone(), + None, + None, + ); Self { assets, @@ -518,7 +630,7 @@ impl Inner { documents, http_client, maybe_cache_path: None, - maybe_config_file: None, + maybe_config_file_info: None, maybe_import_map: None, maybe_import_map_uri: None, maybe_package_json: None, @@ -527,10 +639,13 @@ impl Inner { maybe_testing_server: None, module_registries, module_registries_location, - npm_api, - npm_cache, - npm_resolution, - npm_resolver, + npm: LspNpmServices { + config_hash: LspNpmConfigHash(0), // this will be updated in initialize + api: npm_api, + cache: npm_cache, + resolution: npm_resolution, + resolver: npm_resolver, + }, performance, ts_fixable_diagnostics: Default::default(), ts_server, @@ -622,7 +737,7 @@ impl Inner { }?; lsp_log!(" Resolved configuration file: \"{}\"", config_url); - let config_file = ConfigFile::from_specifier(&config_url)?; + let config_file = ConfigFile::from_specifier(config_url)?; return Ok(Some(config_file)); } } @@ -649,6 +764,10 @@ impl Inner { &self, maybe_config_file: Option<&ConfigFile>, ) -> Result, AnyError> { + if crate::args::has_flag_env_var("DENO_NO_PACKAGE_JSON") { + return Ok(None); + } + // It is possible that root_uri is not set, for example when having a single // file open and not a workspace. In those situations we can't // automatically discover the configuration @@ -696,7 +815,7 @@ impl Inner { &self, tsconfig: &mut TsConfig, ) -> Result<(), AnyError> { - if let Some(config_file) = self.maybe_config_file.as_ref() { + if let Some(config_file) = self.maybe_config_file() { let (value, maybe_ignored_options) = config_file.to_compiler_options()?; tsconfig.merge(&value); if let Some(ignored_options) = maybe_ignored_options { @@ -712,9 +831,9 @@ impl Inner { pub fn snapshot(&self) -> Arc { // create a new snapshotted npm resolution and resolver let npm_resolution = Arc::new(NpmResolution::new( - self.npm_api.clone(), - self.npm_resolution.snapshot(), - None, + self.npm.api.clone(), + self.npm.resolution.snapshot(), + self.maybe_lockfile().cloned(), )); let node_fs = Arc::new(deno_fs::RealFs); let npm_resolver = Arc::new(CliNpmResolver::new( @@ -722,14 +841,14 @@ impl Inner { npm_resolution.clone(), create_npm_fs_resolver( node_fs.clone(), - self.npm_cache.clone(), + self.npm.cache.clone(), &ProgressBar::new(ProgressBarStyle::TextOnly), - self.npm_api.base_url().clone(), + self.npm.api.base_url().clone(), npm_resolution, - None, + self.maybe_node_modules_dir_path().cloned(), NpmSystemInfo::default(), ), - None, + self.maybe_lockfile().cloned(), )); let node_resolver = Arc::new(NodeResolver::new(node_fs, npm_resolver.clone())); @@ -743,7 +862,7 @@ impl Inner { }) } - pub fn update_cache(&mut self) -> Result<(), AnyError> { + pub async fn update_cache(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_cache", None::<()>); self.performance.measure(mark); let maybe_cache = &self.config.workspace_settings().cache; @@ -773,20 +892,19 @@ impl Inner { None }; if self.maybe_cache_path != maybe_cache_path { - self.recreate_http_client_and_dependents(maybe_cache_path)?; + self + .recreate_http_client_and_dependents(maybe_cache_path) + .await?; } Ok(()) } /// Recreates the http client and all dependent structs. - fn recreate_http_client_and_dependents( + async fn recreate_http_client_and_dependents( &mut self, new_cache_path: Option, ) -> Result<(), AnyError> { - let maybe_custom_root = new_cache_path - .clone() - .or_else(|| env::var("DENO_DIR").map(String::into).ok()); - let dir = DenoDir::new(maybe_custom_root)?; + let dir = self.deno_dir_from_maybe_cache_path(new_cache_path.clone())?; let workspace_settings = self.config.workspace_settings(); let maybe_root_path = self .config @@ -812,12 +930,6 @@ impl Inner { self.http_client.clone(), ); self.module_registries_location = module_registries_location; - ( - self.npm_api, - self.npm_cache, - self.npm_resolver, - self.npm_resolution, - ) = create_lsp_structs(&dir, self.http_client.clone()); // update the cache path let location = dir.deps_folder_path(); self.documents.set_location(&location); @@ -826,6 +938,64 @@ impl Inner { Ok(()) } + fn deno_dir_from_maybe_cache_path( + &self, + cache_path: Option, + ) -> std::io::Result { + let maybe_custom_root = + cache_path.or_else(|| env::var("DENO_DIR").map(String::into).ok()); + DenoDir::new(maybe_custom_root) + } + + async fn recreate_npm_services_if_necessary(&mut self) { + let deno_dir = match self + .deno_dir_from_maybe_cache_path(self.maybe_cache_path.clone()) + { + Ok(deno_dir) => deno_dir, + Err(err) => { + lsp_warn!("Error getting deno dir: {}", err); + return; + } + }; + let config_hash = LspNpmConfigHash::from_inner(self); + if config_hash == self.npm.config_hash { + return; // no need to do anything + } + + let registry_url = CliNpmRegistryApi::default_url(); + let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly); + (self.npm.api, self.npm.cache) = create_npm_api_and_cache( + &deno_dir, + self.http_client.clone(), + registry_url, + &progress_bar, + ); + let maybe_snapshot = match self.maybe_lockfile() { + Some(lockfile) => { + match snapshot_from_lockfile(lockfile.clone(), &self.npm.api).await { + Ok(snapshot) => Some(snapshot), + Err(err) => { + lsp_warn!("Failed getting npm snapshot from lockfile: {}", err); + None + } + } + } + None => None, + }; + (self.npm.resolver, self.npm.resolution) = + create_npm_resolver_and_resolution( + registry_url, + progress_bar, + self.npm.api.clone(), + self.npm.cache.clone(), + self.maybe_node_modules_dir_path().cloned(), + maybe_snapshot, + ); + + // update the hash + self.npm.config_hash = config_hash; + } + pub async fn update_import_map(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_import_map", None::<()>); @@ -855,7 +1025,7 @@ impl Inner { ) -> Result { resolve_import_map_from_specifier( import_map_url, - self.maybe_config_file.as_ref(), + self.maybe_config_file(), &self.create_file_fetcher(cache_setting), ) .await @@ -896,7 +1066,7 @@ impl Inner { "Setting import map from workspace settings: \"{}\"", import_map_str ); - if let Some(config_file) = &self.maybe_config_file { + if let Some(config_file) = self.maybe_config_file() { if let Some(import_map_path) = config_file.to_import_map_path() { lsp_log!("Warning: Import map \"{}\" configured in \"{}\" being ignored due to an import map being explicitly configured in workspace settings.", import_map_path, config_file.specifier); } @@ -922,7 +1092,7 @@ impl Inner { import_map_str )); } - } else if let Some(config_file) = &self.maybe_config_file { + } else if let Some(config_file) = self.maybe_config_file() { if config_file.is_an_import_map() { lsp_log!( "Setting import map defined in configuration file: \"{}\"", @@ -968,7 +1138,9 @@ impl Inner { async fn update_registries(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_registries", None::<()>); - self.recreate_http_client_and_dependents(self.maybe_cache_path.clone())?; + self + .recreate_http_client_and_dependents(self.maybe_cache_path.clone()) + .await?; let workspace_settings = self.config.workspace_settings(); for (registry, enabled) in workspace_settings.suggest.imports.hosts.iter() { if *enabled { @@ -982,11 +1154,10 @@ impl Inner { Ok(()) } - fn update_config_file(&mut self) -> Result<(), AnyError> { - self.maybe_config_file = None; + async fn update_config_file(&mut self) -> Result<(), AnyError> { + self.maybe_config_file_info = None; self.fmt_options = Default::default(); self.lint_options = Default::default(); - if let Some(config_file) = self.get_config_file()? { let lint_options = config_file .to_lint_config() @@ -1005,7 +1176,11 @@ impl Inner { anyhow!("Unable to update formatter configuration: {:?}", err) })?; - self.maybe_config_file = Some(config_file); + self.maybe_config_file_info = Some(LspConfigFileInfo { + maybe_lockfile: self.resolve_lockfile_from_config(&config_file), + maybe_node_modules_dir: resolve_node_modules_dir(&config_file), + config_file, + }); self.lint_options = lint_options; self.fmt_options = fmt_options; } @@ -1013,12 +1188,45 @@ impl Inner { Ok(()) } + fn resolve_lockfile_from_config( + &self, + config_file: &ConfigFile, + ) -> Option>> { + let lockfile_path = match config_file.resolve_lockfile_path() { + Ok(Some(value)) => value, + Ok(None) => return None, + Err(err) => { + lsp_warn!("Error resolving lockfile: {:#}", err); + return None; + } + }; + resolve_lockfile_from_path(lockfile_path) + } + + fn maybe_node_modules_dir_path(&self) -> Option<&PathBuf> { + self + .maybe_config_file_info + .as_ref() + .and_then(|p| p.maybe_node_modules_dir.as_ref()) + } + + fn maybe_config_file(&self) -> Option<&ConfigFile> { + self.maybe_config_file_info.as_ref().map(|c| &c.config_file) + } + + fn maybe_lockfile(&self) -> Option<&Arc>> { + self + .maybe_config_file_info + .as_ref() + .and_then(|c| c.maybe_lockfile.as_ref()) + } + /// Updates the package.json. Always ensure this is done after updating /// the configuration file as the resolution of this depends on that. fn update_package_json(&mut self) -> Result<(), AnyError> { self.maybe_package_json = None; self.maybe_package_json = - self.get_package_json(self.maybe_config_file.as_ref())?; + self.get_package_json(self.maybe_config_file())?; Ok(()) } @@ -1126,10 +1334,10 @@ impl Inner { self.update_debug_flag(); // Check to see if we need to change the cache path - if let Err(err) = self.update_cache() { + if let Err(err) = self.update_cache().await { self.client.show_message(MessageType::WARNING, err); } - if let Err(err) = self.update_config_file() { + if let Err(err) = self.update_config_file().await { self.client.show_message(MessageType::WARNING, err); } if let Err(err) = self.update_package_json() { @@ -1156,6 +1364,7 @@ impl Inner { self.client.show_message(MessageType::WARNING, err); } + self.recreate_npm_services_if_necessary().await; self.assets.intitialize(self.snapshot()).await; self.performance.measure(mark); @@ -1174,10 +1383,13 @@ impl Inner { .workspace_settings() .document_preload_limit, maybe_import_map: self.maybe_import_map.clone(), - maybe_config_file: self.maybe_config_file.as_ref(), + maybe_config_file: self + .maybe_config_file_info + .as_ref() + .map(|c| &c.config_file), maybe_package_json: self.maybe_package_json.as_ref(), - npm_registry_api: self.npm_api.clone(), - npm_resolution: self.npm_resolution.clone(), + npm_registry_api: self.npm.api.clone(), + npm_resolution: self.npm.resolution.clone(), }); } @@ -1245,7 +1457,7 @@ impl Inner { async fn refresh_npm_specifiers(&mut self) { let package_reqs = self.documents.npm_package_reqs(); - let npm_resolver = self.npm_resolver.clone(); + let npm_resolver = self.npm.resolver.clone(); // spawn to avoid the LSP's Send requirements let handle = spawn(async move { npm_resolver.set_package_reqs(&package_reqs).await }); @@ -1303,13 +1515,13 @@ impl Inner { } self.update_debug_flag(); - if let Err(err) = self.update_cache() { + if let Err(err) = self.update_cache().await { self.client.show_message(MessageType::WARNING, err); } if let Err(err) = self.update_registries().await { self.client.show_message(MessageType::WARNING, err); } - if let Err(err) = self.update_config_file() { + if let Err(err) = self.update_config_file().await { self.client.show_message(MessageType::WARNING, err); } if let Err(err) = self.update_package_json() { @@ -1322,8 +1534,10 @@ impl Inner { self.client.show_message(MessageType::WARNING, err); } + self.recreate_npm_services_if_necessary().await; self.refresh_documents_config(); + self.diagnostics_server.invalidate_all(); self.send_diagnostics_update(); self.send_testing_update(); } @@ -1343,9 +1557,9 @@ impl Inner { .collect(); // if the current deno.json has changed, we need to reload it - if let Some(config_file) = &self.maybe_config_file { + if let Some(config_file) = self.maybe_config_file() { if changes.contains(&config_file.specifier) { - if let Err(err) = self.update_config_file() { + if let Err(err) = self.update_config_file().await { self.client.show_message(MessageType::WARNING, err); } if let Err(err) = self.update_tsconfig().await { @@ -1354,6 +1568,24 @@ impl Inner { touched = true; } } + if let Some(config_info) = self.maybe_config_file_info.as_mut() { + if let Some(lockfile) = config_info.maybe_lockfile.as_ref() { + let lockfile_path = lockfile.lock().filename.clone(); + let maybe_specifier = + ModuleSpecifier::from_file_path(&lockfile_path).ok(); + if let Some(specifier) = maybe_specifier { + if changes.contains(&specifier) { + let lockfile_hash = hash_lockfile(Some(lockfile)); + let new_lockfile = resolve_lockfile_from_path(lockfile_path); + // only update if the lockfile has changed + if lockfile_hash != hash_lockfile(new_lockfile.as_ref()) { + config_info.maybe_lockfile = new_lockfile; + touched = true; + } + } + } + } + } if let Some(package_json) = &self.maybe_package_json { // always update the package json if the deno config changes if touched || changes.contains(&package_json.specifier()) { @@ -1374,6 +1606,7 @@ impl Inner { } } if touched { + self.recreate_npm_services_if_necessary().await; self.refresh_documents_config(); self.refresh_npm_specifiers().await; self.diagnostics_server.invalidate_all(); @@ -2793,9 +3026,13 @@ impl tower_lsp::LanguageServer for LanguageServer { } } - if !self.refresh_specifiers_from_client().await { - // force update config - self.0.write().await.refresh_documents_config(); + self.refresh_specifiers_from_client().await; + + { + let mut ls = self.0.write().await; + ls.refresh_documents_config(); + ls.diagnostics_server.invalidate_all(); + ls.send_diagnostics_update(); } lsp_log!("Server ready."); @@ -2949,7 +3186,12 @@ impl tower_lsp::LanguageServer for LanguageServer { (ls.performance.clone(), mark) }; - self.refresh_specifiers_from_client().await; + if self.refresh_specifiers_from_client().await { + let mut ls = self.0.write().await; + ls.refresh_documents_config(); + ls.diagnostics_server.invalidate_all(); + ls.send_diagnostics_update(); + } performance.measure(mark); } @@ -3147,23 +3389,24 @@ impl Inner { vec![referrer] }; + let workspace_settings = self.config.workspace_settings(); let mut cli_options = CliOptions::new( Flags { cache_path: self.maybe_cache_path.clone(), - ca_stores: None, - ca_data: None, - unsafely_ignore_certificate_errors: None, - // this is to allow loading npm specifiers, so we can remove this - // once stabilizing them - unstable: true, + ca_stores: workspace_settings.certificate_stores.clone(), + ca_data: workspace_settings.tls_certificate.clone().map(CaData::File), + unsafely_ignore_certificate_errors: workspace_settings + .unsafely_ignore_certificate_errors + .clone(), + node_modules_dir: Some(self.maybe_node_modules_dir_path().is_some()), + // bit of a hack to force the lsp to cache the @types/node package + type_check_mode: crate::args::TypeCheckMode::Local, ..Default::default() }, std::env::current_dir().with_context(|| "Failed getting cwd.")?, - self.maybe_config_file.clone(), - // TODO(#16510): add support for lockfile - None, - // TODO(bartlomieju): handle package.json dependencies here - None, + self.maybe_config_file().cloned(), + self.maybe_lockfile().cloned(), + self.maybe_package_json.clone(), )?; cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone()); @@ -3195,12 +3438,7 @@ impl Inner { } fn get_tasks(&self) -> LspResult> { - Ok( - self - .maybe_config_file - .as_ref() - .and_then(|cf| cf.to_lsp_tasks()), - ) + Ok(self.maybe_config_file().and_then(|cf| cf.to_lsp_tasks())) } async fn inlay_hint( diff --git a/cli/npm/installer.rs b/cli/npm/installer.rs index 8590feb9c24dd2..adb4344ee48a2c 100644 --- a/cli/npm/installer.rs +++ b/cli/npm/installer.rs @@ -64,6 +64,12 @@ impl PackageJsonDepsInstaller { })) } + /// Creates an installer that never installs local packages during + /// resolution. A top level install will be a no-op. + pub fn no_op() -> Self { + Self(None) + } + /// Installs the top level dependencies in the package.json file /// without going through and resolving the descendant dependencies yet. pub async fn ensure_top_level_install(&self) -> Result<(), AnyError> { diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 1687864078a429..26d95482439509 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -91,6 +91,16 @@ impl CliNpmResolver { self.fs_resolver.node_modules_path() } + /// Checks if the provided package req's folder is cached. + pub fn is_pkg_req_folder_cached(&self, req: &NpmPackageReq) -> bool { + self + .resolve_pkg_id_from_pkg_req(req) + .ok() + .and_then(|id| self.fs_resolver.package_folder(&id).ok()) + .map(|folder| folder.exists()) + .unwrap_or(false) + } + pub fn resolve_pkg_id_from_pkg_req( &self, req: &NpmPackageReq, diff --git a/cli/tests/integration/inspector_tests.rs b/cli/tests/integration/inspector_tests.rs index 8fa9ec85c0ed1d..f94dd221b89dc3 100644 --- a/cli/tests/integration/inspector_tests.rs +++ b/cli/tests/integration/inspector_tests.rs @@ -18,6 +18,7 @@ use test_util as util; use test_util::TempDir; use tokio::net::TcpStream; use url::Url; +use util::assert_starts_with; use util::http_server; use util::DenoChild; @@ -217,15 +218,6 @@ impl InspectorTester { } } -macro_rules! assert_starts_with { - ($string:expr, $($test:expr),+) => { - let string = $string; // This might be a function call or something - if !($(string.starts_with($test))||+) { - panic!("{:?} does not start with {:?}", string, [$($test),+]); - } - } - } - fn assert_stderr( stderr_lines: &mut impl std::iter::Iterator, expected_lines: &[&str], diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 656ec9ade7d6c3..db009999b6fe6f 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -9,8 +9,10 @@ use deno_core::url::Url; use pretty_assertions::assert_eq; use std::fs; use std::process::Stdio; +use test_util::assert_starts_with; use test_util::deno_cmd_with_deno_dir; use test_util::env_vars_for_npm_tests; +use test_util::lsp::LspClient; use test_util::testdata_path; use test_util::TestContextBuilder; use tower_lsp::lsp_types as lsp; @@ -7540,3 +7542,145 @@ fn lsp_data_urls_with_jsx_compiler_option() { client.shutdown(); } + +#[test] +fn lsp_node_modules_dir() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + + // having a package.json should have no effect on whether + // a node_modules dir is created + temp_dir.write("package.json", "{}"); + + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let file_uri = temp_dir.uri().join("file.ts").unwrap(); + client.did_open(json!({ + "textDocument": { + "uri": file_uri, + "languageId": "typescript", + "version": 1, + "text": "import chalk from 'npm:chalk';\nimport path from 'node:path';\n\nconsole.log(chalk.green(path.join('a', 'b')));", + } + })); + let cache = |client: &mut LspClient| { + client.write_request( + "deno/cache", + json!({ + "referrer": { + "uri": file_uri, + }, + "uris": [ + { + "uri": "npm:chalk", + }, + { + "uri": "npm:@types/node", + } + ] + }), + ); + }; + + cache(&mut client); + + assert!(!temp_dir.path().join("node_modules").exists()); + + temp_dir.write( + temp_dir.path().join("deno.json"), + "{ \"nodeModulesDir\": true, \"lock\": false }\n", + ); + let refresh_config = |client: &mut LspClient| { + client.write_notification( + "workspace/didChangeConfiguration", + json!({ + "settings": { + "enable": true, + "config": "./deno.json", + } + }), + ); + + let request = json!([{ + "enable": true, + "config": "./deno.json", + "codeLens": { + "implementations": true, + "references": true + }, + "importMap": null, + "lint": false, + "suggest": { + "autoImports": true, + "completeFunctionCalls": false, + "names": true, + "paths": true, + "imports": {} + }, + "unstable": false + }]); + // one for the workspace + client.handle_configuration_request(request.clone()); + // one for the specifier + client.handle_configuration_request(request); + }; + refresh_config(&mut client); + + let diagnostics = client.read_diagnostics(); + assert_eq!(diagnostics.viewed().len(), 2); // not cached + + cache(&mut client); + + assert!(temp_dir.path().join("node_modules/chalk").exists()); + assert!(temp_dir.path().join("node_modules/@types/node").exists()); + assert!(!temp_dir.path().join("deno.lock").exists()); + + // now add a lockfile and cache + temp_dir.write( + temp_dir.path().join("deno.json"), + "{ \"nodeModulesDir\": true }\n", + ); + refresh_config(&mut client); + cache(&mut client); + + let diagnostics = client.read_diagnostics(); + assert_eq!(diagnostics.viewed().len(), 0, "{:#?}", diagnostics); + + assert!(temp_dir.path().join("deno.lock").exists()); + + // the declaration should be found in the node_modules directory + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": file_uri, + }, + "position": { "line": 0, "character": 7 }, // chalk + "context": { + "includeDeclaration": false + } + }), + ); + + // ensure that it's using the node_modules directory + let references = res.as_array().unwrap(); + assert_eq!(references.len(), 2, "references: {:#?}", references); + let uri = references[1] + .as_object() + .unwrap() + .get("uri") + .unwrap() + .as_str() + .unwrap(); + // canonicalize for mac + let path = temp_dir.path().join("node_modules").canonicalize().unwrap(); + assert_starts_with!( + uri, + ModuleSpecifier::from_file_path(&path).unwrap().as_str() + ); + + client.shutdown(); +} diff --git a/test_util/src/assertions.rs b/test_util/src/assertions.rs index a004530b6e21f7..29066ded083aea 100644 --- a/test_util/src/assertions.rs +++ b/test_util/src/assertions.rs @@ -1,5 +1,15 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +#[macro_export] +macro_rules! assert_starts_with { + ($string:expr, $($test:expr),+) => { + let string = $string; // This might be a function call or something + if !($(string.starts_with($test))||+) { + panic!("{:?} does not start with {:?}", string, [$($test),+]); + } + } +} + #[macro_export] macro_rules! assert_ends_with { ($left:expr, $right:expr $(,)?) => {