From 0c14531966523b0d9197d86f853f8e1003ee03a5 Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Wed, 3 Dec 2025 22:39:15 +0000 Subject: [PATCH] fix(oxlint/lsp): revalidate all known files after internal restart (#16407) Before: Linted all files which are not ignored & supported After: Lint all files which are got open inside the editor --- crates/oxc_language_server/src/backend.rs | 10 +-- .../src/formatter/server_formatter.rs | 14 +--- .../src/linter/server_linter.rs | 29 +------ crates/oxc_language_server/src/tests.rs | 41 ++-------- crates/oxc_language_server/src/tool.rs | 2 - crates/oxc_language_server/src/worker.rs | 80 +++++++++++++------ 6 files changed, 71 insertions(+), 105 deletions(-) diff --git a/crates/oxc_language_server/src/backend.rs b/crates/oxc_language_server/src/backend.rs index d9aa42cc7a9b8..79f401f142bfd 100644 --- a/crates/oxc_language_server/src/backend.rs +++ b/crates/oxc_language_server/src/backend.rs @@ -340,8 +340,9 @@ impl LanguageServer for Backend { continue; }; - let (diagnostics, registrations, unregistrations) = - worker.did_change_configuration(option.options).await; + let (diagnostics, registrations, unregistrations) = worker + .did_change_configuration(option.options, &*self.file_system.read().await) + .await; if let Some(diagnostics) = diagnostics { new_diagnostics.extend(diagnostics); @@ -390,7 +391,7 @@ impl LanguageServer for Backend { continue; }; let (diagnostics, registrations, unregistrations) = - worker.did_change_watched_files(file_event).await; + worker.did_change_watched_files(file_event, &*self.file_system.read().await).await; if let Some(diagnostics) = diagnostics { new_diagnostics.extend(diagnostics); @@ -503,9 +504,6 @@ impl LanguageServer for Backend { return; }; - // saving the file means we can read again from the file system - self.file_system.write().await.remove(uri); - if let Some(diagnostics) = worker.run_diagnostic_on_save(uri, params.text.as_deref()).await { self.client.publish_diagnostics(uri.clone(), diagnostics, None).await; diff --git a/crates/oxc_language_server/src/formatter/server_formatter.rs b/crates/oxc_language_server/src/formatter/server_formatter.rs index ec9248f906eff..01dfd7cb5d298 100644 --- a/crates/oxc_language_server/src/formatter/server_formatter.rs +++ b/crates/oxc_language_server/src/formatter/server_formatter.rs @@ -183,18 +183,13 @@ impl Tool for ServerFormatter { }; if old_option == new_option { - return ToolRestartChanges { - tool: None, - diagnostic_reports: None, - watch_patterns: None, - }; + return ToolRestartChanges { tool: None, watch_patterns: None }; } let new_formatter = ServerFormatterBuilder::build(root_uri, new_options_json.clone()); let watch_patterns = new_formatter.get_watcher_patterns(new_options_json); ToolRestartChanges { tool: Some(Box::new(new_formatter)), - diagnostic_reports: None, watch_patterns: Some(watch_patterns), } } @@ -228,11 +223,7 @@ impl Tool for ServerFormatter { options: serde_json::Value, ) -> ToolRestartChanges { if !self.should_run { - return ToolRestartChanges { - tool: None, - diagnostic_reports: None, - watch_patterns: None, - }; + return ToolRestartChanges { tool: None, watch_patterns: None }; } // TODO: Check if the changed file is actually a config file @@ -241,7 +232,6 @@ impl Tool for ServerFormatter { ToolRestartChanges { tool: Some(Box::new(new_formatter)), - diagnostic_reports: None, // TODO: update watch patterns if config_path changed watch_patterns: None, } diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index 822a455b9b3da..f8fb4b5ce4030 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -345,17 +345,11 @@ impl Tool for ServerLinter { }; if !Self::needs_restart(&old_option, &new_options) { - return ToolRestartChanges { - tool: None, - diagnostic_reports: None, - watch_patterns: None, - }; + return ToolRestartChanges { tool: None, watch_patterns: None }; } // get the cached files before refreshing the linter, and revalidate them after - let cached_files = self.get_cached_files_of_diagnostics(); let new_linter = ServerLinterBuilder::build(root_uri, new_options_json.clone()); - let diagnostics = Some(new_linter.revalidate_diagnostics(cached_files)); let patterns = { if old_option.config_path == new_options.config_path @@ -367,11 +361,7 @@ impl Tool for ServerLinter { } }; - ToolRestartChanges { - tool: Some(Box::new(new_linter)), - diagnostic_reports: diagnostics, - watch_patterns: patterns, - } + ToolRestartChanges { tool: Some(Box::new(new_linter)), watch_patterns: patterns } } fn get_watcher_patterns(&self, options: serde_json::Value) -> Vec { @@ -410,13 +400,8 @@ impl Tool for ServerLinter { // TODO: Check if the changed file is actually a config file (including extended paths) let new_linter = ServerLinterBuilder::build(root_uri, options); - // get the cached files before refreshing the linter, and revalidate them after - let cached_files = self.get_cached_files_of_diagnostics(); - let diagnostics = Some(new_linter.revalidate_diagnostics(cached_files)); - ToolRestartChanges { tool: Some(Box::new(new_linter)), - diagnostic_reports: diagnostics, // TODO: update watch patterns if config_path changed, or the extended paths changed watch_patterns: None, } @@ -591,16 +576,6 @@ impl ServerLinter { self.diagnostics.pin().keys().filter_map(|s| Uri::from_str(s).ok()).collect() } - fn revalidate_diagnostics(&self, uris: Vec) -> Vec<(String, Vec)> { - let mut diagnostics = Vec::with_capacity(uris.len()); - for uri in uris { - if let Some(file_diagnostic) = self.run_diagnostic(&uri, None) { - diagnostics.push((uri.to_string(), file_diagnostic)); - } - } - diagnostics - } - fn is_ignored(&self, uri: &Uri) -> bool { let Some(uri_path) = uri.to_file_path() else { return true; diff --git a/crates/oxc_language_server/src/tests.rs b/crates/oxc_language_server/src/tests.rs index e432a6a3b7ea9..bb01c574f7899 100644 --- a/crates/oxc_language_server/src/tests.rs +++ b/crates/oxc_language_server/src/tests.rs @@ -55,34 +55,19 @@ impl Tool for FakeTool { _old_options_json: &serde_json::Value, new_options_json: serde_json::Value, ) -> ToolRestartChanges { - if new_options_json.as_u64() == Some(1) { + if new_options_json.as_u64() == Some(1) || new_options_json.as_u64() == Some(3) { return ToolRestartChanges { tool: Some(FakeToolBuilder.build_boxed(root_uri, new_options_json)), - diagnostic_reports: None, watch_patterns: None, }; } if new_options_json.as_u64() == Some(2) { return ToolRestartChanges { tool: None, - diagnostic_reports: None, watch_patterns: Some(vec!["**/new_watcher.config".to_string()]), }; } - if new_options_json.as_u64() == Some(3) { - return ToolRestartChanges { - tool: None, - diagnostic_reports: Some(vec![( - root_uri.to_string(), - vec![Diagnostic { - message: "Fake diagnostic".to_string(), - ..Default::default() - }], - )]), - watch_patterns: None, - }; - } - ToolRestartChanges { tool: None, diagnostic_reports: None, watch_patterns: None } + ToolRestartChanges { tool: None, watch_patterns: None } } fn get_watcher_patterns( @@ -101,34 +86,22 @@ impl Tool for FakeTool { root_uri: &Uri, options: serde_json::Value, ) -> ToolRestartChanges { - if changed_uri.as_str().ends_with("tool.config") { + if changed_uri.as_str().ends_with("tool.config") + || changed_uri.as_str().ends_with("diagnostics.config") + { return ToolRestartChanges { tool: Some(FakeToolBuilder.build_boxed(root_uri, options)), - diagnostic_reports: None, watch_patterns: None, }; } if changed_uri.as_str().ends_with("watcher.config") { return ToolRestartChanges { tool: None, - diagnostic_reports: None, watch_patterns: Some(vec!["**/new_watcher.config".to_string()]), }; } - if changed_uri.as_str().ends_with("diagnostics.config") { - return ToolRestartChanges { - tool: None, - diagnostic_reports: Some(vec![( - changed_uri.to_string(), - vec![Diagnostic { - message: "Fake diagnostic".to_string(), - ..Default::default() - }], - )]), - watch_patterns: None, - }; - } - ToolRestartChanges { tool: None, diagnostic_reports: None, watch_patterns: None } + + ToolRestartChanges { tool: None, watch_patterns: None } } fn get_code_actions_or_commands( diff --git a/crates/oxc_language_server/src/tool.rs b/crates/oxc_language_server/src/tool.rs index 7187869668ad0..5667320e4546d 100644 --- a/crates/oxc_language_server/src/tool.rs +++ b/crates/oxc_language_server/src/tool.rs @@ -129,8 +129,6 @@ pub struct ToolRestartChanges { /// The tool that was restarted (linter, formatter). /// If None, no tool was restarted. pub tool: Option>, - /// The diagnostic reports that need to be revalidated after the tool restart - pub diagnostic_reports: Option)>>, /// The patterns that were added during the tool restart /// Old patterns will be automatically unregistered pub watch_patterns: Option>, diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index 6c9346cc30264..35d29db1f8a7c 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -13,6 +13,7 @@ use tower_lsp_server::{ use crate::{ ToolRestartChanges, + file_system::LSPFileSystem, tool::{Tool, ToolBuilder}, }; @@ -213,6 +214,7 @@ impl WorkspaceWorker { pub async fn did_change_watched_files( &self, file_event: &FileEvent, + file_system: &LSPFileSystem, ) -> ( // Diagnostic reports that need to be revalidated Option)>>, @@ -227,7 +229,7 @@ impl WorkspaceWorker { options_guard.clone().unwrap_or_default() }; - self.handle_tool_changes(|tool| { + self.handle_tool_changes(file_system, |tool| { tool.handle_watched_file_change(&file_event.uri, &self.root_uri, options.clone()) }) .await @@ -240,6 +242,7 @@ impl WorkspaceWorker { pub async fn did_change_configuration( &self, changed_options_json: serde_json::Value, + file_system: &LSPFileSystem, ) -> ( // Diagnostic reports that need to be revalidated Option)>>, @@ -266,7 +269,7 @@ impl WorkspaceWorker { *options_guard = Some(changed_options_json.clone()); } - self.handle_tool_changes(|tool| { + self.handle_tool_changes(file_system, |tool| { tool.handle_configuration_change( &self.root_uri, &old_options, @@ -280,6 +283,7 @@ impl WorkspaceWorker { /// diagnostics updates, watcher registrations/unregistrations, and tool replacement async fn handle_tool_changes( &self, + file_system: &LSPFileSystem, change_handler: F, ) -> (Option)>>, Vec, Vec) where @@ -291,13 +295,7 @@ impl WorkspaceWorker { for tool in self.tools.write().await.iter_mut() { let change = change_handler(tool); - if let Some(reports) = change.diagnostic_reports { - if let Some(existing_diagnostics) = &mut diagnostics { - existing_diagnostics.extend(reports); - } else { - diagnostics = Some(reports); - } - } + if let Some(patterns) = change.watch_patterns { unregistrations.push(unregistration_tool_watcher_id(tool.name(), &self.root_uri)); if !patterns.is_empty() { @@ -310,6 +308,18 @@ impl WorkspaceWorker { } if let Some(replaced_tool) = change.tool { *tool = replaced_tool; + + for uri in file_system.keys() { + if let Some(reports) = + tool.run_diagnostic(&uri, file_system.get(&uri).as_deref()) + { + if let Some(existing_diagnostics) = &mut diagnostics { + existing_diagnostics.push((uri.to_string(), reports)); + } else { + diagnostics = Some(vec![(uri.to_string(), reports)]); + } + } + } } } @@ -371,6 +381,7 @@ mod tests { use crate::{ ToolBuilder, + file_system::LSPFileSystem, tests::{FAKE_COMMAND, FakeToolBuilder}, worker::WorkspaceWorker, }; @@ -454,11 +465,20 @@ mod tests { let tools: Vec> = vec![Box::new(FakeToolBuilder)]; worker.start_worker(serde_json::Value::Null, &tools).await; + let fs = LSPFileSystem::default(); + fs.set( + &Uri::from_str("file:///root/diagnostics.config").unwrap(), + "hello world".to_string(), + ); + let (diagnostics, registrations, unregistrations) = worker - .did_change_watched_files(&FileEvent { - uri: Uri::from_str("file:///root/unknown.file").unwrap(), - typ: FileChangeType::CHANGED, - }) + .did_change_watched_files( + &FileEvent { + uri: Uri::from_str("file:///root/unknown.file").unwrap(), + typ: FileChangeType::CHANGED, + }, + &fs, + ) .await; // Since FakeToolBuilder does not know about "unknown.file", no diagnostics or registrations are expected @@ -467,10 +487,13 @@ mod tests { assert_eq!(unregistrations.len(), 0); // No unregistrations expected let (diagnostics, registrations, unregistrations) = worker - .did_change_watched_files(&FileEvent { - uri: Uri::from_str("file:///root/watcher.config").unwrap(), - typ: FileChangeType::CHANGED, - }) + .did_change_watched_files( + &FileEvent { + uri: Uri::from_str("file:///root/watcher.config").unwrap(), + typ: FileChangeType::CHANGED, + }, + &fs, + ) .await; // Since FakeToolBuilder knows about "watcher.config", registrations are expected @@ -481,10 +504,13 @@ mod tests { assert_eq!(registrations[0].id, "watcher-FakeTool-file:///root/"); let (diagnostics, registrations, unregistrations) = worker - .did_change_watched_files(&FileEvent { - uri: Uri::from_str("file:///root/diagnostics.config").unwrap(), - typ: FileChangeType::CHANGED, - }) + .did_change_watched_files( + &FileEvent { + uri: Uri::from_str("file:///root/diagnostics.config").unwrap(), + typ: FileChangeType::CHANGED, + }, + &fs, + ) .await; // Since FakeToolBuilder knows about "diagnostics.config", diagnostics are expected assert!(diagnostics.is_some()); @@ -502,8 +528,14 @@ mod tests { .start_worker(serde_json::json!({"some_option": true}), &[Box::new(FakeToolBuilder)]) .await; + let fs = LSPFileSystem::default(); + fs.set( + &Uri::from_str("file:///root/diagnostics.config").unwrap(), + "hello world".to_string(), + ); + let (diagnostics, registrations, unregistrations) = - worker.did_change_configuration(serde_json::json!({"some_option": false})).await; + worker.did_change_configuration(serde_json::json!({"some_option": false}), &fs).await; // Since FakeToolBuilder does not change anything based on configuration, no diagnostics or registrations are expected assert!(diagnostics.is_none()); @@ -511,7 +543,7 @@ mod tests { assert_eq!(unregistrations.len(), 0); // No unregistrations expected let (diagnostics, registrations, unregistrations) = - worker.did_change_configuration(serde_json::json!(2)).await; + worker.did_change_configuration(serde_json::json!(2), &fs).await; // Since FakeToolBuilder changes watcher patterns based on configuration, registrations are expected assert!(diagnostics.is_none()); @@ -521,7 +553,7 @@ mod tests { assert_eq!(registrations[0].id, "watcher-FakeTool-file:///root/"); let (diagnostics, registrations, unregistrations) = - worker.did_change_configuration(serde_json::json!(3)).await; + worker.did_change_configuration(serde_json::json!(3), &fs).await; // Since FakeToolBuilder changes diagnostics based on configuration, diagnostics are expected assert!(diagnostics.is_some());