From 235fdc243faa6d0fc008dc7fa482c29e511c2fa0 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 20 Jul 2023 14:05:52 -0400 Subject: [PATCH] fix(lsp): auto-discover deno.json in more cases (#19894) We weren't auto-discovering the deno.json in two cases: 1. A project that didn't have a deno.json and just added one. 2. After a syntax error in the deno.json. This now rediscovers it in both these cases. Closes https://github.com/denoland/vscode_deno/issues/867 --- cli/lsp/config.rs | 7 +- cli/lsp/language_server.rs | 40 ++++++----- cli/tests/integration/lsp_tests.rs | 112 +++++++++++++++++++++++++++++ test_util/src/lsp.rs | 61 +++++++++++++++- 4 files changed, 202 insertions(+), 18 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 298c86a55d..03464d1f8f 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -773,7 +773,12 @@ fn resolve_lockfile_from_path( lockfile_path: PathBuf, ) -> Option>> { match Lockfile::new(lockfile_path, false) { - Ok(value) => Some(Arc::new(Mutex::new(value))), + Ok(value) => { + if let Ok(specifier) = ModuleSpecifier::from_file_path(&value.filename) { + lsp_log!(" Resolved lock file: \"{}\"", specifier); + } + Some(Arc::new(Mutex::new(value))) + } Err(err) => { lsp_warn!("Error loading lockfile: {:#}", err); None diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 6dead0fcf6..5a4c3aeaec 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1493,22 +1493,30 @@ impl Inner { .collect(); // if the current deno.json has changed, we need to reload it - if let Some(config_file) = self.config.maybe_config_file() { - if changes.contains(&config_file.specifier) - || self - .config - .maybe_lockfile() - .map(|l| has_lockfile_changed(&l.lock(), &changes)) - .unwrap_or(false) - { - if let Err(err) = self.update_config_file().await { - self.client.show_message(MessageType::WARNING, err); - } - if let Err(err) = self.update_tsconfig().await { - self.client.show_message(MessageType::WARNING, err); - } - touched = true; + let has_config_changed = match self.config.maybe_config_file() { + Some(config_file) => changes.contains(&config_file.specifier), + None => { + // check for auto-discovery + changes.iter().any(|url| { + url.path().ends_with("/deno.json") + || url.path().ends_with("/deno.jsonc") + }) } + } || match self.config.maybe_lockfile() { + Some(lockfile) => has_lockfile_changed(&lockfile.lock(), &changes), + None => { + // check for auto-discovery + changes.iter().any(|url| url.path().ends_with("/deno.lock")) + } + }; + if has_config_changed { + if let Err(err) = self.update_config_file().await { + self.client.show_message(MessageType::WARNING, err); + } + if let Err(err) = self.update_tsconfig().await { + self.client.show_message(MessageType::WARNING, err); + } + touched = true; } if let Some(package_json) = &self.maybe_package_json { @@ -2940,7 +2948,7 @@ impl tower_lsp::LanguageServer for LanguageServer { let watch_registration_options = DidChangeWatchedFilesRegistrationOptions { watchers: vec![FileSystemWatcher { - glob_pattern: "**/*.{json,jsonc}".to_string(), + glob_pattern: "**/*.{json,jsonc,lock}".to_string(), kind: Some(WatchKind::Change), }], }; diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 22463b58aa..261c98b525 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -432,6 +432,118 @@ fn lsp_import_map_embedded_in_config_file_after_initialize() { client.shutdown(); } +#[test] +fn lsp_import_map_config_file_auto_discovered() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.create_dir_all("lib"); + temp_dir.write("lib/b.ts", r#"export const b = "b";"#); + + let mut client = context.new_lsp_command().capture_stderr().build(); + client.initialize_default(); + + // add the deno.json + temp_dir.write("deno.jsonc", r#"{ "imports": { "/~/": "./lib/" } }"#); + client.did_change_watched_files(json!({ + "changes": [{ + "uri": temp_dir.uri().join("deno.jsonc").unwrap(), + "type": 2 + }] + })); + client.wait_until_stderr_line(|line| { + line.contains("Auto-resolved configuration file:") + }); + + let uri = temp_dir.uri().join("a.ts").unwrap(); + + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": uri, + "languageId": "typescript", + "version": 1, + "text": "import { b } from \"/~/b.ts\";\n\nconsole.log(b);\n" + } + })); + + assert_eq!(diagnostics.all().len(), 0); + + let res = client.write_request( + "textDocument/hover", + json!({ + "textDocument": { + "uri": uri + }, + "position": { "line": 2, "character": 12 } + }), + ); + assert_eq!( + res, + json!({ + "contents": [ + { + "language": "typescript", + "value":"(alias) const b: \"b\"\nimport b" + }, + "" + ], + "range": { + "start": { "line": 2, "character": 12 }, + "end": { "line": 2, "character": 13 } + } + }) + ); + + // now cause a syntax error + temp_dir.write("deno.jsonc", r#",,#,#,,"#); + client.did_change_watched_files(json!({ + "changes": [{ + "uri": temp_dir.uri().join("deno.jsonc").unwrap(), + "type": 2 + }] + })); + assert_eq!(client.read_diagnostics().all().len(), 1); + + // now fix it, and things should work again + temp_dir.write("deno.jsonc", r#"{ "imports": { "/~/": "./lib/" } }"#); + client.did_change_watched_files(json!({ + "changes": [{ + "uri": temp_dir.uri().join("deno.jsonc").unwrap(), + "type": 2 + }] + })); + client.wait_until_stderr_line(|line| { + line.contains("Auto-resolved configuration file:") + }); + let res = client.write_request( + "textDocument/hover", + json!({ + "textDocument": { + "uri": uri + }, + "position": { "line": 2, "character": 12 } + }), + ); + assert_eq!( + res, + json!({ + "contents": [ + { + "language": "typescript", + "value":"(alias) const b: \"b\"\nimport b" + }, + "" + ], + "range": { + "start": { "line": 2, "character": 12 }, + "end": { "line": 2, "character": 13 } + } + }) + ); + assert_eq!(client.read_diagnostics().all().len(), 0); + + client.shutdown(); +} + #[test] fn lsp_deno_task() { let context = TestContextBuilder::new().use_temp_cwd().build(); diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs index cd546bd6cd..6fafc234c5 100644 --- a/test_util/src/lsp.rs +++ b/test_util/src/lsp.rs @@ -37,6 +37,8 @@ use serde_json::to_value; use serde_json::Value; use std::collections::HashSet; use std::io; +use std::io::BufRead; +use std::io::BufReader; use std::io::Write; use std::path::Path; use std::process::Child; @@ -44,6 +46,7 @@ use std::process::ChildStdin; use std::process::ChildStdout; use std::process::Command; use std::process::Stdio; +use std::sync::mpsc; use std::sync::Arc; use std::time::Duration; use std::time::Instant; @@ -468,6 +471,7 @@ impl InitializeParamsBuilder { pub struct LspClientBuilder { print_stderr: bool, + capture_stderr: bool, deno_exe: PathRef, context: Option, use_diagnostic_sync: bool, @@ -478,6 +482,7 @@ impl LspClientBuilder { pub fn new() -> Self { Self { print_stderr: false, + capture_stderr: false, deno_exe: deno_exe_path(), context: None, use_diagnostic_sync: true, @@ -497,6 +502,11 @@ impl LspClientBuilder { self } + pub fn capture_stderr(&mut self) -> &mut Self { + self.capture_stderr = true; + self + } + /// Whether to use the synchronization messages to better sync diagnostics /// between the test client and server. pub fn use_diagnostic_sync(&mut self, value: bool) -> &mut Self { @@ -527,7 +537,9 @@ impl LspClientBuilder { .arg("lsp") .stdin(Stdio::piped()) .stdout(Stdio::piped()); - if !self.print_stderr { + if self.capture_stderr { + command.stderr(Stdio::piped()); + } else if !self.print_stderr { command.stderr(Stdio::null()); } let mut child = command.spawn()?; @@ -538,6 +550,31 @@ impl LspClientBuilder { let stdin = child.stdin.take().unwrap(); let writer = io::BufWriter::new(stdin); + let stderr_lines_rx = if self.capture_stderr { + let stderr = child.stderr.take().unwrap(); + let print_stderr = self.print_stderr; + let (tx, rx) = mpsc::channel::(); + std::thread::spawn(move || { + let stderr = BufReader::new(stderr); + for line in stderr.lines() { + match line { + Ok(line) => { + if print_stderr { + eprintln!("{}", line); + } + tx.send(line).unwrap(); + } + Err(err) => { + panic!("failed to read line from stderr: {:#}", err); + } + } + } + }); + Some(rx) + } else { + None + }; + Ok(LspClient { child, reader, @@ -549,6 +586,7 @@ impl LspClientBuilder { .unwrap_or_else(|| TestContextBuilder::new().build()), writer, deno_dir, + stderr_lines_rx, }) } } @@ -561,6 +599,7 @@ pub struct LspClient { writer: io::BufWriter, deno_dir: TempDir, context: TestContext, + stderr_lines_rx: Option>, } impl Drop for LspClient { @@ -594,6 +633,26 @@ impl LspClient { self.reader.pending_len() } + #[track_caller] + pub fn wait_until_stderr_line(&self, condition: impl Fn(&str) -> bool) { + let timeout_time = + Instant::now().checked_add(Duration::from_secs(5)).unwrap(); + let lines_rx = self + .stderr_lines_rx + .as_ref() + .expect("must setup with client_builder.capture_stderr()"); + while Instant::now() < timeout_time { + if let Ok(line) = lines_rx.try_recv() { + if condition(&line) { + return; + } + } + std::thread::sleep(Duration::from_millis(20)); + } + + panic!("Timed out.") + } + pub fn initialize_default(&mut self) { self.initialize(|_| {}) }