From 32ad247bbd1c698717e3230ae18cca7913ecfa62 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 10:36:17 -0700 Subject: [PATCH 01/35] Add the SymbolInfo marker for kind-parameterized lookup --- src/Domain/ClassInfo.php | 2 +- src/Domain/FunctionInfo.php | 2 +- src/Domain/SymbolInfo.php | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 src/Domain/SymbolInfo.php diff --git a/src/Domain/ClassInfo.php b/src/Domain/ClassInfo.php index 0b656815..569d989d 100644 --- a/src/Domain/ClassInfo.php +++ b/src/Domain/ClassInfo.php @@ -7,7 +7,7 @@ /** * Metadata about a class, interface, trait, or enum. */ -final readonly class ClassInfo implements Formattable +final readonly class ClassInfo implements Formattable, SymbolInfo { /** * @param list $interfaces Implemented interfaces diff --git a/src/Domain/FunctionInfo.php b/src/Domain/FunctionInfo.php index e317bed8..48466a73 100644 --- a/src/Domain/FunctionInfo.php +++ b/src/Domain/FunctionInfo.php @@ -11,7 +11,7 @@ /** * Metadata about a standalone function. */ -final readonly class FunctionInfo implements Formattable +final readonly class FunctionInfo implements Formattable, SymbolInfo { /** * @param list $parameters diff --git a/src/Domain/SymbolInfo.php b/src/Domain/SymbolInfo.php new file mode 100644 index 00000000..886dda46 --- /dev/null +++ b/src/Domain/SymbolInfo.php @@ -0,0 +1,16 @@ + Date: Wed, 12 Aug 2026 10:49:11 -0700 Subject: [PATCH 02/35] Confine AST-route kind dispatch to one factory --- .../DeclarationSymbolInfoFactory.php | 81 ++++++++++ .../DeclarationSymbolInfoFactoryTest.php | 144 ++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 src/Knowledge/DeclarationSymbolInfoFactory.php create mode 100644 tests/Knowledge/DeclarationSymbolInfoFactoryTest.php diff --git a/src/Knowledge/DeclarationSymbolInfoFactory.php b/src/Knowledge/DeclarationSymbolInfoFactory.php new file mode 100644 index 00000000..494fbf78 --- /dev/null +++ b/src/Knowledge/DeclarationSymbolInfoFactory.php @@ -0,0 +1,81 @@ +normalize($name); + + return match ($kind) { + NameKind::ClassLike => $this->firstMatching( + $declarations->classLikes, + $target, + $kind, + fn(Stmt\ClassLike $node): SymbolInfo => $this->classes->fromAstNode( + $node, + FileUri::fromPath($filePath), + ), + ), + NameKind::Function_ => $this->firstMatching( + $declarations->functions, + $target, + $kind, + static fn(Stmt\Function_ $node): SymbolInfo => FunctionInfo::fromNode($node, $filePath), + ), + // The declarations are scanned; what is missing is the global-constant + // info type, which S3.8b lands with the Domain\ConstantName naming + // clash it forces (build-manifest S3.8b). + NameKind::Constant => null, + }; + } + + /** + * @template TNode of Node + * @param list> $declarations + * @param callable(TNode): SymbolInfo $build + */ + private function firstMatching(array $declarations, string $target, NameKind $kind, callable $build): ?SymbolInfo + { + foreach ($declarations as $declaration) { + if ($kind->normalize($declaration->name) === $target) { + return $build($declaration->node); + } + } + + return null; + } +} diff --git a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php new file mode 100644 index 00000000..a7c89202 --- /dev/null +++ b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php @@ -0,0 +1,144 @@ +factory = new DeclarationSymbolInfoFactory(new DefaultClassInfoFactory()); + $this->path = $this->fixturePath(self::FIXTURE); + + $ast = (new ParserService())->parseFile($this->path); + self::assertNotNull($ast, 'the fixture must parse so declarations can be scanned'); + $this->declarations = (new DeclarationScanner())->scan($ast); + } + + public function testBuildsClassInfoForAClassLikeDeclaration(): void + { + $info = $this->build('Fixtures\Helpers\HelperRegistry', NameKind::ClassLike); + + self::assertInstanceOf(ClassInfo::class, $info, 'a class-like must build ClassInfo, not another kind\'s type'); + self::assertSame('Fixtures\Helpers\HelperRegistry', $info->name->fqn, 'the located declaration must be built'); + } + + public function testBuildsFunctionInfoForAFunctionDeclaration(): void + { + $info = $this->build('Fixtures\Helpers\helperFormat', NameKind::Function_); + + self::assertInstanceOf(FunctionInfo::class, $info, 'a function must build FunctionInfo'); + self::assertCount(1, $info->parameters, 'the parsed signature must be carried, not just the name'); + self::assertSame($this->path, $info->file, 'the declaring file must be recorded from the path given'); + } + + public function testReturnsNullWhenTheFileDeclaresNoSuchName(): void + { + self::assertNull( + $this->build('Fixtures\Helpers\notDeclaredHere', NameKind::Function_), + 'a name the declarations do not carry is absent (RFC 1 §5.3)', + ); + } + + /** + * The kind selects the declaration list, so a name declared only as one kind is + * not answered when asked for as another. Reading a single merged list — or the + * wrong list — would resolve these, which is the collision PHP's three + * independent symbol namespaces make possible. + * + * @return iterable + */ + public static function crossKindQueries(): iterable + { + yield 'a class asked for as a function' => ['Fixtures\Helpers\HelperRegistry', NameKind::Function_]; + yield 'a function asked for as a class' => ['Fixtures\Helpers\helperFormat', NameKind::ClassLike]; + yield 'a constant asked for as a class' => ['Fixtures\Helpers\HELPER_LIMIT', NameKind::ClassLike]; + } + + #[DataProvider('crossKindQueries')] + public function testAKindOnlyAnswersItsOwnDeclarations(string $fqn, NameKind $kind): void + { + self::assertNull( + $this->build($fqn, $kind), + 'the kind must select the declaration list, so one namespace cannot answer for another', + ); + } + + /** + * @return iterable + */ + public static function caseInsensitiveQueries(): iterable + { + yield 'class-like' => ['FIXTURES\HELPERS\HELPERREGISTRY', NameKind::ClassLike]; + yield 'function' => ['FIXTURES\HELPERS\HELPERFORMAT', NameKind::Function_]; + } + + #[DataProvider('caseInsensitiveQueries')] + public function testMatchingFollowsTheKindsCaseRule(string $fqn, NameKind $kind): void + { + self::assertNotNull( + $this->build($fqn, $kind), + 'PHP matches class and function names case-insensitively, which NameKind::normalize owns', + ); + } + + public function testConstantsAreNotYetBuilt(): void + { + // The fixture does declare this constant, so the null is the missing + // global-constant info type, not a missing declaration. S3.8b adds it, + // together with the Domain\ConstantName naming clash it forces. + self::assertNotSame( + [], + $this->declarations->constants, + 'the fixture must declare constants, or this test would pass vacuously', + ); + self::assertNull( + $this->build('Fixtures\Helpers\HELPER_LIMIT', NameKind::Constant), + 'global-constant metadata arrives with S3.8b', + ); + } + + private function build(string $fqn, NameKind $kind): ?SymbolInfo + { + return $this->factory->fromDeclarations( + $this->declarations, + QualifiedName::fromFullyQualified($fqn), + $kind, + $this->path, + ); + } +} From 5b6787638ac039fd5ee6569c59d9c4986d748133 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 10:59:14 -0700 Subject: [PATCH 03/35] Confine reflection-route kind dispatch to one factory --- phpstan.neon | 1 + src/Knowledge/ReflectionSymbolInfoFactory.php | 72 ++++++++++++++ .../ReflectionSymbolInfoFactoryTest.php | 96 +++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 src/Knowledge/ReflectionSymbolInfoFactory.php create mode 100644 tests/Knowledge/ReflectionSymbolInfoFactoryTest.php diff --git a/phpstan.neon b/phpstan.neon index c3dd1d23..3316041f 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -36,6 +36,7 @@ parameters: message: 'runtime reflection is confined: BuiltinBackend, ReflectionNamespaceSource, and the fromReflection factories' allowIn: - src/Knowledge/BuiltinBackend.php + - src/Knowledge/ReflectionSymbolInfoFactory.php - src/Index/ReflectionNamespaceSource.php - src/Repository/ClassInfoFactory.php - src/Repository/DefaultClassInfoFactory.php diff --git a/src/Knowledge/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php new file mode 100644 index 00000000..9f2486bd --- /dev/null +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -0,0 +1,72 @@ + $this->classInfo($name), + NameKind::Function_ => $this->functionInfo($name), + // Reflection can read a global constant, but there is no info type to + // build; S3.8b lands it (build-manifest S3.8b). + NameKind::Constant => null, + }; + } + + private function classInfo(QualifiedName $name): ?SymbolInfo + { + $fqn = $name->fullyQualifiedName(); + + // All three, because only `class` and `enum` answer to `class_exists`; an + // interface or trait is a class-like this backend must still describe. Each + // autoloads exactly as constructing the reflection would, so this is the + // same absence test, stated in a form that also carries the name's type. + if (!class_exists($fqn) && !interface_exists($fqn) && !trait_exists($fqn)) { + return null; + } + + return $this->classes->fromReflection(new ReflectionClass($fqn)); + } + + private function functionInfo(QualifiedName $name): ?SymbolInfo + { + try { + $reflection = new ReflectionFunction($name->fullyQualifiedName()); + } catch (ReflectionException) { + return null; + } + + // Reflection also sees the functions the server's own dependencies declare, + // which are not the project's. Enumeration is filtered to internal + // (BuiltinFunctionParityTest), so lookup must be too or a name resolves on + // hover while never appearing in completion (RFC 1 §4.2). + return $reflection->isInternal() ? FunctionInfo::fromReflection($reflection) : null; + } +} diff --git a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php new file mode 100644 index 00000000..932185b0 --- /dev/null +++ b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php @@ -0,0 +1,96 @@ +factory = new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()); + } + + public function testBuildsClassInfoForALoadedClass(): void + { + $info = $this->build(\ArrayObject::class, NameKind::ClassLike); + + self::assertInstanceOf(ClassInfo::class, $info, 'a class-like must build ClassInfo'); + self::assertSame('ArrayObject', $info->name->fqn, 'the reflected class must be returned'); + } + + public function testBuildsFunctionInfoForAnInternalFunction(): void + { + $info = $this->build('str_contains', NameKind::Function_); + + self::assertInstanceOf(FunctionInfo::class, $info, 'a function must build FunctionInfo'); + self::assertCount(2, $info->parameters, 'the reflected signature must be carried, not just the name'); + } + + public function testIgnoresFunctionsOnlyTheServerHasLoaded(): void + { + // The server is itself a PHP program, so reflection sees every function its + // own dependencies declare. Those are not the project's, and the backend + // enumerates only internal functions — a lookup answering more broadly would + // resolve a name completion never offers (RFC 1 §4.2). + require_once dirname(__DIR__) . '/Domain/Fixtures/documented_function.php'; + + self::assertNull( + $this->build('testDocumentedFunction', NameKind::Function_), + 'a userland function loaded in the server process is not a built-in', + ); + } + + /** + * @return iterable + */ + public static function absentNames(): iterable + { + yield 'class-like' => ['No\Such\Builtin', NameKind::ClassLike]; + yield 'function' => ['no_such_builtin', NameKind::Function_]; + // The kind selects which reflection is consulted, so a name that exists in + // one of PHP's symbol namespaces is not answered for another. + yield 'a function asked for as a class' => ['str_contains', NameKind::ClassLike]; + yield 'a class asked for as a function' => [\ArrayObject::class, NameKind::Function_]; + } + + #[DataProvider('absentNames')] + public function testReturnsNullWhenReflectionCannotDescribeTheName(string $fqn, NameKind $kind): void + { + self::assertNull( + $this->build($fqn, $kind), + 'a name reflection cannot load for this kind is absent (RFC 1 §5.3)', + ); + } + + public function testConstantsAreNotYetBuilt(): void + { + self::assertNull( + $this->build('PHP_INT_MAX', NameKind::Constant), + 'global-constant metadata arrives with S3.8b', + ); + } + + private function build(string $fqn, NameKind $kind): ?SymbolInfo + { + return $this->factory->fromReflection(QualifiedName::fromFullyQualified($fqn), $kind); + } +} From e064fdf38cabcf713422b45981ed548933588a3b Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 11:49:08 -0700 Subject: [PATCH 04/35] Collapse per-kind backend lookup to one call --- phpstan-baseline.neon | 6 +- phpstan.neon | 3 +- src/Knowledge/BuiltinBackend.php | 60 ++---------- src/Knowledge/CompositeSymbolSource.php | 40 +++++--- src/Knowledge/FilesystemBackend.php | 97 ++++--------------- src/Knowledge/KnowledgeStack.php | 12 +-- src/Knowledge/OpenDocumentBackend.php | 80 +++++++-------- src/Knowledge/SymbolBackend.php | 37 +++---- tests/Knowledge/BuiltinBackendTest.php | 55 +++++------ tests/Knowledge/DocumentSymbolSinkTest.php | 41 +++----- tests/Knowledge/FakeSymbolBackend.php | 18 ++-- tests/Knowledge/FilesystemBackendTest.php | 95 ++++++++---------- .../Knowledge/LooksUpBackendSymbolsTrait.php | 46 +++++++++ tests/Knowledge/OpenDocumentBackendTest.php | 30 +++--- tests/Parity/BuiltinFunctionParityTest.php | 3 +- 15 files changed, 271 insertions(+), 352 deletions(-) create mode 100644 tests/Knowledge/LooksUpBackendSymbolsTrait.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 06641048..b471e156 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -73,19 +73,19 @@ parameters: path: src/Index/WorkspaceIndexer.php - - message: '#^Class ReflectionFunction is forbidden, runtime reflection is confined\: BuiltinBackend, ReflectionNamespaceSource, and the fromReflection factories\. \[ReflectionFunction matches Reflection\*\]$#' + message: '#^Class ReflectionFunction is forbidden, runtime reflection is confined\: ReflectionNamespaceSource and the fromReflection factories\. \[ReflectionFunction matches Reflection\*\]$#' identifier: disallowed.class count: 1 path: src/Repository/DefaultFunctionRepository.php - - message: '#^Namespace ReflectionException is forbidden, runtime reflection is confined\: BuiltinBackend, ReflectionNamespaceSource, and the fromReflection factories\. \[ReflectionException matches Reflection\*\]$#' + message: '#^Namespace ReflectionException is forbidden, runtime reflection is confined\: ReflectionNamespaceSource and the fromReflection factories\. \[ReflectionException matches Reflection\*\]$#' identifier: disallowed.namespace count: 1 path: src/Repository/DefaultFunctionRepository.php - - message: '#^Namespace ReflectionFunction is forbidden, runtime reflection is confined\: BuiltinBackend, ReflectionNamespaceSource, and the fromReflection factories\. \[ReflectionFunction matches Reflection\*\]$#' + message: '#^Namespace ReflectionFunction is forbidden, runtime reflection is confined\: ReflectionNamespaceSource and the fromReflection factories\. \[ReflectionFunction matches Reflection\*\]$#' identifier: disallowed.namespace count: 1 path: src/Repository/DefaultFunctionRepository.php diff --git a/phpstan.neon b/phpstan.neon index 3316041f..3198e7be 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -33,9 +33,8 @@ parameters: - tests/* - namespace: 'Reflection*' - message: 'runtime reflection is confined: BuiltinBackend, ReflectionNamespaceSource, and the fromReflection factories' + message: 'runtime reflection is confined: ReflectionNamespaceSource and the fromReflection factories' allowIn: - - src/Knowledge/BuiltinBackend.php - src/Knowledge/ReflectionSymbolInfoFactory.php - src/Index/ReflectionNamespaceSource.php - src/Repository/ClassInfoFactory.php diff --git a/src/Knowledge/BuiltinBackend.php b/src/Knowledge/BuiltinBackend.php index 879a1c1a..ba784366 100644 --- a/src/Knowledge/BuiltinBackend.php +++ b/src/Knowledge/BuiltinBackend.php @@ -4,19 +4,12 @@ namespace Firehed\PhpLsp\Knowledge; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionInfo; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; -use Firehed\PhpLsp\Repository\ClassInfoFactory; use Psr\SimpleCache\CacheInterface; -use ReflectionClass; -use ReflectionException; -use ReflectionFunction; /** * The lowest-precedence {@see SymbolBackend}: the symbols built into PHP and its @@ -24,7 +17,7 @@ * open-document, workspace, and vendor backends, so a name any of them can resolve * never reaches reflection (RFC 1 §5.3). * - * Built-ins are fixed for a given target environment, so a resolved class is cached + * Built-ins are fixed for a given target environment, so a resolved symbol is cached * (RFC 1 §5.3). This backend is reflection-backed and therefore describes the * *server's* runtime, not the project's target — a known §4.7 gap deferred to Step 5 * (Plan 0002 §5); the interim treats every reflected built-in as available. @@ -36,7 +29,7 @@ final class BuiltinBackend implements SymbolBackend { public function __construct( - private readonly ClassInfoFactory $factory, + private readonly ReflectionSymbolInfoFactory $infoFactory, private readonly NamespaceCatalog $namespaces, private readonly CacheInterface $cache, ) { @@ -47,55 +40,22 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents return $this->namespaces->childrenOf($namespace->path); } - public function lookupClassLike(ClassName $name): ?ClassInfo + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - $cacheKey = SymbolCacheKey::for(QualifiedName::fromClassName($name), NameKind::ClassLike); + $cacheKey = SymbolCacheKey::for($name, $kind); $cached = $this->cache->get($cacheKey); if ($cached !== null) { - assert($cached instanceof ClassInfo); + assert($cached instanceof SymbolInfo); return $cached; } - try { - $classInfo = $this->factory->fromReflection(new ReflectionClass($name->fqn)); - } catch (ReflectionException) { - return null; + $info = $this->infoFactory->fromReflection($name, $kind); + if ($info !== null) { + $this->cache->set($cacheKey, $info); } - $this->cache->set($cacheKey, $classInfo); - return $classInfo; - } - - public function lookupFunction(FunctionName $name): ?FunctionInfo - { - $cacheKey = SymbolCacheKey::for($name->qualifiedName, $name->kind()); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof FunctionInfo); - return $cached; - } - - try { - $function = new ReflectionFunction($name->fullyQualifiedName()); - } catch (ReflectionException) { - return null; - } - - // Reflection sees every function loaded in the *server's* process, which - // includes the ones its own dependencies declare. Those are not the - // project's, and this backend enumerates only internal functions - // (BuiltinFunctionParityTest) — a lookup that answered more broadly would - // resolve a name completion never offers (RFC 1 §4.2). - if (!$function->isInternal()) { - return null; - } - - $functionInfo = FunctionInfo::fromReflection($function); - $this->cache->set($cacheKey, $functionInfo); - - return $functionInfo; + return $info; } /** diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index df376782..aaa3b455 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -10,6 +10,7 @@ use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Index\Symbol; @@ -66,26 +67,18 @@ public function isSubclassOf(ClassName $class, ClassName $potentialParent): bool public function lookupClassLike(ClassName $name): ?ClassInfo { - foreach ($this->backends as $backend) { - $info = $backend->lookupClassLike($name); - if ($info !== null) { - return $info; - } - } + $info = $this->lookup(QualifiedName::fromClassName($name), NameKind::ClassLike); + assert($info === null || $info instanceof ClassInfo); - return null; + return $info; } public function lookupFunction(FunctionName $name): ?FunctionInfo { - foreach ($this->backends as $backend) { - $info = $backend->lookupFunction($name); - if ($info !== null) { - return $info; - } - } + $info = $this->lookup($name->qualifiedName, $name->kind()); + assert($info === null || $info instanceof FunctionInfo); - return null; + return $info; } /** @@ -105,6 +98,25 @@ public function searchClassLikes(string $prefix): array return array_values($byFqn); } + /** + * The first backend that answers wins, which is what makes precedence + * positional. The kind travels as an argument here and is narrowed back to a + * concrete type by each caller above: O(kinds) narrowings at this one site, + * against the O(kinds × backends) methods a per-kind backend would need + * (Plan 0002 §5.6). + */ + private function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo + { + foreach ($this->backends as $backend) { + $info = $backend->lookup($name, $kind); + if ($info !== null) { + return $info; + } + } + + return null; + } + /** * @param array $visited */ diff --git a/src/Knowledge/FilesystemBackend.php b/src/Knowledge/FilesystemBackend.php index ca4f1fee..49dc82ff 100644 --- a/src/Knowledge/FilesystemBackend.php +++ b/src/Knowledge/FilesystemBackend.php @@ -6,18 +6,14 @@ use Firehed\PhpLsp\Cache\Invalidatable; use Firehed\PhpLsp\Document\FileUri; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionInfo; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\DeclarationScanner; use Firehed\PhpLsp\Index\FileDeclarations; use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Parser\ParserService; -use Firehed\PhpLsp\Repository\ClassInfoFactory; use Psr\SimpleCache\CacheInterface; /** @@ -27,12 +23,12 @@ * given (Plan 0002 §3a: the workspace/vendor precedence split), so one lookup * mechanism covers both rather than two hand-written copies. * - * Class-like lookup locates the file for a name and parses that one file — no - * `vendor/` pre-index (RFC 1 §3, lazy-first). Results are held behind the - * replaceable cache seam (RFC 1 §5.3): a file on disk is stable while unchanged, so - * a resolved class is memoized. An on-disk change to a file is signalled through + * Lookup locates the file for a name and parses that one file — no `vendor/` + * pre-index (RFC 1 §3, lazy-first). Results are held behind the replaceable cache + * seam (RFC 1 §5.3): a file on disk is stable while unchanged, so a resolved symbol + * is memoized. An on-disk change to a file is signalled through * {@see invalidate()} ({@see Invalidatable}), which evicts that file's cached - * class-likes and drops cached namespace listings so the next query reflects disk + * symbols and drops cached namespace listings so the next query reflects disk * (RFC 1 §5.2, §5.3). * * Namespace enumeration is a directory listing through the same autoload map @@ -43,10 +39,10 @@ final class FilesystemBackend implements SymbolBackend, Invalidatable { /** - * The class-cache keys derived from each file, so an on-disk change to one - * file evicts exactly its entries. The class cache is keyed by an opaque hash - * of the FQN with no reverse mapping to a path, so the path→key relation is - * recorded here as classes are cached. + * The cache keys derived from each file, so an on-disk change to one file + * evicts exactly its entries. The cache is keyed by an opaque hash of the FQN + * and kind with no reverse mapping to a path, so the path→key relation is + * recorded here as symbols are cached. * * @var array> */ @@ -56,7 +52,7 @@ public function __construct( private readonly SymbolLocator $locator, private readonly NamespaceCatalog $namespaces, private readonly ParserService $parser, - private readonly ClassInfoFactory $factory, + private readonly DeclarationSymbolInfoFactory $infoFactory, private readonly DeclarationScanner $scanner, private readonly CacheInterface $cache, ) { @@ -67,57 +63,32 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents return $this->namespaces->childrenOf($namespace->path); } - public function lookupClassLike(ClassName $name): ?ClassInfo + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - $qualifiedName = QualifiedName::fromClassName($name); - $cacheKey = SymbolCacheKey::for($qualifiedName, NameKind::ClassLike); + $cacheKey = SymbolCacheKey::for($name, $kind); $cached = $this->cache->get($cacheKey); if ($cached !== null) { - assert($cached instanceof ClassInfo); + assert($cached instanceof SymbolInfo); return $cached; } - $filePath = $this->locator->locate($qualifiedName, NameKind::ClassLike); + $filePath = $this->locator->locate($name, $kind); if ($filePath === null) { return null; } - $classInfo = $this->parseClassFrom($name, $filePath); - if ($classInfo !== null) { - $this->cache->set($cacheKey, $classInfo); + $info = $this->infoFactory->fromDeclarations($this->declarationsIn($filePath), $name, $kind, $filePath); + if ($info !== null) { + $this->cache->set($cacheKey, $info); $this->cacheKeysByPath[$filePath][] = $cacheKey; } - return $classInfo; - } - - public function lookupFunction(FunctionName $name): ?FunctionInfo - { - $cacheKey = SymbolCacheKey::for($name->qualifiedName, $name->kind()); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof FunctionInfo); - return $cached; - } - - $filePath = $this->locator->locate($name->qualifiedName, $name->kind()); - if ($filePath === null) { - return null; - } - - $functionInfo = $this->parseFunctionFrom($name, $filePath); - if ($functionInfo !== null) { - $this->cache->set($cacheKey, $functionInfo); - $this->cacheKeysByPath[$filePath][] = $cacheKey; - } - - return $functionInfo; + return $info; } /** - * Evict the file's cached class-likes by their recorded keys and drop cached + * Evict the file's cached symbols by their recorded keys and drop cached * namespace listings, so the next query re-reads disk and the pre-change value * is not restored (RFC 1 §5.2, §5.3). */ @@ -152,34 +123,6 @@ public function searchClassLikes(string $prefix): array return []; } - private function parseFunctionFrom(FunctionName $name, string $filePath): ?FunctionInfo - { - $kind = $name->kind(); - $target = $kind->normalize($name->qualifiedName); - - foreach ($this->declarationsIn($filePath)->functions as $declaration) { - if ($kind->normalize($declaration->name) === $target) { - return FunctionInfo::fromNode($declaration->node, $filePath); - } - } - - return null; - } - - private function parseClassFrom(ClassName $name, string $filePath): ?ClassInfo - { - $kind = NameKind::ClassLike; - $target = $kind->normalize(QualifiedName::fromClassName($name)); - - foreach ($this->declarationsIn($filePath)->classLikes as $declaration) { - if ($kind->normalize($declaration->name) === $target) { - return $this->factory->fromAstNode($declaration->node, FileUri::fromPath($filePath)); - } - } - - return null; - } - /** * A declaration at any depth counts, not just a top-level one: the shape most * `autoload.files` entries take is a polyfill declared inside an diff --git a/src/Knowledge/KnowledgeStack.php b/src/Knowledge/KnowledgeStack.php index f9f00b37..04edf614 100644 --- a/src/Knowledge/KnowledgeStack.php +++ b/src/Knowledge/KnowledgeStack.php @@ -17,7 +17,6 @@ use Firehed\PhpLsp\Index\SymbolExtractor; use Firehed\PhpLsp\Index\SymbolIndex; use Firehed\PhpLsp\Parser\ParserService; -use Firehed\PhpLsp\Repository\ClassInfoFactory; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; /** @@ -54,20 +53,21 @@ public static function forProject( ): self { $index ??= new SymbolIndex(); $classInfoFactory = new DefaultClassInfoFactory(); + $declarationInfoFactory = new DeclarationSymbolInfoFactory($classInfoFactory); [$workspaceMap, $vendorMap] = $autoloadMap->partitionByVendorDirectory($vendorDirectory); $scanner = new DeclarationScanner(); $openDocuments = new OpenDocumentBackend($index); - $workspace = self::filesystemBackend($workspaceMap, $parser, $classInfoFactory, $scanner); - $vendor = self::filesystemBackend($vendorMap, $parser, $classInfoFactory, $scanner); + $workspace = self::filesystemBackend($workspaceMap, $parser, $declarationInfoFactory, $scanner); + $vendor = self::filesystemBackend($vendorMap, $parser, $declarationInfoFactory, $scanner); $source = new CompositeSymbolSource([ $openDocuments, $workspace, $vendor, new BuiltinBackend( - $classInfoFactory, + new ReflectionSymbolInfoFactory($classInfoFactory), new CachedNamespaceCatalog(new ReflectionNamespaceSource(), CacheFactory::inMemory()), CacheFactory::inMemory(), ), @@ -103,7 +103,7 @@ public static function forProject( private static function filesystemBackend( ComposerAutoloadMap $map, ParserService $parser, - ClassInfoFactory $classInfoFactory, + DeclarationSymbolInfoFactory $infoFactory, DeclarationScanner $scanner, ): FilesystemBackend { $autoloadFiles = new AutoloadFilesLocator($map, $parser, $scanner); @@ -121,7 +121,7 @@ private static function filesystemBackend( CacheFactory::inMemory(), ), $parser, - $classInfoFactory, + $infoFactory, $scanner, CacheFactory::inMemory(), ); diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 7e82d57a..92157698 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -5,11 +5,10 @@ namespace Firehed\PhpLsp\Knowledge; use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\ClassName; use Firehed\PhpLsp\Domain\FunctionInfo; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Index\Symbol; use Firehed\PhpLsp\Index\SymbolIndex; @@ -22,12 +21,12 @@ * edits are honored — including edits to a vendored file opened in the editor. * * Open documents change on every keystroke and are never cached (RFC 1 §5.3): the - * backend reads the live symbol index and its own registered class metadata - * directly. Class-like lookup is served from the {@see ClassInfo} registered per - * document by the write path; namespace enumeration and prefix search are served - * from the {@see SymbolIndex} the write path also populates. The write path feeds - * both stores from one parse ({@see DocumentSymbolSink}, Plan 0002 §5.5 Step 3a(iv)); - * here they are read as they stand. + * backend reads the live symbol index and its own registered metadata directly. + * Lookup is served from the {@see SymbolInfo} registered per document by the write + * path; namespace enumeration and prefix search are served from the + * {@see SymbolIndex} the write path also populates. The write path feeds both stores + * from one parse ({@see DocumentSymbolSink}, Plan 0002 §5.5 Step 3a(iv)); here they + * are read as they stand. */ final class OpenDocumentBackend implements SymbolBackend { @@ -44,17 +43,11 @@ final class OpenDocumentBackend implements SymbolBackend SymbolKind::Trait_, ]; - /** @var array Lowercase FQN -> class metadata */ - private array $byFqn = []; + /** @var array Normalized kind-qualified key -> metadata */ + private array $byKey = []; - /** @var array> URI -> the lowercase FQNs it declared */ - private array $fqnsByUri = []; - - /** @var array Lowercase FQN -> function metadata */ - private array $functionsByFqn = []; - - /** @var array> URI -> the lowercase function FQNs it declared */ - private array $functionFqnsByUri = []; + /** @var array> URI -> the keys it declared */ + private array $keysByUri = []; private readonly WorkspaceNamespaceSource $namespaces; @@ -69,14 +62,9 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents return $this->namespaces->childrenOf($namespace->path); } - public function lookupClassLike(ClassName $name): ?ClassInfo - { - return $this->byFqn[self::key(NameKind::ClassLike, $name->fqn)] ?? null; - } - - public function lookupFunction(FunctionName $name): ?FunctionInfo + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - return $this->functionsByFqn[$name->kind()->normalize($name->qualifiedName)] ?? null; + return $this->byKey[self::key($kind, $name)] ?? null; } /** @@ -103,42 +91,42 @@ public function updateDocument(string $uri, array $classes, array $functions = [ $keys = []; foreach ($classes as $classInfo) { - $key = self::key(NameKind::ClassLike, $classInfo->name->fqn); - $this->byFqn[$key] = $classInfo; - $keys[] = $key; + $keys[] = $this->register(NameKind::ClassLike, $classInfo->name->fqn, $classInfo); } - $this->fqnsByUri[$uri] = $keys; - - $functionKeys = []; foreach ($functions as $fqn => $functionInfo) { - $key = self::key(NameKind::Function_, $fqn); - $this->functionsByFqn[$key] = $functionInfo; - $functionKeys[] = $key; + $keys[] = $this->register(NameKind::Function_, $fqn, $functionInfo); } - $this->functionFqnsByUri[$uri] = $functionKeys; + $this->keysByUri[$uri] = $keys; } public function removeDocument(string $uri): void { - foreach ($this->fqnsByUri[$uri] ?? [] as $key) { - unset($this->byFqn[$key]); + foreach ($this->keysByUri[$uri] ?? [] as $key) { + unset($this->byKey[$key]); } - unset($this->fqnsByUri[$uri]); + unset($this->keysByUri[$uri]); + } - foreach ($this->functionFqnsByUri[$uri] ?? [] as $key) { - unset($this->functionsByFqn[$key]); - } - unset($this->functionFqnsByUri[$uri]); + private function register(NameKind $kind, string $fqn, SymbolInfo $info): string + { + $key = self::key($kind, QualifiedName::fromFullyQualified($fqn)); + $this->byKey[$key] = $info; + + return $key; } /** * Registration and lookup must agree on the case rule, and that rule differs by * kind, so both go through {@see NameKind::normalize()} rather than a local - * lowercasing of the whole FQN — which is right for these two kinds and wrong - * for a constant. + * lowercasing of the whole FQN — which is right for class-likes and functions + * and wrong for a constant. + * + * The kind is part of the key because one store now holds every kind, and PHP's + * symbol namespaces are independent: a class and a function may share a + * spelling without being the same symbol. */ - private static function key(NameKind $kind, string $fqn): string + private static function key(NameKind $kind, QualifiedName $name): string { - return $kind->normalize(QualifiedName::fromFullyQualified($fqn)); + return $kind->name . '|' . $kind->normalize($name); } } diff --git a/src/Knowledge/SymbolBackend.php b/src/Knowledge/SymbolBackend.php index f3015e42..f036db68 100644 --- a/src/Knowledge/SymbolBackend.php +++ b/src/Knowledge/SymbolBackend.php @@ -4,10 +4,9 @@ namespace Firehed\PhpLsp\Knowledge; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionInfo; -use Firehed\PhpLsp\Domain\FunctionName; +use Firehed\PhpLsp\Domain\NameKind; +use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Index\Symbol; @@ -26,10 +25,14 @@ * vendored file, and the built-ins — is the composite's concern, not the * backend's: each answers only for its own source. * - * Lookup is per-kind: PHP's symbol namespaces are independent, so one name may be - * both a class and a function, and the query says which is meant. Constant lookup - * and a kind-parameterized search arrive with the slices that first need them - * (Plan 0002 §5.2); a method with no caller is not carried ahead. + * Lookup is **kind-parameterized here and per-kind at the facade**, and the split is + * deliberate (Plan 0002 §5.6). {@see SymbolSource} carries a typed method per kind + * because RFC 1 §5.1 requires a concrete return type; a backend takes the kind as an + * argument because the kind changes only the case rule + * ({@see NameKind::normalize()}) and which factory builds the metadata — never how a + * declaring file is found or how a namespace is listed. So a new kind is a name + * type, an info type, and one factory case, rather than a method on every backend. + * Do not re-derive a per-kind backend method from the facade's closed method set. */ interface SymbolBackend { @@ -41,22 +44,22 @@ interface SymbolBackend public function childrenOf(NamespaceName $namespace): NamespaceContents; /** - * Full metadata for a class-like this backend declares, or `null` when it - * cannot reach a declaration of $name (RFC 1 §5.3: absence is a bare null). - */ - public function lookupClassLike(ClassName $name): ?ClassInfo; - - /** - * Full metadata for a standalone function this backend declares, or `null` when - * it cannot reach a declaration of $name (RFC 1 §5.3). + * Full metadata for the symbol $name names *as a $kind*, or `null` when this + * backend cannot reach such a declaration (RFC 1 §5.3: absence is a bare null). + * + * PHP's three symbol namespaces are independent, so one name may be both a + * class and a function; $kind is what says which is meant. */ - public function lookupFunction(FunctionName $name): ?FunctionInfo; + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo; /** * The class-likes this backend can enumerate whose short name begins with * $prefix. A backend with no affordable prefix enumeration returns an empty * list rather than walking its source (RFC 1 §5.3). * + * A kind parameter arrives with S3.9a, which widens search the way this + * interface's lookup is already widened. + * * @return list */ public function searchClassLikes(string $prefix): array; diff --git a/tests/Knowledge/BuiltinBackendTest.php b/tests/Knowledge/BuiltinBackendTest.php index 15640584..74eb86a5 100644 --- a/tests/Knowledge/BuiltinBackendTest.php +++ b/tests/Knowledge/BuiltinBackendTest.php @@ -5,32 +5,36 @@ namespace Firehed\PhpLsp\Tests\Knowledge; use Firehed\PhpLsp\Cache\CacheFactory; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Knowledge\BuiltinBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; +use Firehed\PhpLsp\Knowledge\ReflectionSymbolInfoFactory; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; use PHPUnit\Framework\TestCase; /** * The built-in backend is the lowest-precedence source (RFC 1 §5.3): it reflects the - * symbols the server runtime has loaded. These prove class-like lookup via - * reflection, its caching, absence for an unknown name, the empty prefix search, and - * that enumeration forwards to the reflection catalog. + * symbols the server runtime has loaded. These prove lookup via reflection, its + * caching, absence for an unknown name, the empty prefix search, and that + * enumeration forwards to the reflection catalog. */ final class BuiltinBackendTest extends TestCase { + use LooksUpBackendSymbolsTrait; + private function backend(NamespaceCatalog $namespaces): BuiltinBackend { - return new BuiltinBackend(new DefaultClassInfoFactory(), $namespaces, CacheFactory::inMemory()); + return new BuiltinBackend( + new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()), + $namespaces, + CacheFactory::inMemory(), + ); } public function testLookupClassLikeReflectsABuiltinClass(): void { - $info = $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupClassLike(self::className(\ArrayObject::class)); + $info = self::classLikeIn($this->backend(self::createStub(NamespaceCatalog::class)), \ArrayObject::class); self::assertNotNull($info, 'a loaded built-in class must resolve through reflection'); self::assertSame('ArrayObject', $info->name->fqn, 'the reflected class must be returned'); @@ -39,8 +43,7 @@ public function testLookupClassLikeReflectsABuiltinClass(): void public function testLookupClassLikeReturnsNullForAnUnknownClass(): void { self::assertNull( - $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupClassLike(self::className('No\Such\Builtin')), + self::classLikeIn($this->backend(self::createStub(NamespaceCatalog::class)), 'No\Such\Builtin'), 'a name reflection cannot load is absent from this backend (RFC 1 §5.3)', ); } @@ -48,10 +51,9 @@ public function testLookupClassLikeReturnsNullForAnUnknownClass(): void public function testLookupClassLikeCachesAResolvedClass(): void { $backend = $this->backend(self::createStub(NamespaceCatalog::class)); - $name = self::className(\ArrayObject::class); - $first = $backend->lookupClassLike($name); - $second = $backend->lookupClassLike($name); + $first = self::classLikeIn($backend, \ArrayObject::class); + $second = self::classLikeIn($backend, \ArrayObject::class); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); self::assertSame($first, $second, 'a second lookup must return the cached instance, not re-reflect'); @@ -59,8 +61,7 @@ public function testLookupClassLikeCachesAResolvedClass(): void public function testLookupFunctionReflectsABuiltinFunction(): void { - $info = $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupFunction(FunctionName::fromFullyQualified('str_contains')); + $info = self::functionIn($this->backend(self::createStub(NamespaceCatalog::class)), 'str_contains'); self::assertNotNull($info, 'a built-in function must resolve through reflection'); self::assertSame('str_contains', $info->name); @@ -70,8 +71,7 @@ public function testLookupFunctionReflectsABuiltinFunction(): void public function testLookupFunctionIsCaseInsensitive(): void { self::assertNotNull( - $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupFunction(FunctionName::fromFullyQualified('STR_CONTAINS')), + self::functionIn($this->backend(self::createStub(NamespaceCatalog::class)), 'STR_CONTAINS'), 'PHP matches function names case-insensitively', ); } @@ -87,8 +87,7 @@ public function testLookupFunctionIgnoresFunctionsOnlyTheServerHasLoaded(): void require_once dirname(__DIR__) . '/Domain/Fixtures/documented_function.php'; self::assertNull( - $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupFunction(FunctionName::fromFullyQualified('testDocumentedFunction')), + self::functionIn($this->backend(self::createStub(NamespaceCatalog::class)), 'testDocumentedFunction'), 'a userland function loaded in the server process is not a built-in', ); } @@ -96,8 +95,7 @@ public function testLookupFunctionIgnoresFunctionsOnlyTheServerHasLoaded(): void public function testLookupFunctionReturnsNullForAnUnknownFunction(): void { self::assertNull( - $this->backend(self::createStub(NamespaceCatalog::class)) - ->lookupFunction(FunctionName::fromFullyQualified('no_such_builtin')), + self::functionIn($this->backend(self::createStub(NamespaceCatalog::class)), 'no_such_builtin'), 'a name reflection cannot load is absent from this backend (RFC 1 §5.3)', ); } @@ -105,10 +103,9 @@ public function testLookupFunctionReturnsNullForAnUnknownFunction(): void public function testLookupFunctionCachesAResolvedFunction(): void { $backend = $this->backend(self::createStub(NamespaceCatalog::class)); - $name = FunctionName::fromFullyQualified('str_contains'); - $first = $backend->lookupFunction($name); - $second = $backend->lookupFunction($name); + $first = self::functionIn($backend, 'str_contains'); + $second = self::functionIn($backend, 'str_contains'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); self::assertSame($first, $second, 'a second lookup must return the cached instance, not re-reflect'); @@ -121,10 +118,10 @@ public function testFunctionAndClassLikeCachesDoNotCollide(): void // ClassInfo to a function lookup. $backend = $this->backend(self::createStub(NamespaceCatalog::class)); - $backend->lookupClassLike(self::className(\ArrayObject::class)); + self::classLikeIn($backend, \ArrayObject::class); self::assertNull( - $backend->lookupFunction(FunctionName::fromFullyQualified('ArrayObject')), + self::functionIn($backend, 'ArrayObject'), 'a cached class-like must not answer a function lookup of the same name', ); } @@ -153,10 +150,4 @@ public function testChildrenOfForwardsToTheReflectionCatalog(): void 'enumeration must forward the namespace path to the catalog and return its result', ); } - - private static function className(string $fqn): ClassName - { - /** @phpstan-ignore argument.type (virtual names are not analyzed) */ - return new ClassName($fqn); - } } diff --git a/tests/Knowledge/DocumentSymbolSinkTest.php b/tests/Knowledge/DocumentSymbolSinkTest.php index f47d8051..abe09e7b 100644 --- a/tests/Knowledge/DocumentSymbolSinkTest.php +++ b/tests/Knowledge/DocumentSymbolSinkTest.php @@ -5,8 +5,6 @@ namespace Firehed\PhpLsp\Tests\Knowledge; use Firehed\PhpLsp\Document\TextDocument; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Index\DeclarationScanner; use Firehed\PhpLsp\Index\DocumentIndexer; use Firehed\PhpLsp\Index\SymbolExtractor; @@ -30,6 +28,7 @@ final class DocumentSymbolSinkTest extends TestCase { use LoadsFixturesTrait; + use LooksUpBackendSymbolsTrait; private SymbolIndex $index; private OpenDocumentBackend $backend; @@ -58,7 +57,7 @@ public function testOpenDocumentRegistersClassesAndIndexesSymbols(): void $this->sink->openDocument(new TextDocument('file:///Widget.php', 'php', 1, $content)); self::assertNotNull( - $this->backend->lookupClassLike(self::className('V\Widget')), + self::classLikeIn($this->backend, 'V\Widget'), 'openDocument must register the class for lookup', ); self::assertNotNull( @@ -74,11 +73,11 @@ public function testOpenDocumentRegistersFunctionsUnderTheirQualifiedNames(): vo $this->sink->openDocument(new TextDocument('file:///helpers.php', 'php', 1, $content)); self::assertNotNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\helper')), + self::functionIn($this->backend, 'V\helper'), 'openDocument must register the document\'s functions for lookup', ); self::assertNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('helper')), + self::functionIn($this->backend, 'helper'), 'a namespaced function must not be registered under its short name', ); } @@ -93,7 +92,7 @@ public function testOpenDocumentRegistersADeclarationBelowTheTopLevel(): void $this->sink->openDocument(new TextDocument('file:///polyfill.php', 'php', 1, $content)); self::assertNotNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('polyfill')), + self::functionIn($this->backend, 'polyfill'), 'a conditionally declared function must be registered like any other declaration', ); } @@ -106,7 +105,7 @@ public function testOpenDocumentRegistersAClassLikeBelowTheTopLevel(): void $this->sink->openDocument(new TextDocument($uri, 'php', 1, $this->loadFixture('MultiClass/MultiClass.php'))); self::assertNotNull( - $this->backend->lookupClassLike(self::className('Fixtures\Completion\ConditionalInMultiFile')), + self::classLikeIn($this->backend, 'Fixtures\Completion\ConditionalInMultiFile'), 'a conditionally declared class must be registered like any other declaration', ); } @@ -120,7 +119,7 @@ public function testTheFirstOfDuplicateClassLikeDeclarationsWins(): void $content = $this->loadFixture('MultiClass/DuplicateDeclarations.php'); $this->sink->openDocument(new TextDocument($uri, 'php', 1, $content)); - $classInfo = $this->backend->lookupClassLike(self::className('Fixtures\MultiClass\Duplicated')); + $classInfo = self::classLikeIn($this->backend, 'Fixtures\MultiClass\Duplicated'); self::assertNotNull($classInfo, 'the duplicated class must still resolve'); self::assertTrue( $classInfo->isFinal, @@ -135,9 +134,7 @@ public function testTheFirstOfDuplicateFunctionDeclarationsWins(): void $content = $this->loadFixture('MultiClass/DuplicateDeclarations.php'); $this->sink->openDocument(new TextDocument($uri, 'php', 1, $content)); - $functionInfo = $this->backend->lookupFunction( - FunctionName::fromFullyQualified('Fixtures\MultiClass\duplicated'), - ); + $functionInfo = self::functionIn($this->backend, 'Fixtures\MultiClass\duplicated'); self::assertNotNull($functionInfo, 'the duplicated function must still resolve'); self::assertSame( 'string', @@ -154,7 +151,7 @@ public function testUpdatingAwayFromAFunctionDropsItsRegistration(): void $this->sink->updateDocument(new TextDocument($uri, 'php', 2, "backend->lookupFunction(FunctionName::fromFullyQualified('helper')), + self::functionIn($this->backend, 'helper'), 'a document that no longer declares the function must drop its registration', ); } @@ -167,7 +164,7 @@ public function testCloseDocumentDropsItsFunctions(): void $this->sink->closeDocument($uri); self::assertNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('helper')), + self::functionIn($this->backend, 'helper'), 'close must drop the registered functions from lookup', ); } @@ -181,11 +178,11 @@ public function testUpdateDocumentReplacesThePriorSymbolsInBothStores(): void self::assertNull($this->index->findByFqn('V\Alpha'), 'update must clear the prior symbols from the index'); self::assertNotNull($this->index->findByFqn('V\Beta'), 'update must index the new symbols'); self::assertNotNull( - $this->backend->lookupClassLike(self::className('V\Beta')), + self::classLikeIn($this->backend, 'V\Beta'), 'update must register the new class for lookup', ); self::assertNull( - $this->backend->lookupClassLike(self::className('V\Alpha')), + self::classLikeIn($this->backend, 'V\Alpha'), 'update must drop the prior class from lookup', ); } @@ -199,7 +196,7 @@ public function testCloseDocumentClearsBothStores(): void self::assertNull($this->index->findByFqn('V\Ephemeral'), 'close must clear the indexed symbols'); self::assertNull( - $this->backend->lookupClassLike(self::className('V\Ephemeral')), + self::classLikeIn($this->backend, 'V\Ephemeral'), 'close must drop the registered class from lookup', ); } @@ -233,7 +230,7 @@ public function testUpdatingAwayFromAllClassesClearsTheBackendNotJustTheIndex(): $uri = 'file:///Doc.php'; $this->sink->openDocument(new TextDocument($uri, 'php', 1, "backend->lookupClassLike(self::className('V\Widget')), + self::classLikeIn($this->backend, 'V\Widget'), 'the class is registered while the document declares it', ); @@ -245,7 +242,7 @@ public function testUpdatingAwayFromAllClassesClearsTheBackendNotJustTheIndex(): $this->sink->updateDocument(new TextDocument($uri, 'php', 2, "backend->lookupClassLike(self::className('V\Widget')), + self::classLikeIn($this->backend, 'V\Widget'), 'a document that no longer declares the class must drop its registration', ); self::assertSame( @@ -266,7 +263,7 @@ public function testEveryRegisteredClassLikeIsAlsoIndexed(string $fixture, strin $this->sink->openDocument(new TextDocument($uri, 'php', 1, $this->loadFixture($fixture))); self::assertNotNull( - $this->backend->lookupClassLike(self::className($fqn)), + self::classLikeIn($this->backend, $fqn), "{$fqn} must be registered for lookup", ); self::assertNotNull( @@ -336,10 +333,4 @@ private function sinkWithOnDiskBackends(Invalidatable ...$onDiskBackends): Docum array_values($onDiskBackends), ); } - - private static function className(string $fqn): ClassName - { - /** @phpstan-ignore argument.type (virtual names are not analyzed) */ - return new ClassName($fqn); - } } diff --git a/tests/Knowledge/FakeSymbolBackend.php b/tests/Knowledge/FakeSymbolBackend.php index 5d36a3c6..e0631b47 100644 --- a/tests/Knowledge/FakeSymbolBackend.php +++ b/tests/Knowledge/FakeSymbolBackend.php @@ -5,9 +5,10 @@ namespace Firehed\PhpLsp\Tests\Knowledge; use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\ClassName; use Firehed\PhpLsp\Domain\FunctionInfo; -use Firehed\PhpLsp\Domain\FunctionName; +use Firehed\PhpLsp\Domain\NameKind; +use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Index\Symbol; use Firehed\PhpLsp\Knowledge\NamespaceName; @@ -39,14 +40,15 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents return $this->namespaces[$namespace->path] ?? new NamespaceContents(); } - public function lookupClassLike(ClassName $name): ?ClassInfo + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - return $this->classLikes[strtolower(ltrim($name->fqn, '\\'))] ?? null; - } + $configured = match ($kind) { + NameKind::ClassLike => $this->classLikes, + NameKind::Function_ => $this->functions, + NameKind::Constant => [], + }; - public function lookupFunction(FunctionName $name): ?FunctionInfo - { - return $this->functions[strtolower($name->fullyQualifiedName())] ?? null; + return $configured[strtolower($name->fullyQualifiedName())] ?? null; } /** diff --git a/tests/Knowledge/FilesystemBackendTest.php b/tests/Knowledge/FilesystemBackendTest.php index d79514b9..fff6b6cd 100644 --- a/tests/Knowledge/FilesystemBackendTest.php +++ b/tests/Knowledge/FilesystemBackendTest.php @@ -6,8 +6,6 @@ use Firehed\PhpLsp\Cache\CacheFactory; use Firehed\PhpLsp\Document\FileUri; -use Firehed\PhpLsp\Domain\ClassName; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Index\AutoloadFilesLocator; use Firehed\PhpLsp\Index\CachedNamespaceCatalog; use Firehed\PhpLsp\Index\ComposerAutoloadMap; @@ -17,18 +15,18 @@ use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Knowledge\CompositeSymbolLocator; +use Firehed\PhpLsp\Knowledge\DeclarationSymbolInfoFactory; use Firehed\PhpLsp\Knowledge\FilesystemBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; use Firehed\PhpLsp\Knowledge\SymbolLocator; use Firehed\PhpLsp\Parser\ParserService; -use Firehed\PhpLsp\Repository\ClassInfoFactory; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; use Firehed\PhpLsp\Tests\Index\CountingNamespaceCatalog; use Psr\SimpleCache\CacheInterface; use PHPUnit\Framework\TestCase; /** - * The filesystem backend resolves class-likes by locating and parsing one file, and + * The filesystem backend resolves symbols by locating and parsing one file, and * enumerates namespaces through the autoload map — the workspace and vendor roles * both run this code, differing only in the map subset they are given. These prove * lookup, its caching, the not-found paths, the empty prefix search, and that @@ -36,20 +34,22 @@ */ final class FilesystemBackendTest extends TestCase { + use LooksUpBackendSymbolsTrait; + private string $fixturesRoot; private ParserService $parser; - private ClassInfoFactory $factory; + private DeclarationSymbolInfoFactory $infoFactory; protected function setUp(): void { $this->fixturesRoot = dirname(__DIR__, 2) . '/tests/Fixtures'; $this->parser = new ParserService(); - $this->factory = new DefaultClassInfoFactory(); + $this->infoFactory = new DeclarationSymbolInfoFactory(new DefaultClassInfoFactory()); } public function testLookupClassLikeResolvesAndParsesAFixtureClass(): void { - $info = $this->backend()->lookupClassLike(self::className('Fixtures\Domain\User')); + $info = self::classLikeIn($this->backend(), 'Fixtures\Domain\User'); self::assertNotNull($info, 'a class reachable through the autoload map must resolve'); self::assertSame('Fixtures\Domain\User', $info->name->fqn, 'the located class must be returned'); @@ -58,7 +58,7 @@ public function testLookupClassLikeResolvesAndParsesAFixtureClass(): void public function testLookupClassLikeReturnsNullForAnAbsentClass(): void { self::assertNull( - $this->backend()->lookupClassLike(self::className('Fixtures\Does\Not\Exist')), + self::classLikeIn($this->backend(), 'Fixtures\Does\Not\Exist'), 'a name the autoload map cannot locate is absent from this backend (RFC 1 §5.3)', ); } @@ -66,10 +66,9 @@ public function testLookupClassLikeReturnsNullForAnAbsentClass(): void public function testLookupClassLikeCachesAResolvedClass(): void { $backend = $this->backend(); - $name = self::className('Fixtures\Domain\User'); - $first = $backend->lookupClassLike($name); - $second = $backend->lookupClassLike($name); + $first = self::classLikeIn($backend, 'Fixtures\Domain\User'); + $second = self::classLikeIn($backend, 'Fixtures\Domain\User'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); self::assertSame($first, $second, 'a second lookup must return the cached instance, not re-parse'); @@ -80,7 +79,7 @@ public function testLookupClassLikeReturnsNullWhenTheLocatedFileIsUnreadable(): $backend = $this->backendWithLocator($this->locatorReturning('/no/such/file/Ghost.php')); self::assertNull( - $backend->lookupClassLike(self::className('Ghost')), + self::classLikeIn($backend, 'Ghost'), 'a located path that is not readable degrades to not-found rather than an error', ); } @@ -94,7 +93,7 @@ public function testLookupClassLikeReturnsNullWhenTheFileDoesNotDeclareTheClass( ); self::assertNull( - $backend->lookupClassLike(self::className('Fixtures\TypeInference\NotDeclaredHere')), + self::classLikeIn($backend, 'Fixtures\TypeInference\NotDeclaredHere'), 'a located file that does not declare the requested class resolves to null', ); } @@ -109,7 +108,7 @@ public function testLookupClassLikeResolvesADeclarationBelowTheTopLevel(): void ); self::assertNotNull( - $backend->lookupClassLike(self::className('Fixtures\Completion\ConditionalInMultiFile')), + self::classLikeIn($backend, 'Fixtures\Completion\ConditionalInMultiFile'), 'a conditionally declared class must resolve like any other declaration', ); } @@ -121,16 +120,14 @@ public function testLookupClassLikeIsCaseInsensitive(): void ); self::assertNotNull( - $backend->lookupClassLike(self::className('fixtures\domain\user')), + self::classLikeIn($backend, 'fixtures\domain\user'), 'PHP matches class names case-insensitively, as the function path already does', ); } public function testLookupFunctionResolvesAFunctionDeclaredInAnAutoloadFilesEntry(): void { - $info = $this->backend()->lookupFunction( - FunctionName::fromFullyQualified('Fixtures\Helpers\helperFormat'), - ); + $info = self::functionIn($this->backend(), 'Fixtures\Helpers\helperFormat'); self::assertNotNull($info, 'a function in the files set must resolve through the derived index'); self::assertCount(1, $info->parameters, 'the parsed signature must be carried'); @@ -148,7 +145,7 @@ public function testLookupFunctionResolvesADeclarationBelowTheTopLevel(): void // narrowed to top-level statements would miss it, and the name would resolve // from an open document but not from disk. self::assertNotNull( - $this->backend()->lookupFunction(FunctionName::fromFullyQualified('fixtureConditionalHelper')), + self::functionIn($this->backend(), 'fixtureConditionalHelper'), 'a conditionally declared function must resolve like any other declaration', ); } @@ -156,9 +153,7 @@ public function testLookupFunctionResolvesADeclarationBelowTheTopLevel(): void public function testLookupFunctionIsCaseInsensitive(): void { self::assertNotNull( - $this->backend()->lookupFunction( - FunctionName::fromFullyQualified('FIXTURES\HELPERS\HELPERFORMAT'), - ), + self::functionIn($this->backend(), 'FIXTURES\HELPERS\HELPERFORMAT'), 'PHP matches function names case-insensitively', ); } @@ -169,9 +164,7 @@ public function testLookupFunctionReturnsNullForAFunctionOnlyAPsr4FileDeclares() // function in an unopened PSR-4 file has no name -> file route at all. That // is Plan 0002 §3's locate-only limitation, not a gap in the backend. self::assertNull( - $this->backend()->lookupFunction( - FunctionName::fromFullyQualified('Fixtures\Completion\calculateSum'), - ), + self::functionIn($this->backend(), 'Fixtures\Completion\calculateSum'), 'no autoload map addresses a function by name outside the files set', ); } @@ -179,7 +172,7 @@ public function testLookupFunctionReturnsNullForAFunctionOnlyAPsr4FileDeclares() public function testLookupFunctionReturnsNullForAnAbsentFunction(): void { self::assertNull( - $this->backend()->lookupFunction(FunctionName::fromFullyQualified('Fixtures\no_such_helper')), + self::functionIn($this->backend(), 'Fixtures\no_such_helper'), 'a name no locator can reach is absent from this backend (RFC 1 §5.3)', ); } @@ -191,7 +184,7 @@ public function testLookupFunctionReturnsNullWhenTheLocatedFileDoesNotDeclareIt( ); self::assertNull( - $backend->lookupFunction(FunctionName::fromFullyQualified('notInThisFile')), + self::functionIn($backend, 'notInThisFile'), 'a located file that does not declare the requested function resolves to null', ); } @@ -201,7 +194,7 @@ public function testLookupFunctionReturnsNullWhenTheLocatedFileIsUnreadable(): v $backend = $this->backendWithLocator($this->locatorReturning('/no/such/file/helpers.php')); self::assertNull( - $backend->lookupFunction(FunctionName::fromFullyQualified('ghostHelper')), + self::functionIn($backend, 'ghostHelper'), 'a located path that is not readable degrades to not-found rather than an error', ); } @@ -209,10 +202,9 @@ public function testLookupFunctionReturnsNullWhenTheLocatedFileIsUnreadable(): v public function testLookupFunctionCachesAResolvedFunction(): void { $backend = $this->backend(); - $name = FunctionName::fromFullyQualified('Fixtures\Helpers\helperFormat'); - $first = $backend->lookupFunction($name); - $second = $backend->lookupFunction($name); + $first = self::functionIn($backend, 'Fixtures\Helpers\helperFormat'); + $second = self::functionIn($backend, 'Fixtures\Helpers\helperFormat'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); self::assertSame($first, $second, 'a second lookup must return the cached instance, not re-parse'); @@ -234,8 +226,8 @@ public function testFunctionAndClassLikeCachesDoNotCollide(): void $backend = $this->backendWithLocator($this->locatorReturning($path)); - $class = $backend->lookupClassLike(self::className('Dual')); - $function = $backend->lookupFunction(FunctionName::fromFullyQualified('Dual')); + $class = self::classLikeIn($backend, 'Dual'); + $function = self::functionIn($backend, 'Dual'); self::assertNotNull($class, 'the class-like must resolve'); self::assertNotNull($function, 'the function must resolve rather than hit the class entry'); @@ -247,13 +239,12 @@ public function testFunctionAndClassLikeCachesDoNotCollide(): void public function testInvalidateEvictsTheCachedFunctionSoTheNextLookupReParses(): void { $backend = $this->backend(); - $name = FunctionName::fromFullyQualified('Fixtures\Helpers\helperFormat'); - $first = $backend->lookupFunction($name); + $first = self::functionIn($backend, 'Fixtures\Helpers\helperFormat'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); $backend->invalidate(FileUri::fromPath($this->fixturesRoot . '/AutoloadFiles/helpers.php')); - $second = $backend->lookupFunction($name); + $second = self::functionIn($backend, 'Fixtures\Helpers\helperFormat'); self::assertNotNull($second, 'the function must resolve again after invalidation'); self::assertNotSame( @@ -266,13 +257,12 @@ public function testInvalidateEvictsTheCachedFunctionSoTheNextLookupReParses(): public function testInvalidateEvictsTheCachedClassSoTheNextLookupReParses(): void { $backend = $this->backend(); - $name = self::className('Fixtures\Domain\User'); - $first = $backend->lookupClassLike($name); + $first = self::classLikeIn($backend, 'Fixtures\Domain\User'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); $backend->invalidate('file://' . $this->fixturesRoot . '/src/Domain/User.php'); - $second = $backend->lookupClassLike($name); + $second = self::classLikeIn($backend, 'Fixtures\Domain\User'); self::assertNotNull($second, 'the class must resolve again after invalidation'); self::assertNotSame( @@ -289,7 +279,7 @@ public function testInvalidateAlsoDropsCachedNamespaceListings(): void self::createStub(SymbolLocator::class), new CachedNamespaceCatalog($counting, CacheFactory::inMemory()), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), CacheFactory::inMemory(), ); @@ -322,13 +312,12 @@ public function testInvalidateDecodesAPercentEncodedUriToMatchTheCachedPath(): v ); $backend = $this->backendWithLocator($this->locatorReturning($path)); - $name = self::className('Spaced'); - $first = $backend->lookupClassLike($name); + $first = self::classLikeIn($backend, 'Spaced'); self::assertNotNull($first, 'the first lookup must resolve so the cache is populated'); $backend->invalidate('file://' . str_replace(' ', '%20', $path)); - $second = $backend->lookupClassLike($name); + $second = self::classLikeIn($backend, 'Spaced'); self::assertNotNull($second, 'the class must resolve again after invalidation'); self::assertNotSame( @@ -369,7 +358,7 @@ public function testInvalidateReachesALocatorHoldingDerivedState(): void ])); self::assertNotNull( - $backend->lookupClassLike(self::className('DerivedBefore')), + self::classLikeIn($backend, 'DerivedBefore'), 'a class-like declared in a files entry must resolve through the derived index', ); @@ -380,7 +369,7 @@ public function testInvalidateReachesALocatorHoldingDerivedState(): void $backend->invalidate(FileUri::fromPath($path)); self::assertNotNull( - $backend->lookupClassLike(self::className('DerivedAfter')), + self::classLikeIn($backend, 'DerivedAfter'), 'invalidate must re-derive the index so a class added on disk resolves', ); } finally { @@ -395,7 +384,7 @@ public function testInvalidateAnUncachedFileIsHarmless(): void $backend->invalidate('file:///never/looked/up.php'); self::assertNotNull( - $backend->lookupClassLike(self::className('Fixtures\Domain\User')), + self::classLikeIn($backend, 'Fixtures\Domain\User'), 'invalidating a file that was never cached must not disturb later lookups', ); } @@ -409,7 +398,7 @@ public function testInvalidateToleratesANonFileUri(): void $backend->invalidate('untitled:Untitled-1'); self::assertNotNull( - $backend->lookupClassLike(self::className('Fixtures\Domain\User')), + self::classLikeIn($backend, 'Fixtures\Domain\User'), 'a non-file:// URI must be handled without error', ); } @@ -436,7 +425,7 @@ public function testChildrenOfForwardsToTheInjectedCatalog(): void self::createStub(SymbolLocator::class), $catalog, $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), CacheFactory::inMemory(), ); @@ -476,7 +465,7 @@ private function backend(): FilesystemBackend ]), new ComposerNamespaceSource($map), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), CacheFactory::inMemory(), ); @@ -488,7 +477,7 @@ private function backendWithLocator(SymbolLocator $locator): FilesystemBackend $locator, self::createStub(NamespaceCatalog::class), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), CacheFactory::inMemory(), ); @@ -501,10 +490,4 @@ private function locatorReturning(string $path): SymbolLocator return $locator; } - - private static function className(string $fqn): ClassName - { - /** @phpstan-ignore argument.type (fixture and virtual names are not analyzed) */ - return new ClassName($fqn); - } } diff --git a/tests/Knowledge/LooksUpBackendSymbolsTrait.php b/tests/Knowledge/LooksUpBackendSymbolsTrait.php new file mode 100644 index 00000000..8b5fa601 --- /dev/null +++ b/tests/Knowledge/LooksUpBackendSymbolsTrait.php @@ -0,0 +1,46 @@ +lookup(QualifiedName::fromFullyQualified($fqn), NameKind::ClassLike); + if ($info === null) { + return null; + } + self::assertInstanceOf(ClassInfo::class, $info, 'a class-like lookup must answer with ClassInfo'); + + return $info; + } + + private static function functionIn(SymbolBackend $backend, string $fqn): ?FunctionInfo + { + $info = $backend->lookup(QualifiedName::fromFullyQualified($fqn), NameKind::Function_); + if ($info === null) { + return null; + } + self::assertInstanceOf(FunctionInfo::class, $info, 'a function lookup must answer with FunctionInfo'); + + return $info; + } +} diff --git a/tests/Knowledge/OpenDocumentBackendTest.php b/tests/Knowledge/OpenDocumentBackendTest.php index e2d09d1d..66be097e 100644 --- a/tests/Knowledge/OpenDocumentBackendTest.php +++ b/tests/Knowledge/OpenDocumentBackendTest.php @@ -4,7 +4,6 @@ namespace Firehed\PhpLsp\Tests\Knowledge; -use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Index\Location; use Firehed\PhpLsp\Index\Symbol; use Firehed\PhpLsp\Index\SymbolIndex; @@ -23,6 +22,7 @@ final class OpenDocumentBackendTest extends TestCase { use BuildsSymbolInfoTrait; + use LooksUpBackendSymbolsTrait; private SymbolIndex $index; private OpenDocumentBackend $backend; @@ -37,7 +37,7 @@ public function testLookupClassLikeReturnsARegisteredClass(): void { $this->backend->updateDocument('file:///Widget.php', [self::classInfo('V\Widget')]); - $info = $this->backend->lookupClassLike(self::className('V\Widget')); + $info = self::classLikeIn($this->backend, 'V\Widget'); self::assertNotNull($info, 'a registered class must resolve'); self::assertSame('V\Widget', $info->name->fqn, 'the registered class must be returned unchanged'); @@ -46,7 +46,7 @@ public function testLookupClassLikeReturnsARegisteredClass(): void public function testLookupClassLikeReturnsNullForAnUnregisteredClass(): void { self::assertNull( - $this->backend->lookupClassLike(self::className('V\Absent')), + self::classLikeIn($this->backend, 'V\Absent'), 'a name no open document declares is absent from this backend (RFC 1 §5.3)', ); } @@ -58,11 +58,11 @@ public function testUpdateDocumentReplacesThePriorClassesForThatUri(): void $this->backend->updateDocument($uri, [self::classInfo('V\Beta')]); self::assertNull( - $this->backend->lookupClassLike(self::className('V\Alpha')), + self::classLikeIn($this->backend, 'V\Alpha'), 'the prior class must be dropped when the document is re-registered', ); self::assertNotNull( - $this->backend->lookupClassLike(self::className('V\Beta')), + self::classLikeIn($this->backend, 'V\Beta'), 'the new class must be registered', ); } @@ -75,7 +75,7 @@ public function testRemoveDocumentDropsItsClasses(): void $this->backend->removeDocument($uri); self::assertNull( - $this->backend->lookupClassLike(self::className('V\Ephemeral')), + self::classLikeIn($this->backend, 'V\Ephemeral'), 'closing a document must drop the classes it registered', ); } @@ -85,7 +85,7 @@ public function testRemoveDocumentIsANoOpForAnUnknownUri(): void $this->backend->removeDocument('file:///never-opened.php'); self::assertNull( - $this->backend->lookupClassLike(self::className('V\Nothing')), + self::classLikeIn($this->backend, 'V\Nothing'), 'removing a document that was never registered must not error', ); } @@ -94,7 +94,7 @@ public function testLookupFunctionReturnsARegisteredFunction(): void { $this->backend->updateDocument('file:///helpers.php', [], ['V\format' => self::functionInfo('format')]); - $info = $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\format')); + $info = self::functionIn($this->backend, 'V\format'); self::assertNotNull($info, 'a registered function must resolve'); self::assertSame('format', $info->name, 'the registered function must be returned unchanged'); @@ -105,7 +105,7 @@ public function testLookupFunctionIsCaseInsensitive(): void $this->backend->updateDocument('file:///helpers.php', [], ['V\format' => self::functionInfo('format')]); self::assertNotNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\FORMAT')), + self::functionIn($this->backend, 'V\FORMAT'), 'PHP matches function names case-insensitively', ); } @@ -113,7 +113,7 @@ public function testLookupFunctionIsCaseInsensitive(): void public function testLookupFunctionReturnsNullForAnUnregisteredFunction(): void { self::assertNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\absent')), + self::functionIn($this->backend, 'V\absent'), 'a name no open document declares is absent from this backend (RFC 1 §5.3)', ); } @@ -127,11 +127,11 @@ public function testFunctionAndClassLikeRegistrationsDoNotCollide(): void ); self::assertNotNull( - $this->backend->lookupClassLike(self::className('V\Dual')), + self::classLikeIn($this->backend, 'V\Dual'), 'the class-like must resolve', ); self::assertNotNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\Dual')), + self::functionIn($this->backend, 'V\Dual'), 'a function sharing the name must resolve too: the symbol namespaces are independent', ); } @@ -143,11 +143,11 @@ public function testUpdateDocumentReplacesThePriorFunctionsForThatUri(): void $this->backend->updateDocument($uri, [], ['V\beta' => self::functionInfo('beta')]); self::assertNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\alpha')), + self::functionIn($this->backend, 'V\alpha'), 'the prior function must be dropped when the document is re-registered', ); self::assertNotNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\beta')), + self::functionIn($this->backend, 'V\beta'), 'the new function must be registered', ); } @@ -160,7 +160,7 @@ public function testRemoveDocumentDropsItsFunctions(): void $this->backend->removeDocument($uri); self::assertNull( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\ephemeral')), + self::functionIn($this->backend, 'V\ephemeral'), 'closing a document must drop the functions it registered', ); } diff --git a/tests/Parity/BuiltinFunctionParityTest.php b/tests/Parity/BuiltinFunctionParityTest.php index c3956522..33479826 100644 --- a/tests/Parity/BuiltinFunctionParityTest.php +++ b/tests/Parity/BuiltinFunctionParityTest.php @@ -11,6 +11,7 @@ use Firehed\PhpLsp\Index\ReflectionNamespaceSource; use Firehed\PhpLsp\Knowledge\BuiltinBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; +use Firehed\PhpLsp\Knowledge\ReflectionSymbolInfoFactory; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; use Firehed\PhpLsp\Utility\NamespacePath; use PHPUnit\Framework\TestCase; @@ -52,7 +53,7 @@ protected function setUp(): void // Assembled exactly as `KnowledgeStack::forProject` assembles the lowest- // precedence backend, so the oracle measures the shipped configuration. $this->backend = new BuiltinBackend( - new DefaultClassInfoFactory(), + new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()), new CachedNamespaceCatalog(new ReflectionNamespaceSource(), CacheFactory::inMemory()), CacheFactory::inMemory(), ); From 6a0b86a96604a0d4e604468aeccf2a92d92e3a17 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 12:00:21 -0700 Subject: [PATCH 05/35] Add the backend x kind x query coverage grid --- docs/architecture/build-manifest.md | 5 + src/Knowledge/CompositeSymbolSource.php | 6 +- tests/Knowledge/GridQuery.php | 25 ++ tests/Knowledge/SymbolCoverageGridTest.php | 290 +++++++++++++++++++++ 4 files changed, 324 insertions(+), 2 deletions(-) create mode 100644 tests/Knowledge/GridQuery.php create mode 100644 tests/Knowledge/SymbolCoverageGridTest.php diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index 84955bd7..a603b164 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -107,6 +107,7 @@ re-runs repo-wide as its completion gate. SC.13 — Settle Domain->Utility type placement — — SC.14 — Filter BuiltinBackend class-like lookup to internal — — SC.15 — Oracle corpus: trait adaptations and enums — — + SC.16 — Index an open document's global constants — — SZ.1 Z Definition of Done gate + repo-wide dup audit all prior — Notes: @@ -294,6 +295,10 @@ Notes: A live defect; owes a regression test against a class the server vendors but the project does not. - **SC.15** — `TypeGraphParityTest`'s corpus has no trait `insteadof`/`as` shapes and no enums, so the reflection oracle cannot see #73's defect class (nor enum-interface members). Fixture-only slice; #73's fix lands on top of it and must fail before, pass after. + - **SC.16** — `SymbolExtractor` emits no `SymbolKind::Constant`, so a global constant in an open document is never indexed and `OpenDocumentBackend::childrenOf` cannot enumerate it — while the on-disk (`AutoloadFilesLocator`) and built-in (`ReflectionNamespaceSource`) backends both do. + `WorkspaceNamespaceSource` already maps the kind, and only a hand-built index in a unit test ever reaches that arm, so the gap is upstream in the extractor rather than in the catalog. + Found by the S3.8d coverage grid on its first run, which is the grid working as intended: the cell is registered against this row until it is drained. + Ungated, and ahead of S3.8b — constant lookup landing on an enumeration that cannot see open documents would rebuild the §4.2 split on the third symbol namespace. - **SC.7** — `MemberResolver` has six near-identical hierarchy walks: `find{Method,Property,Constant}InHierarchy` and `collect{Methods,Properties,Constants}`, each a seen-check, a scan of the class's own members, and a recursion over diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index aaa3b455..44a8d8e4 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -37,10 +37,12 @@ final class CompositeSymbolSource implements SymbolSource /** * @param list $backends In descending precedence: the first * that answers a lookup wins, and the first to report a name wins a - * merge. + * merge. Readable so the §5.1 coverage grid derives its rows from the + * composition that actually ships rather than a hand-kept list + * ({@see \Firehed\PhpLsp\Tests\Knowledge\SymbolCoverageGridTest}). */ public function __construct( - private readonly array $backends, + public readonly array $backends, ) { } diff --git a/tests/Knowledge/GridQuery.php b/tests/Knowledge/GridQuery.php new file mode 100644 index 00000000..edeb168e --- /dev/null +++ b/tests/Knowledge/GridQuery.php @@ -0,0 +1,25 @@ +||`. + * + * @var array + */ + private const array NOT_APPLICABLE = [ + // Global-constant lookup has no info type yet; the kind reaches the + // backends, and S3.8b lands the type and the Domain\ConstantName naming + // decision it forces. + 'OpenDocumentBackend|Constant|lookup' => 'S3.8b', + 'FilesystemBackend|Constant|lookup' => 'S3.8b', + 'BuiltinBackend|Constant|lookup' => 'S3.8b', + + // `searchClassLikes` has no kind parameter: S3.9a widens it, S3.9b makes the + // backends answer function search. + 'OpenDocumentBackend|Function_|search' => 'S3.9a, S3.9b', + 'OpenDocumentBackend|Constant|search' => 'S3.9a, S3.8b', + 'FilesystemBackend|Function_|search' => 'S3.9a, S3.9b', + 'FilesystemBackend|Constant|search' => 'S3.9a, S3.8b', + 'BuiltinBackend|Function_|search' => 'S3.9a, S3.9b', + 'BuiltinBackend|Constant|search' => 'S3.9a, S3.8b', + + // A prefix has no name -> file map, so project-wide search over disk needs + // the workspace walk RFC 1 §3 defers. Built-in search is deliberately empty: + // offering a name that does not resolve unqualified is auto-import. + 'FilesystemBackend|ClassLike|search' => 'RFC 1 §3', + 'BuiltinBackend|ClassLike|search' => 'RFC 1 §3', + + // `SymbolExtractor` emits no `SymbolKind::Constant`, so an open document's + // global constants never reach the index this enumeration reads — while both + // on-disk and built-in enumeration report constants. Found by this grid. + 'OpenDocumentBackend|Constant|childrenOf' => 'SC.16', + ]; + + /** + * The name each backend should resolve for each kind, and the namespace it + * should enumerate it under. A missing entry fails rather than skipping: that + * is how a newly added kind or backend is forced to declare its coverage. + * + * @var array> + */ + private const array PROBES = [ + 'OpenDocumentBackend' => [ + 'ClassLike' => ['name' => 'Grid\GridWidget', 'namespace' => 'Grid'], + 'Function_' => ['name' => 'Grid\gridHelper', 'namespace' => 'Grid'], + 'Constant' => ['name' => 'Grid\GRID_LIMIT', 'namespace' => 'Grid'], + ], + 'FilesystemBackend' => [ + 'ClassLike' => ['name' => 'Fixtures\Domain\User', 'namespace' => 'Fixtures\Domain'], + 'Function_' => ['name' => 'Fixtures\Helpers\helperFormat', 'namespace' => 'Fixtures\Helpers'], + 'Constant' => ['name' => 'Fixtures\Helpers\HELPER_LIMIT', 'namespace' => 'Fixtures\Helpers'], + ], + 'BuiltinBackend' => [ + 'ClassLike' => ['name' => 'ArrayObject', 'namespace' => ''], + 'Function_' => ['name' => 'str_contains', 'namespace' => ''], + 'Constant' => ['name' => 'PHP_INT_MAX', 'namespace' => ''], + ], + ]; + + /** + * Declares one name of each kind, so the open-document row has something to + * answer for. Written as a document rather than a fixture file because the + * point is what the *editor* holds, which no file on disk can stand in for. + */ + private const string OPEN_DOCUMENT = <<<'PHP' + sink->openDocument( + new TextDocument('file:///virtual/Grid.php', 'php', 1, self::OPEN_DOCUMENT), + ); + + self::assertInstanceOf( + CompositeSymbolSource::class, + $knowledge->source, + 'the grid derives its rows from the composite, so the stack must build one', + ); + $this->source = $knowledge->source; + } + + public function testEveryCellAnswersOrNamesItsBlocker(): void + { + ['unregistered' => $unregistered, 'stale' => $stale] = $this->evaluate(self::NOT_APPLICABLE); + + self::assertSame( + [], + $unregistered, + 'every backend x kind x query cell must answer or be registered not-applicable ' + . 'against a named blocker (RFC 1 §5.1, §8.1)', + ); + self::assertSame( + [], + $stale, + 'a cell that now answers must lose its not-applicable registration, ' + . 'or the blocker outlives the gap (Step Z)', + ); + } + + public function testAnUnregisteredCellIsReported(): void + { + // The mechanism itself: with nothing registered, every cell the stack cannot + // answer must surface. A grid that reported none would pass whatever the + // stack did. + ['unregistered' => $unregistered] = $this->evaluate([]); + + $registered = array_keys(self::NOT_APPLICABLE); + sort($registered); + sort($unregistered); + + self::assertSame( + $registered, + $unregistered, + 'the cells that cannot be answered must be exactly the ones registered: ' + . 'an unregistered gap fails, and a registration for a cell that answers is stale', + ); + } + + public function testARegistrationThatNoLongerBlocksIsReported(): void + { + // The other direction: a cell that does answer must not keep a blocker, or a + // closed gap stays recorded as open and Step Z cannot tell the two apart. + $answering = 'BuiltinBackend|ClassLike|lookup'; + ['stale' => $stale] = $this->evaluate([$answering => 'a blocker that no longer applies']); + + self::assertContains( + $answering . ' (registered against a blocker that no longer applies)', + $stale, + 'a registration on a cell that answers must be reported as stale', + ); + } + + public function testEveryRegistrationNamesABlocker(): void + { + foreach (self::NOT_APPLICABLE as $cell => $blocker) { + self::assertNotSame('', $blocker, "the not-applicable cell {$cell} must name its blocker"); + } + } + + /** + * Walk every cell against a registry, reporting the two ways a cell and its + * registration can disagree. Taking the registry as an argument is what lets the + * mechanism be tested rather than only used. + * + * @param array $notApplicable + * @return array{unregistered: list, stale: list} + */ + private function evaluate(array $notApplicable): array + { + $unregistered = []; + $stale = []; + + foreach ($this->rows() as $row => $backend) { + foreach (NameKind::cases() as $kind) { + foreach (GridQuery::cases() as $query) { + $cell = "{$row}|{$kind->name}|{$query->value}"; + $answered = $this->answers($backend, $row, $kind, $query); + + if (!$answered && !array_key_exists($cell, $notApplicable)) { + $unregistered[] = $cell; + } + if ($answered && array_key_exists($cell, $notApplicable)) { + $stale[] = $cell . ' (registered against ' . $notApplicable[$cell] . ')'; + } + } + } + } + + return ['unregistered' => $unregistered, 'stale' => $stale]; + } + + /** + * The grid's rows: one per backend class in the shipped composite, keyed by + * short name. Derived from the composition itself, so adding a backend adds a + * row whose cells are unregistered until they are declared. + * + * @return array + */ + private function rows(): array + { + $rows = []; + foreach ($this->source->backends as $backend) { + $parts = explode('\\', $backend::class); + $rows[end($parts)] ??= $backend; + } + + return $rows; + } + + private function answers(SymbolBackend $backend, string $row, NameKind $kind, GridQuery $query): bool + { + $probe = self::PROBES[$row][$kind->name] ?? null; + self::assertNotNull($probe, "no probe is defined for the {$row} x {$kind->name} cells"); + + $fqn = $probe['name']; + + return match ($query) { + GridQuery::Lookup => $backend->lookup(QualifiedName::fromFullyQualified($fqn), $kind) !== null, + GridQuery::Search => $this->searchFinds($backend, $fqn), + GridQuery::ChildrenOf => $this->enumerates($backend, $probe['namespace'], $kind, $fqn), + }; + } + + private function searchFinds(SymbolBackend $backend, string $fqn): bool + { + $prefix = QualifiedName::fromFullyQualified($fqn)->shortName; + + foreach ($backend->searchClassLikes($prefix) as $symbol) { + if ($symbol->fullyQualifiedName === $fqn) { + return true; + } + } + + return false; + } + + private function enumerates(SymbolBackend $backend, string $namespace, NameKind $kind, string $fqn): bool + { + foreach ($backend->childrenOf(new NamespaceName($namespace))->symbols as $symbol) { + if ($symbol->kind === $kind && $symbol->fullyQualifiedName === $fqn) { + return true; + } + } + + return false; + } +} From 21e287ba93f5360ca1f779a235fd096d6cf04fcb Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 12:18:47 -0700 Subject: [PATCH 06/35] Document the collapsed backend lookup --- CLAUDE.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 187e8afc..53dde4c7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -139,9 +139,13 @@ symbol namespaces are independent, so a class and a function may share a name. Lookup is **per-kind at the `SymbolSource` facade** — a typed method per kind, taking a name type that carries its kind (`ClassName`, `FunctionName`), because RFC 1 §5.1 requires a concrete return type rather than a type-erased union — and **kind-parameterized at -`SymbolBackend`**, so a new kind is never a change to every backend. The backends still -carry a method per kind today; S3.8d collapses them (Plan 0002 §5.6). Do not read the -facade's closed method set as licence to add a per-kind backend method. +`SymbolBackend`**: one `lookup(QualifiedName, NameKind): ?SymbolInfo`. Do NOT read the +facade's closed method set as licence to add a per-kind backend method. The kind +dispatch lives in two factories, one per metadata route: +`DeclarationSymbolInfoFactory` (parsed declarations) and `ReflectionSymbolInfoFactory` +(the loaded runtime), so a new kind is a case in each rather than a method on every +backend. `SymbolCoverageGridTest` enforces §5.1: a backend × kind × query grid, axes +derived, every cell answering or naming its blocker, an unregistered cell failing. `lookupFunction` reaches open documents, the `autoload.files` set, and PHP's built-ins — the last filtered to `isInternal()`, because reflection also sees the functions the *server's* own From 4866b6c5154303609936dbafd9582ba2d383206f Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 13:16:18 -0700 Subject: [PATCH 07/35] Trim comments to what the code does not say --- CLAUDE.md | 11 ++- docs/architecture/build-manifest.md | 8 +-- src/Domain/SymbolInfo.php | 7 +- src/Knowledge/CompositeSymbolSource.php | 11 +-- .../DeclarationSymbolInfoFactory.php | 14 +--- src/Knowledge/FilesystemBackend.php | 6 +- src/Knowledge/OpenDocumentBackend.php | 20 ++---- src/Knowledge/ReflectionSymbolInfoFactory.php | 25 +++---- src/Knowledge/SymbolBackend.php | 20 ++---- .../DeclarationSymbolInfoFactoryTest.php | 21 ++---- tests/Knowledge/GridQuery.php | 11 ++- .../Knowledge/LooksUpBackendSymbolsTrait.php | 10 +-- .../ReflectionSymbolInfoFactoryTest.php | 12 ++-- tests/Knowledge/SymbolCoverageGridTest.php | 69 ++++++------------- 14 files changed, 76 insertions(+), 169 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 53dde4c7..e075383d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,12 +140,11 @@ Lookup is **per-kind at the `SymbolSource` facade** — a typed method per kind, name type that carries its kind (`ClassName`, `FunctionName`), because RFC 1 §5.1 requires a concrete return type rather than a type-erased union — and **kind-parameterized at `SymbolBackend`**: one `lookup(QualifiedName, NameKind): ?SymbolInfo`. Do NOT read the -facade's closed method set as licence to add a per-kind backend method. The kind -dispatch lives in two factories, one per metadata route: -`DeclarationSymbolInfoFactory` (parsed declarations) and `ReflectionSymbolInfoFactory` -(the loaded runtime), so a new kind is a case in each rather than a method on every -backend. `SymbolCoverageGridTest` enforces §5.1: a backend × kind × query grid, axes -derived, every cell answering or naming its blocker, an unregistered cell failing. +facade's closed method set as licence to add a per-kind backend method. Kind dispatch +lives in `DeclarationSymbolInfoFactory` and `ReflectionSymbolInfoFactory`, one per +metadata route, so a new kind is a case in each rather than a method on every backend. +`SymbolCoverageGridTest` enforces §5.1 with a backend × kind × query grid whose axes +are derived: every cell answers or names its blocker, and an unregistered cell fails. `lookupFunction` reaches open documents, the `autoload.files` set, and PHP's built-ins — the last filtered to `isInternal()`, because reflection also sees the functions the *server's* own diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index a603b164..ad30cfe6 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -295,10 +295,10 @@ Notes: A live defect; owes a regression test against a class the server vendors but the project does not. - **SC.15** — `TypeGraphParityTest`'s corpus has no trait `insteadof`/`as` shapes and no enums, so the reflection oracle cannot see #73's defect class (nor enum-interface members). Fixture-only slice; #73's fix lands on top of it and must fail before, pass after. - - **SC.16** — `SymbolExtractor` emits no `SymbolKind::Constant`, so a global constant in an open document is never indexed and `OpenDocumentBackend::childrenOf` cannot enumerate it — while the on-disk (`AutoloadFilesLocator`) and built-in (`ReflectionNamespaceSource`) backends both do. - `WorkspaceNamespaceSource` already maps the kind, and only a hand-built index in a unit test ever reaches that arm, so the gap is upstream in the extractor rather than in the catalog. - Found by the S3.8d coverage grid on its first run, which is the grid working as intended: the cell is registered against this row until it is drained. - Ungated, and ahead of S3.8b — constant lookup landing on an enumeration that cannot see open documents would rebuild the §4.2 split on the third symbol namespace. + - **SC.16** — `SymbolExtractor` emits no `SymbolKind::Constant`, so a global constant in an open document is never indexed and `OpenDocumentBackend::childrenOf` cannot enumerate it, while the on-disk and built-in backends both do. + `WorkspaceNamespaceSource` already maps the kind, so the gap is upstream in the extractor. + Found by the S3.8d coverage grid on its first run. + Ungated, and ahead of S3.8b — constant lookup landing on an enumeration blind to open documents would rebuild the §4.2 split on the third symbol namespace. - **SC.7** — `MemberResolver` has six near-identical hierarchy walks: `find{Method,Property,Constant}InHierarchy` and `collect{Methods,Properties,Constants}`, each a seen-check, a scan of the class's own members, and a recursion over diff --git a/src/Domain/SymbolInfo.php b/src/Domain/SymbolInfo.php index 886dda46..89258c63 100644 --- a/src/Domain/SymbolInfo.php +++ b/src/Domain/SymbolInfo.php @@ -5,11 +5,10 @@ namespace Firehed\PhpLsp\Domain; /** - * Metadata about a symbol declared in one of PHP's three symbol namespaces, as - * returned by a kind-parameterized backend lookup (Plan 0002 §5.6). + * Metadata about a symbol in one of PHP's three symbol namespaces (Plan 0002 §5.6). * - * {@see Formattable} is not reused for this: it says a value can render itself, - * not that it is a symbol, and the two sets only happen to coincide today. + * Not {@see Formattable}, which says a value renders itself rather than that it is a + * symbol; the two sets only coincide today. */ interface SymbolInfo { diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index 44a8d8e4..53cab0f9 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -37,9 +37,7 @@ final class CompositeSymbolSource implements SymbolSource /** * @param list $backends In descending precedence: the first * that answers a lookup wins, and the first to report a name wins a - * merge. Readable so the §5.1 coverage grid derives its rows from the - * composition that actually ships rather than a hand-kept list - * ({@see \Firehed\PhpLsp\Tests\Knowledge\SymbolCoverageGridTest}). + * merge. Readable so the §5.1 coverage grid derives its rows from it. */ public function __construct( public readonly array $backends, @@ -101,11 +99,8 @@ public function searchClassLikes(string $prefix): array } /** - * The first backend that answers wins, which is what makes precedence - * positional. The kind travels as an argument here and is narrowed back to a - * concrete type by each caller above: O(kinds) narrowings at this one site, - * against the O(kinds × backends) methods a per-kind backend would need - * (Plan 0002 §5.6). + * The first backend that answers wins; each caller above narrows the result back + * to a concrete type, at this one site (Plan 0002 §5.6). */ private function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { diff --git a/src/Knowledge/DeclarationSymbolInfoFactory.php b/src/Knowledge/DeclarationSymbolInfoFactory.php index 494fbf78..8327d520 100644 --- a/src/Knowledge/DeclarationSymbolInfoFactory.php +++ b/src/Knowledge/DeclarationSymbolInfoFactory.php @@ -16,14 +16,8 @@ use PhpParser\Node\Stmt; /** - * Builds the metadata for a name a parsed file declares, given the kind it is being - * asked for. - * - * This is the one place a {@see NameKind} decides which declaration list to search - * and which factory builds the result — the whole of what the kind changes on this - * route (Plan 0002 §5.6). Because it is confined here, {@see SymbolBackend} carries a - * single kind-parameterized lookup, and a new kind is a case in this match rather - * than a method on every backend. + * The one place a {@see NameKind} picks a declaration list and a builder, which is + * what lets {@see SymbolBackend} carry a single lookup (Plan 0002 §5.6). */ final readonly class DeclarationSymbolInfoFactory { @@ -56,9 +50,7 @@ public function fromDeclarations( $kind, static fn(Stmt\Function_ $node): SymbolInfo => FunctionInfo::fromNode($node, $filePath), ), - // The declarations are scanned; what is missing is the global-constant - // info type, which S3.8b lands with the Domain\ConstantName naming - // clash it forces (build-manifest S3.8b). + // Scanned, but the global-constant info type lands in S3.8b. NameKind::Constant => null, }; } diff --git a/src/Knowledge/FilesystemBackend.php b/src/Knowledge/FilesystemBackend.php index 49dc82ff..a0f3a2d0 100644 --- a/src/Knowledge/FilesystemBackend.php +++ b/src/Knowledge/FilesystemBackend.php @@ -39,10 +39,8 @@ final class FilesystemBackend implements SymbolBackend, Invalidatable { /** - * The cache keys derived from each file, so an on-disk change to one file - * evicts exactly its entries. The cache is keyed by an opaque hash of the FQN - * and kind with no reverse mapping to a path, so the path→key relation is - * recorded here as symbols are cached. + * The cache keys derived from each file, recorded because a key is an opaque hash + * with no reverse mapping to a path. * * @var array> */ diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 92157698..52b04b09 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -22,11 +22,9 @@ * * Open documents change on every keystroke and are never cached (RFC 1 §5.3): the * backend reads the live symbol index and its own registered metadata directly. - * Lookup is served from the {@see SymbolInfo} registered per document by the write - * path; namespace enumeration and prefix search are served from the - * {@see SymbolIndex} the write path also populates. The write path feeds both stores - * from one parse ({@see DocumentSymbolSink}, Plan 0002 §5.5 Step 3a(iv)); here they - * are read as they stand. + * Lookup is served from the {@see SymbolInfo} the write path registers per document; + * enumeration and prefix search from the {@see SymbolIndex} it also populates. Both + * stores come from one parse ({@see DocumentSymbolSink}, Plan 0002 §5.5 Step 3a(iv)). */ final class OpenDocumentBackend implements SymbolBackend { @@ -116,14 +114,10 @@ private function register(NameKind $kind, string $fqn, SymbolInfo $info): string } /** - * Registration and lookup must agree on the case rule, and that rule differs by - * kind, so both go through {@see NameKind::normalize()} rather than a local - * lowercasing of the whole FQN — which is right for class-likes and functions - * and wrong for a constant. - * - * The kind is part of the key because one store now holds every kind, and PHP's - * symbol namespaces are independent: a class and a function may share a - * spelling without being the same symbol. + * Registration and lookup must agree on the case rule, which differs by kind, so + * both go through {@see NameKind::normalize()} rather than lowercasing the whole + * FQN — right for class-likes and functions, wrong for a constant. The kind is + * part of the key because one store holds all three. */ private static function key(NameKind $kind, QualifiedName $name): string { diff --git a/src/Knowledge/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php index 9f2486bd..1400064a 100644 --- a/src/Knowledge/ReflectionSymbolInfoFactory.php +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -14,13 +14,9 @@ use ReflectionFunction; /** - * Builds the metadata for a symbol the server runtime has loaded, given the kind it - * is being asked for — the reflection counterpart of - * {@see DeclarationSymbolInfoFactory}, and the same confinement: the kind selects - * which reflection describes the name and nothing else (Plan 0002 §5.6). - * - * Reflection describes the *server's* runtime rather than the project's target, - * which is the known §4.7 gap deferred to Step 5. + * The reflection counterpart of {@see DeclarationSymbolInfoFactory}, describing the + * *server's* runtime rather than the project's target — the §4.7 gap deferred to + * Step 5. */ final readonly class ReflectionSymbolInfoFactory { @@ -34,8 +30,7 @@ public function fromReflection(QualifiedName $name, NameKind $kind): ?SymbolInfo return match ($kind) { NameKind::ClassLike => $this->classInfo($name), NameKind::Function_ => $this->functionInfo($name), - // Reflection can read a global constant, but there is no info type to - // build; S3.8b lands it (build-manifest S3.8b). + // Reflectable, but the info type lands in S3.8b. NameKind::Constant => null, }; } @@ -44,10 +39,8 @@ private function classInfo(QualifiedName $name): ?SymbolInfo { $fqn = $name->fullyQualifiedName(); - // All three, because only `class` and `enum` answer to `class_exists`; an - // interface or trait is a class-like this backend must still describe. Each - // autoloads exactly as constructing the reflection would, so this is the - // same absence test, stated in a form that also carries the name's type. + // All three, since `class_exists` answers for classes and enums only; each + // autoloads exactly as constructing the reflection would. if (!class_exists($fqn) && !interface_exists($fqn) && !trait_exists($fqn)) { return null; } @@ -63,10 +56,8 @@ private function functionInfo(QualifiedName $name): ?SymbolInfo return null; } - // Reflection also sees the functions the server's own dependencies declare, - // which are not the project's. Enumeration is filtered to internal - // (BuiltinFunctionParityTest), so lookup must be too or a name resolves on - // hover while never appearing in completion (RFC 1 §4.2). + // Reflection also sees the server's own dependencies; enumeration filters + // those out, so lookup must too (RFC 1 §4.2). return $reflection->isInternal() ? FunctionInfo::fromReflection($reflection) : null; } } diff --git a/src/Knowledge/SymbolBackend.php b/src/Knowledge/SymbolBackend.php index f036db68..3036671c 100644 --- a/src/Knowledge/SymbolBackend.php +++ b/src/Knowledge/SymbolBackend.php @@ -25,14 +25,10 @@ * vendored file, and the built-ins — is the composite's concern, not the * backend's: each answers only for its own source. * - * Lookup is **kind-parameterized here and per-kind at the facade**, and the split is - * deliberate (Plan 0002 §5.6). {@see SymbolSource} carries a typed method per kind - * because RFC 1 §5.1 requires a concrete return type; a backend takes the kind as an - * argument because the kind changes only the case rule - * ({@see NameKind::normalize()}) and which factory builds the metadata — never how a - * declaring file is found or how a namespace is listed. So a new kind is a name - * type, an info type, and one factory case, rather than a method on every backend. - * Do not re-derive a per-kind backend method from the facade's closed method set. + * Lookup is kind-parameterized here but per-kind at the facade, because the kind + * changes only the case rule and which factory builds the metadata, while §5.1 + * requires a concrete return type (Plan 0002 §5.6). Do not re-derive a per-kind + * backend method from the facade's closed set. */ interface SymbolBackend { @@ -45,10 +41,7 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents; /** * Full metadata for the symbol $name names *as a $kind*, or `null` when this - * backend cannot reach such a declaration (RFC 1 §5.3: absence is a bare null). - * - * PHP's three symbol namespaces are independent, so one name may be both a - * class and a function; $kind is what says which is meant. + * backend cannot reach such a declaration (RFC 1 §5.3). */ public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo; @@ -57,8 +50,7 @@ public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo; * $prefix. A backend with no affordable prefix enumeration returns an empty * list rather than walking its source (RFC 1 §5.3). * - * A kind parameter arrives with S3.9a, which widens search the way this - * interface's lookup is already widened. + * A kind parameter arrives with S3.9a. * * @return list */ diff --git a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php index a7c89202..0042b203 100644 --- a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php +++ b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php @@ -19,20 +19,14 @@ use PHPUnit\Framework\TestCase; /** - * The single place a {@see NameKind} selects which declarations to search and which - * factory builds the metadata (Plan 0002 §5.6). Confining that dispatch here is what - * lets {@see \Firehed\PhpLsp\Knowledge\SymbolBackend} carry one lookup rather than - * one per kind, so a new kind is a case here and not a method on every backend. + * The single place a {@see NameKind} selects a declaration list and a builder + * (Plan 0002 §5.6). */ final class DeclarationSymbolInfoFactoryTest extends TestCase { use LoadsFixturesTrait; - /** - * Declares a class, an interface, a trait, an enum, two functions, and both - * `const` and `define()` constants — every kind this factory dispatches on, in - * one file, so a kind reading the wrong declaration list is visible. - */ + /** Every kind this factory dispatches on, so reading the wrong list is visible. */ private const string FIXTURE = 'AutoloadFiles/helpers.php'; private DeclarationSymbolInfoFactory $factory; @@ -75,10 +69,7 @@ public function testReturnsNullWhenTheFileDeclaresNoSuchName(): void } /** - * The kind selects the declaration list, so a name declared only as one kind is - * not answered when asked for as another. Reading a single merged list — or the - * wrong list — would resolve these, which is the collision PHP's three - * independent symbol namespaces make possible. + * A merged list, or the wrong one, would resolve these. * * @return iterable */ @@ -118,9 +109,7 @@ public function testMatchingFollowsTheKindsCaseRule(string $fqn, NameKind $kind) public function testConstantsAreNotYetBuilt(): void { - // The fixture does declare this constant, so the null is the missing - // global-constant info type, not a missing declaration. S3.8b adds it, - // together with the Domain\ConstantName naming clash it forces. + // The fixture declares it, so the null is the missing info type. self::assertNotSame( [], $this->declarations->constants, diff --git a/tests/Knowledge/GridQuery.php b/tests/Knowledge/GridQuery.php index edeb168e..82ada0b6 100644 --- a/tests/Knowledge/GridQuery.php +++ b/tests/Knowledge/GridQuery.php @@ -7,13 +7,10 @@ use Firehed\PhpLsp\Knowledge\SymbolBackend; /** - * The queries every {@see SymbolBackend} answers, as an axis - * {@see SymbolCoverageGridTest} crosses with {@see \Firehed\PhpLsp\Domain\NameKind} - * to derive its columns. - * - * An enum rather than a list of strings so the grid's dispatch is exhaustive: a - * query added to the interface without a probe fails to compile the match instead of - * falling through a default arm that no cell would ever reach. + * The queries every {@see SymbolBackend} answers, crossed with + * {@see \Firehed\PhpLsp\Domain\NameKind} to form {@see SymbolCoverageGridTest}'s + * columns; an enum so a new query breaks the grid's match rather than falling + * through a default. */ enum GridQuery: string { diff --git a/tests/Knowledge/LooksUpBackendSymbolsTrait.php b/tests/Knowledge/LooksUpBackendSymbolsTrait.php index 8b5fa601..6cfc1cd8 100644 --- a/tests/Knowledge/LooksUpBackendSymbolsTrait.php +++ b/tests/Knowledge/LooksUpBackendSymbolsTrait.php @@ -11,14 +11,8 @@ use Firehed\PhpLsp\Knowledge\SymbolBackend; /** - * Typed lookups against a {@see SymbolBackend}, whose own method is - * kind-parameterized and returns the {@see \Firehed\PhpLsp\Domain\SymbolInfo} marker - * (Plan 0002 §5.6). - * - * The narrowing each helper performs is the assertion - * {@see \Firehed\PhpLsp\Knowledge\CompositeSymbolSource} makes in production, so - * every call site here also pins the kind → info-type contract: a backend answering - * a function lookup with a `ClassInfo` fails at the call, not somewhere downstream. + * Typed lookups against a {@see SymbolBackend}, narrowing as `CompositeSymbolSource` + * does in production so every call site also pins the kind → info-type contract. */ trait LooksUpBackendSymbolsTrait { diff --git a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php index 932185b0..1c1022c5 100644 --- a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php +++ b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php @@ -15,10 +15,8 @@ use PHPUnit\Framework\TestCase; /** - * The reflection counterpart of {@see DeclarationSymbolInfoFactoryTest}: the one - * place a {@see NameKind} decides which reflection describes a built-in, so - * {@see \Firehed\PhpLsp\Knowledge\BuiltinBackend} carries a single kind-parameterized - * lookup (Plan 0002 §5.6). + * The reflection counterpart of {@see DeclarationSymbolInfoFactoryTest} + * (Plan 0002 §5.6). */ final class ReflectionSymbolInfoFactoryTest extends TestCase { @@ -47,10 +45,8 @@ public function testBuildsFunctionInfoForAnInternalFunction(): void public function testIgnoresFunctionsOnlyTheServerHasLoaded(): void { - // The server is itself a PHP program, so reflection sees every function its - // own dependencies declare. Those are not the project's, and the backend - // enumerates only internal functions — a lookup answering more broadly would - // resolve a name completion never offers (RFC 1 §4.2). + // Enumeration is filtered to internal, so a broader lookup would resolve a + // name completion never offers (RFC 1 §4.2). require_once dirname(__DIR__) . '/Domain/Fixtures/documented_function.php'; self::assertNull( diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index ba1ca7e5..16ac2241 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -17,25 +17,14 @@ use PHPUnit\Framework\TestCase; /** - * RFC 1 §8.1's mechanism for §5.1 (uniform coverage across kinds): a **backend × - * kind × query** grid over the stack that actually ships. + * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose axes are + * derived rather than listed, so a new kind or backend adds cells this file never + * anticipated. Every cell answers over the fixtures or names a blocker in + * {@see NOT_APPLICABLE}; an unregistered cell fails, and so does a registration on a + * cell that answers. * - * Both axes are **derived, not listed** — rows from - * {@see CompositeSymbolSource::$backends}, columns from {@see NameKind::cases()} - * crossed with {@see GridQuery::cases()} — so a new kind or a new backend adds cells - * that did not exist when this file was written. Every cell either answers over the - * fixtures or is registered in {@see NOT_APPLICABLE} against a named blocker, and an - * **unregistered cell fails**. A hand-listed grid would enforce nothing a - * hand-written prose rule did not. - * - * The registration is checked in both directions: a cell that answers while still - * registered fails too, so a blocker cannot outlive the gap it describes. Step Z - * requires every survivor to still name a live deferral. - * - * A backend appears once per class. The workspace and vendor - * {@see FilesystemBackend}s differ only in which autoload-map subset they hold, so - * their coverage cannot diverge; what differs is reach, which the parity goldens - * cover. + * One row per backend class: the workspace and vendor {@see FilesystemBackend}s + * differ only in autoload-map subset, so their coverage cannot diverge. */ final class SymbolCoverageGridTest extends TestCase { @@ -46,15 +35,12 @@ final class SymbolCoverageGridTest extends TestCase * @var array */ private const array NOT_APPLICABLE = [ - // Global-constant lookup has no info type yet; the kind reaches the - // backends, and S3.8b lands the type and the Domain\ConstantName naming - // decision it forces. + // The kind reaches the backends; the info type does not exist yet. 'OpenDocumentBackend|Constant|lookup' => 'S3.8b', 'FilesystemBackend|Constant|lookup' => 'S3.8b', 'BuiltinBackend|Constant|lookup' => 'S3.8b', - // `searchClassLikes` has no kind parameter: S3.9a widens it, S3.9b makes the - // backends answer function search. + // `searchClassLikes` has no kind parameter until S3.9a. 'OpenDocumentBackend|Function_|search' => 'S3.9a, S3.9b', 'OpenDocumentBackend|Constant|search' => 'S3.9a, S3.8b', 'FilesystemBackend|Function_|search' => 'S3.9a, S3.9b', @@ -62,22 +48,20 @@ final class SymbolCoverageGridTest extends TestCase 'BuiltinBackend|Function_|search' => 'S3.9a, S3.9b', 'BuiltinBackend|Constant|search' => 'S3.9a, S3.8b', - // A prefix has no name -> file map, so project-wide search over disk needs - // the workspace walk RFC 1 §3 defers. Built-in search is deliberately empty: - // offering a name that does not resolve unqualified is auto-import. + // A prefix has no name -> file map, and offering an unqualified built-in is + // auto-import. 'FilesystemBackend|ClassLike|search' => 'RFC 1 §3', 'BuiltinBackend|ClassLike|search' => 'RFC 1 §3', // `SymbolExtractor` emits no `SymbolKind::Constant`, so an open document's - // global constants never reach the index this enumeration reads — while both - // on-disk and built-in enumeration report constants. Found by this grid. + // constants never reach the index this reads. Found by this grid. 'OpenDocumentBackend|Constant|childrenOf' => 'SC.16', ]; /** - * The name each backend should resolve for each kind, and the namespace it - * should enumerate it under. A missing entry fails rather than skipping: that - * is how a newly added kind or backend is forced to declare its coverage. + * The name each backend should resolve per kind, and the namespace it sits in. A + * missing entry fails rather than skipping, which is what forces a new kind or + * backend to declare its coverage. * * @var array> */ @@ -99,11 +83,7 @@ final class SymbolCoverageGridTest extends TestCase ], ]; - /** - * Declares one name of each kind, so the open-document row has something to - * answer for. Written as a document rather than a fixture file because the - * point is what the *editor* holds, which no file on disk can stand in for. - */ + /** One name of each kind for the open-document row, which no on-disk file can stand in for. */ private const string OPEN_DOCUMENT = <<<'PHP' $unregistered] = $this->evaluate([]); $registered = array_keys(self::NOT_APPLICABLE); @@ -181,8 +159,7 @@ public function testAnUnregisteredCellIsReported(): void public function testARegistrationThatNoLongerBlocksIsReported(): void { - // The other direction: a cell that does answer must not keep a blocker, or a - // closed gap stays recorded as open and Step Z cannot tell the two apart. + // A closed gap that keeps its blocker reads as open, which Step Z cannot see. $answering = 'BuiltinBackend|ClassLike|lookup'; ['stale' => $stale] = $this->evaluate([$answering => 'a blocker that no longer applies']); @@ -201,9 +178,7 @@ public function testEveryRegistrationNamesABlocker(): void } /** - * Walk every cell against a registry, reporting the two ways a cell and its - * registration can disagree. Taking the registry as an argument is what lets the - * mechanism be tested rather than only used. + * The registry is an argument so the mechanism can be tested, not only used. * * @param array $notApplicable * @return array{unregistered: list, stale: list} @@ -233,11 +208,7 @@ private function evaluate(array $notApplicable): array } /** - * The grid's rows: one per backend class in the shipped composite, keyed by - * short name. Derived from the composition itself, so adding a backend adds a - * row whose cells are unregistered until they are declared. - * - * @return array + * @return array Backend short name -> the first of its class */ private function rows(): array { From 3dcb56cf1f0331c89cf8c59c9171e2bbacac39ba Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 13:29:35 -0700 Subject: [PATCH 08/35] Stop rewrapping comments the change did not touch --- src/Knowledge/FilesystemBackend.php | 8 ++++---- src/Knowledge/SymbolBackend.php | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Knowledge/FilesystemBackend.php b/src/Knowledge/FilesystemBackend.php index a0f3a2d0..3d640944 100644 --- a/src/Knowledge/FilesystemBackend.php +++ b/src/Knowledge/FilesystemBackend.php @@ -23,10 +23,10 @@ * given (Plan 0002 §3a: the workspace/vendor precedence split), so one lookup * mechanism covers both rather than two hand-written copies. * - * Lookup locates the file for a name and parses that one file — no `vendor/` - * pre-index (RFC 1 §3, lazy-first). Results are held behind the replaceable cache - * seam (RFC 1 §5.3): a file on disk is stable while unchanged, so a resolved symbol - * is memoized. An on-disk change to a file is signalled through + * Lookup locates the file for a name and parses that one file — no + * `vendor/` pre-index (RFC 1 §3, lazy-first). Results are held behind the + * replaceable cache seam (RFC 1 §5.3): a file on disk is stable while unchanged, so + * a resolved symbol is memoized. An on-disk change to a file is signalled through * {@see invalidate()} ({@see Invalidatable}), which evicts that file's cached * symbols and drops cached namespace listings so the next query reflects disk * (RFC 1 §5.2, §5.3). diff --git a/src/Knowledge/SymbolBackend.php b/src/Knowledge/SymbolBackend.php index 3036671c..1794c586 100644 --- a/src/Knowledge/SymbolBackend.php +++ b/src/Knowledge/SymbolBackend.php @@ -50,8 +50,6 @@ public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo; * $prefix. A backend with no affordable prefix enumeration returns an empty * list rather than walking its source (RFC 1 §5.3). * - * A kind parameter arrives with S3.9a. - * * @return list */ public function searchClassLikes(string $prefix): array; From 7a44f3682213d2000b1e2ad5f26657edd7c05a4f Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 14:49:20 -0700 Subject: [PATCH 09/35] Read cached symbols through one read-through --- src/Knowledge/BuiltinBackend.php | 22 ++----- .../DeclarationSymbolInfoFactory.php | 48 +++++++-------- src/Knowledge/FilesystemBackend.php | 36 +++++------ src/Knowledge/KnowledgeStack.php | 4 +- src/Knowledge/OpenDocumentBackend.php | 16 ++--- src/Knowledge/SymbolCache.php | 60 +++++++++++++++++++ src/Knowledge/SymbolCacheKey.php | 25 -------- tests/Knowledge/BuiltinBackendTest.php | 3 +- tests/Knowledge/FilesystemBackendTest.php | 9 +-- tests/Parity/BuiltinFunctionParityTest.php | 3 +- 10 files changed, 118 insertions(+), 108 deletions(-) create mode 100644 src/Knowledge/SymbolCache.php delete mode 100644 src/Knowledge/SymbolCacheKey.php diff --git a/src/Knowledge/BuiltinBackend.php b/src/Knowledge/BuiltinBackend.php index ba784366..1add021f 100644 --- a/src/Knowledge/BuiltinBackend.php +++ b/src/Knowledge/BuiltinBackend.php @@ -9,7 +9,6 @@ use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; -use Psr\SimpleCache\CacheInterface; /** * The lowest-precedence {@see SymbolBackend}: the symbols built into PHP and its @@ -31,7 +30,7 @@ final class BuiltinBackend implements SymbolBackend public function __construct( private readonly ReflectionSymbolInfoFactory $infoFactory, private readonly NamespaceCatalog $namespaces, - private readonly CacheInterface $cache, + private readonly SymbolCache $cache, ) { } @@ -42,20 +41,11 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - $cacheKey = SymbolCacheKey::for($name, $kind); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof SymbolInfo); - return $cached; - } - - $info = $this->infoFactory->fromReflection($name, $kind); - if ($info !== null) { - $this->cache->set($cacheKey, $info); - } - - return $info; + return $this->cache->remember( + $name, + $kind, + fn(): ?SymbolInfo => $this->infoFactory->fromReflection($name, $kind), + ); } /** diff --git a/src/Knowledge/DeclarationSymbolInfoFactory.php b/src/Knowledge/DeclarationSymbolInfoFactory.php index 8327d520..c88faa63 100644 --- a/src/Knowledge/DeclarationSymbolInfoFactory.php +++ b/src/Knowledge/DeclarationSymbolInfoFactory.php @@ -5,15 +5,13 @@ namespace Firehed\PhpLsp\Knowledge; use Firehed\PhpLsp\Document\FileUri; +use Firehed\PhpLsp\Domain\ClassInfo; use Firehed\PhpLsp\Domain\FunctionInfo; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; use Firehed\PhpLsp\Domain\SymbolInfo; -use Firehed\PhpLsp\Index\Declaration; use Firehed\PhpLsp\Index\FileDeclarations; use Firehed\PhpLsp\Repository\ClassInfoFactory; -use PhpParser\Node; -use PhpParser\Node\Stmt; /** * The one place a {@see NameKind} picks a declaration list and a builder, which is @@ -35,36 +33,32 @@ public function fromDeclarations( $target = $kind->normalize($name); return match ($kind) { - NameKind::ClassLike => $this->firstMatching( - $declarations->classLikes, - $target, - $kind, - fn(Stmt\ClassLike $node): SymbolInfo => $this->classes->fromAstNode( - $node, - FileUri::fromPath($filePath), - ), - ), - NameKind::Function_ => $this->firstMatching( - $declarations->functions, - $target, - $kind, - static fn(Stmt\Function_ $node): SymbolInfo => FunctionInfo::fromNode($node, $filePath), - ), + NameKind::ClassLike => $this->classLike($declarations, $target, $filePath), + NameKind::Function_ => self::standaloneFunction($declarations, $target, $filePath), // Scanned, but the global-constant info type lands in S3.8b. NameKind::Constant => null, }; } - /** - * @template TNode of Node - * @param list> $declarations - * @param callable(TNode): SymbolInfo $build - */ - private function firstMatching(array $declarations, string $target, NameKind $kind, callable $build): ?SymbolInfo + private function classLike(FileDeclarations $declarations, string $target, string $filePath): ?ClassInfo { - foreach ($declarations as $declaration) { - if ($kind->normalize($declaration->name) === $target) { - return $build($declaration->node); + foreach ($declarations->classLikes as $declaration) { + if (NameKind::ClassLike->normalize($declaration->name) === $target) { + return $this->classes->fromAstNode($declaration->node, FileUri::fromPath($filePath)); + } + } + + return null; + } + + private static function standaloneFunction( + FileDeclarations $declarations, + string $target, + string $filePath, + ): ?FunctionInfo { + foreach ($declarations->functions as $declaration) { + if (NameKind::Function_->normalize($declaration->name) === $target) { + return FunctionInfo::fromNode($declaration->node, $filePath); } } diff --git a/src/Knowledge/FilesystemBackend.php b/src/Knowledge/FilesystemBackend.php index 3d640944..51ed4d37 100644 --- a/src/Knowledge/FilesystemBackend.php +++ b/src/Knowledge/FilesystemBackend.php @@ -14,7 +14,6 @@ use Firehed\PhpLsp\Index\NamespaceCatalog; use Firehed\PhpLsp\Index\NamespaceContents; use Firehed\PhpLsp\Parser\ParserService; -use Psr\SimpleCache\CacheInterface; /** * A {@see SymbolBackend} over PHP files on disk, resolved through Composer's @@ -52,7 +51,7 @@ public function __construct( private readonly ParserService $parser, private readonly DeclarationSymbolInfoFactory $infoFactory, private readonly DeclarationScanner $scanner, - private readonly CacheInterface $cache, + private readonly SymbolCache $cache, ) { } @@ -63,26 +62,19 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - $cacheKey = SymbolCacheKey::for($name, $kind); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof SymbolInfo); - return $cached; - } - - $filePath = $this->locator->locate($name, $kind); - if ($filePath === null) { - return null; - } - - $info = $this->infoFactory->fromDeclarations($this->declarationsIn($filePath), $name, $kind, $filePath); - if ($info !== null) { - $this->cache->set($cacheKey, $info); - $this->cacheKeysByPath[$filePath][] = $cacheKey; - } - - return $info; + return $this->cache->remember($name, $kind, function () use ($name, $kind): ?SymbolInfo { + $filePath = $this->locator->locate($name, $kind); + if ($filePath === null) { + return null; + } + + $info = $this->infoFactory->fromDeclarations($this->declarationsIn($filePath), $name, $kind, $filePath); + if ($info !== null) { + $this->cacheKeysByPath[$filePath][] = $this->cache->keyFor($name, $kind); + } + + return $info; + }); } /** diff --git a/src/Knowledge/KnowledgeStack.php b/src/Knowledge/KnowledgeStack.php index 04edf614..fbf2bda7 100644 --- a/src/Knowledge/KnowledgeStack.php +++ b/src/Knowledge/KnowledgeStack.php @@ -69,7 +69,7 @@ public static function forProject( new BuiltinBackend( new ReflectionSymbolInfoFactory($classInfoFactory), new CachedNamespaceCatalog(new ReflectionNamespaceSource(), CacheFactory::inMemory()), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ), ]); @@ -123,7 +123,7 @@ private static function filesystemBackend( $parser, $infoFactory, $scanner, - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } } diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 52b04b09..6046dd95 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -89,10 +89,14 @@ public function updateDocument(string $uri, array $classes, array $functions = [ $keys = []; foreach ($classes as $classInfo) { - $keys[] = $this->register(NameKind::ClassLike, $classInfo->name->fqn, $classInfo); + $key = self::key(NameKind::ClassLike, QualifiedName::fromClassName($classInfo->name)); + $this->byKey[$key] = $classInfo; + $keys[] = $key; } foreach ($functions as $fqn => $functionInfo) { - $keys[] = $this->register(NameKind::Function_, $fqn, $functionInfo); + $key = self::key(NameKind::Function_, QualifiedName::fromFullyQualified($fqn)); + $this->byKey[$key] = $functionInfo; + $keys[] = $key; } $this->keysByUri[$uri] = $keys; } @@ -105,14 +109,6 @@ public function removeDocument(string $uri): void unset($this->keysByUri[$uri]); } - private function register(NameKind $kind, string $fqn, SymbolInfo $info): string - { - $key = self::key($kind, QualifiedName::fromFullyQualified($fqn)); - $this->byKey[$key] = $info; - - return $key; - } - /** * Registration and lookup must agree on the case rule, which differs by kind, so * both go through {@see NameKind::normalize()} rather than lowercasing the whole diff --git a/src/Knowledge/SymbolCache.php b/src/Knowledge/SymbolCache.php new file mode 100644 index 00000000..4a89eeb8 --- /dev/null +++ b/src/Knowledge/SymbolCache.php @@ -0,0 +1,60 @@ +cache->delete($key); + } + + public function keyFor(QualifiedName $name, NameKind $kind): string + { + return CacheKey::from($kind->name . '|' . $kind->normalize($name)); + } + + /** + * @param callable(): ?SymbolInfo $resolve Consulted only on a miss + */ + public function remember(QualifiedName $name, NameKind $kind, callable $resolve): ?SymbolInfo + { + $key = $this->keyFor($name, $kind); + + $cached = $this->cache->get($key); + if ($cached !== null) { + assert($cached instanceof SymbolInfo); + return $cached; + } + + $info = $resolve(); + if ($info !== null) { + $this->cache->set($key, $info); + } + + return $info; + } +} diff --git a/src/Knowledge/SymbolCacheKey.php b/src/Knowledge/SymbolCacheKey.php deleted file mode 100644 index 5ffaeca0..00000000 --- a/src/Knowledge/SymbolCacheKey.php +++ /dev/null @@ -1,25 +0,0 @@ -name . '|' . $kind->normalize($name)); - } -} diff --git a/tests/Knowledge/BuiltinBackendTest.php b/tests/Knowledge/BuiltinBackendTest.php index 74eb86a5..3514dd52 100644 --- a/tests/Knowledge/BuiltinBackendTest.php +++ b/tests/Knowledge/BuiltinBackendTest.php @@ -10,6 +10,7 @@ use Firehed\PhpLsp\Knowledge\BuiltinBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; use Firehed\PhpLsp\Knowledge\ReflectionSymbolInfoFactory; +use Firehed\PhpLsp\Knowledge\SymbolCache; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; use PHPUnit\Framework\TestCase; @@ -28,7 +29,7 @@ private function backend(NamespaceCatalog $namespaces): BuiltinBackend return new BuiltinBackend( new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()), $namespaces, - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } diff --git a/tests/Knowledge/FilesystemBackendTest.php b/tests/Knowledge/FilesystemBackendTest.php index fff6b6cd..557f31c7 100644 --- a/tests/Knowledge/FilesystemBackendTest.php +++ b/tests/Knowledge/FilesystemBackendTest.php @@ -18,6 +18,7 @@ use Firehed\PhpLsp\Knowledge\DeclarationSymbolInfoFactory; use Firehed\PhpLsp\Knowledge\FilesystemBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; +use Firehed\PhpLsp\Knowledge\SymbolCache; use Firehed\PhpLsp\Knowledge\SymbolLocator; use Firehed\PhpLsp\Parser\ParserService; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; @@ -281,7 +282,7 @@ public function testInvalidateAlsoDropsCachedNamespaceListings(): void $this->parser, $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); $backend->childrenOf(new NamespaceName('Psr\Log')); @@ -427,7 +428,7 @@ public function testChildrenOfForwardsToTheInjectedCatalog(): void $this->parser, $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); self::assertSame( @@ -467,7 +468,7 @@ private function backend(): FilesystemBackend $this->parser, $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } @@ -479,7 +480,7 @@ private function backendWithLocator(SymbolLocator $locator): FilesystemBackend $this->parser, $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } diff --git a/tests/Parity/BuiltinFunctionParityTest.php b/tests/Parity/BuiltinFunctionParityTest.php index 33479826..b3ffb757 100644 --- a/tests/Parity/BuiltinFunctionParityTest.php +++ b/tests/Parity/BuiltinFunctionParityTest.php @@ -12,6 +12,7 @@ use Firehed\PhpLsp\Knowledge\BuiltinBackend; use Firehed\PhpLsp\Knowledge\NamespaceName; use Firehed\PhpLsp\Knowledge\ReflectionSymbolInfoFactory; +use Firehed\PhpLsp\Knowledge\SymbolCache; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; use Firehed\PhpLsp\Utility\NamespacePath; use PHPUnit\Framework\TestCase; @@ -55,7 +56,7 @@ protected function setUp(): void $this->backend = new BuiltinBackend( new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()), new CachedNamespaceCatalog(new ReflectionNamespaceSource(), CacheFactory::inMemory()), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } From e25e8b54b3edd47927d04bdd515d8f0ab8ef720e Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 17:25:10 -0700 Subject: [PATCH 10/35] Keep built-in class-like lookup on confined reflection --- src/Knowledge/ReflectionSymbolInfoFactory.php | 10 ++++------ .../ReflectionSymbolInfoFactoryTest.php | 19 ++++++++++++++++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/Knowledge/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php index 1400064a..73a5cf62 100644 --- a/src/Knowledge/ReflectionSymbolInfoFactory.php +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -37,15 +37,13 @@ public function fromReflection(QualifiedName $name, NameKind $kind): ?SymbolInfo private function classInfo(QualifiedName $name): ?SymbolInfo { - $fqn = $name->fullyQualifiedName(); - - // All three, since `class_exists` answers for classes and enums only; each - // autoloads exactly as constructing the reflection would. - if (!class_exists($fqn) && !interface_exists($fqn) && !trait_exists($fqn)) { + try { + $reflection = new ReflectionClass($name->fullyQualifiedName()); + } catch (ReflectionException) { return null; } - return $this->classes->fromReflection(new ReflectionClass($fqn)); + return $this->classes->fromReflection($reflection); } private function functionInfo(QualifiedName $name): ?SymbolInfo diff --git a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php index 1c1022c5..7c6c0c82 100644 --- a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php +++ b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php @@ -27,12 +27,25 @@ protected function setUp(): void $this->factory = new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()); } - public function testBuildsClassInfoForALoadedClass(): void + /** + * PHP declares no internal trait, so the fourth flavour cannot be probed here. + * + * @return iterable + */ + public static function loadedClassLikes(): iterable + { + yield 'class' => [\ArrayObject::class]; + yield 'interface' => [\Countable::class]; + yield 'enum' => [\Random\IntervalBoundary::class]; + } + + #[DataProvider('loadedClassLikes')] + public function testBuildsClassInfoForEveryClassLikeFlavour(string $fqn): void { - $info = $this->build(\ArrayObject::class, NameKind::ClassLike); + $info = $this->build($fqn, NameKind::ClassLike); self::assertInstanceOf(ClassInfo::class, $info, 'a class-like must build ClassInfo'); - self::assertSame('ArrayObject', $info->name->fqn, 'the reflected class must be returned'); + self::assertSame($fqn, $info->name->fqn, 'the reflected class-like must be returned'); } public function testBuildsFunctionInfoForAnInternalFunction(): void From 703bbc1bd524c16e5ee4f9f437aeda0bc73851e7 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 17:34:40 -0700 Subject: [PATCH 11/35] Point the docs at the relocated cache and factory --- CLAUDE.md | 2 +- docs/architecture/build-manifest.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e075383d..dfce270c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -133,7 +133,7 @@ flow through the **`SymbolSource`** read seam (`src/Knowledge/`), implemented by A lookup takes the first backend that answers; enumeration and search merge every backend, the earlier (more authoritative) one winning a name clash. Caching is a per-backend PSR-16 policy (`src/Cache/`); on-disk and built-in results are cached, open -documents never. A cache key carries the `NameKind` (`SymbolCacheKey`): PHP's three +documents never. A cache key carries the `NameKind` (`SymbolCache`): PHP's three symbol namespaces are independent, so a class and a function may share a name. Lookup is **per-kind at the `SymbolSource` facade** — a typed method per kind, taking a diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index ad30cfe6..d6276691 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -291,7 +291,7 @@ Notes: - **SC.13** — Domain factories reach into Utility (`TypeFactory`, `NamespacePath`); decide the direction in-slice (move the utility into Domain, or the factory methods out) and drain the frozen edges. Related: `ClassName::shortName`/`getNamespace` hand-roll the split `NamespacePath` owns, so the direction chosen also settles that duplicate. Likewise `NameKind::normalize` re-implements the path fold `NamespacePath::normalize` owns — layer-blocked from routing through it until this move — so the direction also collapses the two folds into one, and the case-folding allowlist follows the file. - - **SC.14** — `BuiltinBackend::lookupClassLike` lacks the `isInternal()` guard its function sibling has, so hover resolves any class the *server's own* autoloader can load while completion never offers it — the §4.2 lookup/enumeration split, live on the class namespace. + - **SC.14** — `ReflectionSymbolInfoFactory`'s class-like branch lacks the `isInternal()` guard its function sibling has, so hover resolves any class the *server's own* autoloader can load while completion never offers it — the §4.2 lookup/enumeration split, live on the class namespace. A live defect; owes a regression test against a class the server vendors but the project does not. - **SC.15** — `TypeGraphParityTest`'s corpus has no trait `insteadof`/`as` shapes and no enums, so the reflection oracle cannot see #73's defect class (nor enum-interface members). Fixture-only slice; #73's fix lands on top of it and must fail before, pass after. From eee839768f8d9740b21f906faa8bd9932a526e65 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 17:45:33 -0700 Subject: [PATCH 12/35] Hold each grid blocker to a real registry row --- tests/Knowledge/SymbolCoverageGridTest.php | 64 ++++++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index 16ac2241..eed60b2d 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -20,14 +20,17 @@ * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose axes are * derived rather than listed, so a new kind or backend adds cells this file never * anticipated. Every cell answers over the fixtures or names a blocker in - * {@see NOT_APPLICABLE}; an unregistered cell fails, and so does a registration on a - * cell that answers. + * {@see NOT_APPLICABLE}; an unregistered cell fails, so does a registration on a cell + * that answers, and so does a blocker naming no row of the slice registry. * * One row per backend class: the workspace and vendor {@see FilesystemBackend}s * differ only in autoload-map subset, so their coverage cannot diverge. */ final class SymbolCoverageGridTest extends TestCase { + /** The other form a blocker may take, when no slice owns the gap. */ + private const string SECTION_REFERENCE = '/^(RFC 1|Plan 0002) §\d+(\.\d+)*$/u'; + /** * Cells the shipped stack cannot answer, each naming a slice id or an RFC * section. Keyed `||`. @@ -170,11 +173,62 @@ public function testARegistrationThatNoLongerBlocksIsReported(): void ); } - public function testEveryRegistrationNamesABlocker(): void + public function testEveryRegistrationNamesALiveBlocker(): void + { + self::assertSame( + [], + self::danglingBlockers(self::NOT_APPLICABLE), + 'a not-applicable cell must name a slice that is still in the registry, or a section: ' + . 'a blocker nobody owns is the permanent exemption Step Z exists to prevent', + ); + } + + public function testABlockerNamingNoSliceIsReported(): void { - foreach (self::NOT_APPLICABLE as $cell => $blocker) { - self::assertNotSame('', $blocker, "the not-applicable cell {$cell} must name its blocker"); + // A registry that accepted any non-empty string would outlive the slice it names. + self::assertSame( + ['BuiltinBackend|Constant|lookup names S9.99'], + self::danglingBlockers(['BuiltinBackend|Constant|lookup' => 'S9.99']), + 'a blocker matching no registry row and no section must be reported', + ); + } + + /** + * @param array $notApplicable + * @return list The ` names ` pairs that resolve to nothing + */ + private static function danglingBlockers(array $notApplicable): array + { + $slices = self::sliceIds(); + $dangling = []; + + foreach ($notApplicable as $cell => $blocker) { + foreach (explode(', ', $blocker) as $named) { + if (in_array($named, $slices, true) || preg_match(self::SECTION_REFERENCE, $named) === 1) { + continue; + } + $dangling[] = "{$cell} names {$named}"; + } } + + return $dangling; + } + + /** + * The registry is the manifest itself, so a blocker cannot outlive the row it + * names by the row being renamed or dropped. + * + * @return list + */ + private static function sliceIds(): array + { + $manifest = file_get_contents(dirname(__DIR__, 2) . '/docs/architecture/build-manifest.md'); + self::assertNotFalse($manifest, 'the slice registry must be readable'); + + preg_match_all('/^ {4}([A-Z][A-Z0-9]\.\d+[a-z]?) /m', $manifest, $matches); + self::assertNotEmpty($matches[1], 'the slice table must be parseable, or every blocker reads as dangling'); + + return $matches[1]; } /** From a67f641f8d0ea00e6937c8559248525b31b69237 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 17:57:57 -0700 Subject: [PATCH 13/35] Confine runtime symbol-existence checks --- CLAUDE.md | 2 +- phpstan.neon | 12 ++++++++++++ src/Knowledge/ReflectionSymbolInfoFactory.php | 12 ++++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dfce270c..fa019d87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ composer phpcs -- -q --report=emacs # run code style checks (PSR-12) Two CI-enforced mechanisms confine where code may live; a rule firing on your change is design feedback, not an obstacle. -- **Capability confinement** (`phpstan.neon`): AST traversal, symbol-name case folding, regex, runtime reflection, and filesystem reads are each usable only in their named homes (allowlists inline, each with its rationale). +- **Capability confinement** (`phpstan.neon`): AST traversal, symbol-name case folding, regex, runtime reflection, runtime symbol existence/enumeration, and filesystem reads are each usable only in their named homes (allowlists inline, each with its rationale). - **Layer contract** (`deptrac.yaml`): an inter-layer dependency not in the ruleset fails analysis. When a rule fires on your change, in order of preference: diff --git a/phpstan.neon b/phpstan.neon index 3198e7be..5665ee37 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -91,6 +91,18 @@ parameters: - src/Knowledge/BuiltinBackend.php - src/Index/ReflectionNamespaceSource.php - tests/* + - + function: + - 'class_exists()' + - 'defined()' + - 'enum_exists()' + - 'function_exists()' + - 'interface_exists()' + - 'trait_exists()' + message: 'asking the runtime whether a symbol exists is a SymbolSource query (RFC 1 §4.2); ReflectionSymbolInfoFactory is the one place it may be the server''s own runtime' + allowIn: + - src/Knowledge/ReflectionSymbolInfoFactory.php + - tests/* - function: - 'file_get_contents()' diff --git a/src/Knowledge/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php index 73a5cf62..16966b9b 100644 --- a/src/Knowledge/ReflectionSymbolInfoFactory.php +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -37,13 +37,17 @@ public function fromReflection(QualifiedName $name, NameKind $kind): ?SymbolInfo private function classInfo(QualifiedName $name): ?SymbolInfo { - try { - $reflection = new ReflectionClass($name->fullyQualifiedName()); - } catch (ReflectionException) { + $fqn = $name->fullyQualifiedName(); + + // Also what narrows the name to a `class-string`, which is why this kind + // cannot use the sibling's try/catch. All three, since `class_exists` + // answers for classes and enums only; each autoloads exactly as + // constructing the reflection would. + if (!class_exists($fqn) && !interface_exists($fqn) && !trait_exists($fqn)) { return null; } - return $this->classes->fromReflection($reflection); + return $this->classes->fromReflection(new ReflectionClass($fqn)); } private function functionInfo(QualifiedName $name): ?SymbolInfo From 0f8666f6de4b697788b317a755dbcc4da7d13dd5 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 18:28:03 -0700 Subject: [PATCH 14/35] Add the kind-carrying declared-symbol record --- src/Domain/DeclaredSymbol.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/Domain/DeclaredSymbol.php diff --git a/src/Domain/DeclaredSymbol.php b/src/Domain/DeclaredSymbol.php new file mode 100644 index 00000000..7724fd0c --- /dev/null +++ b/src/Domain/DeclaredSymbol.php @@ -0,0 +1,23 @@ + Date: Wed, 12 Aug 2026 18:29:35 -0700 Subject: [PATCH 15/35] Derive by-name lookup from the whole-file scan --- .../DeclarationSymbolInfoFactory.php | 77 +++++++++++++------ .../DeclarationSymbolInfoFactoryTest.php | 55 +++++++++++++ 2 files changed, 107 insertions(+), 25 deletions(-) diff --git a/src/Knowledge/DeclarationSymbolInfoFactory.php b/src/Knowledge/DeclarationSymbolInfoFactory.php index c88faa63..779262e1 100644 --- a/src/Knowledge/DeclarationSymbolInfoFactory.php +++ b/src/Knowledge/DeclarationSymbolInfoFactory.php @@ -5,7 +5,7 @@ namespace Firehed\PhpLsp\Knowledge; use Firehed\PhpLsp\Document\FileUri; -use Firehed\PhpLsp\Domain\ClassInfo; +use Firehed\PhpLsp\Domain\DeclaredSymbol; use Firehed\PhpLsp\Domain\FunctionInfo; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; @@ -15,7 +15,13 @@ /** * The one place a {@see NameKind} picks a declaration list and a builder, which is - * what lets {@see SymbolBackend} carry a single lookup (Plan 0002 §5.6). + * what lets {@see SymbolBackend} carry a single lookup and a single registration + * (Plan 0002 §5.6). + * + * Lookup is a filter over {@see allIn()} rather than its own scan: RFC 1 §5.1 + * forbids a derived verb forking from the one it derives from, and a second scan is + * how the on-disk read path and the open-document write path came to disagree about + * which declarations count. */ final readonly class DeclarationSymbolInfoFactory { @@ -24,6 +30,31 @@ public function __construct( ) { } + /** + * Every symbol the file declares, at any depth. Of duplicates the first wins — + * the one PHP would define. + * + * Global constants are scanned but not built: their info type lands in S3.8b. + * + * @return list + */ + public function allIn(FileDeclarations $declarations, string $filePath): array + { + $symbols = []; + $seen = []; + + foreach ($declarations->classLikes as $declaration) { + $info = $this->classes->fromAstNode($declaration->node, FileUri::fromPath($filePath)); + self::collect($symbols, $seen, $declaration->name, NameKind::ClassLike, $info); + } + foreach ($declarations->functions as $declaration) { + $info = FunctionInfo::fromNode($declaration->node, $filePath); + self::collect($symbols, $seen, $declaration->name, NameKind::Function_, $info); + } + + return $symbols; + } + public function fromDeclarations( FileDeclarations $declarations, QualifiedName $name, @@ -32,36 +63,32 @@ public function fromDeclarations( ): ?SymbolInfo { $target = $kind->normalize($name); - return match ($kind) { - NameKind::ClassLike => $this->classLike($declarations, $target, $filePath), - NameKind::Function_ => self::standaloneFunction($declarations, $target, $filePath), - // Scanned, but the global-constant info type lands in S3.8b. - NameKind::Constant => null, - }; - } - - private function classLike(FileDeclarations $declarations, string $target, string $filePath): ?ClassInfo - { - foreach ($declarations->classLikes as $declaration) { - if (NameKind::ClassLike->normalize($declaration->name) === $target) { - return $this->classes->fromAstNode($declaration->node, FileUri::fromPath($filePath)); + foreach ($this->allIn($declarations, $filePath) as $symbol) { + if ($symbol->kind === $kind && $kind->normalize($symbol->name) === $target) { + return $symbol->info; } } return null; } - private static function standaloneFunction( - FileDeclarations $declarations, - string $target, - string $filePath, - ): ?FunctionInfo { - foreach ($declarations->functions as $declaration) { - if (NameKind::Function_->normalize($declaration->name) === $target) { - return FunctionInfo::fromNode($declaration->node, $filePath); - } + /** + * @param list $symbols + * @param array $seen + */ + private static function collect( + array &$symbols, + array &$seen, + QualifiedName $name, + NameKind $kind, + SymbolInfo $info, + ): void { + $key = $kind->name . '|' . $kind->normalize($name); + if (array_key_exists($key, $seen)) { + return; } - return null; + $seen[$key] = true; + $symbols[] = new DeclaredSymbol($name, $kind, $info); } } diff --git a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php index 0042b203..d44f40eb 100644 --- a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php +++ b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php @@ -4,6 +4,7 @@ namespace Firehed\PhpLsp\Tests\Knowledge; +use Firehed\PhpLsp\Document\TextDocument; use Firehed\PhpLsp\Domain\ClassInfo; use Firehed\PhpLsp\Domain\FunctionInfo; use Firehed\PhpLsp\Domain\NameKind; @@ -121,6 +122,60 @@ public function testConstantsAreNotYetBuilt(): void ); } + public function testAllInReportsEveryBuildableDeclarationWithItsKind(): void + { + $reported = []; + foreach ($this->factory->allIn($this->declarations, $this->path) as $symbol) { + $reported[] = $symbol->kind->name . '|' . $symbol->name->fullyQualifiedName(); + } + + self::assertContains( + 'ClassLike|Fixtures\Helpers\HelperRegistry', + $reported, + 'a class-like the file declares must be reported for registration', + ); + self::assertContains( + 'Function_|Fixtures\Helpers\helperFormat', + $reported, + 'a function the file declares must be reported, under its own kind', + ); + self::assertNotContains( + 'Constant|Fixtures\Helpers\HELPER_LIMIT', + $reported, + 'a scanned kind with no info type yet must be omitted rather than reported empty-handed', + ); + } + + public function testAllInKeepsTheFirstOfDuplicateDeclarations(): void + { + $content = $this->loadFixture('MultiClass/DuplicateDeclarations.php'); + $ast = (new ParserService())->parse(new TextDocument('file:///dupes.php', 'php', 1, $content)); + self::assertNotNull($ast, 'the fixture must parse'); + + $names = []; + foreach ($this->factory->allIn((new DeclarationScanner())->scan($ast), '/dupes.php') as $symbol) { + $names[] = $symbol->kind->name . '|' . $symbol->name->fullyQualifiedName(); + } + + self::assertSame( + array_unique($names), + $names, + 'PHP defines the first declaration of a name, so a second must not register over it', + ); + } + + public function testLookupAgreesWithTheFullScan(): void + { + // RFC 1 §5.1: a derived verb must not fork from the one it derives from. + foreach ($this->factory->allIn($this->declarations, $this->path) as $symbol) { + self::assertEquals( + $symbol->info, + $this->build($symbol->name->fullyQualifiedName(), $symbol->kind), + 'every symbol the scan reports must be reachable by name, with the same metadata', + ); + } + } + private function build(string $fqn, NameKind $kind): ?SymbolInfo { return $this->factory->fromDeclarations( From 057a792f671f2b02ef229dea58546421ae536b66 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 12 Aug 2026 18:30:05 -0700 Subject: [PATCH 16/35] Register open-document symbols by kind, not by bucket --- src/Knowledge/DocumentSymbolSink.php | 71 ++++----------------- src/Knowledge/KnowledgeStack.php | 2 +- src/Knowledge/OpenDocumentBackend.php | 27 +++----- tests/Knowledge/DocumentSymbolSinkTest.php | 5 +- tests/Knowledge/OpenDocumentBackendTest.php | 67 +++++++++++++++---- 5 files changed, 82 insertions(+), 90 deletions(-) diff --git a/src/Knowledge/DocumentSymbolSink.php b/src/Knowledge/DocumentSymbolSink.php index 308a924c..c6bcbef3 100644 --- a/src/Knowledge/DocumentSymbolSink.php +++ b/src/Knowledge/DocumentSymbolSink.php @@ -5,14 +5,13 @@ namespace Firehed\PhpLsp\Knowledge; use Firehed\PhpLsp\Cache\Invalidatable; +use Firehed\PhpLsp\Document\FileUri; use Firehed\PhpLsp\Document\TextDocument; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\FunctionInfo; +use Firehed\PhpLsp\Domain\DeclaredSymbol; +use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Index\DeclarationScanner; use Firehed\PhpLsp\Index\DocumentIndexer; -use Firehed\PhpLsp\Index\FileDeclarations; use Firehed\PhpLsp\Index\SymbolIndex; -use Firehed\PhpLsp\Repository\ClassInfoFactory; use Firehed\PhpLsp\Parser\ParserService; /** @@ -37,7 +36,7 @@ public function __construct( private readonly OpenDocumentBackend $backend, private readonly DocumentIndexer $indexer, private readonly SymbolIndex $index, - private readonly ClassInfoFactory $classInfoFactory, + private readonly DeclarationSymbolInfoFactory $infoFactory, private readonly ParserService $parser, private readonly DeclarationScanner $scanner, private readonly array $onDiskBackends = [], @@ -80,12 +79,11 @@ private function write(TextDocument $document): void $ast = $this->parser->parse($document) ?? []; $declarations = $this->scanner->scan($ast); - $classes = $this->classesIn($declarations, $document->uri); - $functions = $this->functionsIn($declarations, $document->uri); - $this->backend->updateDocument($document->uri, $classes, $functions); + $symbols = $this->infoFactory->allIn($declarations, FileUri::toPath($document->uri)); + $this->backend->updateDocument($document->uri, ...$symbols); $this->indexer->indexParsed($document, $ast); - $this->assertStoresAgree($classes, $functions); + $this->assertStoresAgree($symbols); } /** @@ -101,21 +99,16 @@ private function write(TextDocument $document): void * {@see \Firehed\PhpLsp\Index\SymbolExtractor} — so agreement is a property of two * implementations rather than of one. * - * @param list $classes - * @param array $functions + * @param list $symbols */ - private function assertStoresAgree(array $classes, array $functions): void + private function assertStoresAgree(array $symbols): void { - foreach ($classes as $classInfo) { - $this->assertIndexed('class-like', $classInfo->name->fqn); - } - - foreach (array_keys($functions) as $fqn) { - $this->assertIndexed('function', $fqn); + foreach ($symbols as $symbol) { + $this->assertIndexed($symbol->kind, $symbol->name->fullyQualifiedName()); } } - private function assertIndexed(string $kind, string $fqn): void + private function assertIndexed(NameKind $kind, string $fqn): void { if ($this->index->findByFqn($fqn) !== null) { return; @@ -129,47 +122,9 @@ private function assertIndexed(string $kind, string $fqn): void 'Write-path divergence: %s %s is registered for lookup but absent from the ' . 'symbol index; the two stores are written from one parse and must agree ' . '(RFC 1 §4.3).', - $kind, + $kind->name, $fqn, )); // @codeCoverageIgnoreEnd } - - /** - * A declaration at any depth counts, matching what the on-disk backends resolve - * (a polyfill guarded by `function_exists` is the common shape). Opening a file - * must not make a name that already resolved disappear (RFC 1 §4.2). Of duplicate - * declarations, the first wins — the one PHP would define, and the one the - * on-disk backends return. - * - * @return array Fully-qualified name -> metadata - */ - private function functionsIn(FileDeclarations $declarations, string $uri): array - { - $functions = []; - foreach ($declarations->functions as $declaration) { - $fqn = $declaration->name->fullyQualifiedName(); - $functions[$fqn] ??= FunctionInfo::fromNode($declaration->node, $uri); - } - - return $functions; - } - - /** - * Class-likes follow the same depth and duplicate rules as functions above, for - * the same reasons: a `class_exists`-guarded declaration is one the on-disk - * backends resolve, and of duplicates they return the first. - * - * @return list - */ - private function classesIn(FileDeclarations $declarations, string $uri): array - { - $classes = []; - foreach ($declarations->classLikes as $declaration) { - $fqn = $declaration->name->fullyQualifiedName(); - $classes[$fqn] ??= $this->classInfoFactory->fromAstNode($declaration->node, $uri); - } - - return array_values($classes); - } } diff --git a/src/Knowledge/KnowledgeStack.php b/src/Knowledge/KnowledgeStack.php index fbf2bda7..8f0603fd 100644 --- a/src/Knowledge/KnowledgeStack.php +++ b/src/Knowledge/KnowledgeStack.php @@ -77,7 +77,7 @@ public static function forProject( $openDocuments, new DocumentIndexer($parser, new SymbolExtractor(), $index), $index, - $classInfoFactory, + $declarationInfoFactory, $parser, $scanner, // External-change and close-after-edit invalidation drops the on-disk diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 6046dd95..3058c87e 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -4,8 +4,7 @@ namespace Firehed\PhpLsp\Knowledge; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\FunctionInfo; +use Firehed\PhpLsp\Domain\DeclaredSymbol; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; use Firehed\PhpLsp\Domain\SymbolInfo; @@ -74,28 +73,20 @@ public function searchClassLikes(string $prefix): array } /** - * Register the class-likes and functions declared in an open document for - * lookup, replacing any previously registered for the same URI. + * Register the symbols declared in an open document for lookup, replacing any + * previously registered for the same URI. * - * Functions arrive keyed because {@see FunctionInfo} carries only the short - * name; the caller read the qualified one from the declaration. - * - * @param list $classes - * @param array $functions Fully-qualified name -> metadata + * Each symbol carries its own kind, so this backend never enumerates the kinds + * and a new one reaches it without a signature change (Plan 0002 §5.6). */ - public function updateDocument(string $uri, array $classes, array $functions = []): void + public function updateDocument(string $uri, DeclaredSymbol ...$symbols): void { $this->removeDocument($uri); $keys = []; - foreach ($classes as $classInfo) { - $key = self::key(NameKind::ClassLike, QualifiedName::fromClassName($classInfo->name)); - $this->byKey[$key] = $classInfo; - $keys[] = $key; - } - foreach ($functions as $fqn => $functionInfo) { - $key = self::key(NameKind::Function_, QualifiedName::fromFullyQualified($fqn)); - $this->byKey[$key] = $functionInfo; + foreach ($symbols as $symbol) { + $key = self::key($symbol->kind, $symbol->name); + $this->byKey[$key] = $symbol->info; $keys[] = $key; } $this->keysByUri[$uri] = $keys; diff --git a/tests/Knowledge/DocumentSymbolSinkTest.php b/tests/Knowledge/DocumentSymbolSinkTest.php index abe09e7b..55fc0576 100644 --- a/tests/Knowledge/DocumentSymbolSinkTest.php +++ b/tests/Knowledge/DocumentSymbolSinkTest.php @@ -10,6 +10,7 @@ use Firehed\PhpLsp\Index\SymbolExtractor; use Firehed\PhpLsp\Index\SymbolIndex; use Firehed\PhpLsp\Cache\Invalidatable; +use Firehed\PhpLsp\Knowledge\DeclarationSymbolInfoFactory; use Firehed\PhpLsp\Knowledge\DocumentSymbolSink; use Firehed\PhpLsp\Knowledge\OpenDocumentBackend; use Firehed\PhpLsp\Parser\ParserService; @@ -43,7 +44,7 @@ protected function setUp(): void $this->backend, new DocumentIndexer($parser, new SymbolExtractor(), $this->index), $this->index, - new DefaultClassInfoFactory(), + new DeclarationSymbolInfoFactory(new DefaultClassInfoFactory()), $parser, new DeclarationScanner(), ); @@ -327,7 +328,7 @@ private function sinkWithOnDiskBackends(Invalidatable ...$onDiskBackends): Docum $this->backend, new DocumentIndexer($parser, new SymbolExtractor(), $this->index), $this->index, - new DefaultClassInfoFactory(), + new DeclarationSymbolInfoFactory(new DefaultClassInfoFactory()), $parser, new DeclarationScanner(), array_values($onDiskBackends), diff --git a/tests/Knowledge/OpenDocumentBackendTest.php b/tests/Knowledge/OpenDocumentBackendTest.php index 66be097e..46c72183 100644 --- a/tests/Knowledge/OpenDocumentBackendTest.php +++ b/tests/Knowledge/OpenDocumentBackendTest.php @@ -4,6 +4,10 @@ namespace Firehed\PhpLsp\Tests\Knowledge; +use Firehed\PhpLsp\Domain\DeclaredSymbol; +use Firehed\PhpLsp\Domain\NameKind; +use Firehed\PhpLsp\Domain\QualifiedName; +use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Index\Location; use Firehed\PhpLsp\Index\Symbol; use Firehed\PhpLsp\Index\SymbolIndex; @@ -35,7 +39,7 @@ protected function setUp(): void public function testLookupClassLikeReturnsARegisteredClass(): void { - $this->backend->updateDocument('file:///Widget.php', [self::classInfo('V\Widget')]); + $this->backend->updateDocument('file:///Widget.php', self::declaredClass('V\Widget')); $info = self::classLikeIn($this->backend, 'V\Widget'); @@ -54,8 +58,8 @@ public function testLookupClassLikeReturnsNullForAnUnregisteredClass(): void public function testUpdateDocumentReplacesThePriorClassesForThatUri(): void { $uri = 'file:///Doc.php'; - $this->backend->updateDocument($uri, [self::classInfo('V\Alpha')]); - $this->backend->updateDocument($uri, [self::classInfo('V\Beta')]); + $this->backend->updateDocument($uri, self::declaredClass('V\Alpha')); + $this->backend->updateDocument($uri, self::declaredClass('V\Beta')); self::assertNull( self::classLikeIn($this->backend, 'V\Alpha'), @@ -70,7 +74,7 @@ public function testUpdateDocumentReplacesThePriorClassesForThatUri(): void public function testRemoveDocumentDropsItsClasses(): void { $uri = 'file:///Ephemeral.php'; - $this->backend->updateDocument($uri, [self::classInfo('V\Ephemeral')]); + $this->backend->updateDocument($uri, self::declaredClass('V\Ephemeral')); $this->backend->removeDocument($uri); @@ -92,7 +96,7 @@ public function testRemoveDocumentIsANoOpForAnUnknownUri(): void public function testLookupFunctionReturnsARegisteredFunction(): void { - $this->backend->updateDocument('file:///helpers.php', [], ['V\format' => self::functionInfo('format')]); + $this->backend->updateDocument('file:///helpers.php', self::declaredFunction('V\format')); $info = self::functionIn($this->backend, 'V\format'); @@ -102,7 +106,7 @@ public function testLookupFunctionReturnsARegisteredFunction(): void public function testLookupFunctionIsCaseInsensitive(): void { - $this->backend->updateDocument('file:///helpers.php', [], ['V\format' => self::functionInfo('format')]); + $this->backend->updateDocument('file:///helpers.php', self::declaredFunction('V\format')); self::assertNotNull( self::functionIn($this->backend, 'V\FORMAT'), @@ -118,12 +122,37 @@ public function testLookupFunctionReturnsNullForAnUnregisteredFunction(): void ); } + public function testRegistrationCarriesAKindItKnowsNothingAbout(): void + { + // The point of the kind-parameterized write path: a kind whose metadata type + // this backend has never heard of round-trips, so adding one is a change to + // the info factories alone (Plan 0002 §5.6). + $info = new class implements SymbolInfo { + }; + $name = QualifiedName::fromFullyQualified('V\LIMIT'); + + $this->backend->updateDocument( + 'file:///consts.php', + new DeclaredSymbol($name, NameKind::Constant, $info), + ); + + self::assertSame( + $info, + $this->backend->lookup($name, NameKind::Constant), + 'a registered symbol of any kind must resolve for that kind', + ); + self::assertNull( + $this->backend->lookup($name, NameKind::Function_), + 'and must not answer for another symbol namespace', + ); + } + public function testFunctionAndClassLikeRegistrationsDoNotCollide(): void { $this->backend->updateDocument( 'file:///Dual.php', - [self::classInfo('V\Dual')], - ['V\Dual' => self::functionInfo('Dual')], + self::declaredClass('V\Dual'), + self::declaredFunction('V\Dual'), ); self::assertNotNull( @@ -139,8 +168,8 @@ public function testFunctionAndClassLikeRegistrationsDoNotCollide(): void public function testUpdateDocumentReplacesThePriorFunctionsForThatUri(): void { $uri = 'file:///helpers.php'; - $this->backend->updateDocument($uri, [], ['V\alpha' => self::functionInfo('alpha')]); - $this->backend->updateDocument($uri, [], ['V\beta' => self::functionInfo('beta')]); + $this->backend->updateDocument($uri, self::declaredFunction('V\alpha')); + $this->backend->updateDocument($uri, self::declaredFunction('V\beta')); self::assertNull( self::functionIn($this->backend, 'V\alpha'), @@ -155,7 +184,7 @@ public function testUpdateDocumentReplacesThePriorFunctionsForThatUri(): void public function testRemoveDocumentDropsItsFunctions(): void { $uri = 'file:///helpers.php'; - $this->backend->updateDocument($uri, [], ['V\ephemeral' => self::functionInfo('ephemeral')]); + $this->backend->updateDocument($uri, self::declaredFunction('V\ephemeral')); $this->backend->removeDocument($uri); @@ -204,6 +233,22 @@ public function testChildrenOfEnumeratesTheOpenDocumentNamespace(): void ); } + private static function declaredClass(string $fqn): DeclaredSymbol + { + return new DeclaredSymbol( + QualifiedName::fromFullyQualified($fqn), + NameKind::ClassLike, + self::classInfo($fqn), + ); + } + + private static function declaredFunction(string $fqn): DeclaredSymbol + { + $name = QualifiedName::fromFullyQualified($fqn); + + return new DeclaredSymbol($name, NameKind::Function_, self::functionInfo($name->shortName)); + } + private function addSymbol(string $name, string $fqn, SymbolKind $kind): void { $this->index->add(new Symbol($name, $fqn, $kind, new Location('file:///' . $name . '.php', 0, 0, 0, 0))); From 7cef217e4531eef6f081dca073fd76de96c7d102 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 10:34:39 -0700 Subject: [PATCH 17/35] Point built-in class search at the gap that blocks it --- tests/Knowledge/SymbolCoverageGridTest.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index eed60b2d..18c21477 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -28,8 +28,8 @@ */ final class SymbolCoverageGridTest extends TestCase { - /** The other form a blocker may take, when no slice owns the gap. */ - private const string SECTION_REFERENCE = '/^(RFC 1|Plan 0002) §\d+(\.\d+)*$/u'; + /** The forms a blocker may take when no slice owns the gap: an issue, or a section. */ + private const string UNOWNED_BLOCKER = '/^(#\d+|(RFC 1|Plan 0002) §\d+(\.\d+)*)$/u'; /** * Cells the shipped stack cannot answer, each naming a slice id or an RFC @@ -51,10 +51,12 @@ final class SymbolCoverageGridTest extends TestCase 'BuiltinBackend|Function_|search' => 'S3.9a, S3.9b', 'BuiltinBackend|Constant|search' => 'S3.9a, S3.8b', - // A prefix has no name -> file map, and offering an unqualified built-in is - // auto-import. + // A prefix has no name -> file map on disk. The built-in row is blocked on + // something else entirely: the name it would offer does not resolve + // unqualified, so the item is only useful once completion can insert the + // import with it. 'FilesystemBackend|ClassLike|search' => 'RFC 1 §3', - 'BuiltinBackend|ClassLike|search' => 'RFC 1 §3', + 'BuiltinBackend|ClassLike|search' => '#23', // `SymbolExtractor` emits no `SymbolKind::Constant`, so an open document's // constants never reach the index this reads. Found by this grid. @@ -178,7 +180,7 @@ public function testEveryRegistrationNamesALiveBlocker(): void self::assertSame( [], self::danglingBlockers(self::NOT_APPLICABLE), - 'a not-applicable cell must name a slice that is still in the registry, or a section: ' + 'a not-applicable cell must name a slice still in the registry, an issue, or a section: ' . 'a blocker nobody owns is the permanent exemption Step Z exists to prevent', ); } @@ -189,7 +191,7 @@ public function testABlockerNamingNoSliceIsReported(): void self::assertSame( ['BuiltinBackend|Constant|lookup names S9.99'], self::danglingBlockers(['BuiltinBackend|Constant|lookup' => 'S9.99']), - 'a blocker matching no registry row and no section must be reported', + 'a blocker matching no registry row, issue, or section must be reported', ); } @@ -204,7 +206,7 @@ private static function danglingBlockers(array $notApplicable): array foreach ($notApplicable as $cell => $blocker) { foreach (explode(', ', $blocker) as $named) { - if (in_array($named, $slices, true) || preg_match(self::SECTION_REFERENCE, $named) === 1) { + if (in_array($named, $slices, true) || preg_match(self::UNOWNED_BLOCKER, $named) === 1) { continue; } $dangling[] = "{$cell} names {$named}"; From 04d0f1d749039b9f516b160b18aa6c52251f7427 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 10:50:00 -0700 Subject: [PATCH 18/35] Record that registration collapsed with lookup --- CLAUDE.md | 13 ++++++++----- docs/architecture/build-manifest.md | 3 +++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fa019d87..71c6a275 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,11 +180,14 @@ directory listing by `CompositeNamespaceCatalog`. Enumeration is not optional: requires lookup and enumeration to draw on the same backends, so a name that resolved on hover while being invisible to completion is the split this tier exists to prevent. -The write path is **`SymbolSink`** (`DocumentSymbolSink`), which registers class and -function metadata and indexes symbols from one document. A declaration at any depth is -registered, not just a top-level one — a class or function guarded by -`class_exists`/`function_exists` is a name the file validly declares, and the on-disk -backends resolve one, so opening the file must not make it disappear. +The write path is **`SymbolSink`** (`DocumentSymbolSink`), which registers a document's +symbols and indexes them. Registration is kind-parameterized like lookup: the sink hands +`OpenDocumentBackend` `DeclaredSymbol`s built by `DeclarationSymbolInfoFactory`, the same +factory the on-disk read path uses, so a new kind is a case there rather than another +parameter on the backend. A declaration at any depth is registered, not just a top-level +one — a class or function guarded by `class_exists`/`function_exists` is a name the file +validly declares, and the on-disk backends resolve one, so opening the file must not make +it disappear. **`KnowledgeStack::forProject`** assembles the read composite and the write sink, sharing one open-document backend and symbol index. diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index d6276691..a0ff6fb8 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -148,6 +148,9 @@ Notes: pairs; after SC.5 those differ only in which factory builds the metadata. S3.8d also carries the §8.1 mechanism for §5.1 (see 0002), per the rule that a seam ships with its enforcement. + `OpenDocumentBackend`'s *registration* is collapsed with its lookup, for the same + reason: a per-kind parameter there would force S3.8b to edit a backend even though the + read seam held. - **S3.8b is the proof.** Its acceptance carries one criterion that cannot be met by appearance: **its diff must touch no `SymbolBackend` implementation.** If it does, S3.8d did not work. From 4c3ce0249064ca22fcbdd715fbd89db3361305e9 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 11:17:15 -0700 Subject: [PATCH 19/35] Drop BuiltinBackend from the enumeration allowlist --- phpstan-baseline.neon | 2 +- phpstan.neon | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b471e156..0a83afec 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,7 +1,7 @@ parameters: ignoreErrors: - - message: '#^Calling get_defined_functions\(\) is forbidden, runtime symbol enumeration is confined to BuiltinBackend and ReflectionNamespaceSource\.$#' + message: '#^Calling get_defined_functions\(\) is forbidden, runtime symbol enumeration is confined to ReflectionNamespaceSource\.$#' identifier: disallowed.function count: 1 path: src/Completion/FunctionCandidates.php diff --git a/phpstan.neon b/phpstan.neon index 5665ee37..91ee3a34 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -86,9 +86,8 @@ parameters: - 'get_defined_functions()' - 'get_declared_classes()' - 'get_defined_constants()' - message: 'runtime symbol enumeration is confined to BuiltinBackend and ReflectionNamespaceSource' + message: 'runtime symbol enumeration is confined to ReflectionNamespaceSource' allowIn: - - src/Knowledge/BuiltinBackend.php - src/Index/ReflectionNamespaceSource.php - tests/* - From 4a1f97efc502fb886d48c45e88ad3a71ea228824 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 11:18:43 -0700 Subject: [PATCH 20/35] Key the fake backend by the kind's own case rule --- tests/Knowledge/FakeSymbolBackend.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Knowledge/FakeSymbolBackend.php b/tests/Knowledge/FakeSymbolBackend.php index e0631b47..599236f5 100644 --- a/tests/Knowledge/FakeSymbolBackend.php +++ b/tests/Knowledge/FakeSymbolBackend.php @@ -22,10 +22,10 @@ final class FakeSymbolBackend implements SymbolBackend { /** - * @param array $classLikes Lowercased FQN -> info + * @param array $classLikes FQN under the kind's case rule -> info * @param array $namespaces Path -> contents * @param list $searchResults Returned (prefix-filtered on short name) - * @param array $functions Lowercased FQN -> info + * @param array $functions FQN under the kind's case rule -> info */ public function __construct( private readonly array $classLikes = [], @@ -48,7 +48,7 @@ public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo NameKind::Constant => [], }; - return $configured[strtolower($name->fullyQualifiedName())] ?? null; + return $configured[$kind->normalize($name)] ?? null; } /** From ca6ab832686331233e434cd571226621cf7c9a8e Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:11:25 -0700 Subject: [PATCH 21/35] Confine reading a constant's runtime value --- phpstan.neon | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index 91ee3a34..6bbe3140 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -93,12 +93,13 @@ parameters: - function: - 'class_exists()' + - 'constant()' - 'defined()' - 'enum_exists()' - 'function_exists()' - 'interface_exists()' - 'trait_exists()' - message: 'asking the runtime whether a symbol exists is a SymbolSource query (RFC 1 §4.2); ReflectionSymbolInfoFactory is the one place it may be the server''s own runtime' + message: 'asking the runtime whether a symbol exists, or what it holds, is a SymbolSource query (RFC 1 §4.2); ReflectionSymbolInfoFactory is the one place it may be the server''s own runtime' allowIn: - src/Knowledge/ReflectionSymbolInfoFactory.php - tests/* From 6c8e1853116710fdd30329619a0f8912432667cb Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:12:45 -0700 Subject: [PATCH 22/35] Say which of the grid's axes are derived --- CLAUDE.md | 5 +++-- tests/Knowledge/GridQuery.php | 8 ++++++-- tests/Knowledge/SymbolCoverageGridTest.php | 7 ++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 71c6a275..af5c55e1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,8 +143,9 @@ a concrete return type rather than a type-erased union — and **kind-parameteri facade's closed method set as licence to add a per-kind backend method. Kind dispatch lives in `DeclarationSymbolInfoFactory` and `ReflectionSymbolInfoFactory`, one per metadata route, so a new kind is a case in each rather than a method on every backend. -`SymbolCoverageGridTest` enforces §5.1 with a backend × kind × query grid whose axes -are derived: every cell answers or names its blocker, and an unregistered cell fails. +`SymbolCoverageGridTest` enforces §5.1 with a backend × kind × query grid whose backend +and kind axes are derived: every cell answers or names its blocker, and an unregistered +cell fails. `lookupFunction` reaches open documents, the `autoload.files` set, and PHP's built-ins — the last filtered to `isInternal()`, because reflection also sees the functions the *server's* own diff --git a/tests/Knowledge/GridQuery.php b/tests/Knowledge/GridQuery.php index 82ada0b6..60b10953 100644 --- a/tests/Knowledge/GridQuery.php +++ b/tests/Knowledge/GridQuery.php @@ -9,8 +9,12 @@ /** * The queries every {@see SymbolBackend} answers, crossed with * {@see \Firehed\PhpLsp\Domain\NameKind} to form {@see SymbolCoverageGridTest}'s - * columns; an enum so a new query breaks the grid's match rather than falling - * through a default. + * columns. + * + * This is the grid's one listed axis: a query needs an argument list and a way to + * read its answer, which no derivation supplies. So a query added to + * {@see SymbolBackend} does not add cells on its own — S3.9a, which reshapes + * `searchClassLikes`, has to add the case. */ enum GridQuery: string { diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index 18c21477..58cadb9b 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -17,9 +17,10 @@ use PHPUnit\Framework\TestCase; /** - * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose axes are - * derived rather than listed, so a new kind or backend adds cells this file never - * anticipated. Every cell answers over the fixtures or names a blocker in + * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose backend and + * kind axes are derived rather than listed, so a new kind or backend adds cells this + * file never anticipated. The query axis is listed ({@see GridQuery}). Every cell + * answers over the fixtures or names a blocker in * {@see NOT_APPLICABLE}; an unregistered cell fails, so does a registration on a cell * that answers, and so does a blocker naming no row of the slice registry. * From be469680447f2e0a93e8727d02fe3bf7f3a6dfec Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:22:06 -0700 Subject: [PATCH 23/35] Hold a grid lookup to the kind's own info type --- tests/Knowledge/SymbolCoverageGridTest.php | 39 +++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index 58cadb9b..6607cc01 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -5,6 +5,8 @@ namespace Firehed\PhpLsp\Tests\Knowledge; use Firehed\PhpLsp\Document\TextDocument; +use Firehed\PhpLsp\Domain\ClassInfo; +use Firehed\PhpLsp\Domain\FunctionInfo; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; use Firehed\PhpLsp\Index\ComposerAutoloadMap; @@ -89,6 +91,20 @@ final class SymbolCoverageGridTest extends TestCase ], ]; + /** + * The concrete type a lookup of each kind must answer with, so a cell counts as + * covered only when the backend answered for the kind it was asked about — §5.1 + * requires a concrete return type, and the composite's narrowing `assert()` is + * gone in production. Null while the kind has no info type yet. + * + * @var array + */ + private const array INFO_TYPES = [ + 'ClassLike' => ClassInfo::class, + 'Function_' => FunctionInfo::class, + 'Constant' => null, + ]; + /** One name of each kind for the open-document row, which no on-disk file can stand in for. */ private const string OPEN_DOCUMENT = <<<'PHP' $backend->lookup(QualifiedName::fromFullyQualified($fqn), $kind) !== null, + GridQuery::Lookup => $this->looksUp($backend, $fqn, $kind), GridQuery::Search => $this->searchFinds($backend, $fqn), GridQuery::ChildrenOf => $this->enumerates($backend, $probe['namespace'], $kind, $fqn), }; } + private function looksUp(SymbolBackend $backend, string $fqn, NameKind $kind): bool + { + $info = $backend->lookup(QualifiedName::fromFullyQualified($fqn), $kind); + if ($info === null) { + return false; + } + + $expected = self::INFO_TYPES[$kind->name] ?? null; + self::assertNotNull( + $expected, + "{$kind->name} has no info type declared, so no backend may answer a lookup of it", + ); + self::assertInstanceOf( + $expected, + $info, + "a {$kind->name} lookup must answer with that kind's own metadata type (RFC 1 §5.1)", + ); + + return true; + } + private function searchFinds(SymbolBackend $backend, string $fqn): bool { $prefix = QualifiedName::fromFullyQualified($fqn)->shortName; From 1bb0b4fd91d8ba55ae3ddfe3934827c362413f36 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:22:48 -0700 Subject: [PATCH 24/35] Hold open-document keys to each kind's case rule --- tests/Knowledge/OpenDocumentBackendTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Knowledge/OpenDocumentBackendTest.php b/tests/Knowledge/OpenDocumentBackendTest.php index 46c72183..caf19af3 100644 --- a/tests/Knowledge/OpenDocumentBackendTest.php +++ b/tests/Knowledge/OpenDocumentBackendTest.php @@ -47,6 +47,16 @@ public function testLookupClassLikeReturnsARegisteredClass(): void self::assertSame('V\Widget', $info->name->fqn, 'the registered class must be returned unchanged'); } + public function testLookupClassLikeIsCaseInsensitive(): void + { + $this->backend->updateDocument('file:///Widget.php', self::declaredClass('V\Widget')); + + self::assertNotNull( + self::classLikeIn($this->backend, 'v\WIDGET'), + 'PHP matches class-like names case-insensitively', + ); + } + public function testLookupClassLikeReturnsNullForAnUnregisteredClass(): void { self::assertNull( @@ -145,6 +155,15 @@ public function testRegistrationCarriesAKindItKnowsNothingAbout(): void $this->backend->lookup($name, NameKind::Function_), 'and must not answer for another symbol namespace', ); + self::assertSame( + $info, + $this->backend->lookup(QualifiedName::fromFullyQualified('v\LIMIT'), NameKind::Constant), + 'the namespace of a constant is still matched case-insensitively', + ); + self::assertNull( + $this->backend->lookup(QualifiedName::fromFullyQualified('V\limit'), NameKind::Constant), + 'but its own name is not: constants are the one kind PHP matches case-sensitively', + ); } public function testFunctionAndClassLikeRegistrationsDoNotCollide(): void From 3b45c8e2460722df85f3f8084fd7665bf851f65c Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:40:49 -0700 Subject: [PATCH 25/35] Probe the fourth class-like flavour --- tests/Knowledge/ReflectionSymbolInfoFactoryTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php index 7c6c0c82..387389f6 100644 --- a/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php +++ b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php @@ -11,6 +11,7 @@ use Firehed\PhpLsp\Domain\SymbolInfo; use Firehed\PhpLsp\Knowledge\ReflectionSymbolInfoFactory; use Firehed\PhpLsp\Repository\DefaultClassInfoFactory; +use Firehed\PhpLsp\Resolution\ResolvesFromInfo; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -28,8 +29,6 @@ protected function setUp(): void } /** - * PHP declares no internal trait, so the fourth flavour cannot be probed here. - * * @return iterable */ public static function loadedClassLikes(): iterable @@ -37,6 +36,10 @@ public static function loadedClassLikes(): iterable yield 'class' => [\ArrayObject::class]; yield 'interface' => [\Countable::class]; yield 'enum' => [\Random\IntervalBoundary::class]; + // PHP declares no internal trait, so the only probe for the fourth flavour is + // one the server process loaded — which this branch answers for until SC.14 + // filters it to internal, after which the branch is dead and goes with it. + yield 'trait' => [ResolvesFromInfo::class]; } #[DataProvider('loadedClassLikes')] From ca6b976ef776a8acc63d00fd7ddfa1e99e6f1445 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:42:01 -0700 Subject: [PATCH 26/35] Name what the comment's 'all three' refers to --- src/Knowledge/ReflectionSymbolInfoFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Knowledge/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php index 16966b9b..695fd959 100644 --- a/src/Knowledge/ReflectionSymbolInfoFactory.php +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -39,9 +39,9 @@ private function classInfo(QualifiedName $name): ?SymbolInfo { $fqn = $name->fullyQualifiedName(); - // Also what narrows the name to a `class-string`, which is why this kind - // cannot use the sibling's try/catch. All three, since `class_exists` - // answers for classes and enums only; each autoloads exactly as + // These also narrow the name to a `class-string`, which is why this kind + // cannot use the sibling's try/catch. All three are needed: `class_exists` + // answers for classes and enums only. Each autoloads exactly as // constructing the reflection would. if (!class_exists($fqn) && !interface_exists($fqn) && !trait_exists($fqn)) { return null; From 444265a9bfabf8d7af683b6469123546cef48b78 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 16:49:38 -0700 Subject: [PATCH 27/35] Share the declared-symbol builders --- tests/BuildsSymbolInfoTrait.php | 30 +++++++++++++++++++++ tests/Knowledge/OpenDocumentBackendTest.php | 16 ----------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/tests/BuildsSymbolInfoTrait.php b/tests/BuildsSymbolInfoTrait.php index a1f96a8b..ed49b862 100644 --- a/tests/BuildsSymbolInfoTrait.php +++ b/tests/BuildsSymbolInfoTrait.php @@ -7,15 +7,45 @@ use Firehed\PhpLsp\Domain\ClassInfo; use Firehed\PhpLsp\Domain\ClassKind; use Firehed\PhpLsp\Domain\ClassName; +use Firehed\PhpLsp\Domain\DeclaredSymbol; use Firehed\PhpLsp\Domain\FunctionInfo; +use Firehed\PhpLsp\Domain\NameKind; +use Firehed\PhpLsp\Domain\QualifiedName; /** * Builds minimal domain value objects for tests that need symbols without a real * parse — only the identity, the declaring file where precedence is under test, * and, for a class-like, the parent and interface edges a subtype walk follows. + * + * The `declared*` pair wraps the info in the {@see DeclaredSymbol} the kind-agnostic + * write and lookup paths take, so a test states the kind once rather than picking a + * per-kind slot. */ trait BuildsSymbolInfoTrait { + /** + * @param list $interfaces + */ + private static function declaredClass( + string $fqn, + ?string $parent = null, + array $interfaces = [], + ?string $file = null, + ): DeclaredSymbol { + return new DeclaredSymbol( + QualifiedName::fromFullyQualified($fqn), + NameKind::ClassLike, + self::classInfo($fqn, parent: $parent, interfaces: $interfaces, file: $file), + ); + } + + private static function declaredFunction(string $fqn, ?string $file = null): DeclaredSymbol + { + $name = QualifiedName::fromFullyQualified($fqn); + + return new DeclaredSymbol($name, NameKind::Function_, self::functionInfo($name->shortName, $file)); + } + /** * @param list $interfaces */ diff --git a/tests/Knowledge/OpenDocumentBackendTest.php b/tests/Knowledge/OpenDocumentBackendTest.php index caf19af3..c481c91d 100644 --- a/tests/Knowledge/OpenDocumentBackendTest.php +++ b/tests/Knowledge/OpenDocumentBackendTest.php @@ -252,22 +252,6 @@ public function testChildrenOfEnumeratesTheOpenDocumentNamespace(): void ); } - private static function declaredClass(string $fqn): DeclaredSymbol - { - return new DeclaredSymbol( - QualifiedName::fromFullyQualified($fqn), - NameKind::ClassLike, - self::classInfo($fqn), - ); - } - - private static function declaredFunction(string $fqn): DeclaredSymbol - { - $name = QualifiedName::fromFullyQualified($fqn); - - return new DeclaredSymbol($name, NameKind::Function_, self::functionInfo($name->shortName)); - } - private function addSymbol(string $name, string $fqn, SymbolKind $kind): void { $this->index->add(new Symbol($name, $fqn, $kind, new Location('file:///' . $name . '.php', 0, 0, 0, 0))); From 79e3c7142a783e898a597d1385a50a281e762cb3 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 17:02:03 -0700 Subject: [PATCH 28/35] Give the fake backend one kind-agnostic store --- tests/Knowledge/CompositeSymbolSourceTest.php | 40 +++++++++---------- tests/Knowledge/FakeSymbolBackend.php | 31 ++++++++------ 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/tests/Knowledge/CompositeSymbolSourceTest.php b/tests/Knowledge/CompositeSymbolSourceTest.php index 08c78724..4eae31a9 100644 --- a/tests/Knowledge/CompositeSymbolSourceTest.php +++ b/tests/Knowledge/CompositeSymbolSourceTest.php @@ -30,8 +30,8 @@ final class CompositeSymbolSourceTest extends TestCase public function testLookupClassLikeTakesTheFirstBackendThatAnswers(): void { - $open = new FakeSymbolBackend(['app\widget' => self::classInfo('App\Widget', file: 'open.php')]); - $vendor = new FakeSymbolBackend(['app\widget' => self::classInfo('App\Widget', file: 'vendor.php')]); + $open = new FakeSymbolBackend([self::declaredClass('App\Widget', file: 'open.php')]); + $vendor = new FakeSymbolBackend([self::declaredClass('App\Widget', file: 'vendor.php')]); $source = new CompositeSymbolSource([$open, $vendor]); $info = $source->lookupClassLike(self::className('App\Widget')); @@ -47,7 +47,7 @@ public function testLookupClassLikeTakesTheFirstBackendThatAnswers(): void public function testLookupClassLikeFallsThroughToALaterBackend(): void { $open = new FakeSymbolBackend(); - $vendor = new FakeSymbolBackend(['app\widget' => self::classInfo('App\Widget', file: 'vendor.php')]); + $vendor = new FakeSymbolBackend([self::declaredClass('App\Widget', file: 'vendor.php')]); $source = new CompositeSymbolSource([$open, $vendor]); $info = $source->lookupClassLike(self::className('App\Widget')); @@ -68,8 +68,8 @@ public function testLookupClassLikeReturnsNullWhenNoBackendAnswers(): void public function testLookupFunctionTakesTheFirstBackendThatAnswers(): void { - $open = new FakeSymbolBackend(functions: ['app\format' => self::functionInfo('format', 'open.php')]); - $vendor = new FakeSymbolBackend(functions: ['app\format' => self::functionInfo('format', 'vendor.php')]); + $open = new FakeSymbolBackend([self::declaredFunction('App\format', 'open.php')]); + $vendor = new FakeSymbolBackend([self::declaredFunction('App\format', 'vendor.php')]); $source = new CompositeSymbolSource([$open, $vendor]); $info = $source->lookupFunction(FunctionName::fromFullyQualified('App\format')); @@ -85,7 +85,7 @@ public function testLookupFunctionTakesTheFirstBackendThatAnswers(): void public function testLookupFunctionFallsThroughToALaterBackend(): void { $open = new FakeSymbolBackend(); - $vendor = new FakeSymbolBackend(functions: ['app\format' => self::functionInfo('format', 'vendor.php')]); + $vendor = new FakeSymbolBackend([self::declaredFunction('App\format', 'vendor.php')]); $source = new CompositeSymbolSource([$open, $vendor]); $info = $source->lookupFunction(FunctionName::fromFullyQualified('App\format')); @@ -195,8 +195,8 @@ public function testIsSubclassOfMatchesEdgesUnderTheClassCaseRule(): void { // The declared parent spelling differs in case from the queried target. $backend = new FakeSymbolBackend([ - 'app\child' => self::classInfo('App\Child', parent: 'APP\PARENTCLASS'), - 'app\parentclass' => self::classInfo('App\ParentClass'), + self::declaredClass('App\Child', parent: 'APP\PARENTCLASS'), + self::declaredClass('App\ParentClass'), ]); $source = new CompositeSymbolSource([$backend]); @@ -221,7 +221,7 @@ public function testIsSubclassOfSkipsUnresolvableSupertypes(): void // Orphan's parent and interface are named but nothing declares them: the walk // must skip the unresolved edges rather than crash. $backend = new FakeSymbolBackend([ - 'app\orphan' => self::classInfo( + self::declaredClass( 'App\Orphan', parent: 'App\MissingParent', interfaces: ['App\MissingInterface'], @@ -240,8 +240,8 @@ public function testIsSubclassOfTerminatesOnACyclicParentGraph(): void // Illegal in PHP but reachable in mid-edit code: A extends B extends A. The // visited set must break the cycle rather than recurse forever. $backend = new FakeSymbolBackend([ - 'app\cyclea' => self::classInfo('App\CycleA', parent: 'App\CycleB'), - 'app\cycleb' => self::classInfo('App\CycleB', parent: 'App\CycleA'), + self::declaredClass('App\CycleA', parent: 'App\CycleB'), + self::declaredClass('App\CycleB', parent: 'App\CycleA'), ]); $source = new CompositeSymbolSource([$backend]); @@ -256,10 +256,10 @@ public function testIsSubclassOfTerminatesOnADiamondInterfaceGraph(): void // Two interfaces both extend the same base: the base is reached twice and the // visited set must skip the second visit rather than re-walk it. $backend = new FakeSymbolBackend([ - 'app\diamond' => self::classInfo('App\Diamond', interfaces: ['App\IfaceA', 'App\IfaceB']), - 'app\ifacea' => self::classInfo('App\IfaceA', interfaces: ['App\IfaceBase']), - 'app\ifaceb' => self::classInfo('App\IfaceB', interfaces: ['App\IfaceBase']), - 'app\ifacebase' => self::classInfo('App\IfaceBase'), + self::declaredClass('App\Diamond', interfaces: ['App\IfaceA', 'App\IfaceB']), + self::declaredClass('App\IfaceA', interfaces: ['App\IfaceBase']), + self::declaredClass('App\IfaceB', interfaces: ['App\IfaceBase']), + self::declaredClass('App\IfaceBase'), ]); $source = new CompositeSymbolSource([$backend]); @@ -272,17 +272,17 @@ public function testIsSubclassOfTerminatesOnADiamondInterfaceGraph(): void private static function openWithChild(): FakeSymbolBackend { return new FakeSymbolBackend([ - 'app\child' => self::classInfo('App\Child', parent: 'App\ParentClass', interfaces: ['App\IfaceA']), + self::declaredClass('App\Child', parent: 'App\ParentClass', interfaces: ['App\IfaceA']), ]); } private static function vendorGraph(): FakeSymbolBackend { return new FakeSymbolBackend([ - 'app\parentclass' => self::classInfo('App\ParentClass', parent: 'App\Grandparent'), - 'app\grandparent' => self::classInfo('App\Grandparent'), - 'app\ifacea' => self::classInfo('App\IfaceA', interfaces: ['App\IfaceBase']), - 'app\ifacebase' => self::classInfo('App\IfaceBase'), + self::declaredClass('App\ParentClass', parent: 'App\Grandparent'), + self::declaredClass('App\Grandparent'), + self::declaredClass('App\IfaceA', interfaces: ['App\IfaceBase']), + self::declaredClass('App\IfaceBase'), ]); } diff --git a/tests/Knowledge/FakeSymbolBackend.php b/tests/Knowledge/FakeSymbolBackend.php index 599236f5..8dbaa6cd 100644 --- a/tests/Knowledge/FakeSymbolBackend.php +++ b/tests/Knowledge/FakeSymbolBackend.php @@ -4,8 +4,7 @@ namespace Firehed\PhpLsp\Tests\Knowledge; -use Firehed\PhpLsp\Domain\ClassInfo; -use Firehed\PhpLsp\Domain\FunctionInfo; +use Firehed\PhpLsp\Domain\DeclaredSymbol; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\QualifiedName; use Firehed\PhpLsp\Domain\SymbolInfo; @@ -18,21 +17,28 @@ * An in-memory {@see SymbolBackend} configured with fixed answers, so * {@see \Firehed\PhpLsp\Tests\Knowledge\CompositeSymbolSourceTest} can prove the * composite's precedence and merge behavior without standing up real sources. + * + * Kind-agnostic like the real backends: a symbol carries its own kind, so a kind this + * file has never heard of is configurable without a new parameter (Plan 0002 §5.6). */ final class FakeSymbolBackend implements SymbolBackend { + /** @var array Kind-qualified key -> info */ + private array $byKey = []; + /** - * @param array $classLikes FQN under the kind's case rule -> info + * @param list $symbols Keyed here by each one's own case rule * @param array $namespaces Path -> contents * @param list $searchResults Returned (prefix-filtered on short name) - * @param array $functions FQN under the kind's case rule -> info */ public function __construct( - private readonly array $classLikes = [], + array $symbols = [], private readonly array $namespaces = [], private readonly array $searchResults = [], - private readonly array $functions = [], ) { + foreach ($symbols as $symbol) { + $this->byKey[self::key($symbol->name, $symbol->kind)] = $symbol->info; + } } public function childrenOf(NamespaceName $namespace): NamespaceContents @@ -42,13 +48,7 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - $configured = match ($kind) { - NameKind::ClassLike => $this->classLikes, - NameKind::Function_ => $this->functions, - NameKind::Constant => [], - }; - - return $configured[$kind->normalize($name)] ?? null; + return $this->byKey[self::key($name, $kind)] ?? null; } /** @@ -64,4 +64,9 @@ public function searchClassLikes(string $prefix): array ), )); } + + private static function key(QualifiedName $name, NameKind $kind): string + { + return $kind->name . '|' . $kind->normalize($name); + } } From fcb9df94b083a732c702374ecbb497638a4be9a5 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 17:52:52 -0700 Subject: [PATCH 29/35] File the invalidation fan-out row --- docs/architecture/build-manifest.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index a0ff6fb8..1779e598 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -108,6 +108,7 @@ re-runs repo-wide as its completion gate. SC.14 — Filter BuiltinBackend class-like lookup to internal — — SC.15 — Oracle corpus: trait adaptations and enums — — SC.16 — Index an open document's global constants — — + SC.17 — Collapse the hand-routed invalidation fan-out — — SZ.1 Z Definition of Done gate + repo-wide dup audit all prior — Notes: @@ -302,6 +303,12 @@ Notes: `WorkspaceNamespaceSource` already maps the kind, so the gap is upstream in the extractor. Found by the S3.8d coverage grid on its first run. Ungated, and ahead of S3.8b — constant lookup landing on an enumeration blind to open documents would rebuild the §4.2 split on the third symbol namespace. + - **SC.17** — telling the parts that hold file-derived state that a file changed is written out three times, each steered by an `instanceof` test: `DocumentSymbolSink` over its on-disk backends, `FilesystemBackend` over its catalog and locator, `CompositeSymbolLocator` over its routes. + So adding a holder means finding its parent in that tree by hand, and missing one is silent — the stale value is still served and nothing fails. + Not only caches: the same route drops `AutoloadFilesLocator`'s derived name→file map, which is rebuilt rather than memoized. + `SymbolSink extends Invalidatable` solely to give the handler a way in, which is how the write path came to be named after the response instead of the event. + Scope is one registration list at the composition root, which deletes the three fan-outs and the three type tests. Whether a general published event replaces it is #415 and is deliberately not settled here. + Found while reviewing S3.8d. Ungated. - **SC.7** — `MemberResolver` has six near-identical hierarchy walks: `find{Method,Property,Constant}InHierarchy` and `collect{Methods,Properties,Constants}`, each a seen-check, a scan of the class's own members, and a recursion over From 18b87dae2bb83f1874d5c3823988ac57eef3bb36 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 17:53:41 -0700 Subject: [PATCH 30/35] File the symbol-key duplication row --- docs/architecture/build-manifest.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index 1779e598..1bbd9487 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -109,6 +109,7 @@ re-runs repo-wide as its completion gate. SC.15 — Oracle corpus: trait adaptations and enums — — SC.16 — Index an open document's global constants — — SC.17 — Collapse the hand-routed invalidation fan-out — — + SC.18 — One home for the kind-qualified symbol key SC.13 — SZ.1 Z Definition of Done gate + repo-wide dup audit all prior — Notes: @@ -309,6 +310,11 @@ Notes: `SymbolSink extends Invalidatable` solely to give the handler a way in, which is how the write path came to be named after the response instead of the event. Scope is one registration list at the composition root, which deletes the three fan-outs and the three type tests. Whether a general published event replaces it is #415 and is deliberately not settled here. Found while reviewing S3.8d. Ungated. + - **SC.18** — the key a name has under its kind, `$kind->name . '|' . $kind->normalize($name)`, is written out four times: `SymbolCache::keyFor`, `OpenDocumentBackend::key`, `DeclarationSymbolInfoFactory::collect`, and the composite's test fake. + `SymbolCache::keyFor` and `delete` are public for one caller — `FilesystemBackend` holds hashed key strings to reverse-map a path — so a `forget(QualifiedName, NameKind)` takes both off the surface and lets the backend record what it actually knows. + Duplication rather than a defect: the four stores are independent, so no two features can disagree over it today. It is filed because a fifth copy arrives with each new kind. + Gated on SC.13, which decides where the case fold lives; the key helper belongs beside it. + Found while reviewing S3.8d. - **SC.7** — `MemberResolver` has six near-identical hierarchy walks: `find{Method,Property,Constant}InHierarchy` and `collect{Methods,Properties,Constants}`, each a seen-check, a scan of the class's own members, and a recursion over From 6e78ceecbd3b33b15ccd71c983923cf75db513f3 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 18:34:56 -0700 Subject: [PATCH 31/35] Keep the backend list off the composite's surface --- src/Knowledge/CompositeSymbolSource.php | 4 ++-- tests/Knowledge/SymbolCoverageGridTest.php | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index 53cab0f9..c916ec79 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -37,10 +37,10 @@ final class CompositeSymbolSource implements SymbolSource /** * @param list $backends In descending precedence: the first * that answers a lookup wins, and the first to report a name wins a - * merge. Readable so the §5.1 coverage grid derives its rows from it. + * merge. */ public function __construct( - public readonly array $backends, + private readonly array $backends, ) { } diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index 6607cc01..87c14ed4 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -17,6 +17,7 @@ use Firehed\PhpLsp\Knowledge\SymbolBackend; use Firehed\PhpLsp\Parser\ParserService; use PHPUnit\Framework\TestCase; +use ReflectionProperty; /** * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose backend and @@ -281,12 +282,20 @@ private function evaluate(array $notApplicable): array } /** + * The composite is the sole authority on backend precedence, so it publishes no + * way to reach past it — deriving the rows is the grid's own concern and takes + * reflection rather than a production accessor. + * * @return array Backend short name -> the first of its class */ private function rows(): array { + $backends = (new ReflectionProperty(CompositeSymbolSource::class, 'backends'))->getValue($this->source); + assert(is_iterable($backends)); + $rows = []; - foreach ($this->source->backends as $backend) { + foreach ($backends as $backend) { + assert($backend instanceof SymbolBackend); $parts = explode('\\', $backend::class); $rows[end($parts)] ??= $backend; } From 8481c2cbdae1499abff1be5f3943190d21d15127 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 18:35:59 -0700 Subject: [PATCH 32/35] Say where the invalidation type tests actually are --- docs/architecture/build-manifest.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index 1bbd9487..c8b3f8ec 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -304,7 +304,8 @@ Notes: `WorkspaceNamespaceSource` already maps the kind, so the gap is upstream in the extractor. Found by the S3.8d coverage grid on its first run. Ungated, and ahead of S3.8b — constant lookup landing on an enumeration blind to open documents would rebuild the §4.2 split on the third symbol namespace. - - **SC.17** — telling the parts that hold file-derived state that a file changed is written out three times, each steered by an `instanceof` test: `DocumentSymbolSink` over its on-disk backends, `FilesystemBackend` over its catalog and locator, `CompositeSymbolLocator` over its routes. + - **SC.17** — telling the parts that hold file-derived state that a file changed is written out three times: `DocumentSymbolSink` over a list it is handed, `FilesystemBackend` over its catalog and locator, `CompositeSymbolLocator` over its routes. + The latter two steer by an `instanceof` test, three in all; the sink instead takes a pre-filtered list, so the composition root already decides who holds state and the knowledge is split between the two styles. So adding a holder means finding its parent in that tree by hand, and missing one is silent — the stale value is still served and nothing fails. Not only caches: the same route drops `AutoloadFilesLocator`'s derived name→file map, which is rebuilt rather than memoized. `SymbolSink extends Invalidatable` solely to give the handler a way in, which is how the write path came to be named after the response instead of the event. From f58b907071c5e7d8cf2650e1d9ebcc73d057727f Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 18:51:09 -0700 Subject: [PATCH 33/35] Revert "Keep the backend list off the composite's surface" This reverts commit 6e78ceecbd3b33b15ccd71c983923cf75db513f3. --- src/Knowledge/CompositeSymbolSource.php | 4 ++-- tests/Knowledge/SymbolCoverageGridTest.php | 11 +---------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index c916ec79..53cab0f9 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -37,10 +37,10 @@ final class CompositeSymbolSource implements SymbolSource /** * @param list $backends In descending precedence: the first * that answers a lookup wins, and the first to report a name wins a - * merge. + * merge. Readable so the §5.1 coverage grid derives its rows from it. */ public function __construct( - private readonly array $backends, + public readonly array $backends, ) { } diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php index 87c14ed4..6607cc01 100644 --- a/tests/Knowledge/SymbolCoverageGridTest.php +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -17,7 +17,6 @@ use Firehed\PhpLsp\Knowledge\SymbolBackend; use Firehed\PhpLsp\Parser\ParserService; use PHPUnit\Framework\TestCase; -use ReflectionProperty; /** * RFC 1 §8.1's mechanism for §5.1: a backend × kind × query grid whose backend and @@ -282,20 +281,12 @@ private function evaluate(array $notApplicable): array } /** - * The composite is the sole authority on backend precedence, so it publishes no - * way to reach past it — deriving the rows is the grid's own concern and takes - * reflection rather than a production accessor. - * * @return array Backend short name -> the first of its class */ private function rows(): array { - $backends = (new ReflectionProperty(CompositeSymbolSource::class, 'backends'))->getValue($this->source); - assert(is_iterable($backends)); - $rows = []; - foreach ($backends as $backend) { - assert($backend instanceof SymbolBackend); + foreach ($this->source->backends as $backend) { $parts = explode('\\', $backend::class); $rows[end($parts)] ??= $backend; } From dab67a77b4b0ef4124466208f6a8d1c76f9851e5 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 18:56:05 -0700 Subject: [PATCH 34/35] Point the narrowing note at the callers that do it --- src/Knowledge/CompositeSymbolSource.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index 53cab0f9..b4f08142 100644 --- a/src/Knowledge/CompositeSymbolSource.php +++ b/src/Knowledge/CompositeSymbolSource.php @@ -99,8 +99,9 @@ public function searchClassLikes(string $prefix): array } /** - * The first backend that answers wins; each caller above narrows the result back - * to a concrete type, at this one site (Plan 0002 §5.6). + * Answers with the marker type; each caller above narrows it back to a concrete + * one. That is the O(kinds) narrowing Plan 0002 §5.6 trades against a lookup + * method per kind on every backend. */ private function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { From 8ea7338f2a577407f8706d408347db6ce583eeed Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 13 Aug 2026 20:06:25 -0700 Subject: [PATCH 35/35] Drop comments that restate their own code --- src/Domain/DeclaredSymbol.php | 3 --- src/Knowledge/OpenDocumentBackend.php | 3 +-- tests/Knowledge/DeclarationSymbolInfoFactoryTest.php | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Domain/DeclaredSymbol.php b/src/Domain/DeclaredSymbol.php index 7724fd0c..d93932ca 100644 --- a/src/Domain/DeclaredSymbol.php +++ b/src/Domain/DeclaredSymbol.php @@ -5,9 +5,6 @@ namespace Firehed\PhpLsp\Domain; /** - * One symbol a file declares: its name, which of PHP's three symbol namespaces it - * lives in, and its metadata. - * * Registration carries the kind rather than splitting into a parameter per kind, so * a new kind is a case in the info factories and not a signature change on every * write path (Plan 0002 §5.6). diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 3058c87e..9189a6aa 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -103,8 +103,7 @@ public function removeDocument(string $uri): void /** * Registration and lookup must agree on the case rule, which differs by kind, so * both go through {@see NameKind::normalize()} rather than lowercasing the whole - * FQN — right for class-likes and functions, wrong for a constant. The kind is - * part of the key because one store holds all three. + * FQN — right for class-likes and functions, wrong for a constant. */ private static function key(NameKind $kind, QualifiedName $name): string { diff --git a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php index d44f40eb..7e76e7a4 100644 --- a/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php +++ b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php @@ -27,7 +27,7 @@ final class DeclarationSymbolInfoFactoryTest extends TestCase { use LoadsFixturesTrait; - /** Every kind this factory dispatches on, so reading the wrong list is visible. */ + /** Declares all three kinds, so a lookup reading the wrong list is visible. */ private const string FIXTURE = 'AutoloadFiles/helpers.php'; private DeclarationSymbolInfoFactory $factory;