diff --git a/CLAUDE.md b/CLAUDE.md index 187e8af..af5c55e 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: @@ -133,15 +133,19 @@ 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 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. 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 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 @@ -177,11 +181,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 84955bd..c8b3f8e 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -107,6 +107,9 @@ 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 — — + 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: @@ -147,6 +150,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. @@ -290,10 +296,26 @@ 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. + - **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.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. + 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 diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 0664104..0a83afe 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 @@ -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 c3dd1d2..6bbe314 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -33,9 +33,9 @@ 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 - src/Repository/DefaultClassInfoFactory.php @@ -86,11 +86,23 @@ 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/* + - + function: + - 'class_exists()' + - 'constant()' + - 'defined()' + - 'enum_exists()' + - 'function_exists()' + - 'interface_exists()' + - 'trait_exists()' + 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/* - function: - 'file_get_contents()' diff --git a/src/Domain/ClassInfo.php b/src/Domain/ClassInfo.php index 0b65681..569d989 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/DeclaredSymbol.php b/src/Domain/DeclaredSymbol.php new file mode 100644 index 0000000..d93932c --- /dev/null +++ b/src/Domain/DeclaredSymbol.php @@ -0,0 +1,20 @@ + $parameters diff --git a/src/Domain/SymbolInfo.php b/src/Domain/SymbolInfo.php new file mode 100644 index 0000000..89258c6 --- /dev/null +++ b/src/Domain/SymbolInfo.php @@ -0,0 +1,15 @@ +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); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof ClassInfo); - return $cached; - } - - try { - $classInfo = $this->factory->fromReflection(new ReflectionClass($name->fqn)); - } catch (ReflectionException) { - return null; - } - $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 $this->cache->remember( + $name, + $kind, + fn(): ?SymbolInfo => $this->infoFactory->fromReflection($name, $kind), + ); } /** diff --git a/src/Knowledge/CompositeSymbolSource.php b/src/Knowledge/CompositeSymbolSource.php index df37678..b4f0814 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; @@ -36,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, ) { } @@ -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,23 @@ public function searchClassLikes(string $prefix): array return array_values($byFqn); } + /** + * 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 + { + 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/DeclarationSymbolInfoFactory.php b/src/Knowledge/DeclarationSymbolInfoFactory.php new file mode 100644 index 0000000..779262e --- /dev/null +++ b/src/Knowledge/DeclarationSymbolInfoFactory.php @@ -0,0 +1,94 @@ + + */ + 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, + NameKind $kind, + string $filePath, + ): ?SymbolInfo { + $target = $kind->normalize($name); + + foreach ($this->allIn($declarations, $filePath) as $symbol) { + if ($symbol->kind === $kind && $kind->normalize($symbol->name) === $target) { + return $symbol->info; + } + } + + return null; + } + + /** + * @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; + } + + $seen[$key] = true; + $symbols[] = new DeclaredSymbol($name, $kind, $info); + } +} diff --git a/src/Knowledge/DocumentSymbolSink.php b/src/Knowledge/DocumentSymbolSink.php index 308a924..c6bcbef 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/FilesystemBackend.php b/src/Knowledge/FilesystemBackend.php index ca4f1fe..51ed4d3 100644 --- a/src/Knowledge/FilesystemBackend.php +++ b/src/Knowledge/FilesystemBackend.php @@ -6,19 +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; /** * A {@see SymbolBackend} over PHP files on disk, resolved through Composer's @@ -27,12 +22,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 + * 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 + * 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 +38,8 @@ 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, recorded because a key is an opaque hash + * with no reverse mapping to a path. * * @var array> */ @@ -56,9 +49,9 @@ 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, + private readonly SymbolCache $cache, ) { } @@ -67,57 +60,25 @@ 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); - - $cached = $this->cache->get($cacheKey); - if ($cached !== null) { - assert($cached instanceof ClassInfo); - return $cached; - } - - $filePath = $this->locator->locate($qualifiedName, NameKind::ClassLike); - if ($filePath === null) { - return null; - } - - $classInfo = $this->parseClassFrom($name, $filePath); - if ($classInfo !== null) { - $this->cache->set($cacheKey, $classInfo); - $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; - } + return $this->cache->remember($name, $kind, function () use ($name, $kind): ?SymbolInfo { + $filePath = $this->locator->locate($name, $kind); + if ($filePath === null) { + return null; + } - $functionInfo = $this->parseFunctionFrom($name, $filePath); - if ($functionInfo !== null) { - $this->cache->set($cacheKey, $functionInfo); - $this->cacheKeysByPath[$filePath][] = $cacheKey; - } + $info = $this->infoFactory->fromDeclarations($this->declarationsIn($filePath), $name, $kind, $filePath); + if ($info !== null) { + $this->cacheKeysByPath[$filePath][] = $this->cache->keyFor($name, $kind); + } - 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 +113,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 f9f00b3..8f0603f 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,22 +53,23 @@ 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(), + new SymbolCache(CacheFactory::inMemory()), ), ]); @@ -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 @@ -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,9 +121,9 @@ private static function filesystemBackend( CacheFactory::inMemory(), ), $parser, - $classInfoFactory, + $infoFactory, $scanner, - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } } diff --git a/src/Knowledge/OpenDocumentBackend.php b/src/Knowledge/OpenDocumentBackend.php index 7e82d57..9189a6a 100644 --- a/src/Knowledge/OpenDocumentBackend.php +++ b/src/Knowledge/OpenDocumentBackend.php @@ -4,12 +4,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\DeclaredSymbol; 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 +20,10 @@ * 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} 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 { @@ -44,17 +40,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 +59,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; } /** @@ -88,57 +73,40 @@ 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, $classInfo->name->fqn); - $this->byFqn[$key] = $classInfo; + foreach ($symbols as $symbol) { + $key = self::key($symbol->kind, $symbol->name); + $this->byKey[$key] = $symbol->info; $keys[] = $key; } - $this->fqnsByUri[$uri] = $keys; - - $functionKeys = []; - foreach ($functions as $fqn => $functionInfo) { - $key = self::key(NameKind::Function_, $fqn); - $this->functionsByFqn[$key] = $functionInfo; - $functionKeys[] = $key; - } - $this->functionFqnsByUri[$uri] = $functionKeys; + $this->keysByUri[$uri] = $keys; } public function removeDocument(string $uri): void { - foreach ($this->fqnsByUri[$uri] ?? [] as $key) { - unset($this->byFqn[$key]); - } - unset($this->fqnsByUri[$uri]); - - foreach ($this->functionFqnsByUri[$uri] ?? [] as $key) { - unset($this->functionsByFqn[$key]); + foreach ($this->keysByUri[$uri] ?? [] as $key) { + unset($this->byKey[$key]); } - unset($this->functionFqnsByUri[$uri]); + unset($this->keysByUri[$uri]); } /** - * 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. + * 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. */ - 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/ReflectionSymbolInfoFactory.php b/src/Knowledge/ReflectionSymbolInfoFactory.php new file mode 100644 index 0000000..695fd95 --- /dev/null +++ b/src/Knowledge/ReflectionSymbolInfoFactory.php @@ -0,0 +1,65 @@ + $this->classInfo($name), + NameKind::Function_ => $this->functionInfo($name), + // Reflectable, but the info type lands in S3.8b. + NameKind::Constant => null, + }; + } + + private function classInfo(QualifiedName $name): ?SymbolInfo + { + $fqn = $name->fullyQualifiedName(); + + // 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; + } + + 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 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 f3015e4..1794c58 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,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 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 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 { @@ -41,16 +40,10 @@ 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). + * Full metadata for the symbol $name names *as a $kind*, or `null` when this + * backend cannot reach such a declaration (RFC 1 §5.3). */ - 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). - */ - 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 diff --git a/src/Knowledge/SymbolCache.php b/src/Knowledge/SymbolCache.php new file mode 100644 index 0000000..4a89eeb --- /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 5ffaeca..0000000 --- a/src/Knowledge/SymbolCacheKey.php +++ /dev/null @@ -1,25 +0,0 @@ -name . '|' . $kind->normalize($name)); - } -} diff --git a/tests/BuildsSymbolInfoTrait.php b/tests/BuildsSymbolInfoTrait.php index a1f96a8..ed49b86 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/BuiltinBackendTest.php b/tests/Knowledge/BuiltinBackendTest.php index 1564058..3514dd5 100644 --- a/tests/Knowledge/BuiltinBackendTest.php +++ b/tests/Knowledge/BuiltinBackendTest.php @@ -5,32 +5,37 @@ 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\Knowledge\SymbolCache; 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, + new SymbolCache(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 +44,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 +52,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 +62,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 +72,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 +88,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 +96,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 +104,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 +119,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 +151,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/CompositeSymbolSourceTest.php b/tests/Knowledge/CompositeSymbolSourceTest.php index 08c7872..4eae31a 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/DeclarationSymbolInfoFactoryTest.php b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php new file mode 100644 index 0000000..7e76e7a --- /dev/null +++ b/tests/Knowledge/DeclarationSymbolInfoFactoryTest.php @@ -0,0 +1,188 @@ +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)', + ); + } + + /** + * A merged list, or the wrong one, would resolve these. + * + * @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 declares it, so the null is the missing info type. + 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', + ); + } + + 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( + $this->declarations, + QualifiedName::fromFullyQualified($fqn), + $kind, + $this->path, + ); + } +} diff --git a/tests/Knowledge/DocumentSymbolSinkTest.php b/tests/Knowledge/DocumentSymbolSinkTest.php index f47d805..55fc057 100644 --- a/tests/Knowledge/DocumentSymbolSinkTest.php +++ b/tests/Knowledge/DocumentSymbolSinkTest.php @@ -5,13 +5,12 @@ 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; 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; @@ -30,6 +29,7 @@ final class DocumentSymbolSinkTest extends TestCase { use LoadsFixturesTrait; + use LooksUpBackendSymbolsTrait; private SymbolIndex $index; private OpenDocumentBackend $backend; @@ -44,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(), ); @@ -58,7 +58,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 +74,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 +93,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 +106,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 +120,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 +135,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 +152,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 +165,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 +179,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 +197,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 +231,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 +243,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 +264,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( @@ -330,16 +328,10 @@ 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), ); } - - 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 5d36a3c..8dbaa6c 100644 --- a/tests/Knowledge/FakeSymbolBackend.php +++ b/tests/Knowledge/FakeSymbolBackend.php @@ -4,10 +4,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\DeclaredSymbol; +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; @@ -17,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 Lowercased FQN -> 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 Lowercased FQN -> 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 @@ -39,14 +46,9 @@ public function childrenOf(NamespaceName $namespace): NamespaceContents return $this->namespaces[$namespace->path] ?? new NamespaceContents(); } - public function lookupClassLike(ClassName $name): ?ClassInfo - { - return $this->classLikes[strtolower(ltrim($name->fqn, '\\'))] ?? null; - } - - public function lookupFunction(FunctionName $name): ?FunctionInfo + public function lookup(QualifiedName $name, NameKind $kind): ?SymbolInfo { - return $this->functions[strtolower($name->fullyQualifiedName())] ?? null; + return $this->byKey[self::key($name, $kind)] ?? null; } /** @@ -62,4 +64,9 @@ public function searchClassLikes(string $prefix): array ), )); } + + private static function key(QualifiedName $name, NameKind $kind): string + { + return $kind->name . '|' . $kind->normalize($name); + } } diff --git a/tests/Knowledge/FilesystemBackendTest.php b/tests/Knowledge/FilesystemBackendTest.php index d79514b..557f31c 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,19 @@ 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\SymbolCache; 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 +35,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 +59,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 +67,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 +80,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 +94,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 +109,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 +121,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 +146,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 +154,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 +165,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 +173,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 +185,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 +195,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 +203,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 +227,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 +240,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 +258,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,9 +280,9 @@ public function testInvalidateAlsoDropsCachedNamespaceListings(): void self::createStub(SymbolLocator::class), new CachedNamespaceCatalog($counting, CacheFactory::inMemory()), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); $backend->childrenOf(new NamespaceName('Psr\Log')); @@ -322,13 +313,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 +359,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 +370,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 +385,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 +399,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,9 +426,9 @@ public function testChildrenOfForwardsToTheInjectedCatalog(): void self::createStub(SymbolLocator::class), $catalog, $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); self::assertSame( @@ -476,9 +466,9 @@ private function backend(): FilesystemBackend ]), new ComposerNamespaceSource($map), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } @@ -488,9 +478,9 @@ private function backendWithLocator(SymbolLocator $locator): FilesystemBackend $locator, self::createStub(NamespaceCatalog::class), $this->parser, - $this->factory, + $this->infoFactory, new DeclarationScanner(), - CacheFactory::inMemory(), + new SymbolCache(CacheFactory::inMemory()), ); } @@ -501,10 +491,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/GridQuery.php b/tests/Knowledge/GridQuery.php new file mode 100644 index 0000000..60b1095 --- /dev/null +++ b/tests/Knowledge/GridQuery.php @@ -0,0 +1,26 @@ +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 e2d09d1..c481c91 100644 --- a/tests/Knowledge/OpenDocumentBackendTest.php +++ b/tests/Knowledge/OpenDocumentBackendTest.php @@ -4,7 +4,10 @@ namespace Firehed\PhpLsp\Tests\Knowledge; -use Firehed\PhpLsp\Domain\FunctionName; +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; @@ -23,6 +26,7 @@ final class OpenDocumentBackendTest extends TestCase { use BuildsSymbolInfoTrait; + use LooksUpBackendSymbolsTrait; private SymbolIndex $index; private OpenDocumentBackend $backend; @@ -35,18 +39,28 @@ 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 = $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'); } + 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( - $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)', ); } @@ -54,15 +68,15 @@ 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( - $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', ); } @@ -70,12 +84,12 @@ 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); 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,16 +99,16 @@ 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', ); } 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 = $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'); @@ -102,10 +116,10 @@ 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( - $this->backend->lookupFunction(FunctionName::fromFullyQualified('V\FORMAT')), + self::functionIn($this->backend, 'V\FORMAT'), 'PHP matches function names case-insensitively', ); } @@ -113,25 +127,59 @@ 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)', ); } + 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', + ); + 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 { $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( - $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', ); } @@ -139,15 +187,15 @@ 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( - $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', ); } @@ -155,12 +203,12 @@ 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); 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/Knowledge/ReflectionSymbolInfoFactoryTest.php b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php new file mode 100644 index 0000000..387389f --- /dev/null +++ b/tests/Knowledge/ReflectionSymbolInfoFactoryTest.php @@ -0,0 +1,108 @@ +factory = new ReflectionSymbolInfoFactory(new DefaultClassInfoFactory()); + } + + /** + * @return iterable + */ + 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')] + public function testBuildsClassInfoForEveryClassLikeFlavour(string $fqn): void + { + $info = $this->build($fqn, NameKind::ClassLike); + + self::assertInstanceOf(ClassInfo::class, $info, 'a class-like must build ClassInfo'); + self::assertSame($fqn, $info->name->fqn, 'the reflected class-like 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 + { + // 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( + $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); + } +} diff --git a/tests/Knowledge/SymbolCoverageGridTest.php b/tests/Knowledge/SymbolCoverageGridTest.php new file mode 100644 index 0000000..6607cc0 --- /dev/null +++ b/tests/Knowledge/SymbolCoverageGridTest.php @@ -0,0 +1,355 @@ +||`. + * + * @var array + */ + private const array NOT_APPLICABLE = [ + // 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 until S3.9a. + '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 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' => '#23', + + // `SymbolExtractor` emits no `SymbolKind::Constant`, so an open document's + // constants never reach the index this reads. Found by this grid. + 'OpenDocumentBackend|Constant|childrenOf' => 'SC.16', + ]; + + /** + * 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> + */ + 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' => ''], + ], + ]; + + /** + * 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' + 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 + { + // 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 + { + // 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']); + + 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 testEveryRegistrationNamesALiveBlocker(): void + { + self::assertSame( + [], + self::danglingBlockers(self::NOT_APPLICABLE), + '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', + ); + } + + public function testABlockerNamingNoSliceIsReported(): void + { + // 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, issue, or 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::UNOWNED_BLOCKER, $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]; + } + + /** + * The registry is an argument so the mechanism can be tested, not 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]; + } + + /** + * @return array Backend short name -> the first of its class + */ + 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 => $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; + + 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; + } +} diff --git a/tests/Parity/BuiltinFunctionParityTest.php b/tests/Parity/BuiltinFunctionParityTest.php index c395652..b3ffb75 100644 --- a/tests/Parity/BuiltinFunctionParityTest.php +++ b/tests/Parity/BuiltinFunctionParityTest.php @@ -11,6 +11,8 @@ use Firehed\PhpLsp\Index\ReflectionNamespaceSource; 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; @@ -52,9 +54,9 @@ 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(), + new SymbolCache(CacheFactory::inMemory()), ); }