From 263afb49709bfb5dd306bc1e3848730c2fa83cd0 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 2 Sep 2026 14:07:56 -0700 Subject: [PATCH 1/5] Add failing tests for namespaced-first and global-fallback constant resolution Co-Authored-By: Claude Opus 4.7 --- .../src/Resolution/NamespacedConstant.php | 23 +++++++++++ tests/Handler/DefinitionHandlerTest.php | 33 ++++++++++++++++ tests/Handler/HoverHandlerTest.php | 39 +++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 tests/Fixtures/src/Resolution/NamespacedConstant.php diff --git a/tests/Fixtures/src/Resolution/NamespacedConstant.php b/tests/Fixtures/src/Resolution/NamespacedConstant.php new file mode 100644 index 00000000..870a85a8 --- /dev/null +++ b/tests/Fixtures/src/Resolution/NamespacedConstant.php @@ -0,0 +1,23 @@ +openFixture($fixture); + $cursor = $this->openFixtureAtHoverMarker($fixture, 'namespaced_const_fetch'); + + $result = $this->handler->handle($this->definitionRequestAt($cursor)); + + self::assertIsArray( + $result, + 'unqualified constant in a namespace resolves via the namespace-first candidate', + ); + self::assertSame($uri, $result['uri']); + self::assertSame(12, $result['range']['start']['line']); + } + + public function testGoToGlobalConstantDefinitionFromNamespacedFile(): void + { + $fixture = 'src/Resolution/NamespacedConstant.php'; + $helpersUri = $this->openFixture('AutoloadFiles/helpers.php'); + $this->openFixture($fixture); + $cursor = $this->openFixtureAtHoverMarker($fixture, 'global_const_fetch'); + + $result = $this->handler->handle($this->definitionRequestAt($cursor)); + + self::assertIsArray( + $result, + 'a global constant referenced from a namespace with no `use const` resolves via the global fallback', + ); + self::assertSame($helpersUri, $result['uri']); + self::assertSame(18, $result['range']['start']['line']); + } } diff --git a/tests/Handler/HoverHandlerTest.php b/tests/Handler/HoverHandlerTest.php index 19d52736..4a421d12 100644 --- a/tests/Handler/HoverHandlerTest.php +++ b/tests/Handler/HoverHandlerTest.php @@ -902,6 +902,45 @@ public function testHoverOnAttributeClass(): void self::assertStringContainsString('Defines a route', $result['contents']['value']); } + public function testHoverOnNamespacedConstant(): void + { + $fixture = 'src/Resolution/NamespacedConstant.php'; + $this->openFixture($fixture); + $cursor = $this->openFixtureAtHoverMarker($fixture, 'namespaced_const_fetch'); + + $result = $this->handler->handle($this->hoverRequestAt($cursor)); + + self::assertIsArray( + $result, + 'the namespace-first candidate resolves an unqualified constant declared in the same namespace', + ); + self::assertStringContainsString( + 'NAMESPACED_CONSTANT', + $result['contents']['value'], + 'the resolved constant surfaces its name in the hover signature', + ); + } + + public function testHoverOnGlobalConstantFromNamespacedFile(): void + { + $fixture = 'src/Resolution/NamespacedConstant.php'; + $this->openFixture('AutoloadFiles/helpers.php'); + $this->openFixture($fixture); + $cursor = $this->openFixtureAtHoverMarker($fixture, 'global_const_fetch'); + + $result = $this->handler->handle($this->hoverRequestAt($cursor)); + + self::assertIsArray( + $result, + 'a global constant resolves via the global fallback when no namespaced candidate matches', + ); + self::assertStringContainsString( + 'FIXTURE_HELPER_DEFINED', + $result['contents']['value'], + 'the fallback path surfaces the global constant in the hover signature', + ); + } + private function handlerFor(MarkupKind $hoverMarkupKind): HoverHandler { $capabilities = self::createStub(SessionCapabilitiesProvider::class); From d038b161401bf43cc5e7e406cbc9103b2f6a21ef Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 2 Sep 2026 14:08:09 -0700 Subject: [PATCH 2/5] resolveConstFetch iterates NameContext::candidates for name resolution rules 5-7 Co-Authored-By: Claude Opus 4.7 --- src/Resolution/ExpressionResolver.php | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Resolution/ExpressionResolver.php b/src/Resolution/ExpressionResolver.php index 8c07124e..2b1a9910 100644 --- a/src/Resolution/ExpressionResolver.php +++ b/src/Resolution/ExpressionResolver.php @@ -123,7 +123,7 @@ public function resolve(Expr $expr, array $ast): ?ResolvedSymbol } if ($expr instanceof Expr\ConstFetch) { - return $this->resolveConstFetch($expr); + return $this->resolveConstFetch($expr, $ast); } if ($expr instanceof Clone_) { @@ -466,11 +466,22 @@ private function resolveClassConstFetch(ClassConstFetch $expr): ?ResolvedSymbol return $constant; } - private function resolveConstFetch(Expr\ConstFetch $expr): ?ConstantInfo + /** + * @param array $ast + */ + private function resolveConstFetch(Expr\ConstFetch $expr, array $ast): ?ConstantInfo { - $name = ScopeFinder::resolveName($expr->name); - $info = $this->symbolSource->lookupConstant(GlobalConstantName::fromFullyQualified($name)); - return $info; + $shortName = $expr->name->toString(); + $line = $expr->name->getStartLine() - 1; + $context = NameContextFactory::fromAst($ast, $line); + + foreach ($context->candidates($shortName, NameKind::Constant) as $candidate) { + $info = $this->symbolSource->lookupConstant(GlobalConstantName::fromFullyQualified($candidate)); + if ($info !== null) { + return $info; + } + } + return null; } private function resolveLateBoundReturn(MethodInfo $methodInfo, ClassName $callingClass): MethodInfo From 0e0054a2675bfca8021821b876245fd85477340a Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 2 Sep 2026 14:08:15 -0700 Subject: [PATCH 3/5] Pin SymbolSource::lookupConstant to ExpressionResolver, drain baseline Co-Authored-By: Claude Opus 4.7 --- phpstan-baseline.neon | 6 ------ phpstan.neon | 3 ++- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2b80d8c9..60402605 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -24,12 +24,6 @@ parameters: count: 1 path: src/Handler/SignatureHelpHandler.php - - - message: '#^Calling Firehed\\PhpLsp\\Knowledge\\SymbolSource\:\:lookupConstant\(\) is forbidden, constant lookup iterates NameContext\:\:candidates\(NameKind\:\:Constant\) inside its own helper \(build\-manifest step\-28 adds it and lists it here\); mirrors \#478 for findMethod/findProperty\.$#' - identifier: disallowed.method - count: 1 - path: src/Resolution/ExpressionResolver.php - - message: '#^Calling Firehed\\PhpLsp\\Resolution\\TextFallbackHelper\:\:resolveEnclosingClassName\(\) is forbidden, enclosing\-class resolution goes through one helper that consults the text fallback when the parent chain is detached \(build\-manifest step\-31 adds the helper and lists it here\)\.$#' identifier: disallowed.method diff --git a/phpstan.neon b/phpstan.neon index b9d53ebe..b90a65e7 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -354,8 +354,9 @@ parameters: - tests/* - method: 'Firehed\PhpLsp\Knowledge\SymbolSource::lookupConstant()' - message: 'constant lookup iterates NameContext::candidates(NameKind::Constant) inside its own helper (build-manifest step-28 adds it and lists it here); mirrors #478 for findMethod/findProperty' + message: 'constant lookup iterates NameContext::candidates(NameKind::Constant) inside ExpressionResolver::resolveConstFetch (build-manifest step-28); mirrors #478 for findMethod/findProperty' allowIn: + - src/Resolution/ExpressionResolver.php # resolveConstFetch: the candidate-loop caller - src/Knowledge/CompositeSymbolSource.php # composite delegates to backends - tests/* - From 6ca1ece77e42969571c5dc09d04b0fc67a3d3a11 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 2 Sep 2026 14:08:22 -0700 Subject: [PATCH 4/5] Make ScopeFinder::resolveName private; class-name path stays public Its only remaining src/ caller was resolveClassName, which now inlines the resolved-name attribute read. Test coverage moves to resolveClassName. Co-Authored-By: Claude Opus 4.7 --- src/Utility/ScopeFinder.php | 22 +++++++--------------- tests/Utility/ScopeFinderTest.php | 19 +++---------------- 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/src/Utility/ScopeFinder.php b/src/Utility/ScopeFinder.php index 281f2d54..66ba6e46 100644 --- a/src/Utility/ScopeFinder.php +++ b/src/Utility/ScopeFinder.php @@ -79,29 +79,21 @@ public static function findEnclosingNamespace(Node $node): ?Stmt\Namespace_ return null; } - /** - * Resolve a Name node to its fully qualified name. - * - * Uses the resolved name attribute if available (from NameResolver), - * otherwise falls back to the raw name. - */ - public static function resolveName(Name $name): string - { - $resolvedName = $name->getAttribute('resolvedName'); - return $resolvedName instanceof Name - ? $resolvedName->toString() - : $name->toString(); - } - /** * Resolve a class Name node to its fully qualified class name. * + * Uses the resolved name attribute set by NameResolver when present; + * falls back to the raw name otherwise. + * * @return class-string */ public static function resolveClassName(Name $name): string { + $resolvedName = $name->getAttribute('resolvedName'); /** @var class-string */ - return self::resolveName($name); + return $resolvedName instanceof Name + ? $resolvedName->toString() + : $name->toString(); } /** diff --git a/tests/Utility/ScopeFinderTest.php b/tests/Utility/ScopeFinderTest.php index 81b4be3a..64383d98 100644 --- a/tests/Utility/ScopeFinderTest.php +++ b/tests/Utility/ScopeFinderTest.php @@ -208,7 +208,7 @@ public function testFindEnclosingClassNameReturnsNullForAnonymousClass(): void self::assertNull($className); } - public function testResolveNameReturnsRawNameWhenNoResolvedAttribute(): void + public function testResolveClassNameReturnsRawNameWhenNoResolvedAttribute(): void { $code = $this->loadFixture('src/Inheritance/ParentClass.php'); $ast = self::parseWithParents($code); @@ -218,23 +218,10 @@ public function testResolveNameReturnsRawNameWhenNoResolvedAttribute(): void self::assertNotNull($class); self::assertNotNull($class->extends); - self::assertSame('Fixtures\Inheritance\Grandparent', ScopeFinder::resolveName($class->extends)); + self::assertSame('Fixtures\Inheritance\Grandparent', ScopeFinder::resolveClassName($class->extends)); } - public function testResolveNameUsesResolvedNameWhenAvailable(): void - { - $code = $this->loadFixture('src/Utility/ImportedExtends.php'); - $ast = self::parseWithParents($code); - $namespace = $ast[1]; - self::assertInstanceOf(Stmt\Namespace_::class, $namespace); - $class = self::findFirstClassLike($namespace->stmts, Stmt\Class_::class); - self::assertNotNull($class); - self::assertNotNull($class->extends); - - self::assertSame('Fixtures\Inheritance\ParentClass', ScopeFinder::resolveName($class->extends)); - } - - public function testResolveClassNameDelegatesToResolveName(): void + public function testResolveClassNameUsesResolvedNameWhenAvailable(): void { $code = $this->loadFixture('src/Utility/ImportedExtends.php'); $ast = self::parseWithParents($code); From 58e4b2532cf2c20a3b557751a293d6cd341f7c39 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 2 Sep 2026 14:08:34 -0700 Subject: [PATCH 5/5] Tick step-28 Co-Authored-By: Claude Opus 4.7 --- docs/architecture/build-manifest.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/build-manifest.md b/docs/architecture/build-manifest.md index 0144296e..0fd5e072 100644 --- a/docs/architecture/build-manifest.md +++ b/docs/architecture/build-manifest.md @@ -45,7 +45,7 @@ One signal: both baseline files are deleted, and the issues named below are clos - [x] **step-25** — `ExpressionResolver::docblockForExpression` reads the resolved symbol's docblock through one `resolve(...)?->getDocumentation()` call — no per-kind branch. If the wrapper carries no logic once the branch is gone, delete it and inline the call at every caller. Done: the method either does not exist or is one line with no `match`/`instanceof` on the expression node; `@return list` and `@var` docblock inference works on `FuncCall`, `MethodCall`, `NullsafeMethodCall`, `StaticCall`, `PropertyFetch`, `NullsafePropertyFetch`, `StaticPropertyFetch`, `ClassConstFetch`, and `ConstFetch` the same way it works on `$this->items()`; a test covers each of those node kinds. - [x] **step-26** — The three late-binding keywords (`self`, `static`, `parent`) resolve in one place: `Domain\LateBindingKeyword`. Every reader (`ScopeFinder`, `MemberAccessDetector`'s text and AST paths, any other) identifies a keyword through `LateBindingKeyword::tryFrom(strtolower($name))` and resolves it through one function on the enum; the `parent`-of-non-`Class_` guard exists there once. A test under `tests/Architecture/` fails if a string comparison against `'self'`, `'static'`, or `'parent'` appears in `src/` outside `src/Domain/LateBindingKeyword.php` — the tighten that pins the seam. Done: no `src/` file outside the enum compares against the three keyword literals in a class-name-resolution context; a text-path and an AST-path test exercise the same behavior through one code path; the architecture test above is green. - [x] **step-27** — `ExpressionResolver::resolveMember` (introduced in step-24) iterates every class it gets from the receiver's `Type::getResolvableClassNames()` instead of indexing `[0]`, the same way `SymbolResolver::getAccessibleMembers` iterates. `MemberAccessDetector`'s three instance-receiver sites route through the same helper (or apply the same iteration). Tighten: `disallowedMethodCalls` restricts `Type::getResolvableClassNames()` to the shared helper and to `SymbolResolver::getAccessibleMembers`, so a future direct caller fails PHPStan. Done: no callsite in `src/Resolution/` indexes `[0]` on `getResolvableClassNames()`; hover, definition, and signature-help on `$x->onlyB()` where `$x: A|B` and only `B` declares `onlyB` answer the same way completion offers it; a parity test asserts the four positional handlers and completion agree on union and intersection receivers; the phpstan baseline for the rule reaches zero. -- [ ] **step-28** — `resolveConstFetch` iterates `NameContext::candidates(short, NameKind::Constant)` the way `resolveFuncCall` iterates `NameKind::Function_`, so PHP name-resolution rules 5-7 (namespaced-first, global fallback) apply to constants as they do to functions. Tighten: `disallowedMethodCalls` restricts `SymbolSource::lookupConstant` to `src/Resolution/ExpressionResolver.php` (mirroring the #478 pattern for `findMethod`/`findProperty`), so a future direct `lookupConstant` outside the candidate loop fails PHPStan. Done: hover and definition on `X` in `namespace App; const X = 1; echo X;` answer; hover and definition on `PHP_INT_MAX` in a namespaced file with no `use const` answer; `resolveConstFetch` has no direct `lookupConstant` call that bypasses the candidate loop; a test covers both the namespaced-constant and global-fallback paths. +- [x] **step-28** — `resolveConstFetch` iterates `NameContext::candidates(short, NameKind::Constant)` the way `resolveFuncCall` iterates `NameKind::Function_`, so PHP name-resolution rules 5-7 (namespaced-first, global fallback) apply to constants as they do to functions. Tighten: `disallowedMethodCalls` restricts `SymbolSource::lookupConstant` to `src/Resolution/ExpressionResolver.php` (mirroring the #478 pattern for `findMethod`/`findProperty`), so a future direct `lookupConstant` outside the candidate loop fails PHPStan. Done: hover and definition on `X` in `namespace App; const X = 1; echo X;` answer; hover and definition on `PHP_INT_MAX` in a namespaced file with no `use const` answer; `resolveConstFetch` has no direct `lookupConstant` call that bypasses the candidate loop; a test covers both the namespaced-constant and global-fallback paths. - [ ] **step-29** — `SymbolCandidates` reads a symbol's documentation through the `ResolvedSymbol::getDocumentation()` interface method, not by direct `->docblock` field access plus `DocblockParser::extractDescription`. Tighten: `disallowedMethodCalls` restricts `DocblockParser::extractDescription` to `src/Domain/HasSymbolLocation.php`, so a second bypass of the interface fails PHPStan. Done: `SymbolCandidates` does not name `->docblock` or `DocblockParser` directly; a future change to `getDocumentation()` (e.g. tag stripping) reaches completion detail the same way it reaches hover. - [ ] **step-30** — Signature-plus-documentation assembly lives in one place. A `ResolvedSymbolPresenter` in `src/Resolution/` returns the shape (signature, documentation, deprecation, tags) that `HoverHandler`, `SignatureHelpHandler`, and `CompletionItemFactory` all consume. Each handler and factory maps the presenter output into its LSP shape but does not compose signature-and-documentation itself. Tighten: `disallowedMethodCalls` restricts `ResolvedSymbol::format()` and `ResolvedSymbol::getDocumentation()` to `ResolvedSymbolPresenter`, so a future handler that recomposes signature-plus-doc directly fails PHPStan. Done: no handler or factory calls both `->format()` and `->getDocumentation()` on a resolved symbol; adding a new user-facing field to `ResolvedSymbol` (e.g. `getDeprecation()`) is one edit that all three surfaces read; a parity test asserts hover, signature-help, and completion-detail surface the added attribute the same way; the disallow above is in place. - [ ] **step-31** — `Variable('this')` types through one code path. `ExpressionResolver::resolve(Variable('this'))` reads the enclosing class the same way `MemberAccessDetector` reads it — a shared helper that consults the text fallback when the parent chain is detached — and the `resolvedType` attribute side-channel `MemberAccessDetector` sets on the `$this` node is deleted. Tighten: `disallowedMethodCalls` restricts `TextFallbackHelper::resolveEnclosingClassName` to the shared helper, so a future ad-hoc text-fallback caller fails PHPStan. Done: hover on `$this` and completion on `$this->` answer for the same set of broken-code inputs; a fixture whose enclosing class-like parent chain is detached drives both features through one path; `MemberAccessDetector` does not write `resolvedType` on any AST node; the disallow above is in place.