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
40 changes: 23 additions & 17 deletions docs/architecture/build-manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,18 @@ re-runs repo-wide as its completion gate.
S4.6 4 SymbolResolver -> glue; CodeResolver positional S4.2,S4.3,S4.4,S4.5,S4.8 —
S4.7 4 Step 4 duplication audit all Step 4 —
SC.1 — Delete the dead WorkspaceIndexer — —
SC.3 — Namespace tracking -> the parser's namespacedName — —
SC.3 — SymbolExtractor reads DeclarationScanner — —
SC.4 — Dedupe the hand-rolled file:// conversion — —
SC.7 — Six member-hierarchy walks -> one — —
SC.8 — Prefix matching: SymbolIndex -> PrefixMatcher — —
SC.10 — Enforce the one-declaration-scanner rule SC.3,SC.9 —
SC.12 — Move MemberFilter out of Resolution — —
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 —
SC.19 — Own the four unowned baseline entries — —
SZ.1 Z Definition of Done gate + repo-wide dup audit all prior —

Notes:
Expand Down Expand Up @@ -225,12 +225,21 @@ Notes:
map, so a class and a function sharing a short name collide — and the constant table it
also folds in is exactly what constant resolution will read. `NameContext::importsFor()`
already keeps the three tables apart, so no caller needs Step 4 to move.
- **SC.3** — `SymbolExtractor` hand-tracks
`Stmt\Namespace_` to build FQNs that `NameResolver` already computed into
`namespacedName` (which `DefaultClassInfoFactory`, `DefaultFunctionRepository`,
`ScopeFinder`, and `DeclarationScanner` all read). Behavior-preserving, so the Step P
write-path and class-like-lookup goldens prove it. `SymbolExtractor`'s `Class::method`
FQNs are its own and stay.
- **SC.3** — `SymbolExtractor` is the last hand-written declaration traversal in `src/`:
it walks the AST itself and hand-tracks `Stmt\Namespace_` to rebuild FQNs that
`NameResolver` already computed into `namespacedName`. It reads `DeclarationScanner`
instead, so no consumer can disagree about what a file declares. Its `Class::method`
symbols are **not** an obstacle to that, as this row long claimed: methods come off
each class-like's own node via `ClassLike::getMethods()`, which the scanner already
hands back, so no visitor is needed for them either. Behavior-preserving, so the Step P
write-path and prefix-search goldens prove it.

The confinement this satisfies is **already enforced** — `phpstan.neon` restricts
`NodeTraverser` / `NodeFinder` / `NodeVisitor*` to `ParserService`, the positional
finders, and `DeclarationScanner`. `SymbolExtractor`'s violations were merely frozen in
`phpstan-baseline.neon`, which is why nothing failed while they stood. Do not file a
slice to build that rule; the remaining frozen violations belong to S4.2
(`SymbolResolver`) and S4.8 (`BasicTypeResolver`).
- **SC.4** — `file://` URI and path conversion is hand-rolled in four live places
(`DefaultClassInfoFactory`, `FilesystemBackend` ×2, `Location`, and the dead
`WorkspaceIndexer`), each differing in how it handles the scheme and percent-
Expand Down Expand Up @@ -324,15 +333,12 @@ Notes:
more copies. Outside Step 4's scope — that step decomposes `src/Resolution/`, this is
`src/Repository/`.
The member-name case rule rides along: the collect keys (`strtolower`), `MethodName::equals` (`strcasecmp`), and the fallback's raw merge key disagree today, and the walks' seen-sets key raw FQNs where `ClassName::equals` is case-insensitive.
- **SC.10** — SC.5 states a hard invariant ("Do NOT write a new one; a rule about what
counts as a declaration is a change to the scanner") with no mechanism, which §8.1
forbids where a static rule or test is feasible. One is: a PHPStan rule confining
`NodeVisitorAbstract` / `NodeFinder` / `NodeTraverser` to the parser, the positional
finders, and `DeclarationScanner`, in the shape of the existing
`SymbolDiscoveryAuthorityExtension`. Gated on its two known violators (SC.3's
`SymbolExtractor`, SC.9's `classesIn`) because a rule that must ship with two
exemptions enforces nothing. The analogue to follow is `TypeGraphParityTest`, which is
how the sibling single-traversal invariant on `supertypes()` is held.
- **SC.19** — four `phpstan-baseline.neon` entries that no row above drains:
`ReferenceResolver`'s `strcasecmp`, and the `preg_match` calls in `CompletionHandler`
and `NamedArgumentCandidates`. The case fold belongs to `NameKind`; the two regexes
belong in `CompletionClassifier`, which is where the allowlist already puts
text-pattern analysis. Filed because the baseline must reach zero and an entry with no
owning slice is how it stalls — found by auditing the baseline against this table.
- **SC.8** — `Completion\PrefixMatcher::matches` and `SymbolIndex::findByPrefix` both
hand-roll `str_starts_with(strtolower(...))`. SC.6 owns the `strtolower` half (it is
the same per-kind case rule); what is left here is the duplicated *matching* helper, so
Expand Down
18 changes: 0 additions & 18 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,6 @@ parameters:
count: 2
path: src/Handler/CompletionHandler.php

-
message: '#^Class PhpParser\\NodeTraverser is forbidden, AST traversal is confined\: parser pipeline \(ParserService\), positional finders \(NodeAtPosition, Scope, ScopeFinder\), and DeclarationScanner\.$#'
identifier: disallowed.class
count: 1
path: src/Index/SymbolExtractor.php

-
message: '#^Namespace PhpParser\\NodeTraverser is forbidden, AST traversal is confined\: parser pipeline \(ParserService\), positional finders \(NodeAtPosition, Scope, ScopeFinder\), and DeclarationScanner\.$#'
identifier: disallowed.namespace
count: 1
path: src/Index/SymbolExtractor.php

-
message: '#^Namespace PhpParser\\NodeVisitorAbstract is forbidden, AST traversal is confined\: parser pipeline \(ParserService\), positional finders \(NodeAtPosition, Scope, ScopeFinder\), and DeclarationScanner\. \[PhpParser\\NodeVisitorAbstract matches PhpParser\\NodeVisitor\*\]$#'
identifier: disallowed.namespace
count: 2
path: src/Index/SymbolExtractor.php

-
message: '#^Calling strtolower\(\) is forbidden, case rules for symbol names live in NameKind; non\-symbol uses may be allowlisted here consciously\.$#'
identifier: disallowed.function
Expand Down
163 changes: 52 additions & 111 deletions src/Index/SymbolExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,144 +7,85 @@
use Firehed\PhpLsp\Document\TextDocument;
use PhpParser\Node;
use PhpParser\Node\Stmt;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitorAbstract;

final class SymbolExtractor extends NodeVisitorAbstract
final class SymbolExtractor
{
/** @var list<Symbol> */
private array $symbols = [];
private string $uri = '';
private string $namespace = '';
private ?string $currentClass = null;
private TextDocument $document;
public function __construct(
private readonly DeclarationScanner $declarations = new DeclarationScanner(),
) {
}

/**
* @param array<Stmt> $ast
* @return list<Symbol>
*/
public function extract(TextDocument $document, array $ast): array
{
$this->symbols = [];
$this->uri = $document->uri;
$this->namespace = '';
$this->currentClass = null;
$this->document = $document;

$traverser = new NodeTraverser();
$traverser->addVisitor($this);
$traverser->traverse($ast);

return $this->symbols;
}

public function enterNode(Node $node): null
{
if ($node instanceof Stmt\Namespace_) {
$this->namespace = $node->name?->toString() ?? '';
return null;
}
$declarations = $this->declarations->scan($ast);
$symbols = [];

if ($node instanceof Stmt\Class_) {
$this->addClassLikeSymbol($node, SymbolKind::Class_);
$this->currentClass = $node->name?->toString();
return null;
}

if ($node instanceof Stmt\Interface_) {
$this->addClassLikeSymbol($node, SymbolKind::Interface_);
$this->currentClass = $node->name?->toString();
return null;
}

if ($node instanceof Stmt\Trait_) {
$this->addClassLikeSymbol($node, SymbolKind::Trait_);
$this->currentClass = $node->name?->toString();
return null;
}
foreach ($declarations->classLikes as $declaration) {
$fqn = $declaration->name->fullyQualifiedName();
$symbols[] = new Symbol(
name: $declaration->name->shortName,
fullyQualifiedName: $fqn,
kind: self::kindOf($declaration->node),
location: self::locate($document, $declaration->node),
);

if ($node instanceof Stmt\Enum_) {
$this->addClassLikeSymbol($node, SymbolKind::Enum_);
$this->currentClass = $node->name?->toString();
return null;
foreach ($declaration->node->getMethods() as $method) {
$name = $method->name->toString();
$symbols[] = new Symbol(
name: $name,
fullyQualifiedName: $fqn . '::' . $name,
kind: SymbolKind::Method,
location: self::locate($document, $method),
containerName: $declaration->name->shortName,
);
}
}

if ($node instanceof Stmt\Function_) {
$name = $node->name->toString();
$fqn = $this->namespace !== '' ? $this->namespace . '\\' . $name : $name;
$this->symbols[] = new Symbol(
name: $name,
fullyQualifiedName: $fqn,
foreach ($declarations->functions as $declaration) {
$symbols[] = new Symbol(
name: $declaration->name->shortName,
fullyQualifiedName: $declaration->name->fullyQualifiedName(),
kind: SymbolKind::Function_,
location: $this->createLocation($node),
location: self::locate($document, $declaration->node),
);
return null;
}

if ($node instanceof Stmt\ClassMethod && $this->currentClass !== null) {
$name = $node->name->toString();
$fqn = ($this->namespace !== '' ? $this->namespace . '\\' : '')
. $this->currentClass . '::' . $name;
$this->symbols[] = new Symbol(
name: $name,
fullyQualifiedName: $fqn,
kind: SymbolKind::Method,
location: $this->createLocation($node),
containerName: $this->currentClass,
);
return null;
}
// Callers index `$symbols[0]` as the file's first declaration, which the two
// separate scanner lists would otherwise interleave by kind rather than by
// where each is written.
usort($symbols, self::byPosition(...));

return null;
return $symbols;
}

public function leaveNode(Node $node): null
private static function byPosition(Symbol $a, Symbol $b): int
{
if (
$node instanceof Stmt\Class_
|| $node instanceof Stmt\Interface_
|| $node instanceof Stmt\Trait_
|| $node instanceof Stmt\Enum_
) {
$this->currentClass = null;
}

return null;
return [$a->location->startLine, $a->location->startCharacter]
<=> [$b->location->startLine, $b->location->startCharacter];
}

private function addClassLikeSymbol(
Stmt\Class_|Stmt\Interface_|Stmt\Trait_|Stmt\Enum_ $node,
SymbolKind $kind,
): void {
$name = $node->name?->toString();
if ($name === null) {
return; // Anonymous class
}

$fqn = $this->namespace !== '' ? $this->namespace . '\\' . $name : $name;
$this->symbols[] = new Symbol(
name: $name,
fullyQualifiedName: $fqn,
kind: $kind,
location: $this->createLocation($node),
);
private static function kindOf(Stmt\ClassLike $node): SymbolKind
{
return match (true) {
$node instanceof Stmt\Enum_ => SymbolKind::Enum_,
$node instanceof Stmt\Interface_ => SymbolKind::Interface_,
$node instanceof Stmt\Trait_ => SymbolKind::Trait_,
default => SymbolKind::Class_,
};
}

private function createLocation(Node $node): Location
private static function locate(TextDocument $document, Node $node): Location
{
$startLine = $node->getStartLine() - 1; // LSP is 0-indexed
$endLine = $node->getEndLine() - 1;

// Get character positions from the document
$startPos = $this->document->positionAt($node->getStartFilePos());
$endPos = $this->document->positionAt($node->getEndFilePos() + 1);

return new Location(
uri: $this->uri,
startLine: $startLine,
startCharacter: $startPos['character'],
endLine: $endLine,
endCharacter: $endPos['character'],
uri: $document->uri,
startLine: $node->getStartLine() - 1, // LSP is 0-indexed
startCharacter: $document->positionAt($node->getStartFilePos())['character'],
endLine: $node->getEndLine() - 1,
endCharacter: $document->positionAt($node->getEndFilePos() + 1)['character'],
);
}
}