From f75a17521df053218c0c2b5fb93c5354f9c8f274 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Sat, 9 Sep 2023 19:37:01 +0100 Subject: [PATCH] fix(lsp): respect configured exclusions for testing APIs (#20427) LSP testing APIs now obey the various file inclusion settings: - Modules shown in the text explorer now respect the `exclude`, `test.exclude` and `test.include` fields in `deno.json`, as well as `deno.enablePaths` in VSCode settings. - Modules with testing code lens now respect the `"exclude"`, `test.exclude` and `test.include` fields in `deno.json`. Code lens already respects `deno.enablePaths`. --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/lsp/code_lens.rs | 4 +- cli/lsp/config.rs | 181 ++++++++++++++++++++++++++++++------- cli/lsp/diagnostics.rs | 1 + cli/lsp/language_server.rs | 3 + cli/lsp/testing/server.rs | 3 + cli/lsp/tsc.rs | 2 + 8 files changed, 163 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7403b26dd6..e199ec6f73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1050,9 +1050,9 @@ dependencies = [ [[package]] name = "deno_config" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee6f80798a5460656fbbbcdbe6e7700878a6777739195a205e97a8e5fcd00baf" +checksum = "ed5999e360fec39bbfee5d85bac82c5f557ed93a58660bc255026a90796138c6" dependencies = [ "anyhow", "deno_semver", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index e713a1a643..c08e661805 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -47,7 +47,7 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_cache_dir = "=0.6.0" -deno_config = "=0.3.0" +deno_config = "=0.3.1" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_doc = "=0.66.0" deno_emit = "=0.27.0" diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index c451e30bdc..d64b2014fb 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -407,7 +407,9 @@ fn collect_test( parsed_source: Option, config: &Config, ) -> Result, AnyError> { - if config.specifier_code_lens_test(specifier) { + if config.specifier_enabled_for_test(specifier) + && config.specifier_code_lens_test(specifier) + { if let Some(parsed_source) = parsed_source { let mut collector = DenoTestCollector::new(specifier.clone(), parsed_source.clone()); diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index e8d19b9271..b463a2fcb3 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -396,8 +396,7 @@ impl WorkspaceSettings { #[derive(Debug, Clone, Default)] pub struct ConfigSnapshot { pub client_capabilities: ClientCapabilities, - pub excluded_paths: Option>, - pub has_config_file: bool, + pub config_file: Option, pub settings: Settings, pub workspace_folders: Vec<(ModuleSpecifier, lsp::WorkspaceFolder)>, } @@ -407,12 +406,28 @@ impl ConfigSnapshot { pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { specifier_enabled( specifier, + self.config_file.as_ref(), &self.settings, - self.excluded_paths.as_ref(), &self.workspace_folders, - self.has_config_file, ) } + + pub fn specifier_enabled_for_test( + &self, + specifier: &ModuleSpecifier, + ) -> bool { + if let Some(cf) = &self.config_file { + if let Some(options) = cf.to_test_config().ok().flatten() { + if !options.files.matches_specifier(specifier) { + return false; + } + } + } + if !self.specifier_enabled(specifier) { + return false; + } + true + } } #[derive(Debug, Default, Clone)] @@ -436,7 +451,6 @@ struct LspConfigFileInfo { maybe_lockfile: Option>>>, /// The canonicalized node_modules directory, which is found relative to the config file. maybe_node_modules_dir: Option, - excluded_paths: Vec, } #[derive(Debug)] @@ -551,15 +565,6 @@ impl Config { }, ), maybe_node_modules_dir: resolve_node_modules_dir(&config_file), - excluded_paths: config_file - .to_files_config() - .ok() - .flatten() - .map(|c| c.exclude) - .unwrap_or_default() - .into_iter() - .filter_map(|path| ModuleSpecifier::from_file_path(path).ok()) - .collect(), config_file: WithCanonicalizedSpecifier { canonicalized_specifier: config_file .specifier @@ -594,11 +599,7 @@ impl Config { pub fn snapshot(&self) -> Arc { Arc::new(ConfigSnapshot { client_capabilities: self.client_capabilities.clone(), - excluded_paths: self - .maybe_config_file_info - .as_ref() - .map(|i| i.excluded_paths.clone()), - has_config_file: self.has_config_file(), + config_file: self.maybe_config_file().cloned(), settings: self.settings.clone(), workspace_folders: self.workspace_folders.clone(), }) @@ -611,16 +612,29 @@ impl Config { pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { specifier_enabled( specifier, + self.maybe_config_file(), &self.settings, - self - .maybe_config_file_info - .as_ref() - .map(|i| &i.excluded_paths), &self.workspace_folders, - self.has_config_file(), ) } + pub fn specifier_enabled_for_test( + &self, + specifier: &ModuleSpecifier, + ) -> bool { + if let Some(cf) = self.maybe_config_file() { + if let Some(options) = cf.to_test_config().ok().flatten() { + if !options.files.matches_specifier(specifier) { + return false; + } + } + } + if !self.specifier_enabled(specifier) { + return false; + } + true + } + /// Gets the directories or specifically enabled file paths based on the /// workspace config. /// @@ -750,23 +764,19 @@ impl Config { fn specifier_enabled( specifier: &Url, + config_file: Option<&ConfigFile>, settings: &Settings, - excluded_urls: Option<&Vec>, workspace_folders: &Vec<(Url, lsp::WorkspaceFolder)>, - workspace_has_config_file: bool, ) -> bool { - if let Some(excluded_urls) = excluded_urls { - for excluded_path in excluded_urls { - if specifier.as_str().starts_with(excluded_path.as_str()) { + if let Some(cf) = config_file { + if let Some(files) = cf.to_files_config().ok().flatten() { + if !files.matches_specifier(specifier) { return false; } } } - let root_enable = settings - .workspace - .enable - .unwrap_or(workspace_has_config_file); + let root_enable = settings.workspace.enable.unwrap_or(config_file.is_some()); if let Some(settings) = settings.specifiers.get(specifier) { // TODO(nayeemrmn): We don't know from where to resolve `enable_paths` in @@ -1111,4 +1121,109 @@ mod tests { ); assert_eq!(config.enabled_urls(), vec![root_uri]); } + + #[test] + fn config_specifier_enabled_for_test() { + let root_uri = resolve_url("file:///root/").unwrap(); + let mut config = Config::new_with_root(root_uri.clone()); + config.settings.workspace.enable = Some(true); + + config.settings.workspace.enable_paths = Some(vec!["mod1.ts".to_string()]); + assert!( + config.specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap()) + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod2.ts").unwrap()) + ); + config.settings.workspace.enable_paths = None; + + config.set_config_file( + ConfigFile::new( + &json!({ + "exclude": ["mod2.ts"], + "test": { + "exclude": ["mod3.ts"], + }, + }) + .to_string(), + root_uri.join("deno.json").unwrap(), + ) + .unwrap(), + ); + assert!( + config.specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap()) + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod2.ts").unwrap()) + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod3.ts").unwrap()) + ); + + config.set_config_file( + ConfigFile::new( + &json!({ + "test": { + "include": ["mod1.ts"], + }, + }) + .to_string(), + root_uri.join("deno.json").unwrap(), + ) + .unwrap(), + ); + assert!( + config.specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap()) + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod2.ts").unwrap()) + ); + + config.set_config_file( + ConfigFile::new( + &json!({ + "test": { + "exclude": ["mod2.ts"], + "include": ["mod2.ts"], + }, + }) + .to_string(), + root_uri.join("deno.json").unwrap(), + ) + .unwrap(), + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap()) + ); + assert!( + !config.specifier_enabled_for_test(&root_uri.join("mod2.ts").unwrap()) + ); + } + + #[test] + fn config_snapshot_specifier_enabled_for_test() { + let root_uri = resolve_url("file:///root/").unwrap(); + let mut config = Config::new_with_root(root_uri.clone()); + config.settings.workspace.enable = Some(true); + config.set_config_file( + ConfigFile::new( + &json!({ + "exclude": ["mod2.ts"], + "test": { + "exclude": ["mod3.ts"], + }, + }) + .to_string(), + root_uri.join("deno.json").unwrap(), + ) + .unwrap(), + ); + let config_snapshot = config.snapshot(); + assert!(config_snapshot + .specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap())); + assert!(!config_snapshot + .specifier_enabled_for_test(&root_uri.join("mod2.ts").unwrap())); + assert!(!config_snapshot + .specifier_enabled_for_test(&root_uri.join("mod3.ts").unwrap())); + } } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 7d8368b27c..361dba1966 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1382,6 +1382,7 @@ mod tests { cache_metadata: cache::CacheMetadata::new(Arc::new( GlobalHttpCache::new(location.to_path_buf(), RealDenoCacheEnv), )), + config: Default::default(), maybe_node_resolver: None, maybe_npm_resolver: None, } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 3b19d9288b..7a69d84307 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -44,6 +44,7 @@ use super::client::Client; use super::code_lens; use super::completions; use super::config::Config; +use super::config::ConfigSnapshot; use super::config::SETTINGS_SECTION; use super::diagnostics; use super::diagnostics::DiagnosticServerUpdateMessage; @@ -157,6 +158,7 @@ pub struct LanguageServer(Arc>); pub struct StateSnapshot { pub assets: AssetsSnapshot, pub cache_metadata: cache::CacheMetadata, + pub config: Arc, pub documents: Documents, pub maybe_import_map: Option>, pub maybe_node_resolver: Option>, @@ -812,6 +814,7 @@ impl Inner { Arc::new(StateSnapshot { assets: self.assets.snapshot(), cache_metadata: self.cache_metadata.clone(), + config: self.config.snapshot(), documents: self.documents.clone(), maybe_import_map: self.maybe_import_map.clone(), maybe_node_resolver: Some(node_resolver), diff --git a/cli/lsp/testing/server.rs b/cli/lsp/testing/server.rs index 58ac7a8c49..562bbfcb08 100644 --- a/cli/lsp/testing/server.rs +++ b/cli/lsp/testing/server.rs @@ -98,6 +98,9 @@ impl TestServer { .documents(DocumentsFilter::AllDiagnosable) { let specifier = document.specifier(); + if !snapshot.config.specifier_enabled_for_test(specifier) { + continue; + } keys.remove(specifier); let script_version = document.script_version(); let valid = if let Some(test) = tests.get(specifier) { diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 78e8cb60cb..ae4a00e6a4 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3414,6 +3414,7 @@ deno_core::extension!(deno_tsc, Arc::new(StateSnapshot { assets: Default::default(), cache_metadata: CacheMetadata::new(options.cache.clone()), + config: Default::default(), documents: Documents::new(options.cache.clone()), maybe_import_map: None, maybe_node_resolver: None, @@ -4141,6 +4142,7 @@ mod tests { documents, assets: Default::default(), cache_metadata: CacheMetadata::new(cache), + config: Default::default(), maybe_import_map: None, maybe_node_resolver: None, maybe_npm_resolver: None,