harper-ls: debounce diagnostics after edits

Keep delayed diagnostics simple and bug-focused by debouncing didChange publishes, cleaning up pending task state, and removing the race-prone post-code-action refresh path.

before getting rid of the generation

now generation

pending_diagnostic_task

move publish_immediately

get rid of the test

Author: Dronakurl

harper-ls/Cargo.toml

@@ -21,7 +21,7 @@ harper-html = { path = "../harper-html", version = "2.0.0" }
 harper-python = { path = "../harper-python", version = "2.0.0" }
 harper-asciidoc = { path = "../harper-asciidoc", version = "2.0.0" }
 tower-lsp-server = "0.22.1"
-tokio = { version = "1.52.1", default-features = false, features = ["fs", "io-std", "io-util", "macros", "net", "rt-multi-thread", "sync"] }
+tokio = { version = "1.52.1", default-features = false, features = ["fs", "io-std", "io-util", "macros", "net", "rt-multi-thread", "sync", "test-util", "time"] }
 clap = { version = "4.6.0", default-features = false, features = ["derive", "std"] }
 dirs = "6.0.0"
 anyhow = "1.0.102"

harper-ls/src/backend.rs

@@ -4,6 +4,7 @@ use std::io::{BufWriter, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::sync::atomic::{AtomicU64, Ordering};
+use std::time::Duration;
 
 use crate::config::Config;
 use crate::document_state::DocumentState;
@@ -36,7 +37,7 @@ use tower_lsp_server::lsp_types::notification::{Progress, PublishDiagnostics};
 use tower_lsp_server::lsp_types::request::WorkDoneProgressCreate;
 use tower_lsp_server::lsp_types::{
     CodeActionOrCommand, CodeActionParams, CodeActionProviderCapability, CodeActionResponse,
-    Diagnostic, DidChangeConfigurationParams, DidChangeTextDocumentParams,
+    ConfigurationItem, Diagnostic, DidChangeConfigurationParams, DidChangeTextDocumentParams,
     DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams,
     DidSaveTextDocumentParams, ExecuteCommandOptions, ExecuteCommandParams, FileChangeType,
     InitializeParams, InitializeResult, InitializedParams, MessageType, NumberOrString,
@@ -53,14 +54,16 @@ pub fn ls_version() -> &'static str {
     env!("CARGO_PKG_VERSION")
 }
 
+#[derive(Clone)]
 pub struct Backend {
     client: Client,
-    root: RwLock<PathBuf>,
-    config: RwLock<Config>,
-    stats: RwLock<Stats>,
-    doc_state: Mutex<HashMap<Uri, DocumentState>>,
-    progress_counter: AtomicU64,
-    lang_detect: LanguageDetectionRegistry,
+    root: Arc<RwLock<PathBuf>>,
+    config: Arc<RwLock<Config>>,
+    stats: Arc<RwLock<Stats>>,
+    doc_state: Arc<Mutex<HashMap<Uri, DocumentState>>>,
+    progress_counter: Arc<AtomicU64>,
+    lang_detect: Arc<LanguageDetectionRegistry>,
+    pending_diagnostic_tasks: Arc<Mutex<HashMap<Uri, tokio::task::JoinHandle<()>>>>,
 }
 
 const MIN_WORDS_FOR_LANGUAGE_DETECTION: usize = 10;
@@ -69,12 +72,13 @@ impl Backend {
     pub fn new(client: Client, config: Config) -> Self {
         Self {
             client,
-            root: RwLock::new(".".into()),
-            stats: RwLock::new(Stats::new()),
-            config: RwLock::new(config),
-            doc_state: Mutex::new(HashMap::new()),
-            progress_counter: AtomicU64::new(1),
-            lang_detect: LanguageDetectionRegistry::new(),
+            root: Arc::new(RwLock::new(".".into())),
+            stats: Arc::new(RwLock::new(Stats::new())),
+            config: Arc::new(RwLock::new(config)),
+            doc_state: Arc::new(Mutex::new(HashMap::new())),
+            progress_counter: Arc::new(AtomicU64::new(1)),
+            lang_detect: Arc::new(LanguageDetectionRegistry::new()),
+            pending_diagnostic_tasks: Arc::new(Mutex::new(HashMap::new())),
         }
     }
 
@@ -759,6 +763,13 @@ impl Backend {
         }
     }
 
+    /// The async pending diagnostics are discarded and diagnostics are created
+    /// and published immediately.
+    async fn publish_diagnostics_immediately(&self, uri: &Uri) {
+        self.abort_pending_diagnostics(uri).await;
+        self.publish_diagnostics(uri).await;
+    }
+
     /// Update the configuration of the server and publish document updates that
     /// match it.
     async fn update_config_from_obj(&self, json_obj: Value) {
@@ -795,6 +806,41 @@ impl Backend {
             *config = new_config;
         }
     }
+    async fn pull_config(&self) {
+        let mut new_config = self
+            .client
+            .configuration(vec![ConfigurationItem {
+                scope_uri: None,
+                section: None,
+            }])
+            .await
+            .unwrap_or(vec![json!({ "harper-ls": {} })]);
+
+        if let Some(first) = new_config.pop() {
+            self.update_config_from_obj(first).await;
+        }
+    }
+
+    /// Aborts a pending diagnostic run, e.g. because the document was changed.
+    async fn abort_pending_diagnostics(&self, uri: &Uri) {
+        let mut tasks = self.pending_diagnostic_tasks.lock().await;
+        if let Some(task) = tasks.remove(uri) {
+            task.abort();
+        }
+    }
+
+    /// Schedule diagnostics in `delay_ms` milliseconds
+    pub async fn schedule_diagnostics(&self, uri: &Uri, delay_ms: u64) {
+        let task_uri = uri.clone();
+        let backend = self.clone();
+        let handle = tokio::spawn(async move {
+            tokio::time::sleep(Duration::from_millis(delay_ms)).await;
+            backend.publish_diagnostics(&task_uri).await;
+        });
+
+        let mut tasks = self.pending_diagnostic_tasks.lock().await;
+        tasks.insert(uri.clone(), handle);
+    }
 }
 
 impl LanguageServer for Backend {
@@ -875,7 +921,8 @@ impl LanguageServer for Backend {
             error!("{err}");
         }
 
-        self.publish_diagnostics(&params.text_document.uri).await;
+        self.publish_diagnostics_immediately(&params.text_document.uri)
+            .await;
         self.end_progress(progress, "Analysis complete").await;
     }
 
@@ -895,15 +942,25 @@ impl LanguageServer for Backend {
             error!("{err}")
         }
 
-        self.publish_diagnostics(&params.text_document.uri).await;
+        let delay_ms = self.config.read().await.diagnostic_delay_ms;
+        if delay_ms > 0 {
+            // Diagnostics delay is configured, abort pending diagnostic task
+            // and schedule a new diagnostic run in delay_ms milliseconds.
+            self.abort_pending_diagnostics(&params.text_document.uri)
+                .await;
+            self.schedule_diagnostics(&params.text_document.uri, delay_ms)
+                .await;
+        } else {
+            self.publish_diagnostics(&params.text_document.uri).await;
+        }
         self.end_progress(progress, "Diagnostics updated").await;
     }
 
     async fn did_close(&self, _params: DidCloseTextDocumentParams) {
         let uri = _params.text_document.uri;
         let mut doc_lock = self.doc_state.lock().await;
         doc_lock.remove(&uri);
-
+        self.abort_pending_diagnostics(&uri).await;
         self.client
             .send_notification::<PublishDiagnostics>(PublishDiagnosticsParams {
                 uri: uri.clone(),
@@ -935,6 +992,7 @@ impl LanguageServer for Backend {
         }
 
         for uri in &uris_to_clear {
+            self.abort_pending_diagnostics(uri).await;
             self.client
                 .send_notification::<PublishDiagnostics>(PublishDiagnosticsParams {
                     uri: uri.clone(),
@@ -989,7 +1047,7 @@ impl LanguageServer for Backend {
                     .await
                     .map_err(|err| error!("{err}"))
                     .err();
-                self.publish_diagnostics(&file_uri).await;
+                self.publish_diagnostics_immediately(&file_uri).await;
             }
             "HarperAddToWSDict" => {
                 let word = &first.chars().collect::<Vec<_>>();
@@ -1011,7 +1069,7 @@ impl LanguageServer for Backend {
                     .await
                     .map_err(|err| error!("{err}"))
                     .err();
-                self.publish_diagnostics(&file_uri).await;
+                self.publish_diagnostics_immediately(&file_uri).await;
             }
             "HarperAddToFileDict" => {
                 let word = &first.chars().collect::<Vec<_>>();
@@ -1043,7 +1101,7 @@ impl LanguageServer for Backend {
                     .await
                     .map_err(|err| error!("{err}"))
                     .err();
-                self.publish_diagnostics(&file_uri).await;
+                self.publish_diagnostics_immediately(&file_uri).await;
             }
             "HarperOpen" => match open::that(&first) {
                 Ok(()) => {
@@ -1093,7 +1151,7 @@ impl LanguageServer for Backend {
 
                 drop(doc_lock);
 
-                self.publish_diagnostics(&uri).await;
+                self.publish_diagnostics_immediately(&uri).await;
             }
             _ => (),
         }
@@ -1128,7 +1186,7 @@ impl LanguageServer for Backend {
                 .await
                 .map_err(|err| error!("{err}"))
                 .err();
-            self.publish_diagnostics(&uri).await;
+            self.publish_diagnostics_immediately(&uri).await;
         }
     }
 
@@ -1155,6 +1213,7 @@ impl LanguageServer for Backend {
 
         // Clears the diagnostics for open buffers (without holding lock)
         for uri in &uris {
+            self.abort_pending_diagnostics(uri).await;
             let result = PublishDiagnosticsParams {
                 uri: uri.clone(),
                 diagnostics: vec![],

harper-ls/src/config.rs

@@ -83,6 +83,9 @@ pub struct Config {
     /// Above this limit, the file will not be linted.
     pub max_file_length: usize,
     pub exclude_patterns: GlobSet,
+    /// Delay in milliseconds after typing stops before diagnostics are published.
+    /// Set to 0 to publish diagnostics immediately.
+    pub diagnostic_delay_ms: u64,
 }
 
 impl Config {
@@ -276,6 +279,10 @@ impl Config {
             }
         }
 
+        if let Some(v) = value.get("diagnosticDelayMs") {
+            base.diagnostic_delay_ms = serde_json::from_value(v.clone())?;
+        }
+
         Ok(base)
     }
 }
@@ -298,6 +305,7 @@ impl Default for Config {
             language: Language::default(),
             max_file_length: 120_000,
             exclude_patterns: GlobSet::empty(),
+            diagnostic_delay_ms: 0,
         }
     }
 }
@@ -308,6 +316,28 @@ mod tests {
     use serde_json::json;
     use tempfile::TempDir;
 
+    #[test]
+    fn parses_diagnostic_delay() {
+        let config = Config::from_lsp_config(
+            std::path::Path::new("."),
+            json!({
+                "harper-ls": {
+                    "diagnosticDelayMs": 750
+                }
+            }),
+        )
+        .unwrap();
+
+        assert_eq!(config.diagnostic_delay_ms, 750);
+    }
+
+    #[test]
+    fn defaults_diagnostic_delay_to_zero() {
+        let config = Config::default();
+
+        assert_eq!(config.diagnostic_delay_ms, 0);
+    }
+
     #[test]
     fn stats_path_config_does_not_override_file_dict_path() {
         let tempdir = TempDir::new().unwrap();