Answer document diagnostic pulls with the current text's diagnostics

docs/ARCHITECTURE.md

@@ -1423,6 +1423,18 @@ support sees it. Push delivery still caches each published set with a
 `resultId`, so a pull from a push client answers with what was last
 published instead of an empty set.
 
+A `textDocument/diagnostic` pull is held until the diagnostics reflect the
+document's current text, since editors pull right after every `didChange`
+and some (Neovim) do not pull the document again when the server sends
+`workspace/diagnostic/refresh`. The pull waits for startup to finish, for
+the edit's background parse, for the pass that parse queues, and for the
+pass in flight, without computing anything on the request path. Each stage
+hands the URI on before letting go of it (`edits_awaiting_parse`, then
+`pending_uris`, then `in_flight_uris` on `DiagnosticState`), so the wait
+never sees a gap. A newer pull for the same document releases the held
+one, and shutdown or the worker stopping releases every held pull. The
+`workspace/diagnostic` pull still answers at once from the cache.
+
 Each worker is created during `initialized` via `clone_for_blocking`, which hands it a `Backend` sharing the main one's `BackendState`, so every worker sees all mutations the main `Backend` makes. The PHPStan and PHPCS workers each have their own notify handle, pending-URI slot, and diagnostic cache so they run independently of each other and of the native diagnostic worker.
 
 Non-`Arc` fields are snapshotted at spawn time: `php_version`, `vendor_uri_prefixes`, and `vendor_dir_paths`. These fields are only written during `initialized` (before the workers are spawned) and never change afterwards. If a future feature adds runtime PHP version changes, the workers would need to be notified or re-cloned. This invariant ("init-time fields are write-once") should be verified before adding any post-init mutation to these fields.

docs/CHANGELOG.md

@@ -598,6 +598,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Same answers in the browser build.** Go-to-definition, rename, hover, signature help, and highlights in the WebAssembly build now resolve types as thoroughly as the language server does.
 - **`@see` links in hover.** A `@see` reference now links to the class it names, through the declaring file's `use` imports, rather than to another class with the same short name. Go-to-definition and type hierarchy no longer land in the wrong file in the same situation.
 - **Stale diagnostics after a quick edit.** An editor that pulls diagnostics no longer occasionally keeps showing the previous set when it asks for them just as a new set finishes.
+- **Diagnostics catch up after an edit in Neovim.** Fixing an error, such as closing a class's brace, now clears its diagnostic once the edit has been checked, where Neovim used to keep the old diagnostics until the file was reloaded with `:e`. A file opened while the project is still being indexed now shows its diagnostics once indexing finishes, without needing an edit first. Contributed by @stubbedev.
 - **Completion after an emoji.** Member, variable, array-key, class-name and PHPDoc tag completion now work on a line that has an emoji or other character outside the Basic Multilingual Plane before the cursor.
 - **A call still missing its `)`.** While you type a call or array whose closing bracket is not there yet, the rest of the class or function it is in keeps working: its imports are no longer reported as unused, and its members still complete, hover and resolve. The same goes for a statement being typed at the end of a file, such as the newest route in a routes file, from the moment its `(` is typed.
 - **Constants in `switch` cases.** Hover, go-to-definition, find references and rename now work on a constant or class constant written in a `case` label, and an import used only there is no longer reported as unused.

src/backend/documents.rs

@@ -175,9 +175,11 @@ impl Backend {
                 }
                 return;
             }
+            self.edit_awaiting_parse(&uri, &text);
             let backend = self.clone_for_blocking();
             tokio::spawn(async move {
                 let refresh_backend = backend.clone_for_blocking();
+                let (parsed_uri, parsed_text) = (uri.clone(), Arc::clone(&text));
                 let committed = run_blocking_cancel_safe("did_change resource scan", move || {
                     let is_latest_text = backend
                         .open_files
@@ -194,6 +196,7 @@ impl Backend {
                     true
                 })
                 .await;
+                refresh_backend.edit_parsed(&parsed_uri, &parsed_text);
 
                 if committed == Some(true) {
                     refresh_backend.request_code_lens_refresh().await;
@@ -217,10 +220,12 @@ impl Backend {
             }
             self.schedule_diagnostics(uri.clone());
         } else {
+            self.edit_awaiting_parse(&uri, &text);
             let backend = self.clone_for_blocking();
             tokio::spawn(async move {
                 let refresh_backend = backend.clone_for_blocking();
                 let uri_for_diagnostics = uri.clone();
+                let (parsed_uri, parsed_text) = (uri.clone(), Arc::clone(&text));
                 let committed = run_blocking_cancel_safe("did_change parse", move || {
                     let parse_lock = {
                         let mut locks = backend.did_change_parse_locks.lock();
@@ -260,6 +265,7 @@ impl Backend {
                     true
                 })
                 .await;
+                refresh_backend.edit_parsed(&parsed_uri, &parsed_text);
 
                 // A new symbol map was committed.  Tokens the editor already
                 // holds were computed from the pre-edit map (the

src/backend/startup.rs

@@ -408,10 +408,7 @@ impl Backend {
         // (php_version, vendor_uri_prefixes, vendor_dir_paths) are
         // snapshotted — they are only written during init (above) and
         // never change afterwards.
-        let worker_backend = self.clone_for_blocking();
-        tokio::spawn(async move {
-            worker_backend.diagnostic_worker().await;
-        });
+        self.spawn_diagnostic_worker();
 
         // Spawn the PHPStan worker as a separate background task.
         // PHPStan is extremely slow and resource-intensive, so it runs
@@ -562,6 +559,7 @@ impl Backend {
         // and pull handlers know the project is fully indexed.
         self.init_complete
             .store(true, std::sync::atomic::Ordering::Release);
+        self.diag.pass_done.notify_waiters();
 
         // Files opened during startup (before indexing finished) were
         // not diagnosed because `schedule_diagnostics` skips work when

src/diagnostics/delivery.rs

@@ -8,11 +8,31 @@ use tower_lsp_server::gen_lsp_types::*;
 
 use crate::Backend;
 
+use super::state::DiagnosticState;
 use super::{ignore_rules, incremental, stale, suppression};
 
 /// How long to wait after the last keystroke before publishing diagnostics.
 const DIAGNOSTIC_DEBOUNCE_MS: u64 = 500;
 
+/// Marks the diagnostic worker as running from the moment it is spawned
+/// for as long as its task lives, and wakes held document pulls when it
+/// stops however it stops.
+struct WorkerAlive(DiagnosticState);
+
+impl WorkerAlive {
+    fn start(state: &DiagnosticState) -> Self {
+        state.worker_alive.store(true, Ordering::Release);
+        Self(state.clone())
+    }
+}
+
+impl Drop for WorkerAlive {
+    fn drop(&mut self) {
+        self.0.worker_alive.store(false, Ordering::Release);
+        self.0.pass_done.notify_waiters();
+    }
+}
+
 impl Backend {
     /// Deliver diagnostics for a single file.
     ///
@@ -531,14 +551,12 @@ impl Backend {
         }
 
         // In pull mode we deliberately KEEP the previously cached full set.
-        // The pull handler no longer computes on the request path (that
-        // wedged the transport — see `trigger_diagnostics_for_pull`), so the
-        // editor would otherwise flicker to empty between an edit and the
-        // background recompute. Keeping the stale set means a pull returns
-        // the old diagnostics until the worker recomputes, bumps the
-        // resultId, and requests a refresh, which makes the editor re-pull
-        // the fresh set. The resultId is bumped only by `assemble_and_push`,
-        // so the "unchanged" fast-path stays correct.
+        // A document pull waits for the pass queued here (see
+        // `await_fresh_diagnostics`), but the workspace pull answers at
+        // once, and an empty set there would flicker the editor between
+        // an edit and the background recompute. The resultId is bumped
+        // only by `assemble_and_push`, so the "unchanged" fast-path stays
+        // correct.
 
         // Both modes: queue for the debounced background worker.
         // In push mode the worker pushes the full assembled set.
@@ -557,6 +575,32 @@ impl Backend {
         // at a time).
     }
 
+    /// Record that `text`, the latest edit of `uri`, is being parsed on a
+    /// background task that queues the file's diagnostic pass when done,
+    /// so a document pull sent meanwhile waits for that pass.
+    pub(crate) fn edit_awaiting_parse(&self, uri: &str, text: &Arc<String>) {
+        self.diag
+            .edits_awaiting_parse
+            .lock()
+            .insert(uri.to_string(), Arc::clone(text));
+    }
+
+    /// Called once the background parse of `text` is over, whether it
+    /// queued a pass, found a newer edit, or failed.  A newer edit's entry
+    /// is left to that edit's own parse.
+    pub(crate) fn edit_parsed(&self, uri: &str, text: &Arc<String>) {
+        {
+            let mut awaiting = self.diag.edits_awaiting_parse.lock();
+            if awaiting
+                .get(uri)
+                .is_some_and(|latest| Arc::ptr_eq(latest, text))
+            {
+                awaiting.remove(uri);
+            }
+        }
+        self.diag.pass_done.notify_waiters();
+    }
+
     /// Schedule all external tool runs (PHPStan, PHPCS, PHPMD, Mago) for a
     /// single file.
     ///
@@ -652,13 +696,11 @@ impl Backend {
     /// cached full diagnostics are stale or missing.  Does **not**
     /// compute synchronously; it only queues the file for the debounced
     /// background worker and returns immediately (see the comment inside
-    /// for why). The pull handler reads whatever is currently cached in
-    /// `diag_last_full`, which may be stale until the worker finishes and
-    /// requests a `workspace/diagnostic/refresh`.
+    /// for why).  The pull handler then waits for that pass before it
+    /// answers.
     pub(crate) fn trigger_diagnostics_for_pull(&self, uri_str: &str) {
         // Don't compute diagnostics before initialization is complete.
-        // The pull handler will return empty results; once `initialized`
-        // finishes it schedules all open files which populates the cache.
+        // The pull handler waits for it and calls this again.
         if !self.init_complete.load(Ordering::Acquire) {
             return;
         }
@@ -676,9 +718,9 @@ impl Backend {
         //
         // Instead, ensure the debounced background worker will (re)compute
         // this file off the async runtime; it caches the full set and
-        // requests a workspace diagnostic refresh when done. This pull
-        // returns whatever is currently cached (stale results are kept until
-        // the refresh, so the editor never flickers to empty).
+        // requests a workspace diagnostic refresh when done. The pull
+        // waits for that pass without occupying anything but its own
+        // request (see `await_fresh_diagnostics`).
         {
             let mut pending = self.diag.pending_uris.lock();
             pending.insert(uri_str.to_string());
@@ -712,7 +754,20 @@ impl Backend {
     /// the version counter will have moved, and step 1 picks up
     /// immediately after step 4 finishes — giving the two-slot
     /// (one running + one pending) behaviour.
-    pub(crate) async fn diagnostic_worker(&self) {
+    ///
+    /// The worker counts as running from this call on, so a document pull
+    /// that lands before the task is first polled already waits for it.
+    pub(crate) fn spawn_diagnostic_worker(&self) -> tokio::task::JoinHandle<()> {
+        let alive = WorkerAlive::start(&self.diag);
+        let worker = self.clone_for_blocking();
+        tokio::spawn(async move {
+            let _alive = alive;
+            worker.diagnostic_worker().await;
+        })
+    }
+
+    /// The loop [`Self::spawn_diagnostic_worker`] runs.
+    async fn diagnostic_worker(&self) {
         loop {
             if self.shutdown_flag.load(Ordering::Acquire) {
                 return;
@@ -738,9 +793,14 @@ impl Backend {
             }
 
             // ── Step 3: snapshot all pending URIs ────────────────────
+            // They move to `in_flight_uris` under the `pending_uris`
+            // guard, so a pull held on one of them never sees it as done
+            // before its pass has delivered.
             let uris: std::collections::HashSet<String> = {
                 let mut pending = self.diag.pending_uris.lock();
-                std::mem::take(&mut *pending)
+                let uris = std::mem::take(&mut *pending);
+                self.diag.in_flight_uris.lock().extend(uris.iter().cloned());
+                uris
             };
             if uris.is_empty() {
                 continue;
@@ -751,14 +811,12 @@ impl Backend {
             // read lock before each async publish call so that
             // `did_change` is never blocked.
             for uri in &uris {
-                let content = {
-                    let files = self.open_files.read();
-                    match files.get(uri) {
-                        Some(c) => c.clone(),
-                        None => continue,
-                    }
-                };
-                self.publish_diagnostics_for_file(uri, &content).await;
+                let content = self.open_files.read().get(uri).cloned();
+                if let Some(content) = content {
+                    self.publish_diagnostics_for_file(uri, &content).await;
+                }
+                self.diag.in_flight_uris.lock().remove(uri);
+                self.diag.pass_done.notify_waiters();
             }
         }
     }