From fefb0d0baaf344240fd8f56ebc76b85e83935f97 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Tue, 21 Jul 2026 07:10:09 +0000 Subject: [PATCH 1/2] perf(hover): cut hover latency by caching + warming reflection A hover on a typed variable fanned worse-reflection's reflectOffset out into many PHP-stdlib reflections with memoization disabled, and the first stub reflection paid a cold ~512M phpstorm-stubs map build. Hover took 1.6-3.5s and PhpStorm cancelled it before the popup painted (downstream ticket hover-latency-unindexed-native-symbols). Three fixes: - Enable worse-reflection's reflection cache on the session reflector, collapsing the ~12x same-class reflections a single reflectOffset does. The cache is name-keyed with no source version, so ReflectionCachePurger flushes it on every didChange; purges land between hovers, preserving the intra-hover memoization while an edit always forces re-reflection. - StubCacheWarmer forces the cold phpstorm-stubs map build off the Initialized handshake (async), so the first hover doesn't pay it. - Variable hover fast-path: a bare $var cursor whose type GenericResolver can pin renders directly, skipping reflectOffset. Strictly additive -- any miss falls through to the unchanged path. Unit 1228 green, behat 111 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/LspDispatcherFactory.php | 20 ++- src/Reflection/ReflectionCachePurger.php | 50 ++++++ src/Reflection/ReflectorFactory.php | 55 ++++++- src/Reflection/StubCacheWarmer.php | 88 +++++++++++ src/Resolver/PhpHoverResolver.php | 76 +++++++++ test/Reflection/ReflectionCachePurgerTest.php | 142 +++++++++++++++++ test/Reflection/ReflectorFactoryTest.php | 24 +++ test/Reflection/StubCacheWarmerTest.php | 145 ++++++++++++++++++ test/Resolver/PhpHoverResolverTest.php | 53 +++++++ 9 files changed, 645 insertions(+), 8 deletions(-) create mode 100644 src/Reflection/ReflectionCachePurger.php create mode 100644 src/Reflection/StubCacheWarmer.php create mode 100644 test/Reflection/ReflectionCachePurgerTest.php create mode 100644 test/Reflection/StubCacheWarmerTest.php diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index 06c57dd..72e99d3 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -151,7 +151,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // map, WorkspaceSymbols' open-doc walk, WorkspaceClassLikeLookup's // open-doc walk). Phase-0 of the LSP follow-up roadmap. $fqnIndex = new FqnIndex($workspace, $cache, $xphpParser, $rootPath, $extraSourceRoots, $excludedDirs); - $reflector = (new ReflectorFactory( + $reflectorFactory = new ReflectorFactory( $workspace, $cache, $xphpParser, @@ -159,7 +159,8 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia ReflectorFactory::defaultStubPath(), ReflectorFactory::defaultCacheDir(), $fqnIndex, - ))->build(); + ); + $reflector = $reflectorFactory->build(); // Per-session registry of (namespace, paramName) pairs harvested from // generic ClassLike declarations in open documents. Resolvers query // this when formatting type names so a post-strip placeholder @@ -248,6 +249,21 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // pay the ~500ms filesystem-walk cost in-band. Async via // Amp\asyncCall -- doesn't block the initialize handshake. new \XPHP\Lsp\Reflection\FqnIndexWarmer($fqnIndex), + // Hover-latency fix (downstream ticket + // hover-latency-unindexed-native-symbols): force the cold + // phpstorm-stubs map build off the `Initialized` event so the + // first hover on a typed variable -- which fans out into stdlib + // reflections -- doesn't pay the multi-second build in-band and + // miss PhpStorm's hover-cancel window. Async, like the FQN + // warmer; no-ops gracefully when stubs aren't bundled. + new \XPHP\Lsp\Reflection\StubCacheWarmer($reflector), + // Correctness guard for the reflection cache enabled above: the + // cache is keyed by symbol name with no source-version, so an + // edit to an open buffer must flush it or hover/completion would + // serve pre-edit reflections. Purges on every didChange, which + // lands between user actions and so never undoes the intra-hover + // memoization that makes hover fast. + new \XPHP\Lsp\Reflection\ReflectionCachePurger($reflectorFactory->reflectionCache()), // Multi-root (Track A): when a file from a project OUTSIDE the // workspace root is opened, fold that project's source roots into the // FQN index so navigation resolves its symbols. Keys purely off the diff --git a/src/Reflection/ReflectionCachePurger.php b/src/Reflection/ReflectionCachePurger.php new file mode 100644 index 0000000..931db0c --- /dev/null +++ b/src/Reflection/ReflectionCachePurger.php @@ -0,0 +1,50 @@ +cache->purge(); + } +} diff --git a/src/Reflection/ReflectorFactory.php b/src/Reflection/ReflectorFactory.php index 067c3a7..47646e0 100644 --- a/src/Reflection/ReflectorFactory.php +++ b/src/Reflection/ReflectorFactory.php @@ -7,6 +7,8 @@ use Phpactor\LanguageServer\Core\Workspace\Workspace as PhpactorWorkspace; use Phpactor\WorseReflection\Reflector; use Phpactor\WorseReflection\ReflectorBuilder; +use Phpactor\WorseReflection\Core\Cache; +use Phpactor\WorseReflection\Core\Cache\TtlCache; use Phpactor\WorseReflection\Core\SourceCodeLocator\StubSourceLocator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -35,12 +37,29 @@ * (Iterator, Generator, ArrayAccess, ...) for which worse-reflection * ships purpose-built reflections. * - * Why no caching on the Reflector itself: worse-reflection's `enableCache()` - * uses a TTL cache (default 5s) keyed by reflection class -- useful when - * one logical user action triggers many reflection lookups (completion - * after `$obj->|` queries the class repeatedly). For GTD / hover (one - * lookup per action) it's wasted complexity. Revisit if a profile shows - * repeated lookups in completion. + * Reflection cache: `enableCache()` wraps class reflection in a memoizing + * cache keyed by reflection name. We enable it because the premise of "one + * lookup per action" is false: worse-reflection reflects the same symbol + * many times within a SINGLE `reflectOffset` -- ~12 lookups of one + * `ReflectionMethod` per hover per the {@see FilesystemSourceLocator} note, + * plus a fan-out across every stdlib symbol on a typed variable's + * right-hand side, each of which consults the `StubSourceLocator` and + * unserializes the whole phpstorm-stubs map. Uncached, that work is + * redone every time and dominates hover latency. Memoizing collapses the + * intra-hover repeats to one reflection each -- the dominant win, and the + * reason a hover lands inside PhpStorm's ~200-300ms cancel window. + * Downstream ticket: phpstorm-plugin hover-latency-unindexed-native-symbols. + * + * Invalidation: the cache is keyed by NAME with no source-version + * component, so a stale reflection of a class the user just edited in an + * open buffer would otherwise survive. We inject a shared {@see Cache} + * (exposed via {@see reflectionCache}) that `ReflectionCachePurger` flushes + * on every `TextDocumentUpdated` -- purges happen BETWEEN hovers, never + * during one, so the intra-hover memoization above is fully preserved while + * an edit always forces re-reflection on the next hover. + * + * The throwaway bootstrap reflector below is deliberately left uncached: + * it exists only to build the on-disk stub map once and is discarded. * * Bootstrap caveat: `StubSourceLocator` needs a `Reflector` in its * constructor (it uses it to discover FQNs in stub files when building @@ -62,6 +81,15 @@ final class ReflectorFactory * workspace / filesystem lookup still works. * @param string $cacheDir Writable dir for the stub map cache. */ + /** + * Shared reflection cache handed to the built reflector via + * `withCache`. Held here so the caller can wire it to + * `ReflectionCachePurger` for edit-driven invalidation -- see the + * class docblock. Always created (even for the stubs-less reflector) + * so the purger wiring is unconditional. + */ + private readonly Cache $reflectionCache; + public function __construct( private readonly PhpactorWorkspace $workspace, private readonly ParsedDocumentCache $cache, @@ -71,6 +99,17 @@ public function __construct( private readonly string $cacheDir, private readonly FqnIndex $fqnIndex, ) { + $this->reflectionCache = new TtlCache(); + } + + /** + * The reflection cache backing the reflector this factory builds. + * Flush it (via {@see Cache::purge}) when an open document changes so + * name-keyed reflections of edited classes don't go stale. + */ + public function reflectionCache(): Cache + { + return $this->reflectionCache; } public function build(): Reflector @@ -85,6 +124,8 @@ public function build(): Reflector return ReflectorBuilder::create() ->addLocator($workspaceLocator, priority: 100) ->addLocator($filesystemLocator, priority: 50) + ->enableCache() + ->withCache($this->reflectionCache) ->build(); } @@ -106,6 +147,8 @@ public function build(): Reflector ->addLocator($workspaceLocator, priority: 100) ->addLocator($filesystemLocator, priority: 50) ->addLocator($stubLocator, priority: 25) + ->enableCache() + ->withCache($this->reflectionCache) ->build(); } diff --git a/src/Reflection/StubCacheWarmer.php b/src/Reflection/StubCacheWarmer.php new file mode 100644 index 0000000..5665ff7 --- /dev/null +++ b/src/Reflection/StubCacheWarmer.php @@ -0,0 +1,88 @@ + file map to + * disk -- a multi-second, ~512M-peak operation (see the Makefile note on + * the behat stub-cache pre-warm). Paid in-band, it lands the hover + * response well outside PhpStorm's ~200-300ms hover-cancel window and the + * popup never paints. Building it off the `Initialized` event moves that + * cost off the first hover; once the map is serialized on disk subsequent + * sessions skip the build entirely. + * + * Warm path: reflect a single global stdlib function (`strlen`). Functions + * resolve only through the stub locator (not the InternalLocator, which + * covers a handful of fundamental interfaces), so this reliably drives the + * stub map build. Any failure (stubs genuinely absent, reflection error) + * is swallowed -- warming is best-effort and must never destabilise the + * session. + * + * Mirrors {@see FqnIndexWarmer}: same `Initialized` hook, same + * `Amp\asyncCall` dispatch so the warm-up runs off the synchronous + * initialize handshake. Registered in `LspDispatcherFactory::create` + * unconditionally; when stubs aren't bundled the warm reflection simply + * throws and is swallowed (logged as "skipped"), so there's no need to + * gate the registration. + */ +class StubCacheWarmer implements ListenerProviderInterface +{ + /** + * A global stdlib function guaranteed to live in phpstorm-stubs and to + * resolve exclusively via the stub locator, so reflecting it forces the + * stub map build. + */ + private const WARM_FUNCTION = 'strlen'; + + public function __construct(private readonly Reflector $reflector) + { + } + + /** + * {@inheritDoc} + */ + public function getListenersForEvent(object $event): iterable + { + if ($event instanceof Initialized) { + return [[$this, 'warm']]; + } + return []; + } + + public function warm(Initialized $initialized): void + { + asyncCall(function (): void { + try { + $this->reflector->reflectFunction(self::WARM_FUNCTION); + Stderr::write("[xphp-lsp warmer] stub-cache warmed\n"); + } catch (Throwable $e) { + // Best-effort: a stubs-less build or a reflection hiccup must + // not take the session down. The first real hover will still + // work (just paying the cold cost it would have paid anyway). + Stderr::write(sprintf( + "[xphp-lsp warmer] stub-cache warm skipped: %s\n", + $e->getMessage(), + )); + } + }); + } +} diff --git a/src/Resolver/PhpHoverResolver.php b/src/Resolver/PhpHoverResolver.php index 4b67823..20d7763 100644 --- a/src/Resolver/PhpHoverResolver.php +++ b/src/Resolver/PhpHoverResolver.php @@ -110,6 +110,29 @@ private function resolveInner(string $uri, int $line, int $character, ?Cancellat } $document = $this->workspace->get($uri); $offset = (new PositionMap($document->text))->positionToOffset($line, $character); + + // Variable fast-path (hover-latency fix, downstream ticket + // hover-latency-unindexed-native-symbols): when the cursor sits on a + // plain `$var` whose concrete type the GenericResolver can pin from + // in-file bindings / parameters, render it directly and skip + // reflectOffset -- the single heaviest op in the hover path, which + // otherwise fans a typed-variable hover out into stdlib reflection. + // This produces exactly the same output the Symbol::VARIABLE branch + // would (renderVariable consults resolveVariable first with the same + // offset), just without paying for the reflection walk. Strictly + // additive: on ANY miss we fall through to the full worse-reflection + // path below, unchanged. + $fastVarName = $this->variableNameAtOffset($uri, $offset); + if ($fastVarName !== null) { + $resolved = $this->genericResolver->resolveVariable($uri, $fastVarName, $offset); + if ($resolved !== null) { + return new Hover(new MarkupContent( + MarkupKind::MARKDOWN, + self::format(sprintf('%s $%s', $resolved, $fastVarName), ''), + )); + } + } + $stripped = $this->parser->strip($document->text); $source = TextDocumentBuilder::create($stripped)->uri($uri)->language('php')->build(); @@ -681,6 +704,59 @@ private function hits(Node $name): bool return $visitor->hit; } + /** + * If the cursor sits on a plain `$var` token, return the variable's + * name (without the leading `$`); otherwise null. "Plain" means a + * {@see Variable} node with a literal string name -- variable-variables + * (`$$x`, name is an Expr) are excluded, and property / static-property + * name tokens (`$obj->prop`, `Foo::$bar`) are NOT `Variable` nodes so + * they never match here. Used by the variable fast-path to decide + * whether it can try `resolveVariable` before the expensive + * reflectOffset. + */ + private function variableNameAtOffset(string $uri, int $byteOffset): ?string + { + if (!$this->workspace->has($uri)) { + return null; + } + $item = $this->workspace->get($uri); + try { + $ast = $this->parser->parseTolerant($item->text); + } catch (Throwable) { + return null; + } + if ($ast === null) { + return null; + } + $visitor = new class($byteOffset) extends NodeVisitorAbstract { + public ?string $name = null; + + public function __construct(private readonly int $offset) + { + } + + public function enterNode(Node $node): null + { + if ($this->name !== null) { + return null; + } + if (!$node instanceof Variable || !is_string($node->name)) { + return null; + } + $start = $node->getStartFilePos(); + $end = $node->getEndFilePos(); + if ($start >= 0 && $this->offset >= $start && $this->offset <= $end) { + $this->name = $node->name; + } + return null; + } + }; + $traverser = new NodeTraverser(); + $traverser->addVisitor($visitor); + $traverser->traverse($ast); + return $visitor->name; + } + /** * Mirror of `PhpDefinitionResolver::useFunctionFqnAtOffset`: detect a * `use function App\foo;` (or group-use variant) cursor and return the diff --git a/test/Reflection/ReflectionCachePurgerTest.php b/test/Reflection/ReflectionCachePurgerTest.php new file mode 100644 index 0000000..91cf15e --- /dev/null +++ b/test/Reflection/ReflectionCachePurgerTest.php @@ -0,0 +1,142 @@ +spyCache()); + + $listeners = $purger->getListenersForEvent(new \stdClass()); + self::assertSame([], is_array($listeners) ? $listeners : iterator_to_array($listeners)); + + $listeners = $purger->getListenersForEvent($this->updated('file:///X.xphp')); + $listenerList = is_array($listeners) ? $listeners : iterator_to_array($listeners); + self::assertCount(1, $listenerList); + + $listener = $listenerList[0]; + self::assertIsArray($listener); + self::assertCount(2, $listener); + self::assertSame($purger, $listener[0]); + self::assertSame('purge', $listener[1]); + self::assertTrue(is_callable($listener), 'listener must be callable as-is'); + } + + public function testPurgeFlushesTheCache(): void + { + $cache = $this->spyCache(); + $purger = new ReflectionCachePurger($cache); + + self::assertSame(0, $cache->purges); + $purger->purge($this->updated('file:///X.xphp')); + self::assertSame(1, $cache->purges, 'a didChange must flush the reflection cache exactly once'); + } + + public function testEditToOpenBufferIsVisibleAfterPurge(): void + { + // End-to-end regression for the staleness the cache would otherwise + // introduce: reflect a class (populating the name-keyed cache), then + // edit the open buffer to add a method, fire the purger, and confirm + // the re-reflection sees the new member. Without the purger the + // second reflection would return the pre-edit (cached) view. + $workspace = new PhpactorWorkspace(); + $workspace->open(new TextDocumentItem( + 'file:///Coll.xphp', + 'xphp', + 1, + "factory($workspace); + $reflector = $factory->build(); + $purger = new ReflectionCachePurger($factory->reflectionCache()); + + self::assertSame(['alpha'], $this->methodNames($reflector, 'App\\Coll')); + + // Edit the open buffer: add `beta()`. Workspace::update bumps the + // document version so the source locator returns the new text; the + // reflection cache, however, is keyed only by name and still holds + // the pre-edit reflection until purged. + $newText = "update(new VersionedTextDocumentIdentifier(2, 'file:///Coll.xphp'), $newText); + + $purger->purge($this->updated('file:///Coll.xphp', 2, $newText)); + + self::assertSame(['alpha', 'beta'], $this->methodNames($reflector, 'App\\Coll')); + } + + /** + * @return list + */ + private function methodNames(\Phpactor\WorseReflection\Reflector $reflector, string $fqn): array + { + $names = []; + foreach ($reflector->reflectClass($fqn)->methods() as $method) { + $names[] = (string) $method->name(); + } + sort($names); + return $names; + } + + private function updated(string $uri, int $version = 1, string $text = ''): TextDocumentUpdated + { + return new TextDocumentUpdated(new VersionedTextDocumentIdentifier($version, $uri), $text); + } + + private function factory(PhpactorWorkspace $workspace): ReflectorFactory + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + $root = sys_get_temp_dir() . '/xphp-purger-empty'; + if (!is_dir($root)) { + mkdir($root, 0o755, true); + } + return new ReflectorFactory( + $workspace, + $cache, + $parser, + $root, + '', // no stubs -- this test only exercises workspace reflection + ReflectorFactory::defaultCacheDir(), + new FqnIndex($workspace, $cache, $parser, $root), + ); + } + + /** + * @return Cache&object{purges: int} + */ + private function spyCache(): Cache + { + return new class implements Cache { + public int $purges = 0; + + public function getOrSet(string $key, Closure $closure) + { + return $closure(); + } + + public function purge(): void + { + $this->purges++; + } + }; + } +} diff --git a/test/Reflection/ReflectorFactoryTest.php b/test/Reflection/ReflectorFactoryTest.php index eeb41cd..9e05e96 100644 --- a/test/Reflection/ReflectorFactoryTest.php +++ b/test/Reflection/ReflectorFactoryTest.php @@ -97,6 +97,30 @@ public function testReflectsNativeFunctionFromStubs(): void self::assertSame('strlen', (string) $function->name()); } + public function testReflectionCacheMemoizesRepeatedLookups(): void + { + // enableCache() wraps class reflection in a TTL cache, so repeated + // reflection of the same FQN within the lifetime returns the SAME + // instance instead of rebuilding. This is what collapses a single + // typed-variable hover's stdlib fan-out (and the rapid-fire repeat + // hovers PhpStorm issues on one token) from N reflections to one. + // A NullCache (the pre-fix default) would rebuild on every call and + // the two instances would differ. + $workspace = new PhpactorWorkspace(); + $workspace->open(new TextDocumentItem( + '/User.xphp', + 'xphp', + 1, + "newFactory($workspace, rootPath: $this->emptyRoot())->build(); + + $first = $reflector->reflectClass('App\\Models\\User'); + $second = $reflector->reflectClass('App\\Models\\User'); + self::assertSame($first, $second, 'reflection cache must return the memoized instance'); + } + public function testWorkspaceShadowsFilesystemAtSameFqn(): void { // When both workspace AND filesystem could answer the FQN, workspace diff --git a/test/Reflection/StubCacheWarmerTest.php b/test/Reflection/StubCacheWarmerTest.php new file mode 100644 index 0000000..f0e6dbc --- /dev/null +++ b/test/Reflection/StubCacheWarmerTest.php @@ -0,0 +1,145 @@ +reflector(stubs: false)); + + // Unrecognised event -> no listeners. + $listeners = $warmer->getListenersForEvent(new \stdClass()); + self::assertSame([], is_array($listeners) ? $listeners : iterator_to_array($listeners)); + + // Initialized -> exactly one `[$warmer, 'warm']` listener. Asserting + // the shape (not just the count) kills an ArrayItemRemoval mutant on + // `[[$this, 'warm']]` -> `[['warm']]`, which keeps the count at 1. + $listeners = $warmer->getListenersForEvent($this->initialized()); + $listenerList = is_array($listeners) ? $listeners : iterator_to_array($listeners); + self::assertCount(1, $listenerList); + + $listener = $listenerList[0]; + self::assertIsArray($listener); + self::assertCount(2, $listener); + self::assertSame($warmer, $listener[0]); + self::assertSame('warm', $listener[1]); + self::assertTrue(is_callable($listener), 'listener must be callable as-is'); + } + + public function testWarmBuildsTheStubMap(): void + { + // Prove the WARMER drives the stub map build -- not merely that a map + // happens to exist in the shared durable cache from a prior run. Use + // a tiny scratch stub dir (one function) + a fresh cache dir so the + // map is provably ABSENT before warming and PRESENT after, and the + // build is cheap (no ~512M phpstorm-stubs walk that would risk an OOM + // under the unit-test memory limit). + $stubDir = sys_get_temp_dir() . '/xphp-stubwarm-stubs-' . bin2hex(random_bytes(6)); + $cacheDir = sys_get_temp_dir() . '/xphp-stubwarm-cache-' . bin2hex(random_bytes(6)); + mkdir($stubDir, 0o755, true); + file_put_contents($stubDir . '/functions.php', "reflectorWith(stubPath: $stubDir, cacheDir: $cacheDir); + $warmer = new StubCacheWarmer($reflector); + + self::assertFileDoesNotExist($mapFile, 'precondition: the stub map must not exist yet'); + + wait(call(function () use ($warmer): \Generator { + $warmer->warm($this->initialized()); + // Let asyncCall's enqueued callback run. + yield new \Amp\Delayed(10); + })); + + self::assertFileExists($mapFile, 'warming must build and serialize the stub map'); + } finally { + $this->rmrf($stubDir); + if (is_dir($cacheDir)) { + $this->rmrf($cacheDir); + } + } + } + + public function testWarmSwallowsReflectionFailures(): void + { + // A stubs-less reflector can't resolve `strlen`, so reflectFunction + // throws. The async warm body must swallow it -- warming is + // best-effort and can never destabilise the session. Reaching the + // assertion without an uncaught exception IS the test. + $warmer = new StubCacheWarmer($this->reflector(stubs: false)); + + wait(call(function () use ($warmer): \Generator { + $warmer->warm($this->initialized()); + yield new \Amp\Delayed(10); + })); + + $this->addToAssertionCount(1); + } + + private function initialized(): Initialized + { + return new Initialized(new InitializeParams(new ClientCapabilities())); + } + + private function reflector(bool $stubs): Reflector + { + return $this->reflectorWith( + stubPath: $stubs ? ReflectorFactory::defaultStubPath() : '', + cacheDir: ReflectorFactory::defaultCacheDir(), + ); + } + + private function reflectorWith(string $stubPath, string $cacheDir): Reflector + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + $workspace = new PhpactorWorkspace(); + $root = sys_get_temp_dir() . '/xphp-stubwarm-empty'; + if (!is_dir($root)) { + mkdir($root, 0o755, true); + } + return (new ReflectorFactory( + $workspace, + $cache, + $parser, + $root, + $stubPath, + $cacheDir, + new FqnIndex($workspace, $cache, $parser, $root), + ))->build(); + } + + private function rmrf(string $dir): void + { + foreach (scandir($dir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $p = $dir . '/' . $entry; + is_dir($p) ? $this->rmrf($p) : unlink($p); + } + rmdir($dir); + } +} diff --git a/test/Resolver/PhpHoverResolverTest.php b/test/Resolver/PhpHoverResolverTest.php index 6c895df..8dab74f 100644 --- a/test/Resolver/PhpHoverResolverTest.php +++ b/test/Resolver/PhpHoverResolverTest.php @@ -757,6 +757,59 @@ public function first(): ?T { return null; } self::assertStringNotContainsString('App\\Containers\\T', $markdown); } + public function testVariableFastPathRendersResolvedBindingExactly(): void + { + // Hover-latency fast-path: hovering a plain `$var` whose type the + // GenericResolver can pin from an in-file binding renders directly, + // BEFORE (and without) the expensive reflectOffset. Locks the exact + // ` $` output the fast-path emits so a Concat mutant on + // its `sprintf('%s $%s', ...)` is caught. + $workspace = $this->workspace(); + $this->open($workspace, '/Collection.xphp', <<<'XPHP' + { + public function first(): ?T { return null; } + } + XPHP); + $this->open($workspace, '/User.xphp', "();\necho \$users;\n"; + $this->open($workspace, '/Use.xphp', $useSource); + + $hover = $this->hoverAt($workspace, '/Use.xphp', $useSource, 'echo $users', strlen('echo ')); + + self::assertSame( + "```php\nApp\\Containers\\Collection \$users\n```", + $this->markdown($hover), + ); + } + + public function testVariableFastPathDoesNotHijackMethodHover(): void + { + // Scoping guard: the fast-path must fire ONLY when the cursor is + // inside a `Variable` node's span. A cursor on the method name in + // `$users->first` is NOT on the `$users` Variable, so the fast-path + // must decline and the method hover must still render. + $workspace = $this->workspace(); + $this->open($workspace, '/Collection.xphp', <<<'XPHP' + { + public function first(): ?T { return null; } + } + XPHP); + $this->open($workspace, '/User.xphp', "();\n\$users->first();\n"; + $this->open($workspace, '/Use.xphp', $useSource); + + $hover = $this->hoverAt($workspace, '/Use.xphp', $useSource, '$users->first', strlen('$users->first')); + $markdown = $this->markdown($hover); + + // Method signature -- not a bare `... $users` variable render. + self::assertStringContainsString('function first', $markdown); + self::assertStringContainsString('): ?App\\Models\\User', $markdown); + } + public function testMethodHoverSubstitutesReturnTypeAtCallSite(): void { // Cursor on the `first` token in `$users->first()` -- the method From 5543747949b5e4573c3d3f69e9ec1abe2c61d4dd Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Tue, 21 Jul 2026 07:19:32 +0000 Subject: [PATCH 2/2] ci(release): bump server info to 0.3.1 + document reflection cache/warming serverInfo.version 0.3.0 -> 0.3.1 for the v0.3.1 release, which carries the hover-latency work (7098727): the reflection cache + edit-driven purge, the stub-map startup warmer, and the variable hover fast-path. Also: - docs(features): document the reflection cache (per-session, edit-invalidated) and the stub-map startup warming under Performance. - docs(readme): the server targets xphp 0.3.x, not 0.2.x (stale since 0.3.0; composer already requires ^v0.3.0). Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- docs/features/index.md | 22 ++++++++++++++++++++++ src/LspDispatcherFactory.php | 2 +- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7901663..37009bd 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The server reuses the parent `xphp` package's AST, generic-instantiation `Registry`, and `TypeHierarchy` directly -- no second parser, no duplicated language semantics. -Targets **xphp 0.2.x**, including the turbofish call-site syntax +Targets **xphp 0.3.x**, including the turbofish call-site syntax (`new Box::()`, `Foo::method::(...)`), variance markers, default type arguments, and composite (intersection / union) bounds. diff --git a/docs/features/index.md b/docs/features/index.md index bcfaa45..c0b8762 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -483,6 +483,28 @@ cache directory resolved from `$XPHP_LSP_CACHE_DIR` -> XDG -> fallback. Survives reboots and `/tmp` reaping, so cold-start cost is paid once per machine, not once per session. +The stub map is also **warmed off the `initialize` handshake**: on a +cold machine, building it walks the entire phpstorm-stubs tree, so +paying it on the first hover would push that hover past the editor's +hover-cancel window and the popup would never paint. The warmer moves +the build to a background task at startup, so the first hover over a +variable whose type involves native functions (`strlen`, `array_map`, +…) responds promptly. + +### Reflection cache (per-session, edit-invalidated) + +A single hover on a typed variable makes worse-reflection reflect the +same symbols many times over -- both the receiver class (repeatedly) +and every native symbol on the value's right-hand side. Those +reflections are memoized per session so the repeats within one hover, +and across the rapid repeat hovers an editor issues as the cursor +settles, collapse to one reflection each -- the difference between a +multi-second hover and one that lands inside the hover-cancel window. +The cache is keyed by symbol name, so it is flushed on every +`didChange`: an edit to an open buffer is always re-reflected on the +next hover, never served stale. Flushes land between user actions, so +they never undo the intra-hover memoization. + ### Tolerant-parse fallback In-memory locators recover from trailing parse errors so mid-edit diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index 72e99d3..2b98c51 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -434,7 +434,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia new ErrorHandlingMiddleware($this->logger), new InitializeMiddleware($handlers, $eventDispatcher, [ 'name' => 'xphp-lsp', - 'version' => '0.3.0', + 'version' => '0.3.1', ]), new ShutdownMiddleware($eventDispatcher), new ResponseHandlingMiddleware($responseWatcher),