From d6f35ba4f14cd373c4c8311a7dfa145bb84fb15e Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Tue, 25 Aug 2026 02:31:43 -0300 Subject: [PATCH] Drop stale analyses when the index becomes ready A file opened while the workspace scan is still running is analyzed against whatever has been indexed by then, so it can report a symbol as missing purely because the scan had not yet reached the file declaring it. `compute_open_file_diagnostics` knows that and holds UndefinedClass and UndefinedFunction back until `is_index_ready()` flips, and `republish_after_index_ready` sends the fuller set once it has. Holding them back is only half of it. The analysis cache is keyed on `(source, decl_version)` and a finishing scan changes neither, so the republish re-renders the same memo through an open gate -- and publishes exactly the stale findings the gate existed to hide. They stay through pull requests as well as pushes, until somebody types a character in the file, which changes the source, misses the cache, and produces the right answer. This is the case 9149c80a left. That commit fixed the same staleness for `didChangeWatchedFiles` by calling `note_new_file_declarations` for every CREATED/CHANGED file; the initial scan reaches the store through `mirror_text`/`ingest_from_doc` and calls neither. Issue #242's third acceptance criterion -- "Full semantic diagnostics are available after indexing completes" -- is not met until both paths invalidate. So `mark_index_ready` bumps `decl_version` on its way through. That is what the counter is for: `evict_analysis_all` already carries the same meaning for a PHP-version change, and "the workspace is now complete" is the same claim about the same cache. Bumping once at the transition also answers the objection recorded in ae512858, which is why this was not fixed there: bumping per scanned file would thrash the cache during a large scan, and one bump at readiness costs one analysis per open file, once per session. The new test is the deterministic sibling of `stale_cached_analysis_not_invalidated_by_new_dependency_file`, with the scan's `mirror_text`-and-nothing-else in place of that path's explicit `note_new_file_declarations`. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 ++++ src/document/document_store.rs | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 942ddb75..b961e413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to php-lsp are documented here. ## [Unreleased] +### Fixed + +- **Stale diagnostics survive the initial workspace scan**: a file opened while indexing was still running was analyzed against a partial index, and the analysis cache — keyed on `(source, decl_version)` — was never invalidated when the scan finished, so the post-index republish re-served the same memo. Symbols declared in files the scan had not yet reached stayed reported as undefined until the file was edited. `mark_index_ready` now bumps `decl_version`, the same invalidation `note_new_file_declarations` performs on the `didChangeWatchedFiles` path. + ## [0.25.1] — 2026-08-22 ### Fixed diff --git a/src/document/document_store.rs b/src/document/document_store.rs index 875dc0d5..26420512 100644 --- a/src/document/document_store.rs +++ b/src/document/document_store.rs @@ -470,6 +470,10 @@ impl DocumentStore { /// Mark the workspace reference index as fully built. Called by the scan /// when its final phase completes (alongside `$/php-lsp/indexReady`). pub fn mark_index_ready(&self) { + // Every cached analysis predates the finished workspace, and the + // scan bumped nothing on its way through — `note_new_file_declarations` + // is the same invalidation on the `didChangeWatchedFiles` path. + self.caches.bump_decl_version(); self.index_ready.store(true, Ordering::Release); } @@ -4022,6 +4026,39 @@ mod tests { ); } + /// The same staleness, reached by the other caller on that path: the + /// initial workspace scan mirrors every file it reads and never calls + /// `note_new_file_declarations`, so a file opened while the scan is still + /// running keeps the memo it left behind against a partial index. + #[test] + fn stale_cached_analysis_not_invalidated_by_finished_scan() { + let store = DocumentStore::new(); + let consumer_uri = uri("/app.php"); + let dep_uri = uri("/Mage.php"); + + store.mirror_text(&consumer_uri, "