Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/architecture/build-manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ One signal: both baseline files are deleted, and the issues named below are clos
- [x] **step-24** — Member lookup in `ExpressionResolver` is one function taking the receiver expression, the member name, and a kind-specific finder; `resolveMethodCall`, `resolveStaticCall`, `resolvePropertyFetch`, and `resolveStaticPropertyFetch` call it. Done: the four methods share one member-lookup helper; adding a fifth member-access node kind is one call site, not four; the existing hover, definition, completion, and signature-help suites remain green.
- [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<T>` 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.
- [ ] **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.
- [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.
- [ ] **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.
Expand Down
12 changes: 0 additions & 12 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,12 @@ parameters:
count: 1
path: src/Handler/SignatureHelpHandler.php

-
message: '#^Calling Firehed\\PhpLsp\\Domain\\Type\:\:getResolvableClassNames\(\) is forbidden, union/intersection receivers\: member lookup must iterate every class, not index \[0\]\. Route through the shared member\-on\-any\-type helper \(build\-manifest step\-27\)\.$#'
identifier: disallowed.method
count: 1
path: src/Resolution/ExpressionResolver.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\\Domain\\Type\:\:getResolvableClassNames\(\) is forbidden, union/intersection receivers\: member lookup must iterate every class, not index \[0\]\. Route through the shared member\-on\-any\-type helper \(build\-manifest step\-27\)\.$#'
identifier: disallowed.method
count: 3
path: src/Resolution/MemberAccessDetector.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
Expand Down
3 changes: 2 additions & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,10 @@ parameters:
- tests/*
-
method: 'Firehed\PhpLsp\Domain\Type::getResolvableClassNames()'
message: 'union/intersection receivers: member lookup must iterate every class, not index [0]. Route through the shared member-on-any-type helper (build-manifest step-27)'
message: 'union/intersection receivers: member lookup must iterate every class, not index [0]. Route through ExpressionResolver::receiverClassNames or SymbolResolver::getAccessibleMembers (build-manifest step-27)'
allowIn:
- src/Resolution/SymbolResolver.php # getAccessibleMembers: the legitimate every-class iterator
- src/Resolution/ExpressionResolver.php # receiverClassNames: the shared helper for member-access receivers
- src/Domain/UnionType.php # recursive composition
- src/Domain/IntersectionType.php # recursive composition
- tests/*
Expand Down
24 changes: 16 additions & 8 deletions src/Resolution/ExpressionResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -410,16 +410,24 @@ private function resolveMember(
if ($classNameStr === null) {
return null;
}
$className = TypeFactory::className($classNameStr);
} else {
$receiverType = $this->resolve($receiver, $ast)?->getType();
$classNames = $receiverType?->getResolvableClassNames() ?? [];
if ($classNames === []) {
return null;
return $find(TypeFactory::className($classNameStr), $memberName);
}
$receiverType = $this->resolve($receiver, $ast)?->getType();
foreach (self::receiverClassNames($receiverType) as $className) {
$member = $find($className, $memberName);
if ($member !== null) {
return $member;
}
$className = $classNames[0];
}
return $find($className, $memberName);
return null;
}

/**
* @return list<ClassName>
*/
public static function receiverClassNames(?Type $type): array
{
return $type?->getResolvableClassNames() ?? [];
}

private function findMethod(ClassName $className, string $name): ?MethodInfo
Expand Down
63 changes: 37 additions & 26 deletions src/Resolution/MemberAccessDetector.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,11 @@ public function detect(

$prefix = $node->name instanceof Identifier ? $node->name->toString() : '';
$type = $this->resolveInstanceAccessType($node, $ast, $document, $line);
$target = $type?->getResolvableClassNames()[0] ?? null;
if ($type !== null && $target !== null) {
$enclosingName = ScopeFinder::findEnclosingClassName($node);
$vantage = $enclosingName !== null ? TypeFactory::className($enclosingName) : null;
return MemberAccessContext::forInstance(
$type,
$this->visibilityBetween($vantage, $target),
$prefix,
);
$enclosingName = ScopeFinder::findEnclosingClassName($node);
$vantage = $enclosingName !== null ? TypeFactory::className($enclosingName) : null;
$visibility = $this->visibilityForReceiver($vantage, $type);
if ($type !== null && $visibility !== null) {
return MemberAccessContext::forInstance($type, $visibility, $prefix);
}
return $this->fromText($document, $ast, $line, $character);
}
Expand Down Expand Up @@ -182,6 +178,30 @@ public function resolveInstanceAccessType(
return $exprResolver->resolve($node->var, $ast)?->getType();
}

/**
* Null when the receiver resolves to no classes; otherwise the most
* restrictive visibility across the constituents — a member must be
* visible on every possible runtime class to be safe to offer.
*/
private function visibilityForReceiver(?ClassName $vantage, ?Type $type): ?Visibility
{
$classes = ExpressionResolver::receiverClassNames($type);
if ($classes === []) {
return null;
}
$visibility = Visibility::Private;
foreach ($classes as $target) {
$per = $this->visibilityBetween($vantage, $target);
if ($per->value > $visibility->value) {
$visibility = $per;
}
if ($visibility === Visibility::Public) {
break;
}
}
return $visibility;
}

/**
* The one function that decides how visible a target class is to a vantage
* class. Same class: private. Subclass (any depth): protected. Otherwise
Expand Down Expand Up @@ -249,15 +269,11 @@ private function resolveTextMatch(
return null;
}
$type = $this->resolveChainReceiverType($match['chain'], $enclosingClass, $document, $ast);
$target = $type?->getResolvableClassNames()[0] ?? null;
if ($type === null || $target === null) {
$visibility = $this->visibilityForReceiver(TypeFactory::className($enclosingClass), $type);
if ($type === null || $visibility === null) {
return null;
}
return MemberAccessContext::forInstance(
$type,
$this->visibilityBetween(TypeFactory::className($enclosingClass), $target),
$match['prefix'],
);
return MemberAccessContext::forInstance($type, $visibility, $match['prefix']);
}

if ($match['kind'] === 'instance') {
Expand Down Expand Up @@ -360,18 +376,13 @@ private function resolveVariableAccessWithAst(
return null;
}

$target = $type->getResolvableClassNames()[0] ?? null;
if ($target === null) {
return null;
}
$enclosingClassName = $scope->getSelfContext();
$vantage = $enclosingClassName !== null ? TypeFactory::className($enclosingClassName) : null;

return MemberAccessContext::forInstance(
$type,
$this->visibilityBetween($vantage, $target),
$match['prefix'],
);
$visibility = $this->visibilityForReceiver($vantage, $type);
if ($visibility === null) {
return null;
}
return MemberAccessContext::forInstance($type, $visibility, $match['prefix']);
}

/**
Expand Down
36 changes: 36 additions & 0 deletions tests/Fixtures/src/Intersection/IntersectionReceiver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

declare(strict_types=1);

namespace Fixtures\Intersection;

use Fixtures\Domain\Entity;
use Fixtures\Domain\Person;

/**
* Intersection receiver where each constituent declares a distinct member:
* Entity contributes getId(), Person contributes getName(). A value of the
* intersection satisfies both, so either member must resolve.
*/
class IntersectionReceiver
{
public function triggerPersonMember(Entity&Person $value): void
{
$value->getName(); //hover:intersection_person_member
}

public function triggerEntityMember(Entity&Person $value): void
{
$value->getId(); //hover:intersection_entity_member
}

public function completePersonMember(Entity&Person $value): void
{
$value->/*|intersection_completion*/
}

public function signaturePersonMember(Entity&Person $value): void
{
$value->getName(/*|intersection_signature*/);
}
}
36 changes: 36 additions & 0 deletions tests/Fixtures/src/Union/UnionReceiver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

declare(strict_types=1);

namespace Fixtures\Union;

use Fixtures\Domain\Entity;
use Fixtures\Domain\Person;

/**
* Union receiver where each constituent declares a distinct member:
* Entity contributes getId(), Person contributes getName(). Either member
* must resolve regardless of constituent order.
*/
class UnionReceiver
{
public function triggerPersonMember(Entity|Person $value): void
{
$value->getName(); //hover:union_person_member
}

public function triggerEntityMember(Entity|Person $value): void
{
$value->getId(); //hover:union_entity_member
}

public function completePersonMember(Entity|Person $value): void
{
$value->/*|union_completion*/
}

public function signaturePersonMember(Entity|Person $value): void
{
$value->getName(/*|union_signature*/);
}
}
Loading