Skip to content

Commit 39e28ef

Browse files
committed
Activate chain resolution cache for all LSP handlers
1 parent 6e735b7 commit 39e28ef

6 files changed

Lines changed: 16 additions & 68 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727

2828
- **Rewritten variable resolver.** Variable type resolution now uses a single top-to-bottom pass through each function body on both the diagnostic and completion paths, with zero recursion and no depth limit.
2929
- **Broader type narrowing.** `instanceof`, type-guard functions (`is_array`, `is_string`, etc.), `in_array()` strict mode, `assert()`, `@phpstan-assert-if-true`/`-if-false`, and compound `&&`/`||` conditions now narrow variable types in if/else branches, guard clauses, while-loop bodies, ternary expressions, and `match(true)` arms. Negated guards and compound conditions like `if (!$x instanceof A && !$x instanceof B) { return; }` are handled correctly.
30+
- **Chain resolution cache.** The per-request chain resolution cache that eliminates redundant re-resolution of shared chain prefixes is now active for all LSP handlers (completion, hover, go-to-definition, signature help, etc.), not just diagnostics.
3031
- **Closure and arrow function parameter inference.** Untyped closure and arrow function parameters are inferred from the enclosing call's callable signature, including through method chains that return `static`. Generic type substitution flows through to inferred parameters (e.g. a `callable(T)` parameter on a `Repository<Product>` resolves `T` to `Product`).
3132
- **Editing responsiveness.** Classes evicted from the resolved-class cache after a file edit are now eagerly re-populated in dependency order, eliminating lazy resolution delays for diagnostics and completions.
3233
- **Virtual member resolution.** Virtual member providers (PHPDoc mixins, Laravel model, Eloquent Builder) now resolve completely on every class, eliminating cases where mixins or Eloquent accessors were missing after edits.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ within the same impact tier.
2525

2626
| # | Item | Impact | Effort |
2727
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
28-
| P26 | [Chain resolution cache only active during diagnostics](todo/performance.md#p26-chain-resolution-cache-only-active-during-diagnostics) | Medium | Low |
2928
| P24 | [`IN_ARRAY_KEY_ASSIGN` re-entry guard in forward walker](todo/performance.md#p24-in_array_key_assign-re-entry-guard-in-forward-walker) | Low | Low |
3029
| P20 | [Forward walker: bounded iteration and depth-cap cleanup](todo/performance.md#p20-forward-walker-bounded-iteration-and-depth-cap-cleanup) | High | Medium |
3130
| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5--mago-style-separated-metadata) | High | High |

docs/todo/performance.md

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -884,67 +884,4 @@ path entirely and the `IN_ARRAY_KEY_ASSIGN` guard can be removed.
884884

885885
---
886886

887-
## P26. Chain resolution cache only active during diagnostics
888887

889-
**Impact: Medium · Effort: Low**
890-
891-
`completion/resolver.rs` lines ~41-65 define a `CHAIN_CACHE`
892-
thread-local that caches `resolve_target_classes` results by subject
893-
text. This cache is activated by `with_chain_resolution_cache` and
894-
is only enabled during diagnostic passes (via `analyse.rs`). During
895-
completion and hover, the cache is inactive and every chain prefix
896-
is re-resolved from scratch.
897-
898-
This means that typing `$model->where(...)->orderBy(...)->` in a
899-
completion context re-resolves `$model->where(...)` even though it
900-
was just resolved moments ago for `$model->where(...)->orderBy(...)`.
901-
902-
Similarly, the `DIAGNOSTIC_SCOPE` cache (forward walker scope
903-
snapshots, lines ~57-156) is only populated during diagnostic
904-
passes. Hover and completion use a separate code path
905-
(`resolve_variable_types` with backward scanning or forward walk
906-
without caching), which can produce subtly different results because
907-
the diagnostic path's forward walker records scope at every
908-
statement boundary while the completion path only walks to the
909-
cursor position.
910-
911-
### What this causes
912-
913-
1. **Completion is slower than diagnostics** for chain expressions,
914-
because diagnostics benefit from the chain cache but completion
915-
does not.
916-
2. **Hover and completion can disagree** on a variable's type when
917-
the forward walker's single-cursor walk produces a different
918-
scope state than the diagnostic pass's full-file walk (e.g. due
919-
to branch merging order or loop depth cutoffs).
920-
921-
### How the reference projects handle this
922-
923-
PHPStan's `MutatingScope` stores a `$resolvedTypes` array that
924-
caches every expression type resolved via `getType()`. The scope is
925-
the same object regardless of whether the caller is a rule
926-
(diagnostic), a return type extension, or any other consumer. There
927-
is no consumer-specific mode.
928-
929-
Phpactor has three caching layers that are all consumer-agnostic:
930-
(1) `NodeContextResolver` caches per AST node via `spl_object_id`.
931-
(2) `FrameResolver` caches the entire variable frame per scope node.
932-
(3) `MemonizedReflector` caches reflected classes. All three are
933-
active for every operation (completion, hover, go-to-definition).
934-
The source comment notes: "The cache should only have a lifetime of
935-
the current operation."
936-
937-
### Fix
938-
939-
1. Activate the chain resolution cache for completion and hover
940-
requests, not just diagnostics. The cache is cheap (a HashMap
941-
cleared per request) and the chain prefix re-resolution is the
942-
dominant cost for fluent API chains.
943-
2. Unify the scope resolution path: when the diagnostic scope cache
944-
is available (file has been analyzed), use it for hover and
945-
completion as well. Fall back to cursor-targeted forward walk
946-
only when no cached scopes exist.
947-
3. Long-term: move toward a per-request expression type cache
948-
(similar to PHPStan's `$resolvedTypes` or Phpactor's
949-
`spl_object_id` cache) that all consumers share, eliminating
950-
the need for separate chain and scope caches.

src/completion/handler.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,11 @@ impl Backend {
269269
let content = self.get_file_content(&uri);
270270

271271
if let Some(content) = content {
272+
// Activate the chain resolution cache so that shared chain
273+
// prefixes are resolved once and reused within this completion
274+
// request. The guard is re-entrant safe.
275+
let _chain_guard = super::resolver::with_chain_resolution_cache();
276+
272277
// Gather per-file context (classes, use-map, namespace) in one
273278
// call instead of three separate lock-and-unwrap blocks.
274279
let ctx = self.file_context(&uri);

src/completion/resolver.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ use crate::virtual_members::resolve_class_fully_maybe_cached;
5151
// the first method call 5 times, etc. — O(depth²) total work.
5252
//
5353
// The chain cache stores `resolve_target_classes` results keyed by the
54-
// raw subject text string. It is activated at the diagnostic loop level
55-
// (via [`with_chain_resolution_cache`]) and consulted by
56-
// `resolve_target_classes` before doing any work. When the cache is not
57-
// active (completion, hover, etc.) the function behaves exactly as before.
54+
// raw subject text string. It is activated per-request for all LSP
55+
// handlers (completion, hover, definition, diagnostics, etc.) via
56+
// [`with_chain_resolution_cache`] and consulted by
57+
// `resolve_target_classes` before doing any work.
5858

5959
thread_local! {
6060
/// When `Some`, `resolve_target_classes` will consult and populate

src/server.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,12 @@ impl Backend {
10221022
f: impl FnOnce(&str) -> T,
10231023
) -> Option<T> {
10241024
let content = self.get_file_content(uri)?;
1025+
// Activate the chain resolution cache so that shared chain prefixes
1026+
// (e.g. `$model->where(...)` in `$model->where(...)->orderBy(...)`)
1027+
// are resolved once and reused across all LSP handlers, not just
1028+
// diagnostics. The guard is re-entrant safe: if a diagnostic pass
1029+
// already activated the cache, this is a no-op.
1030+
let _chain_guard = crate::completion::resolver::with_chain_resolution_cache();
10251031
crate::util::catch_panic_unwind_safe(handler_name, uri, position, || f(&content))
10261032
}
10271033

0 commit comments

Comments
 (0)