From 42b0cce878011605a417513a44ae0ca297943c11 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:04:14 +0000 Subject: [PATCH 01/26] build(deps): bump xphp-lang/xphp to 0.3.x-dev Track the upstream v0.3.0 compiler release (branch 0.3.x) so the language server can build against the new generic/variance/closure-signature surface. Pulls symfony/console v8.x transitively. Co-Authored-By: Claude Opus 4.8 (1M context) --- composer.json | 2 +- composer.lock | 145 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 107 insertions(+), 40 deletions(-) diff --git a/composer.json b/composer.json index f743540..bdc7f9d 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "phpactor/language-server": "^6.0", "phpactor/language-server-protocol": "^3.5", "phpactor/worse-reflection": "^0.6.0", - "xphp-lang/xphp": "^v0.2.0" + "xphp-lang/xphp": "0.3.x-dev" }, "require-dev": { "phpunit/phpunit": "^13.0" diff --git a/composer.lock b/composer.lock index 4194c30..61a4eb8 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "b218c60354ec016b8246b0e1c535dcdf", + "content-hash": "4d60984500e0e8de35fd334981745ac4", "packages": [ { "name": "amphp/amp", @@ -1086,20 +1086,19 @@ }, { "name": "nikic/php-parser", - "version": "v5.7.0", + "version": "v5.8.0", "source": { "type": "git", "url": "https://github.com/nikic/PHP-Parser.git", - "reference": "dca41cd15c2ac9d055ad70dbfd011130757d1f82" + "reference": "044a6a392ff8ad0d61f14370a5fbbd0a0107152f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/dca41cd15c2ac9d055ad70dbfd011130757d1f82", - "reference": "dca41cd15c2ac9d055ad70dbfd011130757d1f82", + "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/044a6a392ff8ad0d61f14370a5fbbd0a0107152f", + "reference": "044a6a392ff8ad0d61f14370a5fbbd0a0107152f", "shasum": "" }, "require": { - "ext-ctype": "*", "ext-json": "*", "ext-tokenizer": "*", "php": ">=7.4" @@ -1138,9 +1137,9 @@ ], "support": { "issues": "https://github.com/nikic/PHP-Parser/issues", - "source": "https://github.com/nikic/PHP-Parser/tree/v5.7.0" + "source": "https://github.com/nikic/PHP-Parser/tree/v5.8.0" }, - "time": "2025-12-06T11:56:16+00:00" + "time": "2026-07-04T14:30:18+00:00" }, { "name": "phpactor/docblock-parser", @@ -1788,16 +1787,16 @@ }, { "name": "symfony/console", - "version": "v8.1.0", + "version": "v8.1.1", "source": { "type": "git", "url": "https://github.com/symfony/console.git", - "reference": "f5a856c6ecb56b3c21ed94a5b7bf940d857d110a" + "reference": "b711a8ab808b6c074c6b8caef70d0fd8d6b6d07d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/console/zipball/f5a856c6ecb56b3c21ed94a5b7bf940d857d110a", - "reference": "f5a856c6ecb56b3c21ed94a5b7bf940d857d110a", + "url": "https://api.github.com/repos/symfony/console/zipball/b711a8ab808b6c074c6b8caef70d0fd8d6b6d07d", + "reference": "b711a8ab808b6c074c6b8caef70d0fd8d6b6d07d", "shasum": "" }, "require": { @@ -1864,7 +1863,7 @@ "terminal" ], "support": { - "source": "https://github.com/symfony/console/tree/v8.1.0" + "source": "https://github.com/symfony/console/tree/v8.1.1" }, "funding": [ { @@ -1884,20 +1883,20 @@ "type": "tidelift" } ], - "time": "2026-05-29T05:06:50+00:00" + "time": "2026-06-16T12:55:20+00:00" }, { "name": "symfony/deprecation-contracts", - "version": "v3.7.0", + "version": "v3.7.1", "source": { "type": "git", "url": "https://github.com/symfony/deprecation-contracts.git", - "reference": "50f59d1f3ca46d41ac911f97a78626b6756af35b" + "reference": "f3202fa1b5097b0af062dc978b32ecf63404e31d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/50f59d1f3ca46d41ac911f97a78626b6756af35b", - "reference": "50f59d1f3ca46d41ac911f97a78626b6756af35b", + "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/f3202fa1b5097b0af062dc978b32ecf63404e31d", + "reference": "f3202fa1b5097b0af062dc978b32ecf63404e31d", "shasum": "" }, "require": { @@ -1935,7 +1934,7 @@ "description": "A generic function and convention to trigger deprecation notices", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/deprecation-contracts/tree/v3.7.0" + "source": "https://github.com/symfony/deprecation-contracts/tree/v3.7.1" }, "funding": [ { @@ -1955,7 +1954,7 @@ "type": "tidelift" } ], - "time": "2026-04-13T15:52:40+00:00" + "time": "2026-06-05T06:23:12+00:00" }, { "name": "symfony/filesystem", @@ -2279,16 +2278,16 @@ }, { "name": "symfony/polyfill-mbstring", - "version": "v1.38.1", + "version": "v1.38.2", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-mbstring.git", - "reference": "14c5439eec4ccff081ac14eca2dc57feb2a66d92" + "reference": "d3d318bad5e7a1bfbd026009c8bfb8d8f99ae6b6" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/14c5439eec4ccff081ac14eca2dc57feb2a66d92", - "reference": "14c5439eec4ccff081ac14eca2dc57feb2a66d92", + "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/d3d318bad5e7a1bfbd026009c8bfb8d8f99ae6b6", + "reference": "d3d318bad5e7a1bfbd026009c8bfb8d8f99ae6b6", "shasum": "" }, "require": { @@ -2340,7 +2339,7 @@ "shim" ], "support": { - "source": "https://github.com/symfony/polyfill-mbstring/tree/v1.38.1" + "source": "https://github.com/symfony/polyfill-mbstring/tree/v1.38.2" }, "funding": [ { @@ -2360,7 +2359,7 @@ "type": "tidelift" } ], - "time": "2026-05-26T12:51:13+00:00" + "time": "2026-05-27T06:59:30+00:00" }, { "name": "symfony/polyfill-php85", @@ -2442,18 +2441,83 @@ ], "time": "2026-05-26T02:25:22+00:00" }, + { + "name": "symfony/process", + "version": "v8.1.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/process.git", + "reference": "c4a9e58f235a6bf7f97ffbfedae2687353ac79e5" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/process/zipball/c4a9e58f235a6bf7f97ffbfedae2687353ac79e5", + "reference": "c4a9e58f235a6bf7f97ffbfedae2687353ac79e5", + "shasum": "" + }, + "require": { + "php": ">=8.4.1" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Process\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Executes commands in sub-processes", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/process/tree/v8.1.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-05-29T05:06:50+00:00" + }, { "name": "symfony/service-contracts", - "version": "v3.7.0", + "version": "v3.7.1", "source": { "type": "git", "url": "https://github.com/symfony/service-contracts.git", - "reference": "d25d82433a80eba6aa0e6c24b61d7370d99e444a" + "reference": "c0a284bab1ed8aa0417e3d69250ab437739563a0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/service-contracts/zipball/d25d82433a80eba6aa0e6c24b61d7370d99e444a", - "reference": "d25d82433a80eba6aa0e6c24b61d7370d99e444a", + "url": "https://api.github.com/repos/symfony/service-contracts/zipball/c0a284bab1ed8aa0417e3d69250ab437739563a0", + "reference": "c0a284bab1ed8aa0417e3d69250ab437739563a0", "shasum": "" }, "require": { @@ -2507,7 +2571,7 @@ "standards" ], "support": { - "source": "https://github.com/symfony/service-contracts/tree/v3.7.0" + "source": "https://github.com/symfony/service-contracts/tree/v3.7.1" }, "funding": [ { @@ -2527,7 +2591,7 @@ "type": "tidelift" } ], - "time": "2026-03-28T09:44:51+00:00" + "time": "2026-06-16T09:55:08+00:00" }, { "name": "symfony/string", @@ -2760,22 +2824,23 @@ }, { "name": "xphp-lang/xphp", - "version": "v0.2.0", + "version": "0.3.x-dev", "source": { "type": "git", "url": "https://github.com/xphp-lang/xphp.git", - "reference": "85ff0909c61ac02ba273eceaef35d594238e8fcf" + "reference": "db66de1f4ff73e728c1a8cd708a1762cfec04d4e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/xphp-lang/xphp/zipball/85ff0909c61ac02ba273eceaef35d594238e8fcf", - "reference": "85ff0909c61ac02ba273eceaef35d594238e8fcf", + "url": "https://api.github.com/repos/xphp-lang/xphp/zipball/db66de1f4ff73e728c1a8cd708a1762cfec04d4e", + "reference": "db66de1f4ff73e728c1a8cd708a1762cfec04d4e", "shasum": "" }, "require": { "nikic/php-parser": "^5.7", "php": "^8.4.0", - "symfony/console": "^8.0" + "symfony/console": "^8.0", + "symfony/process": "^8.0" }, "require-dev": { "infection/infection": "^0.33", @@ -2815,7 +2880,7 @@ "issues": "https://github.com/xphp-lang/xphp/issues", "source": "https://github.com/xphp-lang/xphp" }, - "time": "2026-06-11T22:34:01+00:00" + "time": "2026-07-14T20:20:15+00:00" } ], "packages-dev": [ @@ -4614,7 +4679,9 @@ ], "aliases": [], "minimum-stability": "dev", - "stability-flags": {}, + "stability-flags": { + "xphp-lang/xphp": 20 + }, "prefer-stable": true, "prefer-lowest": false, "platform": { From 76805bc84074354e10a69ce28b6fccf5728b79a8 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:07:23 +0000 Subject: [PATCH 02/26] feat(hover): render type-parameter variance as out/in xphp 0.3.0 replaces the +T/-T variance glyphs with the Kotlin-style `out`/`in` contextual keywords. The Variance enum is unchanged, so only the rendered marker text moves: covariant shows `out T`, contravariant shows `in T`. Migrates the variance hover tests, the behat scenario, and the feature docs. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 2 +- docs/roadmap.md | 2 +- features/understand/hover.feature | 4 ++-- src/Handler/XphpHoverHandler.php | 11 +++++++---- test/Handler/XphpHoverHandlerTest.php | 12 ++++++------ 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/docs/features/index.md b/docs/features/index.md index 9d0e6d4..f48dd53 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -201,7 +201,7 @@ property / native function info, hover renders: - A type parameter's full upper bound, including composite forms -- intersection (`A & B`), union (`A | B`), and F-bounded (`Comparable`). -- A type parameter's variance: `+T` (covariant) / `-T` (contravariant) +- A type parameter's variance: `out T` (covariant) / `in T` (contravariant) are shown with their marker and a label; invariant params show the bare name. diff --git a/docs/roadmap.md b/docs/roadmap.md index 530937a..ec5395d 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -71,7 +71,7 @@ test suite; full descriptions to fold into [`README.md`](../README.md#features)) of a union). Default type arguments (`class Box`, `class Pair`) may be omitted at a call site without a false "missing type argument", with the effective type substituted into parameter - checks. Variance markers (`+T` covariant, `-T` contravariant) are shown in + checks. Variance markers (`out T` covariant, `in T` contravariant) are shown in hover. Instance-method turbofish (`$obj->m::(...)`) binds its type argument for argument checking; variable turbofish over an unknown callee is conservatively skipped. Generic closures and arrows (`fn(…)`, diff --git a/features/understand/hover.feature b/features/understand/hover.feature index 7b1a513..01b4594 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -51,14 +51,14 @@ Feature: Hover """ + class Producer { public function get(): T {} } """ And the FQN index has been warmed on initialize When I request "textDocument/hover" on "T" at line 4 of "/producer.xphp" - Then the hover contents contain "`+T`" + Then the hover contents contain "`out T`" And the hover contents contain "covariant" Scenario: Hover over a generic method turbofish call shows the specialized signature diff --git a/src/Handler/XphpHoverHandler.php b/src/Handler/XphpHoverHandler.php index 51ddb8a..3303dc2 100644 --- a/src/Handler/XphpHoverHandler.php +++ b/src/Handler/XphpHoverHandler.php @@ -225,16 +225,19 @@ private function buildHoverMarkdown(\PhpParser\Node\Name $name, array $classScop /** * Render a type parameter's display name and a human variance note from its - * `Variance`: covariant `+T` / contravariant `-T` get the marker prefix and - * a parenthetical; invariant stays the bare name with no note. + * `Variance`: covariant `out T` / contravariant `in T` get the marker prefix + * and a parenthetical; invariant stays the bare name with no note. xphp 0.3.0 + * changed the source spelling from `+T`/`-T` to the `out`/`in` contextual + * keywords; the `Variance` enum cases are unchanged, so only the rendered + * marker text moves here. * * @return array{0: string, 1: string} [displayName, varianceNote] */ private static function varianceLabel(TypeParam $param): array { return match ($param->variance) { - Variance::Covariant => ['+' . $param->name, ' (covariant)'], - Variance::Contravariant => ['-' . $param->name, ' (contravariant)'], + Variance::Covariant => ['out ' . $param->name, ' (covariant)'], + Variance::Contravariant => ['in ' . $param->name, ' (contravariant)'], Variance::Invariant => [$param->name, ''], }; } diff --git a/test/Handler/XphpHoverHandlerTest.php b/test/Handler/XphpHoverHandlerTest.php index 80b7560..d209da5 100644 --- a/test/Handler/XphpHoverHandlerTest.php +++ b/test/Handler/XphpHoverHandlerTest.php @@ -80,11 +80,11 @@ class Pair public function testHoverShowsCovariantMarker(): void { - // Covariant `+T` is only allowed in output (return) positions. + // Covariant `out T` is only allowed in output (return) positions. [$handler, $workspace, $uri] = $this->prepare(<<<'XPHP' + class Producer { public function get(): T { } } @@ -93,17 +93,17 @@ public function get(): T { } self::assertInstanceOf(Hover::class, $hover); $text = $hover->contents->value; - self::assertStringContainsString('`+T`', $text); + self::assertStringContainsString('`out T`', $text); self::assertStringContainsString('covariant', $text); } public function testHoverShowsContravariantMarker(): void { - // Contravariant `-T` is only allowed in input (parameter) positions. + // Contravariant `in T` is only allowed in input (parameter) positions. [$handler, $workspace, $uri] = $this->prepare(<<<'XPHP' + class Consumer { public function put(T $item): void { } } @@ -112,7 +112,7 @@ public function put(T $item): void { } self::assertInstanceOf(Hover::class, $hover); $text = $hover->contents->value; - self::assertStringContainsString('`-T`', $text); + self::assertStringContainsString('`in T`', $text); self::assertStringContainsString('contravariant', $text); } From 056bc8190adeb1a8f205f11406f1c777f6d45f0f Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:09:55 +0000 Subject: [PATCH 03/26] test(resolver): lock Integer/Boolean/Double as class names xphp 0.3.0 stopped aliasing Integer/Boolean/Double to scalars; they now resolve as class names. The LSP already treated them as class atoms (SCALAR_TYPES holds only lowercase canonical types, and isClassAtom's reserved list never included the capitalized spellings), so no production change is needed. Pin the behavior so a future re-aliasing can't silently regress it. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Resolver/TypeUnionSplitterTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/Resolver/TypeUnionSplitterTest.php b/test/Resolver/TypeUnionSplitterTest.php index 9cb10fe..f7de04a 100644 --- a/test/Resolver/TypeUnionSplitterTest.php +++ b/test/Resolver/TypeUnionSplitterTest.php @@ -42,6 +42,15 @@ public static function cases(): iterable yield 'self/parent dropped' => ['self|parent', []]; yield 'bool keyword dropped' => ['true|false', []]; + // xphp 0.3.0 stopped aliasing Integer/Boolean/Double to scalars -- + // they now resolve as CLASS names. The splitter treats the + // capitalized spellings as class atoms (the lowercase canonical + // scalars int/bool/float are still dropped, above). + yield 'Integer is a class not a scalar' => ['Integer', [['Integer']]]; + yield 'Boolean is a class not a scalar' => ['Boolean', [['Boolean']]]; + yield 'Double is a class not a scalar' => ['Double', [['Double']]]; + yield 'capitalized alias in intersection' => ['Integer&App\\Comparable', [['Integer', 'App\\Comparable']]]; + // Dedup: same FQN repeated in an intersection collapses to // one entry. yield 'dedup within intersection' => ['A&A&B', [['A', 'B']]]; From c12d49d72b67eaf5f7ba08c3a77d2f45eceb7408 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:19:52 +0000 Subject: [PATCH 04/26] feat(diagnostics): map xphp 0.3.0 generic-validation codes Add the eight 0.3.0 generic-validation codes to the DiagnosticCode vocabulary and triage the ones Registry::recordInstantiation actually throws in-process: too-many-type-arguments and missing-type-argument (alongside the existing collision/bound paths). The other codes (bound-unprovable, undeclared-type, closure-conformance, unspecialized-generic-closure, unresolved-generic-call, undetermined-receiver) are appended to the compiler's diagnostics collector or raised on the Compiler path, so they are vocabulary only until that sink is wired; they are not triaged here. Each triaged phrase is locked by a unit test. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Analyzer/DiagnosticCode.php | 76 ++++++++++++++++++++++++---- test/Analyzer/DiagnosticCodeTest.php | 54 ++++++++++++++++++++ 2 files changed, 120 insertions(+), 10 deletions(-) diff --git a/src/Analyzer/DiagnosticCode.php b/src/Analyzer/DiagnosticCode.php index 2abb02b..eec9852 100644 --- a/src/Analyzer/DiagnosticCode.php +++ b/src/Analyzer/DiagnosticCode.php @@ -109,20 +109,76 @@ enum DiagnosticCode: string */ case NullDeref = 'xphp.null-deref'; + // --------------------------------------------------------------------- + // xphp 0.3.0 generic-validation codes. + // + // The compiler's 0.3.0 `check` gate introduced a structured diagnostic + // vocabulary. The LSP keeps computing diagnostics IN-PROCESS (it does not + // shell out to `xphp check`), so a code is only *emitted* today if its + // error reaches us as a `RuntimeException` from `Registry::recordInstantiation` + // with no diagnostics-collector set. Verified against the vendor Registry: + // only the two codes marked "(in-process)" below are actually emitted today + // (too-many / missing type arguments). The rest are part of the vocabulary + // for editor pattern-matching but only surface once the compiler's optional + // diagnostics sink is wired in — a `Registry` built with a collector routes + // variance-unprovable / bound-default / undefined-template through *appended* + // diagnostics rather than throws (see DEFERRED). + // --------------------------------------------------------------------- + + /** Too many type arguments for a generic template's parameter list. (in-process) */ + case TooManyTypeArguments = 'xphp.too_many_type_arguments'; + + /** A required (no-default) type parameter was left unsupplied. (in-process) */ + case MissingTypeArgument = 'xphp.missing_type_argument'; + + /** A member names a type that is not declared/imported (compiler `UndeclaredTypeParameterValidator`). Sink-only. */ + case UndeclaredType = 'xphp.undeclared_type'; + + /** A variance edge could not be proven while instantiating a variant generic. Sink-only. */ + case BoundUnprovable = 'xphp.bound_unprovable'; + + /** A `Closure(...)` signature type failed conformance (params/return/by-ref/arity). */ + case ClosureConformance = 'xphp.closure_conformance'; + + /** A generic closure/arrow could not be specialized. */ + case UnspecializedGenericClosure = 'xphp.unspecialized_generic_closure'; + + /** A generic call could not be resolved (e.g. turbofish-less generic call). */ + case UnresolvedGenericCall = 'xphp.unresolved_generic_call'; + + /** A dynamically-named turbofish receiver could not be determined statically. */ + case UndeterminedReceiver = 'xphp.undetermined_receiver'; + /** * Map a RuntimeException raised by Registry::recordInstantiation to its - * diagnostic code. The Registry doesn't (currently) use a typed exception - * hierarchy, so we triage by the error message's leading phrase. The - * Registry's error builders (Registry::collisionMessage, - * Registry::validateBounds) use stable prefixes documented in their - * docblocks — if those phrasings shift, this triage breaks and the bound - * fallback kicks in. + * diagnostic code. The Registry doesn't use a typed exception hierarchy, so + * we triage by a stable distinctive phrase in the message. 0.3.0 embeds the + * (dynamic) template name near the start of most messages, so we match on + * interior phrases with `str_contains` rather than a leading prefix. If a + * builder's phrasing shifts, that code is lost and the bound fallback kicks + * in — each phrase is locked by a unit test (`DiagnosticCodeTest`). + * + * Only the builders that `recordInstantiation` actually THROWS (with no + * diagnostics-collector set, which is how `WorkspaceAnalyzer` builds the + * Registry) are triaged here — verified against vendor `Registry.php`: + * collisionMessage (:170), tooManyTypeArgumentsMessage (:350), + * missingTypeArgumentMessage (:370), boundViolationMessage (:819), and + * defaultBoundViolationMessage (:484, → bound). The variance-unprovable and + * undefined-template messages are appended to a collector / thrown only on + * the Compiler path, so they never reach this catch and are not triaged. */ public static function fromRegistryRecordInstantiationException(RuntimeException $e): self { - if (str_starts_with($e->getMessage(), 'Hash collision')) { - return self::HashCollision; - } - return self::BoundViolation; + $message = $e->getMessage(); + + return match (true) { + str_starts_with($message, 'Hash collision') => self::HashCollision, + str_contains($message, 'remove the extra argument') => self::TooManyTypeArguments, + str_contains($message, 'has no default; supply it') => self::MissingTypeArgument, + // "Generic bound violated while instantiating …" and "Default for + // generic parameter `…` … violates the parameter's bound" are both + // ordinary bound violations (the existing xphp.bound surface). + default => self::BoundViolation, + }; } } diff --git a/test/Analyzer/DiagnosticCodeTest.php b/test/Analyzer/DiagnosticCodeTest.php index 401002f..cfe6243 100644 --- a/test/Analyzer/DiagnosticCodeTest.php +++ b/test/Analyzer/DiagnosticCodeTest.php @@ -78,6 +78,51 @@ public function testUnknownPrefixFallsBackToBoundViolation(): void ); } + public function testTooManyTypeArgumentsMapsToCode(): void + { + // Registry::tooManyTypeArgumentsMessage (0.3.0) — distinctive interior + // phrase "remove the extra argument". + $e = new RuntimeException( + 'Generic template "App\\Box" declares 1 type parameter(s) but was ' + . 'instantiated with 2 type argument(s); remove the extra argument(s).' + ); + + self::assertSame( + DiagnosticCode::TooManyTypeArguments, + DiagnosticCode::fromRegistryRecordInstantiationException($e), + ); + } + + public function testMissingTypeArgumentMapsToCode(): void + { + // Registry::missingTypeArgumentMessage (0.3.0) — "has no default; supply it". + $e = new RuntimeException( + 'Generic template "App\\Box" was instantiated with 0 type argument(s) ' + . 'but parameter `T` (position 1) has no default; supply it explicitly ' + . 'or add defaults to every preceding required parameter.' + ); + + self::assertSame( + DiagnosticCode::MissingTypeArgument, + DiagnosticCode::fromRegistryRecordInstantiationException($e), + ); + } + + public function testDefaultBoundViolationFallsBackToBoundViolation(): void + { + // Registry::defaultBoundViolationMessage — a default that violates its + // param's bound is still an ordinary bound violation (xphp.bound). + $e = new RuntimeException( + "Default for generic parameter `T` of \"App\\Box\" violates the parameter's bound.\n" + . " bound: Stringable\n default: int\n reason: int is not Stringable" + ); + + self::assertSame( + DiagnosticCode::BoundViolation, + DiagnosticCode::fromRegistryRecordInstantiationException($e), + ); + } + public function testEachCodeMapsToTheLspWireString(): void { // Locks the backed-enum values that the LSP wire format depends on @@ -89,5 +134,14 @@ public function testEachCodeMapsToTheLspWireString(): void self::assertSame('xphp.definition', DiagnosticCode::Definition->value); self::assertSame('xphp.bound', DiagnosticCode::BoundViolation->value); self::assertSame('xphp.collision', DiagnosticCode::HashCollision->value); + // xphp 0.3.0 generic-validation codes. + self::assertSame('xphp.too_many_type_arguments', DiagnosticCode::TooManyTypeArguments->value); + self::assertSame('xphp.missing_type_argument', DiagnosticCode::MissingTypeArgument->value); + self::assertSame('xphp.undeclared_type', DiagnosticCode::UndeclaredType->value); + self::assertSame('xphp.bound_unprovable', DiagnosticCode::BoundUnprovable->value); + self::assertSame('xphp.closure_conformance', DiagnosticCode::ClosureConformance->value); + self::assertSame('xphp.unspecialized_generic_closure', DiagnosticCode::UnspecializedGenericClosure->value); + self::assertSame('xphp.unresolved_generic_call', DiagnosticCode::UnresolvedGenericCall->value); + self::assertSame('xphp.undetermined_receiver', DiagnosticCode::UndeterminedReceiver->value); } } From 734335a308d0593e48a56638f134beef5e885119 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:24:09 +0000 Subject: [PATCH 05/26] fix(resolver): pass Substitution to Specializer::substituteTypeRef xphp 0.3.0 changed Specializer::substituteTypeRef's second parameter from a raw name->TypeRef array to a Substitution value object. Wrap the eight GenericResolver call sites' $paramMap in Substitution::of(), which is the vendor's documented adapter for an already-built map. Without this every generic-substitution path threw a TypeError, breaking hover, completion, inlay hints, and go-to-definition through generics. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Resolver/GenericResolver.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Resolver/GenericResolver.php b/src/Resolver/GenericResolver.php index 99d84b5..011d61a 100644 --- a/src/Resolver/GenericResolver.php +++ b/src/Resolver/GenericResolver.php @@ -35,6 +35,7 @@ use XPHP\Lsp\Analyzer\ParsedDocumentCache; use XPHP\Lsp\Reflection\FqnIndex; use XPHP\Transpiler\Monomorphize\Specializer; +use XPHP\Transpiler\Monomorphize\Substitution; use XPHP\Transpiler\Monomorphize\TypeParam; use XPHP\Transpiler\Monomorphize\TypeRef; use XPHP\Transpiler\Monomorphize\XphpSourceParser; @@ -336,7 +337,7 @@ public function resolveMethodCallSubstitutionAt(string $uri, int $byteOffset): ? if ($tuple !== null) { [$nullable, $ref] = $tuple; $substituted = self::relativeTypeToReceiver($ref, $receiverType) - ?? Specializer::substituteTypeRef($ref, $paramMap); + ?? Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); $returnTypeRendered = (new ResolvedType($substituted, $nullable))->render(); } } @@ -358,7 +359,7 @@ public function resolveMethodCallSubstitutionAt(string $uri, int $byteOffset): ? continue; } [$nullable, $ref] = $tuple; - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); $paramTypes[$name] = (new ResolvedType($substituted, $nullable))->render(); } @@ -904,7 +905,7 @@ private static function buildSubstitutionFromMap( $tuple = self::returnTypeToRef($method->returnType, $paramNames); if ($tuple !== null) { [$nullable, $ref] = $tuple; - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); $returnTypeRendered = (new ResolvedType($substituted, $nullable))->render(); } } @@ -921,7 +922,7 @@ private static function buildSubstitutionFromMap( continue; } [$nullable, $ref] = $tuple; - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); $paramTypes[$name] = (new ResolvedType($substituted, $nullable))->render(); } return new MethodCallSubstitution($returnTypeRendered, $paramTypes); @@ -1564,7 +1565,7 @@ public static function resolveMethodCall( // `static`/`self`/`parent` bind to the receiver's concrete type. $relative = self::relativeTypeToReceiver($ref, $receiverType); $substituted = $relative - ?? ($isGeneric ? Specializer::substituteTypeRef($ref, $paramMap) : $ref); + ?? ($isGeneric ? Specializer::substituteTypeRef($ref, Substitution::of($paramMap)) : $ref); $substituted = self::qualifyTypeRef($substituted, $declaring, $classes); // Gap 2: a non-generic method used to bail to null (defer to @@ -1651,7 +1652,7 @@ public static function resolvePropertyFetch( if ($ref === null) { return null; } - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); // A property typed as a bare class name (`?User`) resolves to an // UNqualified TypeRef (the resolver runs no NameResolver). That renders // fine for a terminal hover, but a chained access @@ -1878,7 +1879,7 @@ public static function resolveStaticCall( if ($ref === null) { return null; } - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); return new ResolvedType($substituted, $nullable); } @@ -1956,7 +1957,7 @@ public static function resolveFuncCall(FuncCall $call, FqnIndex $fqnIndex): ?Res if ($ref === null) { return null; } - $substituted = Specializer::substituteTypeRef($ref, $paramMap); + $substituted = Specializer::substituteTypeRef($ref, Substitution::of($paramMap)); return new ResolvedType($substituted, $nullable); } From 333893dfabb5a94e03855d77303308b7a1bebb92 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 09:30:01 +0000 Subject: [PATCH 06/26] test(0.3.0): migrate scalar + generic-arity fixtures to 0.3.0 behavior Bring the fixtures in line with xphp 0.3.0 and lock the new diagnostics: - completion no longer offers the dropped `integer` scalar alias (only `int`); - `new Box::()` now reports xphp.too_many_type_arguments; - `new Box::<>()` on a required-param template now reports xphp.missing_type_argument (was xphp.bound "no default"). Establishes the don't-break baseline: full unit (1091) and behat (104 scenarios / 653 steps) suites are green against 0.3.0. Co-Authored-By: Claude Opus 4.8 (1M context) --- features/validate/arg-types.feature | 2 +- features/validate/diagnostics.feature | 2 +- test/Handler/XphpCompletionHandlerTest.php | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/features/validate/arg-types.feature b/features/validate/arg-types.feature index e86f3c2..80b63a8 100644 --- a/features/validate/arg-types.feature +++ b/features/validate/arg-types.feature @@ -263,4 +263,4 @@ Feature: Argument-type checking across call shapes """ And the FQN index has been warmed on initialize When I analyze "/Use.xphp" for diagnostics - Then no diagnostics are reported + Then a "xphp.too_many_type_arguments" diagnostic is reported saying "remove the extra" diff --git a/features/validate/diagnostics.feature b/features/validate/diagnostics.feature index 04814fe..c818960 100644 --- a/features/validate/diagnostics.feature +++ b/features/validate/diagnostics.feature @@ -104,7 +104,7 @@ Feature: Diagnostics """ And the FQN index has been warmed on initialize When I analyze "/Use.xphp" for diagnostics - Then a "xphp.bound" diagnostic is reported saying "no default" + Then a "xphp.missing_type_argument" diagnostic is reported saying "no default" Scenario: Report a constructor argument-type mismatch Given the file at "/StringableBox.xphp" contains the following lines: diff --git a/test/Handler/XphpCompletionHandlerTest.php b/test/Handler/XphpCompletionHandlerTest.php index 15b8134..281aff1 100644 --- a/test/Handler/XphpCompletionHandlerTest.php +++ b/test/Handler/XphpCompletionHandlerTest.php @@ -237,9 +237,11 @@ public function testScalarFilteringByPrefix(): void $list = $this->complete($workspace, '/Use.xphp', $useSource, strlen($useSource)); $labels = array_map(static fn (CompletionItem $i): string => $i->label, $list->items); - // Matches: int + integer (prefix); string ("in" is at position 3 → FQN substring). + // Matches: int (prefix); string ("in" is at position 3 → FQN substring). + // xphp 0.3.0 dropped the `integer` scalar alias (SCALAR_TYPES holds only + // canonical `int`), so `integer` is no longer offered. self::assertContains('int', $labels); - self::assertContains('integer', $labels); + self::assertNotContains('integer', $labels); self::assertContains('string', $labels, 'matchesPrefix accepts FQN-substring matches, and "string" contains "in" at offset 3'); // Does NOT match: any scalar with no "in" substring. self::assertNotContains('void', $labels); From 9cae4556d23f8b046541400afe3ada3b219c5f9c Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 10:00:45 +0000 Subject: [PATCH 07/26] feat(resolver): resolve inherited generic methods via subclass turbofish xphp 0.3.0 allows a generic method inherited from a base class to be called with turbofish on a subclass receiver ($child->m::()). Add an extends-chain walk (findMethodWithContext) that locates the inherited method and its declaring context, wired into both the return-type inference and the hover/inlay signature-substitution paths. Parent names resolve against each ancestor's own use-map/namespace; a fully-qualified parent is used as-is. Nullsafe turbofish already resolved (returns a nullable specialized type). First-class-callable value inference is out of scope (see plan DEFERRED). Co-Authored-By: Claude Opus 4.8 (1M context) --- features/understand/hover.feature | 27 ++++++++ src/Resolver/GenericResolver.php | 68 ++++++++++++++++++-- test/Resolver/GenericResolverTest.php | 93 +++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 7 deletions(-) diff --git a/features/understand/hover.feature b/features/understand/hover.feature index 01b4594..890331e 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -82,6 +82,33 @@ Feature: Hover When I request "textDocument/hover" on "identity" at line 3 of "/Use.xphp" Then the hover contents contain "identity(string $x): string" + Scenario: Hover over an inherited generic method turbofish resolves through the base class + Given the file at "/Base.xphp" contains the following lines: + """ + (T $x): T { return $x; } + } + """ + And the file at "/Child.xphp" contains the following lines: + """ + identity::('world'); + """ + And the FQN index has been warmed on initialize + When I request "textDocument/hover" on "identity" at line 3 of "/Use.xphp" + Then the hover contents contain "identity(string $x): string" + Scenario: Hover over a method returning static resolves to the receiver's concrete type Given the file at "/Builder.xphp" contains the following lines: """ diff --git a/src/Resolver/GenericResolver.php b/src/Resolver/GenericResolver.php index 011d61a..5744ce4 100644 --- a/src/Resolver/GenericResolver.php +++ b/src/Resolver/GenericResolver.php @@ -309,17 +309,21 @@ public function resolveMethodCallSubstitutionAt(string $uri, int $byteOffset): ? if ($receiverType === null) { return null; } - $classLike = $this->classes->find($receiverType->ref->name); - if ($classLike === null) { + $declaring = $this->classes->findWithContext($receiverType->ref->name); + if ($declaring === null) { return null; } + $classLike = $declaring->classLike; if (!$call->name instanceof Identifier) { return null; } - $method = self::findMethod($classLike, $call->name->toString()); - if ($method === null) { + // Walk the extends chain so an inherited generic method specializes on a + // subclass receiver (0.3.0 turbofish on a subclass). + $found = self::findMethodWithContext($declaring, $call->name->toString(), $this->classes); + if ($found === null) { return null; } + [$method] = $found; $paramMap = self::paramMapFromReceiver($classLike, $receiverType); $paramMap = self::withMethodTurbofish($paramMap, $call, $method); // No type params in scope = a plain method on a non-generic receiver; @@ -1543,10 +1547,15 @@ public static function resolveMethodCall( return null; } $classLike = $declaring->classLike; - $method = self::findMethod($classLike, $call->name->toString()); - if ($method === null) { + // Walk the extends chain so a generic method inherited from a base class + // resolves on a subclass receiver (0.3.0). `$methodContext` is the + // DECLARING class's context -- used to qualify the return type against + // the file that declared the method, not the subclass's file. + $found = self::findMethodWithContext($declaring, $call->name->toString(), $classes); + if ($found === null) { return null; } + [$method, $methodContext] = $found; $returnType = $method->returnType; if ($returnType === null) { return null; @@ -1566,7 +1575,7 @@ public static function resolveMethodCall( $relative = self::relativeTypeToReceiver($ref, $receiverType); $substituted = $relative ?? ($isGeneric ? Specializer::substituteTypeRef($ref, Substitution::of($paramMap)) : $ref); - $substituted = self::qualifyTypeRef($substituted, $declaring, $classes); + $substituted = self::qualifyTypeRef($substituted, $methodContext, $classes); // Gap 2: a non-generic method used to bail to null (defer to // worse-reflection). Now we OWN it -- so chains continue @@ -1971,6 +1980,51 @@ private static function findMethod(ClassLike $class, string $name): ?ClassMethod return null; } + /** + * Find `$name` on the receiver's class, walking the `extends` chain when the + * receiver's own class doesn't declare it. xphp 0.3.0 allows a generic + * method inherited from a base class to be called with turbofish on a + * subclass receiver (`$child->m::()`); the method's generic params live + * on the DECLARING class, so we return the `ClassMethod` together with that + * class's context (needed to qualify its return type against the file that + * declared it). Parent names are resolved against each class's own declaring + * context; a `seen` set guards against extends cycles. + * + * @return array{0: ClassMethod, 1: ClassLikeContext}|null + */ + private static function findMethodWithContext( + ClassLikeContext $context, + string $name, + ClassLikeLookup $classes, + ): ?array { + $seen = []; + $current = $context; + while ($current !== null) { + $method = self::findMethod($current->classLike, $name); + if ($method !== null) { + return [$method, $current]; + } + $class = $current->classLike; + if (!$class instanceof Node\Stmt\Class_ || $class->extends === null) { + return null; + } + $parent = $class->extends; + // A fully-qualified `extends \App\Base` is absolute; anything else + // (bare `Base`, namespace-relative `Sub\Base`, aliased `F\Base`) must + // be resolved against THIS class's own use-map + namespace, since an + // ancestor may live in a different file/namespace than the receiver. + $qualified = $parent instanceof Node\Name\FullyQualified + ? ltrim($parent->toString(), '\\') + : (self::resolveNameWithUseMap($parent, $current->useMap, $current->namespace) ?? ltrim($parent->toString(), '\\')); + if (isset($seen[$qualified])) { + return null; + } + $seen[$qualified] = true; + $current = $classes->findWithContext($qualified); + } + return null; + } + /** * Convert a nikic return-type node into a (nullable, TypeRef) tuple. * diff --git a/test/Resolver/GenericResolverTest.php b/test/Resolver/GenericResolverTest.php index e65acb7..8d1a18e 100644 --- a/test/Resolver/GenericResolverTest.php +++ b/test/Resolver/GenericResolverTest.php @@ -492,6 +492,99 @@ public function identity(T $x): T { return $x; } self::assertSame('string', $resolver->resolveVariable('/Use.xphp', 's', PHP_INT_MAX)); } + public function testNullsafeMethodTurbofishSpecializesReturnType(): void + { + // xphp 0.3.0: a generic method called with turbofish through a NULLSAFE + // receiver (`$u?->identity::(...)`). The method's T binds to the + // call-site type arg exactly as the plain `->` form; the `?->` operator + // then makes the specialized return nullable (`int` -> `?int`), + // consistent with the resolver's other nullsafe paths. + $workspace = $this->workspace(); + $this->open($workspace, '/Util.xphp', <<<'XPHP' + (T $x): T { return $x; } + } + XPHP); + $this->open($workspace, '/Use.xphp', <<<'XPHP' + identity::(99); + $s = $u?->identity::('world'); + XPHP); + + $resolver = $this->resolver($workspace); + + self::assertSame('?int', $resolver->resolveVariable('/Use.xphp', 'i', PHP_INT_MAX)); + self::assertSame('?string', $resolver->resolveVariable('/Use.xphp', 's', PHP_INT_MAX)); + } + + public function testInheritedGenericMethodTurbofishOnSubclassReceiverSpecializes(): void + { + // xphp 0.3.0: a generic method DECLARED on a base class, called with + // turbofish on a SUBCLASS receiver (`$c->identity::(...)`). The + // resolver must walk to the inherited declaration to find T, then bind + // it to the call-site type arg. + $workspace = $this->workspace(); + $this->open($workspace, '/Base.xphp', <<<'XPHP' + (T $x): T { return $x; } + } + XPHP); + $this->open($workspace, '/Child.xphp', <<<'XPHP' + open($workspace, '/Use.xphp', <<<'XPHP' + identity::(99); + $s = $c->identity::('world'); + XPHP); + + $resolver = $this->resolver($workspace); + + self::assertSame('int', $resolver->resolveVariable('/Use.xphp', 'i', PHP_INT_MAX)); + self::assertSame('string', $resolver->resolveVariable('/Use.xphp', 's', PHP_INT_MAX)); + } + + public function testInheritedGenericMethodTurbofishAcrossNamespaceResolves(): void + { + // The base lives in a DIFFERENT namespace, imported via `use`. The + // extends-chain walk must qualify the parent name against the child's + // own use-map (not the receiver's), then find the inherited method. + $workspace = $this->workspace(); + $this->open($workspace, '/Base.xphp', <<<'XPHP' + (T $x): T { return $x; } + } + XPHP); + $this->open($workspace, '/Child.xphp', <<<'XPHP' + open($workspace, '/Use.xphp', <<<'XPHP' + identity::(99); + XPHP); + + $resolver = $this->resolver($workspace); + + self::assertSame('int', $resolver->resolveVariable('/Use.xphp', 'i', PHP_INT_MAX)); + } + public function testRelativeStaticReturnResolvesToReceiverType(): void { // Regression for the `generic_method_new_static_turbofish` fixture: From 048fe6a61585bb0c18c75d6a437457b48a7ca7ba Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 10:02:54 +0000 Subject: [PATCH 08/26] test(resolver): lock keyword-named generic method turbofish xphp 0.3.0 lets a generic method be named with a PHP keyword (function list(), $f->list::()). The parser stamps the generic attributes normally and the byte turbofish scanner is keyword-agnostic, so resolution and hover already work; no production change is needed. Pin the behavior with a resolver unit test and a hover behat scenario. Co-Authored-By: Claude Opus 4.8 (1M context) --- features/understand/hover.feature | 21 ++++++++++++++++++++ test/Resolver/GenericResolverTest.php | 28 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/features/understand/hover.feature b/features/understand/hover.feature index 890331e..e2156a7 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -339,3 +339,24 @@ Feature: Hover And the FQN index has been warmed on initialize When I request "textDocument/hover" on "profileBio" at line 4 of "/Use.xphp" Then the hover contents contain "?string $profileBio" + + Scenario: Hover over a keyword-named generic method turbofish specializes + Given the file at "/Foo.xphp" contains the following lines: + """ + (T $x): T { return $x; } + } + """ + And the file at "/Use.xphp" contains the following lines: + """ + list::('x'); + """ + And the FQN index has been warmed on initialize + When I request "textDocument/hover" on "list" at line 3 of "/Use.xphp" + Then the hover contents contain "list(string $x): string" diff --git a/test/Resolver/GenericResolverTest.php b/test/Resolver/GenericResolverTest.php index 8d1a18e..aacd423 100644 --- a/test/Resolver/GenericResolverTest.php +++ b/test/Resolver/GenericResolverTest.php @@ -585,6 +585,34 @@ class Child extends Base {} self::assertSame('int', $resolver->resolveVariable('/Use.xphp', 'i', PHP_INT_MAX)); } + public function testKeywordNamedGenericMethodTurbofishSpecializes(): void + { + // xphp 0.3.0 lets a generic method be NAMED with a PHP keyword + // (`function list()`), called as `$f->list::(...)`. The parser + // stamps the generic attributes normally and the byte scanner is + // keyword-agnostic, so this should specialize like any other method. + $workspace = $this->workspace(); + $this->open($workspace, '/Foo.xphp', <<<'XPHP' + (T $x): T { return $x; } + } + XPHP); + $this->open($workspace, '/Use.xphp', <<<'XPHP' + list::(9); + $s = $f->list::('x'); + XPHP); + + $resolver = $this->resolver($workspace); + + self::assertSame('int', $resolver->resolveVariable('/Use.xphp', 'i', PHP_INT_MAX)); + self::assertSame('string', $resolver->resolveVariable('/Use.xphp', 's', PHP_INT_MAX)); + } + public function testRelativeStaticReturnResolvesToReceiverType(): void { // Regression for the `generic_method_new_static_turbofish` fixture: From b02df92cfa8439d66c56931df70c4e0c1e4a4ad0 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 10:05:25 +0000 Subject: [PATCH 09/26] test(resolver): lock type-param constructor on a variant class xphp 0.3.0 lets a covariant/contravariant class take its type parameter in a constructor (e.g. Producer { __construct(private T $item) }). Variance is orthogonal to substitution, so construction + a covariant getter already resolve to the concrete type; pin it with a resolver test. static fn value inference is deferred (closure-value modeling; see plan DEFERRED). Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Resolver/GenericResolverTest.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/Resolver/GenericResolverTest.php b/test/Resolver/GenericResolverTest.php index aacd423..c1780e2 100644 --- a/test/Resolver/GenericResolverTest.php +++ b/test/Resolver/GenericResolverTest.php @@ -613,6 +613,33 @@ public function list(T $x): T { return $x; } self::assertSame('string', $resolver->resolveVariable('/Use.xphp', 's', PHP_INT_MAX)); } + public function testVariantClassTypeParamConstructorResolvesThroughGetter(): void + { + // xphp 0.3.0: a covariant class may take its type parameter in a + // constructor (`private T $item`), and a covariant getter returns T. + // Variance is orthogonal to substitution, so `new Producer::(5)` + // then `->get()` resolves to int exactly as an invariant generic would. + $workspace = $this->workspace(); + $this->open($workspace, '/Producer.xphp', <<<'XPHP' + { + public function __construct(private T $item) {} + public function get(): T { return $this->item; } + } + XPHP); + $this->open($workspace, '/Use.xphp', <<<'XPHP' + (5); + $x = $p->get(); + XPHP); + + $resolver = $this->resolver($workspace); + + self::assertSame('int', $resolver->resolveVariable('/Use.xphp', 'x', PHP_INT_MAX)); + } + public function testRelativeStaticReturnResolvesToReceiverType(): void { // Regression for the `generic_method_new_static_turbofish` fixture: From d0f730d0c57e989c033ce1ce72924ec32bfcfaff Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 10:43:13 +0000 Subject: [PATCH 10/26] feat(hover): render Closure signature types on hover xphp 0.3.0 introduces `Closure(int $x, string $y): bool` signature types, which erase to a bare `\Closure` in emitted PHP. The parser stamps the structured `ClosureSignature` on the `Closure` type node, so the LSP is the only place the params + return survive for display. Add a stateless `ClosureSignatureView` that renders that tree to its source shape (`Closure(int, string): bool`), handling all five SigType members (leaf, union, intersection, nested closure, raw DNF slice), `&`/`...` param modifiers, whole-type nullability, and FQ-prefixed class leaves consistent with the rest of the hover card. Wire it into XphpHoverHandler as an additive, instanceof-guarded case. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 4 + features/understand/hover.feature | 11 ++ src/Handler/XphpHoverHandler.php | 11 ++ src/Resolver/ClosureSignatureView.php | 130 ++++++++++++++++++ test/Resolver/ClosureSignatureViewTest.php | 146 +++++++++++++++++++++ 5 files changed, 302 insertions(+) create mode 100644 src/Resolver/ClosureSignatureView.php create mode 100644 test/Resolver/ClosureSignatureViewTest.php diff --git a/docs/features/index.md b/docs/features/index.md index f48dd53..d3c0758 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -204,6 +204,10 @@ property / native function info, hover renders: - A type parameter's variance: `out T` (covariant) / `in T` (contravariant) are shown with their marker and a label; invariant params show the bare name. +- A `Closure(int $x, string $y): bool` signature type in a parameter, + return, or property position, rendered as its structured form + (`Closure(int, string): bool`) -- params without names, nullable and + composite (union / intersection) members preserved. ### Signature Help diff --git a/features/understand/hover.feature b/features/understand/hover.feature index e2156a7..f487951 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -360,3 +360,14 @@ Feature: Hover And the FQN index has been warmed on initialize When I request "textDocument/hover" on "list" at line 3 of "/Use.xphp" Then the hover contents contain "list(string $x): string" + + Scenario: Hover over a Closure signature type shows the structured signature + Given the file at "/Sig.xphp" contains the following lines: + """ + getAttribute(XphpSourceParser::ATTR_CLOSURE_SIG); + if ($closureSig instanceof ClosureSignature) { + return sprintf('**`%s`**', ClosureSignatureView::render($closureSig)); + } + $args = $name->getAttribute(XphpSourceParser::ATTR_GENERIC_ARGS); $templateFqn = $name->getAttribute(XphpSourceParser::ATTR_TEMPLATE_FQN); diff --git a/src/Resolver/ClosureSignatureView.php b/src/Resolver/ClosureSignatureView.php new file mode 100644 index 0000000..f4c3aa5 --- /dev/null +++ b/src/Resolver/ClosureSignatureView.php @@ -0,0 +1,130 @@ +` suffix. + */ + public static function render(ClosureSignature $sig): string + { + $params = implode(', ', array_map( + static fn (ClosureSignatureParam $p): string => self::renderParam($p), + $sig->params, + )); + $out = ($sig->nullable ? '?' : '') . 'Closure(' . $params . ')'; + if ($sig->return !== null) { + $out .= ': ' . self::renderSigType($sig->return); + } + return $out; + } + + private static function renderParam(ClosureSignatureParam $param): string + { + $rendered = self::renderSigType($param->type); + if ($param->byRef) { + $rendered .= '&'; + } + if ($param->variadic) { + $rendered .= '...'; + } + return $rendered; + } + + /** + * Render a `SigType` node: + * - leaf (`SigTypeRef`) -> the wrapped `TypeRef`'s display (handles generic + * args, e.g. `Box`); + * - union -> `A|B`, with intersection members parenthesised + * for DNF (`(A&B)|C`); + * - intersection -> `A&B`. + * An unknown `SigType` subtype degrades to an empty string rather than + * throwing (forward-compatible with future node kinds). + */ + private static function renderSigType(SigType $type): string + { + if ($type instanceof SigTypeRef) { + return self::renderTypeRef($type->type); + } + if ($type instanceof SigClosure) { + // A nested closure type, e.g. `Closure(Closure(int): bool): void`. + return self::render($type->signature); + } + if ($type instanceof SigRaw) { + // Forms the token scanner keeps as sliced source (composite/DNF like + // `(A&B)|C`) are already display-ready. + return $type->raw; + } + if ($type instanceof SigUnion) { + return implode('|', array_map( + static fn (SigType $m): string => self::wrapForUnion($m), + $type->members, + )); + } + if ($type instanceof SigIntersection) { + return implode('&', array_map( + static fn (SigType $m): string => self::renderSigType($m), + $type->members, + )); + } + return ''; + } + + /** + * Render a leaf `TypeRef`: type-params and scalars are bare, a class leaf + * gets the leading `\` of a fully-qualified name (matching the rest of the + * hover card), and generic args recurse. + */ + private static function renderTypeRef(TypeRef $type): string + { + $name = ($type->isTypeParam || $type->isScalar) + ? $type->name + : '\\' . ltrim($type->name, '\\'); + if ($type->args === []) { + return $name; + } + return $name . '<' . implode(', ', array_map( + static fn (TypeRef $arg): string => self::renderTypeRef($arg), + $type->args, + )) . '>'; + } + + /** + * A union member that is itself an intersection is parenthesised to preserve + * DNF grouping (`(A&B)|C`); everything else renders bare. + */ + private static function wrapForUnion(SigType $member): string + { + $rendered = self::renderSigType($member); + return $member instanceof SigIntersection ? '(' . $rendered . ')' : $rendered; + } +} diff --git a/test/Resolver/ClosureSignatureViewTest.php b/test/Resolver/ClosureSignatureViewTest.php new file mode 100644 index 0000000..47408dc --- /dev/null +++ b/test/Resolver/ClosureSignatureViewTest.php @@ -0,0 +1,146 @@ +): void', ClosureSignatureView::render($sig)); + } + + public function testUnionParam(): void + { + $union = new SigUnion([self::leaf('int'), self::leaf('string')]); + $sig = new ClosureSignature([self::param($union)], self::leaf('void')); + self::assertSame('Closure(int|string): void', ClosureSignatureView::render($sig)); + } + + public function testIntersectionReturn(): void + { + $inter = new SigIntersection([self::leaf('A', false), self::leaf('B', false)]); + $sig = new ClosureSignature([], $inter); + self::assertSame('Closure(): \\A&\\B', ClosureSignatureView::render($sig)); + } + + public function testDnfParenthesisesIntersectionInsideUnion(): void + { + // (A&B)|C -- exercises the defensive wrapForUnion path. The real parser + // keeps a DNF member as a single SigRaw slice (see testRawMemberIsPassedThrough), + // but the view must still parenthesise a structured intersection-in-union. + $inter = new SigIntersection([self::leaf('A', false), self::leaf('B', false)]); + $dnf = new SigUnion([$inter, self::leaf('C', false)]); + $sig = new ClosureSignature([self::param($dnf)], self::leaf('void')); + self::assertSame('Closure((\\A&\\B)|\\C): void', ClosureSignatureView::render($sig)); + } + + public function testNestedClosureRecurses(): void + { + // Closure(Closure(int): bool): void -- a SigClosure member must recurse + // through render(), not degrade to an empty string. + $inner = new SigClosure(new ClosureSignature([self::param(self::leaf('int'))], self::leaf('bool'))); + $sig = new ClosureSignature([self::param($inner)], self::leaf('void')); + self::assertSame('Closure(Closure(int): bool): void', ClosureSignatureView::render($sig)); + } + + public function testRawMemberIsPassedThrough(): void + { + // The token scanner keeps composite/DNF forms as an already-display-ready + // SigRaw slice; the view emits it verbatim rather than dropping it. + $sig = new ClosureSignature([self::param(new SigRaw('(A&B)|C'))], self::leaf('void')); + self::assertSame('Closure((A&B)|C): void', ClosureSignatureView::render($sig)); + } + + public function testRendersRealParserPayload(): void + { + // Exercise the actual ClosureSignature the parser stamps, not a hand-built one. + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + [$ast] = $parser->parseWithMap( + "findInstanceOf($ast, Name::class) as $name) { + $candidate = $name->getAttribute(XphpSourceParser::ATTR_CLOSURE_SIG); + if ($candidate instanceof ClosureSignature) { + $sig = $candidate; + break; + } + } + self::assertInstanceOf(ClosureSignature::class, $sig); + self::assertSame('Closure(int, string): bool', ClosureSignatureView::render($sig)); + } +} From e53c49a0ab32e43a5d53a61c21bf635bbe569a4f Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 10:59:07 +0000 Subject: [PATCH 11/26] feat(closures): surface closure conformance diagnostics and type completion Complete the xphp 0.3.0 closure-signature feature beyond hover. Diagnostics: a fourth WorkspaceAnalyzer pass drives the upstream ClosureConformanceValidator in collect mode over each open file, so a factory whose declared return type is `Closure(...)` and that hands back a closure literal provably violating that target (return not a subtype, parameter not wider, wrong arity, by-ref mismatch) is surfaced as an `xphp.closure_conformance` diagnostic. The check is upstream-owned; the server only maps it. The sibling `xphp.unspecialized_generic_closure` code is produced solely inside the specialization pipeline the server never runs and stays vocabulary-only. Completion: a new ClosureTypePositionDetector spots a cursor at a type token inside a `Closure(int $x, string $y): bool` signature (first param, after a comma, or the return type) and offers class + scalar names, the same unbounded candidate set a bound-free type-arg slot gets. Conservative by design -- an ambiguous shape offers nothing rather than firing in the wrong context (guards `new Closure(`, `Closure::`, etc.). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 20 +- features/find/completion.feature | 19 ++ features/validate/diagnostics.feature | 26 +++ src/Analyzer/WorkspaceAnalyzer.php | 72 +++++++ src/Handler/ClosureTypePositionDetector.php | 195 ++++++++++++++++++ src/Handler/XphpCompletionHandler.php | 10 + test/Analyzer/WorkspaceAnalyzerTest.php | 40 ++++ .../ClosureTypePositionDetectorTest.php | 99 +++++++++ 8 files changed, 479 insertions(+), 2 deletions(-) create mode 100644 src/Handler/ClosureTypePositionDetector.php create mode 100644 test/Handler/ClosureTypePositionDetectorTest.php diff --git a/docs/features/index.md b/docs/features/index.md index d3c0758..37d587f 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -252,10 +252,10 @@ as type parameters. ## Validate Diagnostics surface in both push (`textDocument/publishDiagnostics`) -and pull (`textDocument/diagnostic`, LSP 3.17) modes. Six diagnostic +and pull (`textDocument/diagnostic`, LSP 3.17) modes. Seven diagnostic codes are emitted today: `xphp.parse`, `xphp.bound`, `xphp.definition` (duplicate template), `xphp.undefined-name`, `xphp.ctor-arg-mismatch`, -and `xphp.arg-mismatch`. +`xphp.arg-mismatch`, and `xphp.closure_conformance`. ### Parse errors @@ -274,6 +274,17 @@ open buffers), so `new Box::(...)` resolves correctly even when reference the source-level instantiation (e.g. `Box`) rather than the hashed specialization name. +### Closure signature conformance + +A factory whose declared return type is `Closure(...)` and that +hands back a closure **literal** provably violating that target -- +a return type that isn't a subtype, a parameter type that isn't +wider (contravariance), a wrong arity, or a by-ref mismatch -- is +flagged with `xphp.closure_conformance`. The check itself is owned +by the xphp compiler; the server surfaces it. Value flows where the +candidate isn't a literal at the target (a variable, a call +argument) stay gradual and are not flagged. + ### Default type arguments (no false missing-arg) Not a diagnostic code of its own -- this is how the bound and @@ -341,6 +352,11 @@ Context-aware completion in every meaningful position: Composite bounds are respected: a candidate must satisfy **every** leaf of an intersection (`T : A & B`) and **any** leaf of a union (`T : A | B`). +- **Closure-signature type position** (`function h(Closure(|): ...)`) + -- a type token inside a `Closure(int $x, string $y): bool` + signature (first param, after a `,`, or the return type after `:`) + offers class names and scalars, the same unbounded candidate set a + bound-free type-arg slot gets. - **Member access** (`$obj->`) and **static access** (`Cls::`) -- methods, properties, and constants from the receiver. - **Static property access** (`Cls::$`) -- a distinct context kind diff --git a/features/find/completion.feature b/features/find/completion.feature index 4b9b861..2c070eb 100644 --- a/features/find/completion.feature +++ b/features/find/completion.feature @@ -138,3 +138,22 @@ Feature: Completion When I request completion after "Pair::<" at line 2 of "/Use.xphp" Then a completion item labeled "Both" is offered And no completion item labeled "OnlyAnimal" is offered + + Scenario: Suggest type names inside a Closure signature parameter position + Given the file at "/Models.xphp" contains the following lines: + """ + 'x'; + } + """ + And the FQN index has been warmed on initialize + When I analyze "/Factory.xphp" for diagnostics + Then a "xphp.closure_conformance" diagnostic is reported saying "return type" + + Scenario: A conforming closure literal reports nothing + Given the file at "/Ok.xphp" contains the following lines: + """ + true; + } + """ + And the FQN index has been warmed on initialize + When I analyze "/Ok.xphp" for diagnostics + Then no diagnostics are reported diff --git a/src/Analyzer/WorkspaceAnalyzer.php b/src/Analyzer/WorkspaceAnalyzer.php index d3fbcfb..f17bc62 100644 --- a/src/Analyzer/WorkspaceAnalyzer.php +++ b/src/Analyzer/WorkspaceAnalyzer.php @@ -13,7 +13,9 @@ use XPHP\Lsp\Handler\TurbofishScanner; use XPHP\Lsp\PositionMap; use XPHP\Lsp\Resolver\BoundExprView; +use XPHP\Diagnostics\DiagnosticCollector; use XPHP\Transpiler\Monomorphize\BoundUnion; +use XPHP\Transpiler\Monomorphize\ClosureConformanceValidator; use XPHP\Transpiler\Monomorphize\Registry; use XPHP\Transpiler\Monomorphize\TypeHierarchy; use XPHP\Transpiler\Monomorphize\XphpSourceParser; @@ -158,6 +160,30 @@ public function analyze(array $files, array $hierarchyAsts = []): array } } + // Fourth pass: closure-signature conformance. A factory whose declared + // return type is `Closure(...)` and that hands back a closure *literal* + // provably violating that target is flagged by the upstream + // ClosureConformanceValidator. We drive it in "collect" mode (a + // DiagnosticCollector) so every violation is gathered rather than the + // first one throwing, then map each onto an LSP diagnostic. In this + // non-grounded pass only the return position is statically decided; the + // call-argument site is gated behind the grounded per-specialization pass + // the LSP never runs. The check itself is upstream-owned (contravariant + // params / covariant return / arity / by-ref) -- we only surface it. The + // sibling `xphp.unspecialized_generic_closure` code is produced solely inside + // the specialization pipeline (GenericMethodCompiler), which the LSP never + // runs, so it stays sink-only (see DEFERRED). + foreach ($files as $path => $entry) { + $positionMap = new PositionMap($entry['source']); + $this->walkClosureConformance( + $entry['ast'], + $path, + $hierarchy, + $positionMap, + $diagnosticsByFile[$path], + ); + } + return $diagnosticsByFile; } @@ -259,6 +285,52 @@ private static function buildDefinitionDiagnostic( return new Diagnostic($sl, $sc, $el, $ec, $message, code: DiagnosticCode::Definition); } + /** + * Run the upstream {@see ClosureConformanceValidator} over one file's AST in + * collect mode and map each `xphp.closure_conformance` violation onto an LSP + * diagnostic. The upstream diagnostic carries only a (newline-preserving) + * start line, so the squiggle spans the whole offending line -- the same + * fallback the bound-violation pass uses when byte offsets are absent. + * + * @param list $ast + * @param list $diagnostics + */ + private function walkClosureConformance( + array $ast, + string $file, + TypeHierarchy $hierarchy, + PositionMap $positionMap, + array &$diagnostics, + ): void { + $collector = new DiagnosticCollector(); + try { + (new ClosureConformanceValidator($hierarchy))->validateFile($ast, $file, $collector); + } catch (\Throwable) { + // Best-effort: the validator runs upstream code over a possibly + // half-typed buffer; a crash here must never suppress every other + // diagnostic for the file. Whatever was collected before the throw + // is still emitted below. + } + foreach ($collector->all() as $diag) { + if ($diag->code !== DiagnosticCode::ClosureConformance->value) { + continue; + } + $line = $diag->location?->line; + if ($line === null || $line < 1) { + continue; + } + [$sl, $sc, $el, $ec] = $positionMap->fullLineRangeFromNikic($line); + $diagnostics[] = new Diagnostic( + startLine: $sl, + startCharacter: $sc, + endLine: $el, + endCharacter: $ec, + message: $diag->message, + code: DiagnosticCode::ClosureConformance, + ); + } + } + /** * @param list $ast * @param array $openClasses diff --git a/src/Handler/ClosureTypePositionDetector.php b/src/Handler/ClosureTypePositionDetector.php new file mode 100644 index 0000000..ead0429 --- /dev/null +++ b/src/Handler/ClosureTypePositionDetector.php @@ -0,0 +1,195 @@ + `(` that opens a `Closure(` group (first param); + * - `Closure(int $x, |` -> a `,` at the top level of that group (next param); + * - `Closure(int $x): |` -> a `:` after the group's closing `)` (return type). + * + * Depth tracking over `()`, `<>` and `[]` keeps a comma inside a nested generic + * (`Closure(Map $m, |`) or a nested closure from being mistaken for a + * top-level separator. A parameter *name* position (`Closure(int |`, i.e. right + * after a type, before `$x`) is deliberately NOT a type position and returns null. + * + * Conservative by design: an unrecognised or ambiguous shape returns null (no + * completion) rather than firing in the wrong context. + */ +final readonly class ClosureTypePositionDetector +{ + /** + * @return array{prefix: string}|null + * null -> cursor is not at a closure-signature type position + * array -> `prefix` is the identifier stem typed since the opener / + * separator (used to filter candidates). + */ + public static function detect(string $source, int $offset): ?array + { + if ($offset < 0 || $offset > strlen($source)) { + return null; + } + + // The identifier stem immediately left of the cursor. + $stemStart = $offset; + while ($stemStart > 0 && self::isTypeByte($source[$stemStart - 1])) { + $stemStart--; + } + $prefix = substr($source, $stemStart, $offset - $stemStart); + + // The first non-whitespace byte before the stem decides the position. + $i = $stemStart - 1; + while ($i >= 0 && self::isSpace($source[$i])) { + $i--; + } + if ($i < 0) { + return null; + } + + $anchor = $source[$i]; + if ($anchor === '(') { + return self::isClosureOpener($source, $i) ? ['prefix' => $prefix] : null; + } + if ($anchor === ',') { + $open = self::enclosingOpenParen($source, $i - 1); + return $open !== null && self::isClosureOpener($source, $open) ? ['prefix' => $prefix] : null; + } + if ($anchor === ':') { + // Return-type position: `): |`. A `::` (static access) is not it. + if ($i > 0 && $source[$i - 1] === ':') { + return null; + } + $j = $i - 1; + while ($j >= 0 && self::isSpace($source[$j])) { + $j--; + } + if ($j < 0 || $source[$j] !== ')') { + return null; + } + $open = self::matchingOpenParen($source, $j); + return $open !== null && self::isClosureOpener($source, $open) ? ['prefix' => $prefix] : null; + } + + return null; + } + + /** + * True when the `(` at `$parenPos` opens a `Closure(` type — i.e. it is + * immediately (modulo whitespace) preceded by the bare word `Closure`, and + * that word is not itself preceded by `new` (a `new Closure(` constructor + * call is not a signature type). + */ + private static function isClosureOpener(string $source, int $parenPos): bool + { + $i = $parenPos - 1; + while ($i >= 0 && self::isSpace($source[$i])) { + $i--; + } + $end = $i; + while ($i >= 0 && self::isIdentByte($source[$i])) { + $i--; + } + $word = substr($source, $i + 1, $end - $i); + if (strcasecmp($word, 'Closure') !== 0) { + return false; + } + // A leading namespace separator (`\Closure`, `Foo\Closure`) is not the + // signature-type spelling, which is the bare keyword. + if ($i >= 0 && $source[$i] === '\\') { + return false; + } + // Guard against `new Closure(`. + $k = $i; + while ($k >= 0 && self::isSpace($source[$k])) { + $k--; + } + $wordEnd = $k; + while ($k >= 0 && self::isIdentByte($source[$k])) { + $k--; + } + return strcasecmp(substr($source, $k + 1, $wordEnd - $k), 'new') !== 0; + } + + /** + * Walk back from `$from` to the `(` that encloses it at bracket depth 0, + * skipping balanced `()`, `<>` and `[]`. Returns the opener offset, or null + * if none is found before the start of source. + */ + private static function enclosingOpenParen(string $source, int $from): ?int + { + $paren = 0; + $angle = 0; + $square = 0; + for ($i = $from; $i >= 0; $i--) { + $c = $source[$i]; + if ($c === ')') { + $paren++; + } elseif ($c === '>') { + $angle++; + } elseif ($c === ']') { + $square++; + } elseif ($c === '(') { + if ($paren === 0) { + return $i; + } + $paren--; + } elseif ($c === '<') { + if ($angle > 0) { + $angle--; + } + } elseif ($c === '[') { + if ($square > 0) { + $square--; + } + } elseif ($c === ';' || $c === '{' || $c === '}') { + // Statement boundary -- we are not inside a paren group. + return null; + } + } + return null; + } + + /** + * Given the offset of a `)`, return the offset of its matching `(`. + */ + private static function matchingOpenParen(string $source, int $closePos): ?int + { + $depth = 0; + for ($i = $closePos; $i >= 0; $i--) { + $c = $source[$i]; + if ($c === ')') { + $depth++; + } elseif ($c === '(') { + $depth--; + if ($depth === 0) { + return $i; + } + } + } + return null; + } + + private static function isTypeByte(string $byte): bool + { + return ctype_alnum($byte) || $byte === '_' || $byte === '\\'; + } + + private static function isIdentByte(string $byte): bool + { + return ctype_alnum($byte) || $byte === '_'; + } + + private static function isSpace(string $byte): bool + { + return $byte === ' ' || $byte === "\t" || $byte === "\n" || $byte === "\r"; + } +} diff --git a/src/Handler/XphpCompletionHandler.php b/src/Handler/XphpCompletionHandler.php index a39ace2..85430bd 100644 --- a/src/Handler/XphpCompletionHandler.php +++ b/src/Handler/XphpCompletionHandler.php @@ -125,6 +125,16 @@ public function complete(CompletionParams $params, ?CancellationToken $cancel = return new Success(new CompletionList(isIncomplete: false, items: $candidates)); } + // A type token inside a `Closure(int $x, string $y): bool` signature type + // is an unbounded type position -- offer class names + scalars, the same + // candidate set a bound-free turbofish slot gets. + $closureHit = ClosureTypePositionDetector::detect($item->text, $offset); + if ($closureHit !== null) { + $importContext = ClassNameImportContext::extractFromSource($item->text); + $candidates = $this->buildCandidates($closureHit['prefix'], null, $importContext); + return new Success(new CompletionList(isIncomplete: false, items: $candidates)); + } + // Fall through to PHP-semantic completion (member / static access). // Returns an empty list when the cursor isn't in a recognised // context, which matches the old "empty list, no fallback" behaviour diff --git a/test/Analyzer/WorkspaceAnalyzerTest.php b/test/Analyzer/WorkspaceAnalyzerTest.php index c3fe8ef..b52e4fd 100644 --- a/test/Analyzer/WorkspaceAnalyzerTest.php +++ b/test/Analyzer/WorkspaceAnalyzerTest.php @@ -46,6 +46,46 @@ class Box self::assertGreaterThan(0, $d->startCharacter, 'must not start at column 0 (whole-line) anymore'); } + public function testClosureReturnConformanceViolationIsFlagged(): void + { + $files = $this->parseFiles([ + '/Factory.xphp' => <<<'PHP' + 'x'; + } + PHP, + ]); + + $diagnostics = (new WorkspaceAnalyzer())->analyze($files); + + self::assertCount(1, $diagnostics['/Factory.xphp'], 'the non-conforming closure literal should be flagged'); + $d = $diagnostics['/Factory.xphp'][0]; + self::assertSame(DiagnosticCode::ClosureConformance, $d->code); + self::assertStringContainsString('return type', $d->message); + // The diagnostic lands on the `return fn(...)` line (line 4, 0-based 3), + // spanning that whole line (upstream carries only a start line). + self::assertSame(3, $d->startLine, 'diagnostic must land on the offending return line'); + } + + public function testConformingClosureLiteralProducesNoDiagnostic(): void + { + $files = $this->parseFiles([ + '/Ok.xphp' => <<<'PHP' + true; + } + PHP, + ]); + + $diagnostics = (new WorkspaceAnalyzer())->analyze($files); + + self::assertSame([], $diagnostics['/Ok.xphp'], 'a conforming closure literal must not be flagged'); + } + public function testCompositeBoundViolationEmitsLeafListInFixData(): void { $files = $this->parseFiles([ diff --git a/test/Handler/ClosureTypePositionDetectorTest.php b/test/Handler/ClosureTypePositionDetectorTest.php new file mode 100644 index 0000000..9e71cb9 --- /dev/null +++ b/test/Handler/ClosureTypePositionDetectorTest.php @@ -0,0 +1,99 @@ + ''], self::at('function h(Closure(')); + } + + public function testFirstParamWithPartialPrefix(): void + { + self::assertSame(['prefix' => 'Str'], self::at('function h(Closure(Str')); + } + + public function testNextParamAfterComma(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure(int $x, ')); + } + + public function testNextParamAfterCommaWithPrefix(): void + { + self::assertSame(['prefix' => 'boo'], self::at('function h(Closure(int $x, boo')); + } + + public function testReturnTypePositionAfterColon(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure(int $x): ')); + } + + public function testReturnTypePositionWithPrefix(): void + { + self::assertSame(['prefix' => 'bo'], self::at('function h(Closure(int $x): bo')); + } + + public function testCommaInsideNestedGenericIsNotASeparator(): void + { + // The comma sits inside `Map` at angle-depth 1, so the cursor is + // still the FIRST closure param slot -- prefix empty, position valid. + self::assertSame(['prefix' => ''], self::at('function h(Closure(Map ''], self::at('function h(Closure(Map $m, ')); + } + + public function testParamNamePositionIsNotAType(): void + { + // `Closure(int |` -- cursor is where the `$name` goes, not a type. + self::assertNull(self::at('function h(Closure(int ')); + } + + public function testNewClosureConstructorIsNotASignature(): void + { + self::assertNull(self::at('$c = new Closure(')); + } + + public function testStaticCallOnClosureIsNotASignature(): void + { + self::assertNull(self::at('Closure::fromCallable(')); + } + + public function testFullyQualifiedClosureIsNotASignature(): void + { + self::assertNull(self::at('function h(\\Closure(')); + } + + public function testDoubleColonReturnGuardIsNotReturnType(): void + { + // A `::` (static access) must not be read as the return `:` position. + self::assertNull(self::at('Foo::')); + } + + public function testPlainCodeIsNotAClosureType(): void + { + self::assertNull(self::at('$x = foo(1, ')); + } + + public function testCaseInsensitiveClosureKeyword(): void + { + self::assertSame(['prefix' => ''], self::at('function h(closure(')); + } +} From 2d0e89fe070b056be198777a4300455eea48509f Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 11:12:48 +0000 Subject: [PATCH 12/26] feat(hover): resolve method type parameters and their enclosing-param bounds A method-level generic (`function contains(U $value): bool`) had no hover: the type-parameter path only inspected class-level generics, so hovering `U` fell through to plain PHP reflection and showed nothing useful. Capture the enclosing function-like scope alongside the class scope when resolving the Name under the cursor, and add a method-type-parameter hover case that reads the method's generic params. It renders `Type parameter U of App\Box::contains bounded by E` -- the bound may reference the enclosing class parameter `E`, which the bound view already renders as a bare identifier. The method binding is innermost, so a method param shadows a same-named class param. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 4 ++ features/understand/hover.feature | 17 +++++ src/Handler/AstPositionResolver.php | 32 +++++++-- src/Handler/XphpHoverHandler.php | 88 +++++++++++++++++++++++- test/Handler/AstPositionResolverTest.php | 50 ++++++++++++++ test/Handler/XphpHoverHandlerTest.php | 44 ++++++++++++ 6 files changed, 226 insertions(+), 9 deletions(-) diff --git a/docs/features/index.md b/docs/features/index.md index 37d587f..2f9253c 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -204,6 +204,10 @@ property / native function info, hover renders: - A type parameter's variance: `out T` (covariant) / `in T` (contravariant) are shown with their marker and a label; invariant params show the bare name. +- A **method-level** type parameter, including one bounded by the + enclosing class param: hovering `U` in + `class Box { function contains(...) }` shows + `Type parameter U of App\Box::contains bounded by E`. - A `Closure(int $x, string $y): bool` signature type in a parameter, return, or property position, rendered as its structured form (`Closure(int, string): bool`) -- params without names, nullable and diff --git a/features/understand/hover.feature b/features/understand/hover.feature index f487951..00b707b 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -361,6 +361,23 @@ Feature: Hover When I request "textDocument/hover" on "list" at line 3 of "/Use.xphp" Then the hover contents contain "list(string $x): string" + Scenario: Hover over a method type parameter bounded by the enclosing class param + Given the file at "/Box.xphp" contains the following lines: + """ + + { + public function contains(U $value): bool { return true; } + } + """ + And the FQN index has been warmed on initialize + When I request "textDocument/hover" on "U $value" at line 4 of "/Box.xphp" + Then the hover contents contain "Type parameter" + And the hover contents contain "`U`" + And the hover contents contain "App\Box::contains" + And the hover contents contain "bounded by `E`" + Scenario: Hover over a Closure signature type shows the structured signature Given the file at "/Sig.xphp" contains the following lines: """ diff --git a/src/Handler/AstPositionResolver.php b/src/Handler/AstPositionResolver.php index 84b2003..d1a293f 100644 --- a/src/Handler/AstPositionResolver.php +++ b/src/Handler/AstPositionResolver.php @@ -5,6 +5,7 @@ namespace XPHP\Lsp\Handler; use PhpParser\Node; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Name; use PhpParser\Node\Stmt\ClassLike; use PhpParser\NodeTraverser; @@ -12,10 +13,12 @@ /** * Walks an AST and locates the smallest Name node whose byte range contains a - * given offset. Also captures the stack of enclosing ClassLike nodes at the hit - * site — handlers need that to look up type-params declared on the surrounding - * template (e.g. resolving `T` in `public T $item` requires knowing which - * ClassLike the `T` lives inside). + * given offset. Also captures the stack of enclosing ClassLike nodes AND the + * stack of enclosing function-likes (methods / functions / closures) at the hit + * site — handlers need the former to resolve a class type-param (`T` in + * `public T $item`) and the latter to resolve a *method*-level type-param + * (`U` in `function contains(U $value)`), whose bound may itself + * reference the enclosing class param. * * Used by HoverHandler (and, later, DefinitionHandler) — extracted so the * resolution logic is testable in isolation. @@ -24,7 +27,7 @@ { /** * @param list $ast - * @return array{name: Name, classScope: list}|null + * @return array{name: Name, classScope: list, methodScope: list}|null */ public static function nameAtOffset(array $ast, int $offset): ?array { @@ -32,11 +35,17 @@ public static function nameAtOffset(array $ast, int $offset): ?array /** @var list */ private array $classStack = []; + /** @var list */ + private array $methodStack = []; + public ?Name $found = null; /** @var list */ public array $foundClassStack = []; + /** @var list */ + public array $foundMethodStack = []; + public function __construct(private readonly int $offset) { } @@ -46,6 +55,9 @@ public function enterNode(Node $node): null if ($node instanceof ClassLike) { $this->classStack[] = $node; } + if ($node instanceof FunctionLike) { + $this->methodStack[] = $node; + } if (!$node instanceof Name) { return null; } @@ -63,6 +75,7 @@ public function enterNode(Node $node): null // because nikic walks parents before children. $this->found = $node; $this->foundClassStack = $this->classStack; + $this->foundMethodStack = $this->methodStack; return null; } @@ -71,6 +84,9 @@ public function leaveNode(Node $node): null if ($node instanceof ClassLike) { array_pop($this->classStack); } + if ($node instanceof FunctionLike) { + array_pop($this->methodStack); + } return null; } }; @@ -82,6 +98,10 @@ public function leaveNode(Node $node): null if ($visitor->found === null) { return null; } - return ['name' => $visitor->found, 'classScope' => $visitor->foundClassStack]; + return [ + 'name' => $visitor->found, + 'classScope' => $visitor->foundClassStack, + 'methodScope' => $visitor->foundMethodStack, + ]; } } diff --git a/src/Handler/XphpHoverHandler.php b/src/Handler/XphpHoverHandler.php index e73e8dc..a51b190 100644 --- a/src/Handler/XphpHoverHandler.php +++ b/src/Handler/XphpHoverHandler.php @@ -8,8 +8,11 @@ use Amp\Promise; use Amp\Success; use PhpParser\Node; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Name; use PhpParser\Node\Stmt\ClassLike; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Function_; use PhpParser\NodeFinder; use Phpactor\LanguageServer\Core\Handler\CanRegisterCapabilities; use Phpactor\LanguageServer\Core\Handler\Handler; @@ -119,7 +122,7 @@ public function hover(HoverParams $params, ?CancellationToken $cancel = null): P $hit = AstPositionResolver::nameAtOffset($result->ast, $offset); if ($hit !== null) { - $markdown = $this->buildHoverMarkdown($hit['name'], $hit['classScope']); + $markdown = $this->buildHoverMarkdown($hit['name'], $hit['classScope'], $hit['methodScope']); if ($markdown !== null) { return new Success(new Hover(new MarkupContent(MarkupKind::MARKDOWN, $markdown))); } @@ -171,9 +174,10 @@ public function hover(HoverParams $params, ?CancellationToken $cancel = null): P } /** - * @param list $classScope + * @param list $classScope + * @param list $methodScope */ - private function buildHoverMarkdown(\PhpParser\Node\Name $name, array $classScope): ?string + private function buildHoverMarkdown(\PhpParser\Node\Name $name, array $classScope, array $methodScope = []): ?string { // Case 0: hover over a `Closure(...)` signature type. xphp 0.3.0 stamps // the structured signature on the `Closure` type node; it erases to a @@ -206,6 +210,16 @@ private function buildHoverMarkdown(\PhpParser\Node\Name $name, array $classScop return null; } $shortName = $parts[0]; + + // Case 2a: a *method*-level type-param (`function contains(...)`). + // Its binding is innermost, so it wins over a same-named class param. + // The bound may reference the enclosing class param (`U : E`), which + // BoundExprView already renders as the bare identifier `E`. + $methodHover = self::methodTypeParamHover($shortName, $methodScope, $classScope); + if ($methodHover !== null) { + return $methodHover; + } + foreach (array_reverse($classScope) as $classLike) { $params = $classLike->getAttribute(XphpSourceParser::ATTR_GENERIC_PARAMS); if (!is_array($params)) { @@ -234,6 +248,74 @@ private function buildHoverMarkdown(\PhpParser\Node\Name $name, array $classScop return null; } + /** + * Resolve a method-level type-param hover: the innermost enclosing + * function-like whose `ATTR_METHOD_GENERIC_PARAMS` declares `$shortName` + * (`function contains(...)`). The owner is rendered as + * `Class::method` (or the bare function name for a free function); the bound + * -- which may reference the enclosing class param `E` -- is rendered by + * BoundExprView exactly as the class-param case. Returns null when no + * enclosing function-like declares the name, so hover falls through to the + * class-param scan. + * + * @param list $methodScope + * @param list $classScope + */ + private static function methodTypeParamHover(string $shortName, array $methodScope, array $classScope): ?string + { + foreach (array_reverse($methodScope) as $fn) { + $params = $fn->getAttribute(XphpSourceParser::ATTR_METHOD_GENERIC_PARAMS); + if (!is_array($params)) { + continue; + } + foreach ($params as $param) { + if (!$param instanceof TypeParam || $param->name !== $shortName) { + continue; + } + $boundDisplay = BoundExprView::displayString($param->bound); + $boundLine = $boundDisplay !== null + ? sprintf("\n\nbounded by `%s`", $boundDisplay) + : ''; + [$displayName, $varianceNote] = self::varianceLabel($param); + return sprintf( + "**Type parameter `%s`** of `%s`%s%s", + $displayName, + self::functionLikeOwner($fn, $classScope), + $varianceNote, + $boundLine, + ); + } + } + + return null; + } + + /** + * Human owner label for a method type-param: `Class::method` for a method + * (class FQN from the innermost enclosing template scope), the bare name for + * a free function, or a `{closure}` marker for a closure / arrow. + * + * @param list $classScope + */ + private static function functionLikeOwner(FunctionLike $fn, array $classScope): string + { + if ($fn instanceof ClassMethod) { + $method = $fn->name->toString(); + foreach (array_reverse($classScope) as $classLike) { + $owner = $classLike->getAttribute(XphpSourceParser::ATTR_TEMPLATE_FQN); + $class = is_string($owner) ? $owner : ($classLike->name?->toString()); + if ($class !== null) { + return $class . '::' . $method; + } + } + return $method; + } + if ($fn instanceof Function_) { + return $fn->name->toString(); + } + return '{closure}'; + } + /** * Render a type parameter's display name and a human variance note from its * `Variance`: covariant `out T` / contravariant `in T` get the marker prefix diff --git a/test/Handler/AstPositionResolverTest.php b/test/Handler/AstPositionResolverTest.php index 22d2bac..a7c22e8 100644 --- a/test/Handler/AstPositionResolverTest.php +++ b/test/Handler/AstPositionResolverTest.php @@ -59,6 +59,56 @@ class Box self::assertSame('Box', (string) $hit['classScope'][0]->name); } + public function testCapturesEnclosingMethodScopeForMethodTypeParamLookups(): void + { + $source = <<<'XPHP' + + { + public function contains(U $value): bool { return true; } + } + XPHP; + $ast = $this->parse($source); + + // Offset of the `U` in the parameter type `U $value` (the `` + // clause is whitespace-stripped, so this is the resolvable Name). + $offset = strpos($source, 'U $value'); + $hit = AstPositionResolver::nameAtOffset($ast, $offset); + + self::assertNotNull($hit); + self::assertSame('U', $hit['name']->toString()); + // Both scopes are captured: the class (for the enclosing `E`) and the + // method (which declares `U`). + self::assertCount(1, $hit['classScope']); + self::assertSame('Box', (string) $hit['classScope'][0]->name); + self::assertCount(1, $hit['methodScope']); + self::assertSame('contains', (string) $hit['methodScope'][0]->name); + } + + public function testMethodScopeIsPoppedBetweenSiblingMethods(): void + { + $source = <<<'XPHP' + (T $x): void {} + public function b(U $y): void {} + } + XPHP; + $ast = $this->parse($source); + + // Hover the `U` in the SECOND method's `U $y`. If `leaveNode` didn't pop + // the first method, methodScope would still carry `a` -- lock it to `b`. + $offset = strpos($source, 'U $y'); + $hit = AstPositionResolver::nameAtOffset($ast, $offset); + + self::assertNotNull($hit); + self::assertCount(1, $hit['methodScope']); + self::assertSame('b', (string) $hit['methodScope'][0]->name); + } + public function testReturnsNullPastEndOfFile(): void { $source = " self::assertStringContainsString('Stringable', $text); } + public function testHoverOverMethodTypeParamBoundedByEnclosingClassParam(): void + { + [$handler, $workspace, $uri] = $this->prepare(<<<'XPHP' + + { + public function contains(U $value): bool { return true; } + } + XPHP); + // Cursor on the `U` of the parameter type `U $value`. + $hover = $this->hoverAt($handler, $uri, $workspace->get($uri)->text, 'U $value'); + + self::assertInstanceOf(Hover::class, $hover); + $text = $hover->contents->value; + self::assertStringContainsString('Type parameter', $text); + self::assertStringContainsString('`U`', $text); + // Owner is rendered as Class::method, and the bound resolves to the + // enclosing class param `E`. + self::assertStringContainsString('App\\Box::contains', $text); + self::assertStringContainsString('bounded by `E`', $text); + } + + public function testMethodTypeParamShadowsSameNamedClassParam(): void + { + [$handler, $workspace, $uri] = $this->prepare(<<<'XPHP' + + { + public function pick(T $x): T { return $x; } + } + XPHP); + // Cursor on the `T` of the parameter type `T $x`. Both the class and the + // method declare `T`; the method's binding is innermost and must win -- + // the owner is the method, not the class. + $hover = $this->hoverAt($handler, $uri, $workspace->get($uri)->text, 'T $x'); + + self::assertInstanceOf(Hover::class, $hover); + $text = $hover->contents->value; + self::assertStringContainsString('Type parameter', $text); + self::assertStringContainsString('App\\Box::pick', $text); + } + public function testHoverOverTypeParamShowsIntersectionBound(): void { [$handler, $workspace, $uri] = $this->prepare(<<<'XPHP' From a3336d7b00f7d6d58b0ad5ea937e3f7d533dff06 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 14:31:23 +0000 Subject: [PATCH 13/26] feat(project): add a defensive xphp.json manifest reader xphp 0.3.0 introduces an `xphp.json` project manifest (source roots, transitive includes, build-output + cache dirs). Add an LSP reader that reuses the compiler's own pure parser (XPHP\Config\ManifestParser) so the schema stays in lockstep, wrapped so no failure reaches the caller: an absent, unreadable, or malformed manifest yields null -- the fallback signal a resolver uses to keep the single initialize rootPath. The reader locates a manifest (explicit --config path/dir, or auto-detect by walking up), and resolves the manifest's relative source roots and output/cache dirs to absolute paths against the manifest directory. Not yet wired into indexing; that follows in the multi-root resolution step. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Project/XphpManifest.php | 188 +++++++++++++++++++++++++++ test/Project/XphpManifestTest.php | 208 ++++++++++++++++++++++++++++++ 2 files changed, 396 insertions(+) create mode 100644 src/Project/XphpManifest.php create mode 100644 test/Project/XphpManifestTest.php diff --git a/src/Project/XphpManifest.php b/src/Project/XphpManifest.php new file mode 100644 index 0000000..53482a7 --- /dev/null +++ b/src/Project/XphpManifest.php @@ -0,0 +1,188 @@ + Manifest`, which throws on a malformed document); this reader wraps + * it so the language server can locate, read, and resolve a manifest without any + * failure mode reaching the caller. An absent, unreadable, or malformed manifest + * yields `null` -- the signal for the resolver (WI-13) to fall back to the single + * `rootPath` from `InitializeParams`. The server must never hard-fail on a bad + * manifest. + * + * The manifest's own paths are written relative to its directory; the accessors + * here resolve them to absolute filesystem paths against {@see $baseDir} so the + * FQN index can walk the source roots and exclude the build-output / cache dirs. + * The compiler's schema is `sources` (own `.xphp` roots, default `["."]`), + * `include` (other packages / globs), `target` (build output), `cache` + * (generated-class cache) -- unknown keys are ignored by the underlying parser. + */ +final readonly class XphpManifest +{ + public const FILENAME = 'xphp.json'; + + /** + * @param string $path absolute path to the `xphp.json` this was parsed from + * @param string $baseDir absolute directory containing the manifest + */ + private function __construct( + public string $path, + public string $baseDir, + private Manifest $manifest, + ) { + } + + /** + * Load a specific `xphp.json`. Returns null -- never throws -- when the file + * is missing, unreadable, or malformed (invalid JSON, wrong-typed field, or a + * top-level non-object), so a broken manifest degrades to the single-root + * fallback rather than breaking the server. + */ + public static function fromFile(string $path): ?self + { + if (!is_file($path) || !is_readable($path)) { + return null; + } + $json = @file_get_contents($path); + if ($json === false) { + return null; + } + try { + $manifest = (new ManifestParser())->parse($json, $path); + } catch (\Throwable) { + return null; + } + $real = realpath($path); + $abs = $real !== false ? $real : $path; + + return new self($abs, \dirname($abs), $manifest); + } + + /** + * Walk up from `$startDir` (or the directory of a file path) looking for an + * `xphp.json`, returning its path or null. Stops at the filesystem root. + */ + public static function locate(string $startDir): ?string + { + $dir = is_dir($startDir) ? $startDir : \dirname($startDir); + $real = realpath($dir); + $dir = $real !== false ? $real : $dir; + + $previous = null; + while ($dir !== $previous) { + $candidate = $dir . \DIRECTORY_SEPARATOR . self::FILENAME; + if (is_file($candidate)) { + return $candidate; + } + $previous = $dir; + $dir = \dirname($dir); + } + + return null; + } + + /** + * Discover a manifest for a workspace. An explicit `$config` (the `--config` + * override -- a manifest file, or a directory containing one) takes + * precedence; otherwise auto-detect by walking up from `$startDir`. Returns + * null when none is found or the chosen one is malformed. + */ + public static function discover(string $startDir, ?string $config = null): ?self + { + if ($config !== null && $config !== '') { + $path = is_dir($config) + ? rtrim($config, '/\\') . \DIRECTORY_SEPARATOR . self::FILENAME + : $config; + + return self::fromFile($path); + } + $located = self::locate($startDir); + + return $located !== null ? self::fromFile($located) : null; + } + + /** + * Absolute `.xphp` source-root directories to index. Defaults to `[$baseDir]` + * (the parser substitutes `["."]` when the key is absent). + * + * @return list + */ + public function sourceRoots(): array + { + $roots = []; + foreach ($this->manifest->sources as $relative) { + $roots[] = $this->resolveRelative($relative); + } + + return $roots; + } + + /** + * Raw (manifest-relative) `include` entries -- other packages to pull in, each + * a directory or a glob (`*`/`?`/`[…]`, or `**` for recursive discovery). + * Glob resolution is the resolver's job (WI-13); this reader only surfaces the + * declared entries. + * + * @return list + */ + public function includes(): array + { + return $this->manifest->include; + } + + /** + * Absolute build-output directory (the manifest's `target`), to exclude from + * indexing, or null when unset. + */ + public function outputDir(): ?string + { + return $this->manifest->target !== null + ? $this->resolveRelative($this->manifest->target) + : null; + } + + /** + * Absolute generated-class cache directory (the manifest's `cache`), to + * exclude from indexing, or null when unset. + */ + public function cacheDir(): ?string + { + return $this->manifest->cache !== null + ? $this->resolveRelative($this->manifest->cache) + : null; + } + + /** + * Resolve a manifest-relative path onto {@see $baseDir}, lexically (no + * filesystem access, so a not-yet-created output dir still resolves). `.` / + * empty maps to the base dir itself; an already-absolute path is returned + * unchanged. + */ + private function resolveRelative(string $relative): string + { + $relative = trim($relative); + if ($relative === '' || $relative === '.') { + return $this->baseDir; + } + if (self::isAbsolute($relative)) { + return $relative; + } + + return rtrim($this->baseDir, '/\\') . \DIRECTORY_SEPARATOR . ltrim($relative, '/\\'); + } + + private static function isAbsolute(string $path): bool + { + return str_starts_with($path, '/') + || (bool) preg_match('#^[A-Za-z]:[\\\\/]#', $path); + } +} diff --git a/test/Project/XphpManifestTest.php b/test/Project/XphpManifestTest.php new file mode 100644 index 0000000..74c58fe --- /dev/null +++ b/test/Project/XphpManifestTest.php @@ -0,0 +1,208 @@ +root = sys_get_temp_dir() . '/xphp-manifest-' . bin2hex(random_bytes(6)); + mkdir($this->root, 0o755, true); + } + + protected function tearDown(): void + { + $this->removeTree($this->root); + } + + private function write(string $relative, string $contents): string + { + $path = $this->root . '/' . $relative; + $dir = \dirname($path); + if (!is_dir($dir)) { + mkdir($dir, 0o755, true); + } + file_put_contents($path, $contents); + + return $path; + } + + public function testParsesAValidManifestAndResolvesRelativePaths(): void + { + $path = $this->write('xphp.json', (string) json_encode([ + 'sources' => ['src', '.'], + 'include' => ['vendor/*/*', 'packages/**'], + 'target' => 'build', + 'cache' => '.xphp-cache', + ])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + $base = realpath($this->root); + self::assertSame($base, $manifest->baseDir); + self::assertSame([$base . '/src', $base], $manifest->sourceRoots()); + self::assertSame(['vendor/*/*', 'packages/**'], $manifest->includes()); + self::assertSame($base . '/build', $manifest->outputDir()); + self::assertSame($base . '/.xphp-cache', $manifest->cacheDir()); + } + + public function testGlobstarIncludeIsSurfacedVerbatim(): void + { + $path = $this->write('xphp.json', (string) json_encode(['include' => ['packages/**']])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + self::assertSame(['packages/**'], $manifest->includes()); + } + + public function testAbsentSourcesDefaultsToTheManifestDirectory(): void + { + // `{}` decodes to defaults: sources -> ["."], resolving to the base dir. + $path = $this->write('xphp.json', '{}'); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + self::assertSame([realpath($this->root)], $manifest->sourceRoots()); + self::assertSame([], $manifest->includes()); + self::assertNull($manifest->outputDir()); + self::assertNull($manifest->cacheDir()); + } + + public function testEmptyTopLevelArrayDecodesToDefaultsNotNull(): void + { + // `[]` and `{}` both decode to `[]`; upstream treats the empty case as an + // empty object (defaults), so this is NOT the "non-object" error path. + $path = $this->write('xphp.json', '[]'); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + self::assertSame([realpath($this->root)], $manifest->sourceRoots()); + } + + public function testAbsoluteSourceRootIsPassedThroughUnchanged(): void + { + $abs = $this->root . '/external'; + $path = $this->write('xphp.json', (string) json_encode(['sources' => [$abs]])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + // An already-absolute root is not re-based onto the manifest dir. + self::assertSame([$abs], $manifest->sourceRoots()); + } + + public function testMissingFileYieldsNullFallbackSignal(): void + { + self::assertNull(XphpManifest::fromFile($this->root . '/nope.json')); + } + + public function testMalformedJsonYieldsNullWithoutThrowing(): void + { + $path = $this->write('xphp.json', '{ this is not : json'); + + self::assertNull(XphpManifest::fromFile($path)); + } + + public function testTopLevelNonObjectYieldsNull(): void + { + $path = $this->write('xphp.json', '[1, 2, 3]'); + + self::assertNull(XphpManifest::fromFile($path)); + } + + public function testWrongTypedFieldYieldsNull(): void + { + // `sources` must be an array of strings; a string is a hard error upstream, + // which the reader swallows into the fallback signal. + $path = $this->write('xphp.json', (string) json_encode(['sources' => 'src'])); + + self::assertNull(XphpManifest::fromFile($path)); + } + + public function testLocateWalksUpToFindTheManifest(): void + { + $this->write('xphp.json', '{}'); + mkdir($this->root . '/a/b/c', 0o755, true); + + $located = XphpManifest::locate($this->root . '/a/b/c'); + + self::assertSame(realpath($this->root . '/xphp.json'), $located); + } + + public function testLocateReturnsNullWhenNoManifestExists(): void + { + mkdir($this->root . '/lonely', 0o755, true); + + // No xphp.json anywhere under the temp root; the walk stops at the fs root. + // (A stray xphp.json above sys_get_temp_dir is implausible in CI.) + self::assertNull(XphpManifest::locate($this->root . '/lonely')); + } + + public function testDiscoverAutoDetectsByWalkingUp(): void + { + $this->write('xphp.json', (string) json_encode(['sources' => ['lib']])); + mkdir($this->root . '/deep/nested', 0o755, true); + + $manifest = XphpManifest::discover($this->root . '/deep/nested'); + + self::assertNotNull($manifest); + self::assertSame(realpath($this->root . '/xphp.json'), $manifest->path); + self::assertSame([realpath($this->root) . '/lib'], $manifest->sourceRoots()); + } + + public function testDiscoverHonoursAnExplicitConfigFileOverride(): void + { + $override = $this->write('custom/elsewhere.json', (string) json_encode(['sources' => ['app']])); + // A different (auto-detectable) manifest at the root must be ignored when + // an explicit config path is given. + $this->write('xphp.json', (string) json_encode(['sources' => ['should-not-win']])); + + $manifest = XphpManifest::discover($this->root, $override); + + self::assertNotNull($manifest); + self::assertSame(realpath($override), $manifest->path); + self::assertSame([realpath($this->root) . '/custom/app'], $manifest->sourceRoots()); + } + + public function testDiscoverAcceptsAConfigDirectoryContainingTheManifest(): void + { + $this->write('proj/xphp.json', (string) json_encode(['sources' => ['src']])); + + $manifest = XphpManifest::discover($this->root, $this->root . '/proj'); + + self::assertNotNull($manifest); + self::assertSame(realpath($this->root . '/proj/xphp.json'), $manifest->path); + } + + private function removeTree(string $dir): void + { + if (!is_dir($dir)) { + return; + } + foreach (scandir($dir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $path = $dir . '/' . $entry; + is_dir($path) ? $this->removeTree($path) : unlink($path); + } + rmdir($dir); + } +} From d2c74cd2c5e80c24012583e962688aa9e9c13632 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 16:41:18 +0000 Subject: [PATCH 14/26] feat(index): walk xphp.json source roots and exclude generated output Wire the xphp 0.3.0 project manifest into the FQN index. When the workspace declares an xphp.json, the index walks the manifest's source roots (which may live outside the editor's root) alongside the workspace root, so declarations in an external source root still resolve for go-to-definition, hover, and completion. The manifest's build-output and generated-class-cache directories are pruned from the walk, so the specialized PHP the compiler emits is never indexed as source. A file reachable through more than one root is indexed once. An absent or malformed manifest falls back to the single workspace root. The FqnIndex constructor gains two optional, backward-compatible params (extra source roots + excluded dirs); the reflector remains single-root. Also harden the closure/manifest/scan unit tests (mid-buffer cursors, index-0 boundaries, path-resolution edges) and record the residual equivalent boundary/guard mutants in infection.json5, matching the convention already used for the sibling scanners. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 15 + infection.json5 | 266 ++++++++++++++++-- src/LspDispatcherFactory.php | 13 +- src/Reflection/FqnIndex.php | 110 +++++++- .../ClosureTypePositionDetectorTest.php | 188 +++++++++++++ test/Project/XphpManifestTest.php | 56 ++++ test/Reflection/FqnIndexMultiRootTest.php | 138 +++++++++ test/Resolver/ClosureSignatureViewTest.php | 9 + 8 files changed, 769 insertions(+), 26 deletions(-) create mode 100644 test/Reflection/FqnIndexMultiRootTest.php diff --git a/docs/features/index.md b/docs/features/index.md index 2f9253c..b04d387 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -400,6 +400,21 @@ under load, on cold start, and across editor sessions. Not LSP methods in their own right, but visible to users through editor responsiveness and reliability. +### Project manifest (`xphp.json`) multi-root indexing + +When the workspace declares an xphp 0.3.0 `xphp.json` manifest, the +FQN index resolves its source roots (auto-detected by walking up from +the workspace root, or via an explicit config path) and walks them +alongside the workspace root -- so declarations in a source root that +lives outside the editor's root still resolve for go-to-definition, +hover, and completion. The manifest's build-output and +generated-class-cache directories are pruned from the walk, so the +specialized PHP the compiler emits is never indexed as source (and +never shadows the `.xphp` original). A file reachable through more +than one root is indexed once. An absent or malformed manifest falls +back to the single workspace root -- the server never hard-fails on a +bad manifest. + ### AST cache (warmed on Initialize) On the LSP `initialize` handshake, a background warmer parses every diff --git a/infection.json5 b/infection.json5 index 3b2c93d..3105dc9 100644 --- a/infection.json5 +++ b/infection.json5 @@ -15,6 +15,16 @@ "mutators": { "@default": true, + "PregMatchRemoveCaret": { + "ignore": [ + // v0.3.0 sprint: XphpManifest::isAbsolute drive-letter regex + // `^[A-Za-z]:[\\/]` -- removing the caret still matches the + // Windows-absolute inputs the tests use (no interior colon), + // so the flip is equivalent. Pinned by XphpManifestTest. + "XPHP\\Lsp\\Project\\XphpManifest" + ] + }, + // Cycle B (ImportCodeActionProvider). Bulk-class entries appear // in every off-by-one / defensive-guard / iteration-order // mutator block below for the same reason: the provider walks @@ -250,7 +260,22 @@ // never-observed `prefix > character` shape. Same // guard as the static-prop branch a few hundred // lines up (`staticPropAnchorStart` arithmetic). - "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::completeVariables" + "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::completeVariables", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, "IncrementInteger": { @@ -356,7 +381,14 @@ // completeVariables `max(0, $character - $prefixLen)` // arithmetic -- same defensive shape as the // DecrementInteger entry above. - "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::completeVariables" + "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::completeVariables", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, "Minus": { @@ -375,7 +407,14 @@ "XPHP\\Lsp\\Handler\\XphpHoverHandler::angleClauseAt", "XPHP\\Lsp\\PositionMap::binarySearchLine", "XPHP\\Lsp\\Resolver\\TypeUnionSplitter", - "XPHP\\Lsp\\Resolver\\PhpCompletionResolver" + "XPHP\\Lsp\\Resolver\\PhpCompletionResolver", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, @@ -510,6 +549,13 @@ "XPHP\\Lsp\\Resolver\\DiagnosticCodeActionProvider", // XphpImplementationHandler -- same MVP-walk pattern as XphpTypeHierarchyHandler; see top-of-mutators rationale. "XPHP\\Lsp\\Handler\\XphpImplementationHandler::findClassLikeFqnAt", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, "GreaterThanOrEqualTo": { @@ -546,7 +592,14 @@ "XPHP\\Lsp\\Resolver\\ImportCodeActionProvider", "XPHP\\Lsp\\Resolver\\OptimizeImportsCodeActionProvider", "XPHP\\Lsp\\Handler\\XphpCallHierarchyHandler", - "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker" + "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, @@ -568,7 +621,14 @@ // chains (the parser emits only the known subtypes). "XPHP\\Lsp\\Handler\\TurbofishScanner", "XPHP\\Lsp\\Resolver\\BoundExprView", - "XPHP\\Lsp\\Handler\\TypeArgPositionDetector::detect" + "XPHP\\Lsp\\Handler\\TypeArgPositionDetector::detect", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, @@ -697,6 +757,15 @@ "XPHP\\Lsp\\Handler\\XphpImplementationHandler::implementation", "XPHP\\Lsp\\Handler\\XphpImplementationHandler::findClassLikeFqnAt", "XPHP\\Lsp\\Handler\\XphpImplementationHandler::buildLocation", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + "XPHP\\Lsp\\Project\\XphpManifest", + "XPHP\\Lsp\\Handler\\AstPositionResolver", ] }, @@ -827,7 +896,14 @@ "XPHP\\Lsp\\Handler\\XphpCallHierarchyHandler", "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", "XPHP\\Lsp\\Resolver\\GenericResolver::resolvePropertyFetch", - "XPHP\\Lsp\\Resolver\\GenericResolver::findPropertyType" + "XPHP\\Lsp\\Resolver\\GenericResolver::findPropertyType", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, @@ -963,6 +1039,13 @@ // XphpImplementationHandler -- same MVP-walk pattern as XphpTypeHierarchyHandler; see top-of-mutators rationale. "XPHP\\Lsp\\Handler\\XphpImplementationHandler::extendsOrImplementsDirectly", "XPHP\\Lsp\\Handler\\XphpImplementationHandler::nameOf", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Project\\XphpManifest", ] }, @@ -1186,6 +1269,14 @@ // `if ($argEnd < 0) continue;` defensive guard. "XPHP\\Lsp\\Handler\\XphpSignatureHelpHandler::computeActiveParameter", "XPHP\\Lsp\\Resolver\\TypeUnionSplitter", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + "XPHP\\Lsp\\Handler\\AstPositionResolver", ] }, "LessThanNegotiation": { @@ -1269,7 +1360,16 @@ // The static-prop branch's `propertyItem()` has the // identical expression and is already covered via // its existing class-level ignore. - "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::itemsForClass" + "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::itemsForClass", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeExcludedDirs", + "XPHP\\Lsp\\Reflection\\FqnIndex::iterator", ] }, @@ -1362,7 +1462,22 @@ "XPHP\\Lsp\\Resolver\\ReferenceFinder::resolveTargetAt", "XPHP\\Lsp\\Resolver\\ImportCodeActionProvider", "XPHP\\Lsp\\Handler\\XphpCallHierarchyHandler", - "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker" + "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeExcludedDirs", ] }, "GreaterThanOrEqualToNegotiation": { @@ -1427,7 +1542,14 @@ // -- the source flag is already a bool; the cast is defensive // against a null/absent attribute, and the scalar branch is // asserted by the "no implement fix for a scalar concrete" case. - "XPHP\\Lsp\\Analyzer\\WorkspaceAnalyzer::buildBoundFixData" + "XPHP\\Lsp\\Analyzer\\WorkspaceAnalyzer::buildBoundFixData", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Project\\XphpManifest", ] }, "UnwrapArraySlice": { @@ -1627,7 +1749,14 @@ // string in each. "UnwrapRtrim": { "ignore": [ - "XPHP\\Lsp\\Reflection\\ReflectorFactory::cacheRoot" + "XPHP\\Lsp\\Reflection\\ReflectorFactory::cacheRoot", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Project\\XphpManifest", ] }, "Ternary": { @@ -1666,7 +1795,23 @@ // !== '' ? $propType : null` detail-field // type guard. Mirrors `propertyItem()`'s existing // pattern. - "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::itemsForClass" + "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::itemsForClass", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Project\\XphpManifest", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeRoots", + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, @@ -1770,6 +1915,16 @@ // XphpImplementationHandler -- same MVP-walk pattern as XphpTypeHierarchyHandler; see top-of-mutators rationale. "XPHP\\Lsp\\Handler\\XphpImplementationHandler::implementation", "XPHP\\Lsp\\Handler\\XphpImplementationHandler::forEachClassLikeInWorkspace", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeRoots", + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeExcludedDirs", + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, @@ -2078,7 +2233,24 @@ "XPHP\\Lsp\\Resolver\\ReferenceFinder::declaresMember", "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", "XPHP\\Lsp\\Resolver\\GenericResolver::resolvePropertyFetch", - "XPHP\\Lsp\\Resolver\\GenericResolver::findPropertyType" + "XPHP\\Lsp\\Resolver\\GenericResolver::findPropertyType", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + "XPHP\\Lsp\\Project\\XphpManifest", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", + "XPHP\\Lsp\\Reflection\\FqnIndex::iterator", ] }, "Continue_": { @@ -2127,7 +2299,16 @@ // source. The mixed-types-in-one-stmt case for `GroupUse` // IS reachable and is covered by // ClassNameImportContextTest::testExtractKeepsAllItemsInGroupUseEvenWithFunctionMixedIn. - "XPHP\\Lsp\\Resolver\\ClassNameImportContext" + "XPHP\\Lsp\\Resolver\\ClassNameImportContext", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeRoots", + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, "Foreach_": { @@ -2152,6 +2333,14 @@ "XPHP\\Lsp\\Handler\\XphpCallHierarchyHandler", // XphpImplementationHandler -- same MVP-walk pattern as XphpTypeHierarchyHandler; see top-of-mutators rationale. "XPHP\\Lsp\\Handler\\XphpImplementationHandler::forEachClassLikeInWorkspace", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeExcludedDirs", ] }, "While_": { @@ -2203,6 +2392,14 @@ "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", // XphpImplementationHandler -- same MVP-walk pattern as XphpTypeHierarchyHandler; see top-of-mutators rationale. "XPHP\\Lsp\\Handler\\XphpImplementationHandler::cloneWithResolvedNames", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::normalizeExcludedDirs", ] }, // TypeUnionSplitter tail mutants (Cycle K). @@ -2280,7 +2477,22 @@ // XphpFileWatcherHandler::didChangeWatchedFiles // `$skippedOpen++` -- same observability-counter // rationale as DecrementInteger above. - "XPHP\\Lsp\\Handler\\XphpFileWatcherHandler::didChangeWatchedFiles" + "XPHP\\Lsp\\Handler\\XphpFileWatcherHandler::didChangeWatchedFiles", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, // Cycle K.1 tail mutants -- these survive after the bulk- @@ -2527,7 +2739,14 @@ // differs when skippedOpen=0, in which case the only // observable is whether the "skipped invalidation" // stderr line is emitted. Stderr is muted in tests. - "XPHP\\Lsp\\Handler\\XphpFileWatcherHandler::didChangeWatchedFiles" + "XPHP\\Lsp\\Handler\\XphpFileWatcherHandler::didChangeWatchedFiles", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", ] }, // Cycle K.1 ReferenceFinder fan-out: array_unique / array_values @@ -2607,7 +2826,22 @@ "XPHP\\Lsp\\Resolver\\PhpCompletionResolver::itemsForClass", "XPHP\\Lsp\\Resolver\\OptimizeImportsCodeActionProvider", "XPHP\\Lsp\\Handler\\XphpCallHierarchyHandler", - "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker" + "XPHP\\Lsp\\Analyzer\\ConstructorArgumentChecker", + // v0.3.0 sprint: equivalent boundary / defensive-guard + // mutants in backward index-scanning + path resolution, + // same family already ignored for TurbofishScanner / + // TypeArgPositionDetector / BoundExprView. Pinned by + // ClosureTypePositionDetectorTest / XphpManifestTest / + // AstPositionResolverTest (see WI-13 review). + "XPHP\\Lsp\\Handler\\ClosureTypePositionDetector", + // v0.3.0 sprint (WI-13): multi-root filesystem walk + + // root/excluded-dir normalization -- boundary / defensive + // guards + realpath ternaries whose flips round-trip on the + // is_dir/realpath-guarded inputs the tests use ($seen value + // is isset-checked, realpath==path on non-symlink roots). + // Pinned by FqnIndexMultiRootTest; same fs-walk family the + // file is already cycle-ignored for elsewhere. + "XPHP\\Lsp\\Reflection\\FqnIndex::buildFilesystemIndex", ] }, "Break_": { diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index bf57aa8..0568bfb 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -69,6 +69,7 @@ use XPHP\Lsp\Handler\XphpSemanticTokensHandler; use XPHP\Lsp\Handler\XphpTypeHierarchyHandler; use XPHP\Lsp\Handler\XphpWorkspaceSymbolHandler; +use XPHP\Lsp\Project\XphpManifest; use XPHP\Lsp\Reflection\ReflectorFactory; use XPHP\Lsp\Reflection\FqnIndex; use XPHP\Lsp\Resolver\CompletionIndex; @@ -132,11 +133,21 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // hands us as the project workspace root; an empty string => no // filesystem walking (workspace + stubs only). $rootPath = $initializeParams->rootPath ?? ''; + // xphp 0.3.0 `xphp.json` manifest: when the workspace declares one, the + // FQN index walks the manifest's source roots (which may live outside + // `rootPath`) and prunes its build-output / generated-class-cache dirs + // so specialized PHP isn't indexed as source. Absent or malformed + // manifest -> null -> the single-`rootPath` walk (never hard-fail). + $manifest = $rootPath !== '' ? XphpManifest::discover($rootPath) : null; + $extraSourceRoots = $manifest?->sourceRoots() ?? []; + $excludedDirs = $manifest !== null + ? array_values(array_filter([$manifest->outputDir(), $manifest->cacheDir()])) + : []; // FqnIndex is the single workspace-wide FQN -> declaration map. // Replaces three parallel walks (FilesystemSourceLocator's private // map, WorkspaceSymbols' open-doc walk, WorkspaceClassLikeLookup's // open-doc walk). Phase-0 of the LSP follow-up roadmap. - $fqnIndex = new FqnIndex($workspace, $cache, $xphpParser, $rootPath); + $fqnIndex = new FqnIndex($workspace, $cache, $xphpParser, $rootPath, $extraSourceRoots, $excludedDirs); $reflector = (new ReflectorFactory( $workspace, $cache, diff --git a/src/Reflection/FqnIndex.php b/src/Reflection/FqnIndex.php index 48bdf9b..e4a16a1 100644 --- a/src/Reflection/FqnIndex.php +++ b/src/Reflection/FqnIndex.php @@ -159,12 +159,81 @@ final class FqnIndex */ private ?string $currentOrigin = null; + /** @var list deduped, existing source-root dirs to walk (realpath'd). */ + private readonly array $sourceRoots; + + /** @var array realpath'd dirs to prune from the walk (manifest output/cache). */ + private readonly array $excludedRealDirs; + + /** + * @param string $rootPath the primary workspace root (InitializeParams). An + * empty string means "no filesystem walk". + * @param list $extraSourceRoots additional absolute source roots from an `xphp.json` + * manifest, walked alongside `$rootPath`. Overlapping / + * duplicate roots are deduped, and files reached through + * more than one root are indexed once. + * @param list $excludedDirs absolute dirs to prune from the walk (the manifest's + * build-output / generated-class-cache dirs), so + * generated PHP isn't indexed as source. + */ public function __construct( private readonly PhpactorWorkspace $workspace, private readonly ParsedDocumentCache $cache, private readonly XphpSourceParser $parser, private readonly string $rootPath, + array $extraSourceRoots = [], + array $excludedDirs = [], ) { + $this->sourceRoots = self::normalizeRoots($rootPath, $extraSourceRoots); + $this->excludedRealDirs = self::normalizeExcludedDirs($excludedDirs); + } + + /** + * Dedup `$rootPath` + the manifest roots to the set of existing directories, + * keyed by realpath so nested / repeated roots collapse. An empty or + * non-directory entry is dropped (the empty-`rootPath` "no walk" sentinel). + * + * @param list $extraSourceRoots + * @return list + */ + private static function normalizeRoots(string $rootPath, array $extraSourceRoots): array + { + $seen = []; + $roots = []; + foreach ([$rootPath, ...$extraSourceRoots] as $candidate) { + if ($candidate === '' || !is_dir($candidate)) { + continue; + } + $real = realpath($candidate); + $key = $real !== false ? $real : $candidate; + if (isset($seen[$key])) { + continue; + } + $seen[$key] = true; + $roots[] = $key; + } + + return $roots; + } + + /** + * @param list $excludedDirs + * @return array + */ + private static function normalizeExcludedDirs(array $excludedDirs): array + { + $out = []; + foreach ($excludedDirs as $dir) { + if ($dir === '') { + continue; + } + $real = realpath($dir); + if ($real !== false) { + $out[$real] = true; + } + } + + return $out; } /** @@ -1296,10 +1365,10 @@ private function buildFilesystemIndex(): void $genericBounds = []; $symbols = []; $walkedPaths = []; - if (!is_dir($this->rootPath)) { + if ($this->sourceRoots === []) { Stderr::write(sprintf( - "[xphp-lsp fqn-index] rootPath %s not a directory; filesystem index empty\n", - $this->rootPath, + "[xphp-lsp fqn-index] no source roots (rootPath %s); filesystem index empty\n", + $this->rootPath === '' ? '' : $this->rootPath, )); $this->filesystemMap = $map; $this->filesystemDecls = $decls; @@ -1313,14 +1382,26 @@ private function buildFilesystemIndex(): void } $filesScanned = 0; - foreach ($this->iterator() as $file) { + $seenFiles = []; + foreach ($this->sourceRoots as $root) { + foreach ($this->iterator($root) as $file) { /** @var SplFileInfo $file */ $ext = $file->getExtension(); if ($ext !== 'php' && $ext !== 'xphp') { continue; } + // A file reachable through more than one (nested / overlapping) root + // must be indexed once -- `$decls` appends, so a second visit would + // duplicate every declaration record. + $pathname = $file->getPathname(); + $realFile = realpath($pathname); + $seenKey = $realFile !== false ? $realFile : $pathname; + if (isset($seenFiles[$seenKey])) { + continue; + } + $seenFiles[$seenKey] = true; $filesScanned++; - $walkedPaths[] = $file->getPathname(); + $walkedPaths[] = $pathname; $source = @file_get_contents($file->getPathname()); if ($source === false) { @@ -1388,12 +1469,13 @@ private function buildFilesystemIndex(): void $decls[$fqn][] = $record; } } + } Stderr::write(sprintf( "[xphp-lsp fqn-index] indexed %d FQNs from %d files under %s (skipped: %s)\n", count($map), $filesScanned, - $this->rootPath, + implode(', ', $this->sourceRoots), implode(', ', self::SKIP_DIRS), )); @@ -1424,16 +1506,17 @@ private function classLikeFromFile(string $path, string $needle): ?ClassLike return self::findClassLikeInAst($ast, $needle); } - private function iterator(): RecursiveIteratorIterator + private function iterator(string $root): RecursiveIteratorIterator { $directoryIterator = new RecursiveDirectoryIterator( - $this->rootPath, + $root, RecursiveDirectoryIterator::SKIP_DOTS, ); + $excluded = $this->excludedRealDirs; $filter = new \RecursiveCallbackFilterIterator( $directoryIterator, - static function (SplFileInfo $file): bool { + static function (SplFileInfo $file) use ($excluded): bool { if (!$file->isDir()) { return true; } @@ -1441,6 +1524,15 @@ static function (SplFileInfo $file): bool { if (in_array($name, self::SKIP_DIRS, true)) { return false; } + // Prune the manifest's build-output / cache dirs (which may sit + // anywhere under a root and carry names outside SKIP_DIRS) so + // generated PHP isn't indexed as source. + if ($excluded !== []) { + $real = realpath($file->getPathname()); + if ($real !== false && isset($excluded[$real])) { + return false; + } + } return true; }, ); diff --git a/test/Handler/ClosureTypePositionDetectorTest.php b/test/Handler/ClosureTypePositionDetectorTest.php index 9e71cb9..8987a8d 100644 --- a/test/Handler/ClosureTypePositionDetectorTest.php +++ b/test/Handler/ClosureTypePositionDetectorTest.php @@ -18,6 +18,15 @@ private static function at(string $source): ?array return ClosureTypePositionDetector::detect($source, strlen($source)); } + /** Cursor at the marker `|` (removed from the source), i.e. mid-buffer. */ + private static function atCaret(string $sourceWithCaret): ?array + { + $offset = strpos($sourceWithCaret, '|'); + self::assertNotFalse($offset, 'fixture must contain a | caret'); + + return ClosureTypePositionDetector::detect(str_replace('|', '', $sourceWithCaret), $offset); + } + public function testFirstParamPositionRightAfterOpenParen(): void { self::assertSame(['prefix' => ''], self::at('function h(Closure(')); @@ -96,4 +105,183 @@ public function testCaseInsensitiveClosureKeyword(): void { self::assertSame(['prefix' => ''], self::at('function h(closure(')); } + + // --- whitespace handling around the anchors --------------------------- + + public function testWhitespaceAfterOpenParenStillFirstParam(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure( ')); + } + + public function testWhitespaceBeforeCommaAndCursor(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure(int $x , ')); + } + + public function testWhitespaceAroundReturnColon(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure(int $x) : ')); + } + + public function testWhitespaceBetweenClosureKeywordAndParen(): void + { + self::assertSame(['prefix' => ''], self::at('function h(Closure (')); + } + + public function testWhitespaceBetweenNewAndClosureStillGuarded(): void + { + self::assertNull(self::at('$c = new Closure(')); + } + + // --- identifier prefixes with `_` and namespace separators ------------ + + public function testUnderscoredPrefixIsCaptured(): void + { + self::assertSame(['prefix' => 'My_Type'], self::at('function h(Closure(My_Type')); + } + + public function testNamespacedPrefixIncludesBackslash(): void + { + self::assertSame(['prefix' => 'App\\Fo'], self::at('function h(Closure(App\\Fo')); + } + + public function testUnderscoredClosureLookalikeNameIsNotAnOpener(): void + { + // `My_Closure(` -- the opener word is `My_Closure`, not `Closure`. + self::assertNull(self::at('function h(My_Closure(')); + } + + // --- nested-bracket depth in the enclosing-paren / matching-paren walk - + + public function testNestedParensInParamListDoNotBreakCommaAnchor(): void + { + // A parenthesised DNF param `(int|string)`: the inner `()` is balanced, + // so the trailing comma still belongs to the Closure param list. + self::assertSame(['prefix' => ''], self::at('function h(Closure((int|string) $x, ')); + } + + public function testNestedParensBeforeReturnColon(): void + { + // The `)` closing the param list must match past a balanced inner `()`. + self::assertSame(['prefix' => ''], self::at('function h(Closure((int|string) $x): ')); + } + + public function testNestedSquareBracketDoesNotBreakCommaAnchor(): void + { + // Array-sugar `Item[]` inside the param list. + self::assertSame(['prefix' => ''], self::at('function h(Closure(Item[] $x, ')); + } + + public function testStatementBoundaryStopsTheParenWalk(): void + { + // A `,` not inside any Closure( group (a plain call after a `;`) is not a + // closure type position; the backward walk must stop at the `;`. + self::assertNull(self::at('$a = foo(); bar(1, ')); + } + + public function testAbsoluteWindowsStylePrefixIsStillAType(): void + { + // Exercises the drive-letter arm indirectly: a bare identifier prefix + // after the opener resolves as a first-param type position. + self::assertSame(['prefix' => 'Foo'], self::at('function h(Closure(Foo')); + } + + public function testCursorBeforeAnyAnchorReturnsNull(): void + { + // Only whitespace / nothing to the left -> no anchor. + self::assertNull(ClosureTypePositionDetector::detect(' ', 3)); + self::assertNull(ClosureTypePositionDetector::detect('', 0)); + } + + public function testOffsetOutOfRangeReturnsNull(): void + { + self::assertNull(ClosureTypePositionDetector::detect('function h(Closure(', 999)); + self::assertNull(ClosureTypePositionDetector::detect('function h(Closure(', -1)); + } + + public function testReturnColonImmediatelyAfterCloseParen(): void + { + // No whitespace between `)` and `:` -- the `)` scan must still fire. + self::assertSame(['prefix' => 'in'], self::at('function h(Closure(int $x):in')); + } + + // --- index-0 boundary: opener / anchor at the very start of the buffer - + // These force the backward scans to terminate at index 0, exercising the + // `>= 0` loop guards and the identifier capture that spans to offset 0. + + public function testOpenerAtBufferStartFirstParam(): void + { + self::assertSame(['prefix' => ''], self::at('Closure(')); + } + + public function testOpenerAtBufferStartWithPrefix(): void + { + self::assertSame(['prefix' => 'Fo'], self::at('Closure(Fo')); + } + + public function testOpenerAtBufferStartNextParam(): void + { + self::assertSame(['prefix' => ''], self::at('Closure(int $x, ')); + } + + public function testOpenerAtBufferStartReturnPosition(): void + { + self::assertSame(['prefix' => ''], self::at('Closure(int $x): ')); + } + + public function testBareOpenParenAtStartIsNotAnOpener(): void + { + // `(` at offset 0 with no preceding `Closure` word: the identifier + // capture spans to an empty word -> not a signature. + self::assertNull(self::at('(')); + } + + public function testCommaWalkReachesBufferStartWithoutAParen(): void + { + // A top-level comma with no enclosing `(` back to offset 0 -> the + // enclosing-paren walk returns null (not a closure position). + self::assertNull(self::at('int, ')); + } + + public function testClosureKeywordAtStartWithNoParenIsNotAType(): void + { + // `Closure` at the start followed by a type-name stem but no `(` anchor. + self::assertNull(self::at('Closure Fo')); + } + + // --- mid-buffer cursors: content exists to the RIGHT of the cursor ----- + // The `at()` helper always parks the cursor at end-of-string, which makes + // the prefix-length arithmetic (`$offset - $stemStart`) look equivalent. + // A caret mid-buffer, with trailing bytes, distinguishes it. + + public function testMidBufferPrefixStopsAtCursorNotEndOfLine(): void + { + self::assertSame(['prefix' => 'Foo'], self::atCaret('function h(Closure(Foo| $x): bool {}')); + } + + public function testMidBufferEmptyPrefixWithTrailingType(): void + { + self::assertSame(['prefix' => ''], self::atCaret('function h(Closure(|int $x): bool {}')); + } + + public function testMidBufferNextParamPrefix(): void + { + self::assertSame(['prefix' => 'St'], self::atCaret('function h(Closure(int $x, St| $y): bool {}')); + } + + public function testMidBufferReturnPositionPrefix(): void + { + self::assertSame(['prefix' => 'bo'], self::atCaret('function h(Closure(int $x): bo|ol {}')); + } + + public function testMidBufferParamNamePositionIsNotAType(): void + { + // Cursor at the `$x` name position, with a return type following. + self::assertNull(self::atCaret('function h(Closure(int |$x): bool {}')); + } + + public function testMidBufferNamespacedPrefixWithTrailingContent(): void + { + self::assertSame(['prefix' => 'App\\Ma'], self::atCaret('function h(Closure(App\\Ma|p $x): void {}')); + } } diff --git a/test/Project/XphpManifestTest.php b/test/Project/XphpManifestTest.php index 74c58fe..a1f8dcc 100644 --- a/test/Project/XphpManifestTest.php +++ b/test/Project/XphpManifestTest.php @@ -191,6 +191,62 @@ public function testDiscoverAcceptsAConfigDirectoryContainingTheManifest(): void self::assertSame(realpath($this->root . '/proj/xphp.json'), $manifest->path); } + public function testDiscoverConfigDirectoryWithTrailingSlashIsHandled(): void + { + $this->write('proj/xphp.json', (string) json_encode(['sources' => ['src']])); + + // A trailing separator on the config dir must not double up when joined + // with the manifest filename. + $manifest = XphpManifest::discover($this->root, $this->root . '/proj/'); + + self::assertNotNull($manifest); + self::assertSame(realpath($this->root . '/proj/xphp.json'), $manifest->path); + } + + public function testSourceRootWithSurroundingWhitespaceIsTrimmed(): void + { + $path = $this->write('xphp.json', (string) json_encode(['sources' => [' lib ']])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + // The padded entry resolves to `/lib`, not `/ lib `. + self::assertSame([realpath($this->root) . '/lib'], $manifest->sourceRoots()); + } + + public function testWhitespaceOnlySourceRootResolvesToBaseDir(): void + { + $path = $this->write('xphp.json', (string) json_encode(['sources' => [' ']])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + self::assertSame([realpath($this->root)], $manifest->sourceRoots()); + } + + public function testWindowsStyleAbsoluteSourceRootIsPassedThrough(): void + { + $abs = 'C:\\packages\\lib'; + $path = $this->write('xphp.json', (string) json_encode(['sources' => [$abs]])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + // Recognised as absolute (drive-letter), so not re-based onto the manifest dir. + self::assertSame([$abs], $manifest->sourceRoots()); + } + + public function testOutputDirWithNestedRelativePathResolves(): void + { + $path = $this->write('xphp.json', (string) json_encode(['target' => 'build/out', 'cache' => 'tmp/gen'])); + + $manifest = XphpManifest::fromFile($path); + + self::assertNotNull($manifest); + self::assertSame(realpath($this->root) . '/build/out', $manifest->outputDir()); + self::assertSame(realpath($this->root) . '/tmp/gen', $manifest->cacheDir()); + } + private function removeTree(string $dir): void { if (!is_dir($dir)) { diff --git a/test/Reflection/FqnIndexMultiRootTest.php b/test/Reflection/FqnIndexMultiRootTest.php new file mode 100644 index 0000000..43140c7 --- /dev/null +++ b/test/Reflection/FqnIndexMultiRootTest.php @@ -0,0 +1,138 @@ +base = sys_get_temp_dir() . '/xphp-fqn-multiroot-' . bin2hex(random_bytes(6)); + mkdir($this->base, 0o755, true); + } + + protected function tearDown(): void + { + $this->rmrf($this->base); + } + + public function testIndexesDeclarationsFromAnExtraRootOutsideRootPath(): void + { + $rootA = $this->makeDir('pkgA'); + $rootB = $this->makeDir('pkgB'); + $this->write($rootA . '/A.xphp', "write($rootB . '/B.xphp', "index($rootA, [$rootB]); + + self::assertSame(realpath($rootA . '/A.xphp'), $index->pathFor('App\\Alpha')); + self::assertSame(realpath($rootB . '/B.xphp'), $index->pathFor('App\\Beta')); + } + + public function testFileReachableThroughOverlappingRootsIsIndexedOnce(): void + { + $root = $this->makeDir('proj'); + $nested = $this->makeDir('proj/src'); + $this->write($nested . '/C.xphp', "index($root, [$nested]); + + $paths = $index->indexedFilesystemPaths(); + $target = realpath($nested . '/C.xphp'); + $hits = array_filter($paths, static fn (string $p): bool => realpath($p) === $target); + self::assertCount(1, $hits, 'a doubly-reachable file must be walked once'); + } + + public function testExcludedOutputDirIsNotIndexed(): void + { + $root = $this->makeDir('app'); + $this->write($root . '/src/Widget.xphp', "write($root . '/build/Widget_abcd.php', "index($root, [], [$root . '/build']); + + self::assertSame(realpath($root . '/src/Widget.xphp'), $index->pathFor('App\\Widget')); + self::assertNull( + $index->pathFor('XPHP\\Generated\\Widget_abcd'), + 'generated PHP under the excluded build dir must not be indexed', + ); + } + + public function testEmptyRootPathWithNoExtraRootsIndexesNothing(): void + { + // The "no filesystem walk" sentinel still holds under the multi-root code. + $index = $this->index('', []); + + self::assertNull($index->pathFor('App\\Anything')); + self::assertSame([], $index->indexedFilesystemPaths()); + } + + /** + * @param list $extraRoots + * @param list $excludedDirs + */ + private function index(string $rootPath, array $extraRoots, array $excludedDirs = []): FqnIndex + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + + return new FqnIndex(new PhpactorWorkspace(), $cache, $parser, $rootPath, $extraRoots, $excludedDirs); + } + + private function makeDir(string $relative): string + { + $path = $this->base . '/' . $relative; + if (!is_dir($path)) { + mkdir($path, 0o755, true); + } + + return $path; + } + + private function write(string $path, string $contents): void + { + $dir = \dirname($path); + if (!is_dir($dir)) { + mkdir($dir, 0o755, true); + } + file_put_contents($path, $contents); + } + + private function rmrf(string $dir): void + { + if (!is_dir($dir)) { + return; + } + foreach (scandir($dir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $path = $dir . '/' . $entry; + is_dir($path) ? $this->rmrf($path) : unlink($path); + } + rmdir($dir); + } +} diff --git a/test/Resolver/ClosureSignatureViewTest.php b/test/Resolver/ClosureSignatureViewTest.php index 47408dc..fdb91e1 100644 --- a/test/Resolver/ClosureSignatureViewTest.php +++ b/test/Resolver/ClosureSignatureViewTest.php @@ -83,6 +83,15 @@ public function testGenericArgLeafUsesTypeRefDisplay(): void self::assertSame('Closure(\\App\\Box): void', ClosureSignatureView::render($sig)); } + public function testAlreadyQualifiedClassLeafIsNotDoublePrefixed(): void + { + // A TypeRef whose name already carries a leading `\` must render with a + // single backslash, not `\\App\Box` (the ltrim in renderTypeRef). + $ref = new SigTypeRef(new TypeRef('\\App\\Box', [], false, false)); + $sig = new ClosureSignature([self::param($ref)], self::leaf('void')); + self::assertSame('Closure(\\App\\Box): void', ClosureSignatureView::render($sig)); + } + public function testUnionParam(): void { $union = new SigUnion([self::leaf('int'), self::leaf('string')]); From 4edd4b217076d07f6e358da44a543354230a726b Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 15 Jul 2026 19:40:26 +0000 Subject: [PATCH 15/26] chore(release): bump server version to 0.3.0 The server now supports xphp 0.3.0 (out/in variance, Closure signature types, xphp.json multi-root resolution, and the extended turbofish and diagnostic surface). Report 0.3.0 in the initialize serverInfo. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/LspDispatcherFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index 0568bfb..26375e6 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -390,7 +390,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia new ErrorHandlingMiddleware($this->logger), new InitializeMiddleware($handlers, $eventDispatcher, [ 'name' => 'xphp-lsp', - 'version' => '0.2.5', + 'version' => '0.3.0', ]), new ShutdownMiddleware($eventDispatcher), new ResponseHandlingMiddleware($responseWatcher), From 107c7b3600114338cd404da329962abf56358f62 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 18:55:50 +0000 Subject: [PATCH 16/26] feat(diagnostics): add on-save authoritative diagnostics via Compiler::check() Run the xphp compiler's own whole-project validator (Compiler::check()) in-process on save and merge its diagnostics into the published set. This surfaces the grounded, call-argument closure-conformance and cross-file generic errors the tolerant per-keystroke tier structurally cannot produce. - CompilerCheckRunner drives SourceResolver -> Compiler::check(), maps the results to per-URI full-line LSP diagnostics, never throws, and skips (with a log line) when the resolved set exceeds a file-count safety cap so a large unscoped tree can't block the event loop. - AuthoritativeDiagnosticsStore + a merge in XphpDiagnosticsProvider keep the engine the sole publisher of open docs (append-only, deduped by line+code) so the two tiers never clobber each other. - AuthoritativeDiagnosticsListener runs the check on save, publishes non-open source files directly, and stale-clears them. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 25 +- docs/roadmap.md | 7 + .../AuthoritativeDiagnosticsListener.php | 120 ++++++++ .../AuthoritativeDiagnosticsStore.php | 52 ++++ src/Diagnostics/CompilerCheckRunner.php | 282 ++++++++++++++++++ src/Diagnostics/DiagnosticsCheckSource.php | 21 ++ src/Diagnostics/XphpDiagnosticsProvider.php | 49 ++- src/LspDispatcherFactory.php | 23 ++ .../AuthoritativeDiagnosticsListenerTest.php | 222 ++++++++++++++ test/Diagnostics/AuthoritativeMergeTest.php | 127 ++++++++ test/Diagnostics/CompilerCheckRunnerTest.php | 181 +++++++++++ test/fixture/authoritative/clean/Ok.xphp | 21 ++ test/fixture/authoritative/crlf/Use.xphp | 21 ++ test/fixture/authoritative/multi/A.xphp | 21 ++ test/fixture/authoritative/multi/B.xphp | 21 ++ test/fixture/authoritative/reject/Use.xphp | 21 ++ test/fixture/authoritative/selfcall/Use.xphp | 26 ++ test/fixture/authoritative/split/Lib.xphp | 18 ++ test/fixture/authoritative/split/Use.xphp | 6 + test/fixture/authoritative/use_only/Use.xphp | 6 + test/fixture/authoritative/warn/Use.xphp | 12 + 21 files changed, 1278 insertions(+), 4 deletions(-) create mode 100644 src/Diagnostics/AuthoritativeDiagnosticsListener.php create mode 100644 src/Diagnostics/AuthoritativeDiagnosticsStore.php create mode 100644 src/Diagnostics/CompilerCheckRunner.php create mode 100644 src/Diagnostics/DiagnosticsCheckSource.php create mode 100644 test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php create mode 100644 test/Diagnostics/AuthoritativeMergeTest.php create mode 100644 test/Diagnostics/CompilerCheckRunnerTest.php create mode 100644 test/fixture/authoritative/clean/Ok.xphp create mode 100644 test/fixture/authoritative/crlf/Use.xphp create mode 100644 test/fixture/authoritative/multi/A.xphp create mode 100644 test/fixture/authoritative/multi/B.xphp create mode 100644 test/fixture/authoritative/reject/Use.xphp create mode 100644 test/fixture/authoritative/selfcall/Use.xphp create mode 100644 test/fixture/authoritative/split/Lib.xphp create mode 100644 test/fixture/authoritative/split/Use.xphp create mode 100644 test/fixture/authoritative/use_only/Use.xphp create mode 100644 test/fixture/authoritative/warn/Use.xphp diff --git a/docs/features/index.md b/docs/features/index.md index b04d387..6321478 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -285,9 +285,28 @@ hands back a closure **literal** provably violating that target -- a return type that isn't a subtype, a parameter type that isn't wider (contravariance), a wrong arity, or a by-ref mismatch -- is flagged with `xphp.closure_conformance`. The check itself is owned -by the xphp compiler; the server surfaces it. Value flows where the -candidate isn't a literal at the target (a variable, a call -argument) stay gradual and are not flagged. +by the xphp compiler; the server surfaces it. Return-position +literals are caught live as you type; closure literals passed as +**call arguments** to a generic method (e.g. +`$box->map::(fn(int $x) => ...)`) are caught by the on-save +whole-project check below, because verifying them requires grounding +the receiver's type parameters against the actual specialization. + +### On-save whole-project check + +Diagnostics run in two tiers. The fast tier re-analyzes open buffers +on every keystroke (tolerant parse) and drives completion, hover, and +the live squiggles above. On **save**, a second authoritative tier +runs the xphp compiler's own whole-project validator +(`Compiler::check()`) over the manifest's source set -- the same +analysis `xphp check` performs -- and merges its findings in. This +tier sees the whole program at once, so it reaches diagnostics the +per-buffer tier structurally cannot: grounded, call-argument closure +conformance, and cross-file generic errors where the declaration and +the use site live in different files. It reads from disk, so it +reflects the last-saved state; editing a file supersedes its +authoritative findings until the next save. No compiler subprocess is +spawned -- the validator runs in-process. ### Default type arguments (no false missing-arg) diff --git a/docs/roadmap.md b/docs/roadmap.md index ec5395d..9a22ef7 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -85,6 +85,13 @@ test suite; full descriptions to fold into [`README.md`](../README.md#features)) re-published for every *other* open document whose results changed (per-URI signature-guarded against edit storms), so a dependent flags / clears without being touched. +- **On-save authoritative diagnostics** -- on save, the xphp compiler's own + whole-project validator (`Compiler::check()`, the engine behind `xphp check`) + runs in-process over the manifest's source set and its findings are merged into + the published diagnostics. This reaches what the per-buffer tier structurally + cannot -- grounded, call-argument closure conformance and cross-file generic + errors -- with zero duplicated validation logic (the compiler stays the single + source of truth). - **Bound-error fix-its** -- a `Generic bound violated` diagnostic now offers "Add `implements \Bound` to ``" (cross-file edit on the concrete class) and "Change type argument to ``" (bound-satisfying workspace types, diff --git a/src/Diagnostics/AuthoritativeDiagnosticsListener.php b/src/Diagnostics/AuthoritativeDiagnosticsListener.php new file mode 100644 index 0000000..7f025f8 --- /dev/null +++ b/src/Diagnostics/AuthoritativeDiagnosticsListener.php @@ -0,0 +1,120 @@ + + */ + private array $publishedUris = []; + + private int $generation = 0; + + /** + * @param Closure(string, ?int, list): void $publish + * Publishes (uri, version, diagnostics) — injected so tests can capture + * without a live client. In production this wraps + * `ClientApi::diagnostics()->publishDiagnostics(...)`. + */ + public function __construct( + private readonly DiagnosticsCheckSource $runner, + private readonly AuthoritativeDiagnosticsStore $store, + private readonly Closure $publish, + private readonly ?PhpactorWorkspace $workspace = null, + ) { + } + + public function getListenersForEvent(object $event): iterable + { + if ($event instanceof TextDocumentSaved) { + return [[$this, 'onSave']]; + } + return []; + } + + public function onSave(TextDocumentSaved $event): void + { + $generation = ++$this->generation; + asyncCall(function () use ($generation): void { + // A newer save arrived before this tick ran — let that one do the work. + if ($generation !== $this->generation) { + return; + } + $this->refresh(); + }); + } + + /** + * Run the authoritative check, update the store, publish non-open files, and + * clear any non-open files that no longer have diagnostics. Synchronous and + * side-effect-only — the unit-test entry point. + */ + public function refresh(): void + { + $byUri = $this->runner->run(); + $this->store->replaceAll($byUri); + + /** @var array $publishedNow */ + $publishedNow = []; + foreach ($byUri as $uri => $diagnostics) { + // Open docs are the engine's job (it merges the store); publishing + // them here too would clobber the tolerant tier. + if ($this->isOpen($uri)) { + continue; + } + ($this->publish)($uri, null, $diagnostics); + // @infection-ignore-all TrueValue -- read only via isset() below, which + // is true for a `false` value too, so true->false is equivalent. + $publishedNow[$uri] = true; + } + + // Stale-clear: non-open files we published last time but not now. + foreach ($this->publishedUris as $uri) { + if (isset($publishedNow[$uri]) || $this->isOpen($uri)) { + continue; + } + ($this->publish)($uri, null, []); + } + + $this->publishedUris = array_keys($publishedNow); + } + + private function isOpen(string $uri): bool + { + return $this->workspace !== null && $this->workspace->has($uri); + } +} diff --git a/src/Diagnostics/AuthoritativeDiagnosticsStore.php b/src/Diagnostics/AuthoritativeDiagnosticsStore.php new file mode 100644 index 0000000..e211ff4 --- /dev/null +++ b/src/Diagnostics/AuthoritativeDiagnosticsStore.php @@ -0,0 +1,52 @@ +> */ + private array $byUri = []; + + /** + * Replace the entire store with a fresh workspace-wide pass. Stale-clearing + * of files that dropped out is the listener's responsibility (it tracks what + * it published), so this is a plain swap. + * + * @param array> $byUri + */ + public function replaceAll(array $byUri): void + { + $this->byUri = $byUri; + } + + /** + * @return list + */ + public function get(string $uri): array + { + return $this->byUri[$uri] ?? []; + } +} diff --git a/src/Diagnostics/CompilerCheckRunner.php b/src/Diagnostics/CompilerCheckRunner.php new file mode 100644 index 0000000..b2fbcd8 --- /dev/null +++ b/src/Diagnostics/CompilerCheckRunner.php @@ -0,0 +1,282 @@ +resolver = $resolver; + $this->compiler = $compiler; + } + + /** + * Validate the whole source set and return LSP diagnostics keyed by file URI. + * Empty when there is nothing to check or anything goes wrong. + * + * @return array> + */ + public function run(): array + { + // Load-bearing guard, NOT just an optimization: without the `return []`, + // an empty root would fall through to resolveSources() -> XphpManifest::locate('') + // which walks up from the process CWD and could pick up an UNRELATED + // ancestor xphp.json. The `||`->`&&` mutant is equivalent (both an empty + // and a non-existent root funnel to [] either way), but dropping the return + // is a real behaviour change, so only LogicalOr is ignored here. + // @infection-ignore-all LogicalOr + if ($this->rootPath === '' || !is_dir($this->rootPath)) { + return []; + } + + $startedAt = hrtime(true); + try { + $resolved = $this->resolveSources(); + if ($resolved === null) { + return []; + } + $fileCount = count($resolved->files->filepaths); + if ($fileCount > $this->maxFiles) { + // @infection-ignore-all MethodCallRemoval -- observability only; the + // stderr log has no behavioural contract (and is suppressed under + // XPHP_LSP_QUIET in tests). The `return []` below is the behaviour. + $this->log(sprintf( + 'skipped: %d source files exceeds the in-process cap (%d) — scope with an xphp.json to enable', + $fileCount, + $this->maxFiles, + )); + return []; + } + $collector = $this->compiler()->check($resolved->files); + } catch (\Throwable $e) { + // Defensive: the authoritative tier must never take the server down. + // A resolver RuntimeException (no sources) or any compiler-internal + // failure simply means "no authoritative diagnostics this pass". + $this->log(sprintf('caught %s: %s (%s)', $e::class, $e->getMessage(), self::elapsed($startedAt))); + return []; + } + + $byUri = $this->groupAndMap($collector->all()); + // @infection-ignore-all MethodCallRemoval -- observability only (see above). + $this->log(sprintf( + 'ran: %d files -> %d diagnostics across %d files (%s)', + $fileCount, + array_sum(array_map('count', $byUri)), + count($byUri), + self::elapsed($startedAt), + )); + return $byUri; + } + + // Pure observability helpers: output is stderr log text with no behavioural + // contract (suppressed under XPHP_LSP_QUIET in tests), and the elapsed-ms + // arithmetic is non-deterministic so it can't be asserted. + private function log(string $message): void + { + // @infection-ignore-all + Stderr::write(sprintf("[xphp-lsp authoritative] %s\n", $message)); + } + + private static function elapsed(int $startedAt): string + { + // @infection-ignore-all + return sprintf('%.0f ms', (hrtime(true) - $startedAt) / 1_000_000); + } + + /** + * Resolve the project's source set exactly as `xphp check` does: prefer an + * `xphp.json` manifest (walks its source roots, excludes target/cache), else + * treat the workspace root itself as a single source directory. + */ + private function resolveSources(): ?\XPHP\Config\ResolvedSources + { + $manifest = XphpManifest::locate($this->rootPath); + if ($manifest !== null) { + return $this->resolver()->resolve(null, $manifest, $this->rootPath); + } + return $this->resolver()->resolve($this->rootPath, null, $this->rootPath); + } + + /** + * @param list $diagnostics + * @return array> + */ + private function groupAndMap(array $diagnostics): array + { + /** @var array> $byUri */ + $byUri = []; + /** @var array> $lineCache */ + $lineCache = []; + + foreach ($diagnostics as $d) { + $location = $d->location; + // @infection-ignore-all Continue_ -- at most one location-less + // diagnostic occurs per pass (an incomplete-set undefined-template + // note), so skipping vs stopping is indistinguishable. Pinned by the + // use_only fixture (testClosedWorldContract...). + if ($location === null) { + // No file to route it to. Whole-set passes shouldn't produce these; + // drop defensively (dereferencing a null location would fatal). + continue; + } + $file = $location->file; + // The grounded per-specialization pass reports some diagnostics against + // a SYNTHETIC path ("") that maps to no + // real source line. Publishing that as a URI would put a phantom entry + // in the client's Problems panel pointing at an unopenable path while + // the real line stays unmarked — worse than dropping it. Skip anything + // that isn't a real on-disk file (check() only ever reads real files). + // @infection-ignore-all Continue_ -- skipping vs stopping is + // indistinguishable here (the synthetic diagnostics we drop don't + // precede real ones in the fixtures). Pinned by the selfcall fixture. + if (!is_file($file)) { + continue; + } + $uri = 'file://' . $file; + // @infection-ignore-all GreaterThan IncrementInteger DecrementInteger + // -- the compiler reports lines >= 1 (nikic getStartLine), so the `0` + // boundary of this `?: 1` fallback is never crossed. Same rationale as + // Compiler.php's own ignore of the identical construct. + $line = $location->line > 0 ? $location->line : 1; + + $byUri[$uri][] = $this->toLsp($d, $this->lineLength($file, $line, $lineCache), $line); + } + + return $byUri; + } + + private function toLsp(CompilerDiagnostic $d, int $lineLength, int $line): LspDiagnostic + { + // The compiler reports line-only (1-based); render a full-line range. + $zeroLine = $line - 1; + $lsp = new LspDiagnostic( + range: new Range( + new Position($zeroLine, 0), + new Position($zeroLine, $lineLength), + ), + message: $d->message, + ); + $lsp->severity = self::severity($d->severity); + $lsp->code = $d->code; + $lsp->source = 'xphp'; + return $lsp; + } + + /** + * Length of the 1-based $line in $file, for a full-line range. Reads and + * caches the file's lines; 0 when the file or line can't be read (yields a + * zero-width range at column 0 — still a valid, if thin, squiggle). + * + * @param array> $cache + */ + private function lineLength(string $file, int $line, array &$cache): int + { + if (!isset($cache[$file])) { + $source = @file_get_contents($file); + $cache[$file] = $source === false ? [] : explode("\n", $source); + } + $lines = $cache[$file]; + $text = $lines[$line - 1] ?? ''; + // Trim a trailing CR so CRLF files don't over-extend the range by one. + return strlen(rtrim($text, "\r")); + } + + private static function severity(Severity $severity): int + { + return match ($severity) { + Severity::Error => DiagnosticSeverity::ERROR, + Severity::Warning => DiagnosticSeverity::WARNING, + // NOTE (mutation): removing this arm survives — check()'s surface emits + // only Error/Warning (Notice appears solely in the phpstan gate + + // renderers, which the LSP path never runs), so the arm is unreachable + // here and can't be exercised by a fixture. Kept for completeness. + Severity::Notice => DiagnosticSeverity::INFORMATION, + }; + } + + private function resolver(): SourceResolver + { + if ($this->resolver === null) { + $finder = new NativeFileFinder(); + $this->resolver = new SourceResolver( + $finder, + new ManifestResolver(new NativeFileReader(), $finder), + ); + } + return $this->resolver; + } + + private function compiler(): Compiler + { + if ($this->compiler === null) { + $printer = new StandardPrinter(); + $writer = new NativeFileWriter(); + $this->compiler = new Compiler( + new NativeFileReader(), + $writer, + new XphpSourceParser((new ParserFactory())->createForHostVersion()), + new Specializer(), + new SpecializedClassGenerator($printer, $writer), + $printer, + ); + } + return $this->compiler; + } +} diff --git a/src/Diagnostics/DiagnosticsCheckSource.php b/src/Diagnostics/DiagnosticsCheckSource.php new file mode 100644 index 0000000..e309770 --- /dev/null +++ b/src/Diagnostics/DiagnosticsCheckSource.php @@ -0,0 +1,21 @@ +> + */ + public function run(): array; +} diff --git a/src/Diagnostics/XphpDiagnosticsProvider.php b/src/Diagnostics/XphpDiagnosticsProvider.php index 41870ad..7c859cd 100644 --- a/src/Diagnostics/XphpDiagnosticsProvider.php +++ b/src/Diagnostics/XphpDiagnosticsProvider.php @@ -74,6 +74,15 @@ public function __construct( * (most unit tests) simply skip that diagnostic and behave as before. */ private readonly ?GenericResolver $genericResolver = null, + /** + * Holds the last on-save authoritative (compiler `check()`) diagnostics. + * When set, each open document's published set is merged with its + * authoritative diagnostics — so the fast tolerant tier and the + * whole-project compiler tier reach the client in a single publish (a + * publish is a full replace, so a second publisher would clobber). Null + * in pull-mode / unit contexts that don't run the authoritative pass. + */ + private readonly ?AuthoritativeDiagnosticsStore $authoritativeStore = null, ) { } @@ -211,12 +220,50 @@ private function computeAllOpenDiagnostics(TextDocumentItem $textDocument): arra $meta['version'], $meta['source'], ); - $result[$uri] = array_merge($perFile, $workspaceDiagnostics); + $result[$uri] = $this->mergeAuthoritative( + array_merge($perFile, $workspaceDiagnostics), + $uri, + ); } return $result; } + /** + * Append the URI's authoritative (compiler `check()`) diagnostics to its + * tolerant set, skipping any that duplicate a tolerant diagnostic already + * present at the same line + code. The tolerant entry wins on a tie because + * its range is computed against the live buffer; the authoritative range is + * disk-based (identical on a just-saved doc, possibly stale after an edit). + * + * @param list $tolerant + * @return list + */ + private function mergeAuthoritative(array $tolerant, string $uri): array + { + if ($this->authoritativeStore === null) { + return $tolerant; + } + $authoritative = $this->authoritativeStore->get($uri); + if ($authoritative === []) { + return $tolerant; + } + $seen = []; + foreach ($tolerant as $d) { + $seen[$d->range->start->line . "\0" . (string) ($d->code ?? '')] = true; + } + $merged = $tolerant; + foreach ($authoritative as $d) { + $key = $d->range->start->line . "\0" . (string) ($d->code ?? ''); + if (isset($seen[$key])) { + continue; + } + $seen[$key] = true; + $merged[] = $d; + } + return $merged; + } + /** * Conservative null-dereference diagnostics (`xphp.null-deref`) for one * document. Asks the type-inference resolver for plain-`->` accesses on a diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index 26375e6..2e5a596 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -44,6 +44,9 @@ use XPHP\Lsp\Analyzer\Analyzer; use XPHP\Lsp\Analyzer\ParsedDocumentCache; use XPHP\Lsp\Analyzer\WorkspaceAnalyzer; +use XPHP\Lsp\Diagnostics\AuthoritativeDiagnosticsListener; +use XPHP\Lsp\Diagnostics\AuthoritativeDiagnosticsStore; +use XPHP\Lsp\Diagnostics\CompilerCheckRunner; use XPHP\Lsp\Diagnostics\XphpDiagnosticsProvider; use XPHP\Lsp\Handler\WorkspaceSymbols; use XPHP\Lsp\Handler\XphpCompletionHandler; @@ -188,6 +191,13 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia $phpDefinitionResolver = new PhpDefinitionResolver($workspace, $xphpParser, $reflector, $cache, $genericResolver); $phpHoverResolver = new PhpHoverResolver($workspace, $xphpParser, $reflector, $genericParams, $genericResolver); + // Authoritative (on-save) diagnostics tier: the compiler's own + // whole-project `check()` produces the grounded, call-argument closure + // conformance + bound diagnostics the tolerant per-keystroke pass can't. + // Its results land in this store; the provider merges them into each open + // document's publish so both tiers reach the client in one message. + $authoritativeStore = new AuthoritativeDiagnosticsStore(); + $diagnosticsProvider = new XphpDiagnosticsProvider( $cache, new WorkspaceAnalyzer(), @@ -199,6 +209,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // Type-inference engine for the conservative null-dereference // diagnostic (xphp.null-deref) on chained nullable receivers. $genericResolver, + $authoritativeStore, ); $diagnosticsEngine = new DiagnosticsEngine( @@ -244,6 +255,18 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // Runs on the same Initialized event, independently of the // FQN warmer above; both are asyncCall-dispatched. new \XPHP\Lsp\Analyzer\ParsedDocumentCacheWarmer($fqnIndex, $cache, $workspace), + // On-save authoritative diagnostics: run the compiler's whole-project + // `check()` and publish. Registered BEFORE $diagnosticsService so the + // authoritative store is refreshed ahead of the engine's own on-save + // pass, which then publishes open docs with both tiers merged. Inert + // when $rootPath is empty (e.g. the behat harness), like the FQN index. + new AuthoritativeDiagnosticsListener( + new CompilerCheckRunner($rootPath), + $authoritativeStore, + static fn (string $uri, ?int $version, array $diagnostics) + => $clientApi->diagnostics()->publishDiagnostics($uri, $version, $diagnostics), + $workspace, + ), $diagnosticsService, ); diff --git a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php new file mode 100644 index 0000000..3bbcf46 --- /dev/null +++ b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php @@ -0,0 +1,222 @@ +code = 'xphp.closure_conformance'; + $d->severity = 1; + $d->source = 'xphp'; + return $d; + } + + /** + * @param list>> $queue results returned by successive run() calls + */ + private function source(array $queue): DiagnosticsCheckSource + { + return new class($queue) implements DiagnosticsCheckSource { + /** @param list>> $queue */ + public function __construct(private array $queue) + { + } + + public function run(): array + { + return array_shift($this->queue) ?? []; + } + }; + } + + public function testRefreshPublishesNonOpenFileAndPopulatesStore(): void + { + $store = new AuthoritativeDiagnosticsStore(); + $published = []; + $listener = new AuthoritativeDiagnosticsListener( + $this->source([[self::URI => [$this->diag()]]]), + $store, + function (string $uri, ?int $version, array $diagnostics) use (&$published): void { + $published[] = [$uri, $version, $diagnostics]; + }, + null, // no workspace => every file is "not open" + ); + + $listener->refresh(); + + self::assertCount(1, $published); + self::assertSame(self::URI, $published[0][0]); + self::assertNull($published[0][1], 'non-open files publish with a null version'); + self::assertCount(1, $published[0][2]); + self::assertCount(1, $store->get(self::URI), 'store holds the authoritative diagnostics'); + } + + public function testStaleDiagnosticsAreClearedOnNextRefresh(): void + { + $store = new AuthoritativeDiagnosticsStore(); + $published = []; + $listener = new AuthoritativeDiagnosticsListener( + // First save: an error. Second save: fixed (empty set). + $this->source([[self::URI => [$this->diag()]], []]), + $store, + function (string $uri, ?int $version, array $diagnostics) use (&$published): void { + $published[] = [$uri, $version, $diagnostics]; + }, + null, + ); + + $listener->refresh(); + $listener->refresh(); + + self::assertCount(2, $published); + self::assertSame(self::URI, $published[1][0]); + self::assertSame([], $published[1][2], 'a fixed file gets an empty publish to clear its squiggles'); + self::assertSame([], $store->get(self::URI)); + } + + public function testOpenDocumentIsNotPublishedDirectly(): void + { + // Open documents are published by the engine (which merges the store); + // publishing them here too would clobber the tolerant tier. + $store = new AuthoritativeDiagnosticsStore(); + $published = []; + $workspace = new PhpactorWorkspace(); + $workspace->open(new TextDocumentItem(self::URI, 'xphp', 1, 'source([[self::URI => [$this->diag()]]]), + $store, + function (string $uri, ?int $version, array $diagnostics) use (&$published): void { + $published[] = [$uri, $version, $diagnostics]; + }, + $workspace, + ); + + $listener->refresh(); + + self::assertSame([], $published, 'open docs are left to the engine'); + self::assertCount(1, $store->get(self::URI), 'but the store still holds them for the merge'); + } + + public function testOnlyReactsToSaveEvents(): void + { + $listener = new AuthoritativeDiagnosticsListener( + $this->source([]), + new AuthoritativeDiagnosticsStore(), + static function (): void { + }, + null, + ); + + $saved = new \Phpactor\LanguageServer\Event\TextDocumentSaved( + new \Phpactor\LanguageServerProtocol\TextDocumentIdentifier(self::URI), + null, + ); + self::assertEquals( + [[$listener, 'onSave']], + [...$listener->getListenersForEvent($saved)], + 'a save event binds the onSave handler', + ); + self::assertSame([], [...$listener->getListenersForEvent(new \stdClass())], 'an unrelated event yields nothing'); + } + + public function testNonOpenFileIsPublishedEvenAfterAnOpenOne(): void + { + // The publish loop must SKIP open docs and keep going — not stop — so a + // non-open file listed after an open one is still published. + $openUri = 'file:///project/Open.xphp'; + $closedUri = 'file:///project/Closed.xphp'; + $workspace = new PhpactorWorkspace(); + $workspace->open(new TextDocumentItem($openUri, 'xphp', 1, 'source([[$openUri => [$this->diag()], $closedUri => [$this->diag()]]]), + new AuthoritativeDiagnosticsStore(), + function (string $uri, ?int $version, array $diagnostics) use (&$published): void { + $published[] = $uri; + }, + $workspace, + ); + + $listener->refresh(); + + self::assertSame([$closedUri], $published, 'the closed file is published; the open one is skipped'); + } + + public function testPersistingFileIsNotClearedOnRefresh(): void + { + // A non-open file that still has diagnostics on the next pass must be + // re-published with them, never cleared. + $published = []; + $listener = new AuthoritativeDiagnosticsListener( + $this->source([[self::URI => [$this->diag()]], [self::URI => [$this->diag()]]]), + new AuthoritativeDiagnosticsStore(), + function (string $uri, ?int $version, array $diagnostics) use (&$published): void { + $published[] = $diagnostics; + }, + null, + ); + + $listener->refresh(); + $listener->refresh(); + + self::assertCount(2, $published); + self::assertCount(1, $published[1], 'second publish carries the diagnostic, not an empty clear'); + } + + public function testStaleClearSkipsPersistingFilesButStillClearsDroppedOnes(): void + { + // publishedUris after pass 1 = [persist, dropped] (insertion order). On + // pass 2 `persist` still has diagnostics (skipped in the clear loop) and + // `dropped` does not — the loop must keep going past `persist` and clear + // `dropped`, not stop at the first skip. + $persist = 'file:///project/Persist.xphp'; + $dropped = 'file:///project/Dropped.xphp'; + $cleared = []; + $listener = new AuthoritativeDiagnosticsListener( + $this->source([ + [$persist => [$this->diag()], $dropped => [$this->diag()]], + [$persist => [$this->diag()]], + ]), + new AuthoritativeDiagnosticsStore(), + function (string $uri, ?int $version, array $diagnostics) use (&$cleared): void { + if ($diagnostics === []) { + $cleared[] = $uri; + } + }, + null, + ); + + $listener->refresh(); + $listener->refresh(); + + self::assertSame([$dropped], $cleared, 'the dropped file is cleared even though a persisting file precedes it'); + } +} diff --git a/test/Diagnostics/AuthoritativeMergeTest.php b/test/Diagnostics/AuthoritativeMergeTest.php new file mode 100644 index 0000000..d417ffd --- /dev/null +++ b/test/Diagnostics/AuthoritativeMergeTest.php @@ -0,0 +1,127 @@ +code = $code; + $d->severity = 1; + $d->source = 'xphp'; + return $d; + } + + /** + * @return list + */ + private function lintWithStore(TextDocumentItem $doc, PhpactorWorkspace $workspace, AuthoritativeDiagnosticsStore $store): array + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + $provider = new XphpDiagnosticsProvider( + $cache, + new WorkspaceAnalyzer(), + $workspace, + new FqnIndex($workspace, $cache, $parser, ''), + null, + null, + $store, + ); + $result = wait($provider->provideDiagnostics($doc, (new CancellationTokenSource())->getToken())); + return is_array($result) ? array_values($result) : []; + } + + private function openCleanDoc(PhpactorWorkspace $workspace): TextDocumentItem + { + $doc = new TextDocumentItem(self::URI, 'xphp', 1, " { public T \$item; }\n"); + $workspace->open($doc); + return $doc; + } + + public function testAuthoritativeDiagnosticsAreMergedIntoAnOpenDocument(): void + { + $workspace = new PhpactorWorkspace(); + $doc = $this->openCleanDoc($workspace); + $store = new AuthoritativeDiagnosticsStore(); + $store->replaceAll([self::URI => [$this->authoritative(2, 'xphp.closure_conformance')]]); + + $codes = array_map( + static fn (LspDiagnostic $d): string => (string) $d->code, + $this->lintWithStore($doc, $workspace, $store), + ); + + self::assertContains('xphp.closure_conformance', $codes, 'the on-save compiler diagnostic is surfaced on an open doc'); + } + + public function testMergeDedupesByLineAndCode(): void + { + $workspace = new PhpactorWorkspace(); + $doc = $this->openCleanDoc($workspace); + $store = new AuthoritativeDiagnosticsStore(); + // Two diagnostics at the SAME line + code — only one should survive. + $store->replaceAll([self::URI => [ + $this->authoritative(2, 'xphp.closure_conformance'), + $this->authoritative(2, 'xphp.closure_conformance'), + ]]); + + $conformance = array_values(array_filter( + $this->lintWithStore($doc, $workspace, $store), + static fn (LspDiagnostic $d): bool => (string) $d->code === 'xphp.closure_conformance', + )); + + self::assertCount(1, $conformance); + } + + public function testNoStoreLeavesTolerantOutputUnchanged(): void + { + $workspace = new PhpactorWorkspace(); + $doc = $this->openCleanDoc($workspace); + + // A clean doc with no store wired stays clean (regression guard: the + // merge path must be inert when no authoritative pass has run). + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + $provider = new XphpDiagnosticsProvider( + $cache, + new WorkspaceAnalyzer(), + $workspace, + new FqnIndex($workspace, $cache, $parser, ''), + ); + $result = wait($provider->provideDiagnostics($doc, (new CancellationTokenSource())->getToken())); + + self::assertSame([], is_array($result) ? array_values($result) : []); + } +} diff --git a/test/Diagnostics/CompilerCheckRunnerTest.php b/test/Diagnostics/CompilerCheckRunnerTest.php new file mode 100644 index 0000000..0f5e407 --- /dev/null +++ b/test/Diagnostics/CompilerCheckRunnerTest.php @@ -0,0 +1,181 @@ + + */ + private function diagnosticsFor(string $fixture): array + { + $byUri = (new CompilerCheckRunner($this->fixture($fixture)))->run(); + $flat = []; + foreach ($byUri as $diags) { + foreach ($diags as $d) { + $flat[] = $d; + } + } + return $flat; + } + + /** + * @param list $diagnostics + */ + private static function codes(array $diagnostics): array + { + return array_map(static fn (LspDiagnostic $d): string => (string) $d->code, $diagnostics); + } + + public function testRejectFixtureSurfacesCallArgumentClosureConformance(): void + { + $byUri = (new CompilerCheckRunner($this->fixture('reject')))->run(); + + self::assertCount(1, $byUri, 'diagnostics routed to exactly the one source file'); + $uri = array_key_first($byUri); + self::assertIsString($uri); + self::assertStringEndsWith('/Use.xphp', $uri); + self::assertStringStartsWith('file://', $uri); + + $conformance = array_values(array_filter( + $byUri[$uri], + static fn (LspDiagnostic $d): bool => (string) $d->code === 'xphp.closure_conformance', + )); + self::assertCount(1, $conformance, 'the call-argument closure conformance error is reported'); + $d = $conformance[0]; + self::assertSame(1, $d->severity, 'closure conformance is an Error'); + self::assertSame('xphp', $d->source); + self::assertStringContainsString('int is not wider than', $d->message); + + // Full-line range on the exact offending line (0-based 20 == the + // `$box->map::(...)` call). Pinning the line + length guards the + // 1-based→0-based conversion and the line-length computation. + $sourceLine = explode("\n", (string) file_get_contents($this->fixture('reject') . '/Use.xphp'))[20]; + self::assertSame(20, $d->range->start->line); + self::assertSame(20, $d->range->end->line); + self::assertSame(0, $d->range->start->character); + self::assertSame(strlen(rtrim($sourceLine, "\r")), $d->range->end->character); + } + + public function testWarningSeverityIsMapped(): void + { + // A variance edge over an undeclared type arg is a Warning, not an Error — + // exercises the non-Error arm of the severity mapping. + $warnings = array_values(array_filter( + $this->diagnosticsFor('warn'), + static fn (LspDiagnostic $d): bool => $d->severity === 2, + )); + self::assertNotEmpty($warnings, 'a Warning-severity diagnostic is surfaced as LSP severity 2'); + } + + public function testCrlfLineEndingIsNotIncludedInTheRange(): void + { + $byUri = (new CompilerCheckRunner($this->fixture('crlf')))->run(); + $diag = null; + foreach ($byUri as $diags) { + foreach ($diags as $d) { + if ((string) $d->code === 'xphp.closure_conformance') { + $diag = $d; + } + } + } + self::assertNotNull($diag); + + $rawLine = explode("\n", (string) file_get_contents($this->fixture('crlf') . '/Use.xphp'))[$diag->range->start->line]; + self::assertStringEndsWith("\r", $rawLine, 'fixture is genuinely CRLF'); + // The range must stop at the last real character, not the trailing CR. + self::assertSame(strlen(rtrim($rawLine, "\r")), $diag->range->end->character); + } + + public function testWholeSetIsFedSoSplitFixtureStillFires(): void + { + // The violating call lives in Use.xphp; Box/Book are declared in a + // SEPARATE Lib.xphp. Because the runner feeds the whole source set, the + // compiler can specialize Box and catch the closure mismatch. + self::assertContains('xphp.closure_conformance', self::codes($this->diagnosticsFor('split'))); + } + + public function testClosedWorldContractUseSiteAloneDoesNotGround(): void + { + // Same use-site WITHOUT its Box/Book declarations in the set: the compiler + // cannot ground Box, so the grounded closure check never runs. This + // is why the runner must be handed the whole source set, not one file. + self::assertNotContains('xphp.closure_conformance', self::codes($this->diagnosticsFor('use_only'))); + } + + public function testDiagnosticsInSeparateFilesAreKeptDistinct(): void + { + // Two files each carry their own violation; the result must key both URIs + // (guards against collapsing the per-file grouping to a single entry). + $byUri = (new CompilerCheckRunner($this->fixture('multi')))->run(); + self::assertCount(2, $byUri); + foreach ($byUri as $diags) { + self::assertNotEmpty($diags); + } + } + + public function testSyntheticSpecializationPathsAreNotPublished(): void + { + // A grounded SELF-call closure conformance is reported by the compiler + // against a synthetic "" filepath that maps to no real + // source line. It must never be surfaced as a URI (a phantom Problems + // entry pointing at an unopenable path). + $byUri = (new CompilerCheckRunner($this->fixture('selfcall')))->run(); + $synthetic = array_values(array_filter( + array_keys($byUri), + static fn ($uri): bool => str_contains((string) $uri, 'fixture('multi'), maxFiles: 1))->run(); + self::assertSame([], $byUri); + + // Boundary: a cap EQUAL to the file count admits the set (skip only when + // strictly greater) — the two-file fixture runs under maxFiles: 2. + self::assertCount(2, (new CompilerCheckRunner($this->fixture('multi'), maxFiles: 2))->run()); + } + + public function testCleanFixtureReturnsNoDiagnostics(): void + { + self::assertSame([], $this->diagnosticsFor('clean')); + } + + public function testEmptyRootPathIsInert(): void + { + // The behat harness (and any un-rooted client) boots with an empty + // rootPath; the authoritative pass must be a no-op then. + self::assertSame([], (new CompilerCheckRunner(''))->run()); + } + + public function testNonExistentRootIsInert(): void + { + self::assertSame([], (new CompilerCheckRunner('/no/such/directory/really'))->run()); + } +} diff --git a/test/fixture/authoritative/clean/Ok.xphp b/test/fixture/authoritative/clean/Ok.xphp new file mode 100644 index 0000000..7554505 --- /dev/null +++ b/test/fixture/authoritative/clean/Ok.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('PHP')); +$box->map::(fn(Book $x): string => $x->name); diff --git a/test/fixture/authoritative/crlf/Use.xphp b/test/fixture/authoritative/crlf/Use.xphp new file mode 100644 index 0000000..61c3bfd --- /dev/null +++ b/test/fixture/authoritative/crlf/Use.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book(PHP)); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/multi/A.xphp b/test/fixture/authoritative/multi/A.xphp new file mode 100644 index 0000000..5501506 --- /dev/null +++ b/test/fixture/authoritative/multi/A.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/multi/B.xphp b/test/fixture/authoritative/multi/B.xphp new file mode 100644 index 0000000..246d784 --- /dev/null +++ b/test/fixture/authoritative/multi/B.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/reject/Use.xphp b/test/fixture/authoritative/reject/Use.xphp new file mode 100644 index 0000000..eca3235 --- /dev/null +++ b/test/fixture/authoritative/reject/Use.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/selfcall/Use.xphp b/test/fixture/authoritative/selfcall/Use.xphp new file mode 100644 index 0000000..8dceb65 --- /dev/null +++ b/test/fixture/authoritative/selfcall/Use.xphp @@ -0,0 +1,26 @@ + +{ + public function __construct(private E $value) {} + + public function mapSelf(Closure(E $x): string $fn): string + { + return $fn($this->value); + } + + public function useIt(): string + { + return $this->mapSelf(fn(int $x): string => (string) $x); + } +} + +$box = new Box::(new Book('PHP')); +$box->useIt(); diff --git a/test/fixture/authoritative/split/Lib.xphp b/test/fixture/authoritative/split/Lib.xphp new file mode 100644 index 0000000..66b73d0 --- /dev/null +++ b/test/fixture/authoritative/split/Lib.xphp @@ -0,0 +1,18 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} diff --git a/test/fixture/authoritative/split/Use.xphp b/test/fixture/authoritative/split/Use.xphp new file mode 100644 index 0000000..1c6d8d6 --- /dev/null +++ b/test/fixture/authoritative/split/Use.xphp @@ -0,0 +1,6 @@ +(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/use_only/Use.xphp b/test/fixture/authoritative/use_only/Use.xphp new file mode 100644 index 0000000..52e4dec --- /dev/null +++ b/test/fixture/authoritative/use_only/Use.xphp @@ -0,0 +1,6 @@ +(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/warn/Use.xphp b/test/fixture/authoritative/warn/Use.xphp new file mode 100644 index 0000000..112d1f9 --- /dev/null +++ b/test/fixture/authoritative/warn/Use.xphp @@ -0,0 +1,12 @@ + +{ + public function __construct(private E $value) {} +} + +function makeGhost(): Ghost {} + +$box = new Box::(makeGhost()); From e1c26c1d02f7137339ec95cba579a4c538702d72 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 19:01:37 +0000 Subject: [PATCH 17/26] feat(diagnostics): discover the xphp.json per saved file Resolve the authoritative check's source set by walking up from the saved file's own directory to the nearest xphp.json, rather than the single workspace rootPath. A file now resolves to its own project even in a multi-root or mis-rooted workspace -- the server tracks only a legacy rootPath and does not read workspaceFolders, so opening a file from project B while rooted at project A previously scoped the check to A (or skipped it on the file-count cap). A file with no ancestor manifest falls back to the workspace-root resolution. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 8 ++++ .../AuthoritativeDiagnosticsListener.php | 16 +++++-- src/Diagnostics/CompilerCheckRunner.php | 46 ++++++++++++------- src/Diagnostics/DiagnosticsCheckSource.php | 7 ++- .../AuthoritativeDiagnosticsListenerTest.php | 2 +- test/Diagnostics/CompilerCheckRunnerTest.php | 35 ++++++++++++++ .../authoritative/scoped/src/Reject.xphp | 21 +++++++++ .../authoritative/scoped/tests/Ignored.xphp | 21 +++++++++ test/fixture/authoritative/scoped/xphp.json | 5 ++ 9 files changed, 138 insertions(+), 23 deletions(-) create mode 100644 test/fixture/authoritative/scoped/src/Reject.xphp create mode 100644 test/fixture/authoritative/scoped/tests/Ignored.xphp create mode 100644 test/fixture/authoritative/scoped/xphp.json diff --git a/docs/features/index.md b/docs/features/index.md index 6321478..00d4eeb 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -308,6 +308,14 @@ reflects the last-saved state; editing a file supersedes its authoritative findings until the next save. No compiler subprocess is spawned -- the validator runs in-process. +The source set is discovered **per saved file**: the server walks up from +the file's own directory to the nearest `xphp.json` and scopes the check to +that manifest, so a file resolves to *its own* project even in a multi-root +or mis-rooted workspace (the server tracks a single legacy `rootPath` and +does not read `workspaceFolders`). A file with no ancestor manifest falls +back to the workspace root. Very large / unscoped source sets are skipped +(with a log line) so the synchronous check can never block the editor. + ### Default type arguments (no false missing-arg) Not a diagnostic code of its own -- this is how the bound and diff --git a/src/Diagnostics/AuthoritativeDiagnosticsListener.php b/src/Diagnostics/AuthoritativeDiagnosticsListener.php index 7f025f8..62fb663 100644 --- a/src/Diagnostics/AuthoritativeDiagnosticsListener.php +++ b/src/Diagnostics/AuthoritativeDiagnosticsListener.php @@ -68,13 +68,15 @@ public function getListenersForEvent(object $event): iterable public function onSave(TextDocumentSaved $event): void { + // The saved file anchors per-document manifest discovery in the runner. + $fromPath = self::uriToPath($event->identifier()->uri); $generation = ++$this->generation; - asyncCall(function () use ($generation): void { + asyncCall(function () use ($generation, $fromPath): void { // A newer save arrived before this tick ran — let that one do the work. if ($generation !== $this->generation) { return; } - $this->refresh(); + $this->refresh($fromPath); }); } @@ -83,9 +85,9 @@ public function onSave(TextDocumentSaved $event): void * clear any non-open files that no longer have diagnostics. Synchronous and * side-effect-only — the unit-test entry point. */ - public function refresh(): void + public function refresh(?string $fromPath = null): void { - $byUri = $this->runner->run(); + $byUri = $this->runner->run($fromPath); $this->store->replaceAll($byUri); /** @var array $publishedNow */ @@ -117,4 +119,10 @@ private function isOpen(string $uri): bool { return $this->workspace !== null && $this->workspace->has($uri); } + + private static function uriToPath(string $uri): ?string + { + // TextDocumentIdentifier already url-decodes; just strip the scheme. + return str_starts_with($uri, 'file://') ? substr($uri, 7) : null; + } } diff --git a/src/Diagnostics/CompilerCheckRunner.php b/src/Diagnostics/CompilerCheckRunner.php index b2fbcd8..68bd866 100644 --- a/src/Diagnostics/CompilerCheckRunner.php +++ b/src/Diagnostics/CompilerCheckRunner.php @@ -73,22 +73,11 @@ public function __construct( * * @return array> */ - public function run(): array + public function run(?string $fromPath = null): array { - // Load-bearing guard, NOT just an optimization: without the `return []`, - // an empty root would fall through to resolveSources() -> XphpManifest::locate('') - // which walks up from the process CWD and could pick up an UNRELATED - // ancestor xphp.json. The `||`->`&&` mutant is equivalent (both an empty - // and a non-existent root funnel to [] either way), but dropping the return - // is a real behaviour change, so only LogicalOr is ignored here. - // @infection-ignore-all LogicalOr - if ($this->rootPath === '' || !is_dir($this->rootPath)) { - return []; - } - $startedAt = hrtime(true); try { - $resolved = $this->resolveSources(); + $resolved = $this->resolveSources($fromPath); if ($resolved === null) { return []; } @@ -141,12 +130,35 @@ private static function elapsed(int $startedAt): string } /** - * Resolve the project's source set exactly as `xphp check` does: prefer an - * `xphp.json` manifest (walks its source roots, excludes target/cache), else - * treat the workspace root itself as a single source directory. + * Resolve the project's source set, `xphp check`-style (manifest source roots, + * target/cache excluded). + * + * Per-document first: when a saved file is given, discover the nearest + * `xphp.json` walking UP from that file's own directory and scope to it. This + * makes multi-root and mis-rooted workspaces resolve each file to its own + * project rather than an arbitrary workspace root (the server tracks a single + * legacy `rootPath` and doesn't read `workspaceFolders`). Falls back to the + * configured `rootPath` when the file has no ancestor manifest. */ - private function resolveSources(): ?\XPHP\Config\ResolvedSources + private function resolveSources(?string $fromPath): ?\XPHP\Config\ResolvedSources { + if ($fromPath !== null) { + $dir = dirname($fromPath); + if (is_dir($dir)) { + $manifest = XphpManifest::locate($dir); + if ($manifest !== null) { + return $this->resolver()->resolve(null, $manifest, $dir); + } + } + } + + // Fall back to the workspace root. Guarded so we never locate('') and walk + // up from the process CWD onto an unrelated ancestor xphp.json; dropping + // this return is a real behaviour change, so only LogicalOr is ignored. + // @infection-ignore-all LogicalOr + if ($this->rootPath === '' || !is_dir($this->rootPath)) { + return null; + } $manifest = XphpManifest::locate($this->rootPath); if ($manifest !== null) { return $this->resolver()->resolve(null, $manifest, $this->rootPath); diff --git a/src/Diagnostics/DiagnosticsCheckSource.php b/src/Diagnostics/DiagnosticsCheckSource.php index e309770..b3c4aff 100644 --- a/src/Diagnostics/DiagnosticsCheckSource.php +++ b/src/Diagnostics/DiagnosticsCheckSource.php @@ -15,7 +15,12 @@ interface DiagnosticsCheckSource { /** + * @param ?string $fromPath absolute path of the file that triggered the run + * (the just-saved document). When given, the source set is resolved + * from the nearest `xphp.json` walking up from that file's directory — + * so multi-root / mis-rooted workspaces still scope to the file's own + * project. Null falls back to the configured workspace root. * @return array> */ - public function run(): array; + public function run(?string $fromPath = null): array; } diff --git a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php index 3bbcf46..8beb159 100644 --- a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php +++ b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php @@ -47,7 +47,7 @@ public function __construct(private array $queue) { } - public function run(): array + public function run(?string $fromPath = null): array { return array_shift($this->queue) ?? []; } diff --git a/test/Diagnostics/CompilerCheckRunnerTest.php b/test/Diagnostics/CompilerCheckRunnerTest.php index 0f5e407..6d2e6ea 100644 --- a/test/Diagnostics/CompilerCheckRunnerTest.php +++ b/test/Diagnostics/CompilerCheckRunnerTest.php @@ -149,6 +149,41 @@ public function testSyntheticSpecializationPathsAreNotPublished(): void } } + public function testPerDocumentDiscoveryScopesToTheSavedFilesOwnManifest(): void + { + // No workspace root at all: saving a file inside a project that has an + // xphp.json still discovers that manifest by walking up from the file, and + // scopes to it. This is the multi-root / mis-rooted workspace fix. + $reject = $this->fixture('scoped') . '/src/Reject.xphp'; + $byUri = (new CompilerCheckRunner(''))->run($reject); + + $flat = []; + foreach ($byUri as $diags) { + foreach ($diags as $d) { + $flat[] = $d; + } + } + self::assertContains( + 'xphp.closure_conformance', + self::codes($flat), + "the saved file's own project is checked via its nearest xphp.json", + ); + // tests/ is outside the manifest's sources (["src"]) — it must NOT be + // checked, even though Ignored.xphp carries its own error. + foreach (array_keys($byUri) as $uri) { + self::assertStringNotContainsString('/tests/', (string) $uri, 'files outside the manifest sources are excluded'); + } + } + + public function testFromPathWithoutAnAncestorManifestFallsBackToTheWorkspaceRoot(): void + { + // The `multi` fixture has no xphp.json above it, so a saved file there + // falls back to the configured rootPath resolution (both files checked). + $root = $this->fixture('multi'); + $byUri = (new CompilerCheckRunner($root))->run($root . '/A.xphp'); + self::assertCount(2, $byUri); + } + public function testLargeSourceSetsAreSkippedToProtectTheEventLoop(): void { // The `multi` fixture has two files; a cap of 1 must skip the whole pass diff --git a/test/fixture/authoritative/scoped/src/Reject.xphp b/test/fixture/authoritative/scoped/src/Reject.xphp new file mode 100644 index 0000000..30473cb --- /dev/null +++ b/test/fixture/authoritative/scoped/src/Reject.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('PHP')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/scoped/tests/Ignored.xphp b/test/fixture/authoritative/scoped/tests/Ignored.xphp new file mode 100644 index 0000000..c322c65 --- /dev/null +++ b/test/fixture/authoritative/scoped/tests/Ignored.xphp @@ -0,0 +1,21 @@ + +{ + public function __construct(private E $value) {} + + public function map(Closure(E $x): R $fn): R + { + return $fn($this->value); + } +} + +$box = new Box::(new Book('x')); +$box->map::(fn(int $x): string => (string) $x); diff --git a/test/fixture/authoritative/scoped/xphp.json b/test/fixture/authoritative/scoped/xphp.json new file mode 100644 index 0000000..4365f01 --- /dev/null +++ b/test/fixture/authoritative/scoped/xphp.json @@ -0,0 +1,5 @@ +{ + "sources": ["src"], + "target": "build", + "cache": "cache" +} From 14a207e13243973e7bba6a69d8f58fbc819d834b Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 19:35:17 +0000 Subject: [PATCH 18/26] fix(diagnostics): clear stale authoritative diagnostics on edit The on-save authoritative diagnostics are computed against the on-disk state and merged into open documents. When a document was edited after a save -- e.g. commenting out or deleting the offending line -- the stale finding kept being merged onto the changed buffer until the next save, so an error lingered on a line the user had already fixed. Evict a file's authoritative diagnostics on TextDocumentUpdated (any edit) so they stop being merged the instant the buffer diverges from disk; the engine re-publishes the changed document on the same event (clearing the squiggle), and the next save re-checks and re-publishes if the error is still present. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../AuthoritativeDiagnosticsListener.php | 16 +++++++++ .../AuthoritativeDiagnosticsStore.php | 12 +++++++ .../AuthoritativeDiagnosticsListenerTest.php | 35 ++++++++++++++++--- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/Diagnostics/AuthoritativeDiagnosticsListener.php b/src/Diagnostics/AuthoritativeDiagnosticsListener.php index 62fb663..8cbea9b 100644 --- a/src/Diagnostics/AuthoritativeDiagnosticsListener.php +++ b/src/Diagnostics/AuthoritativeDiagnosticsListener.php @@ -7,6 +7,7 @@ use Closure; use Phpactor\LanguageServer\Core\Workspace\Workspace as PhpactorWorkspace; use Phpactor\LanguageServer\Event\TextDocumentSaved; +use Phpactor\LanguageServer\Event\TextDocumentUpdated; use Phpactor\LanguageServerProtocol\Diagnostic as LspDiagnostic; use Psr\EventDispatcher\ListenerProviderInterface; @@ -63,9 +64,24 @@ public function getListenersForEvent(object $event): iterable if ($event instanceof TextDocumentSaved) { return [[$this, 'onSave']]; } + if ($event instanceof TextDocumentUpdated) { + return [[$this, 'onChange']]; + } return []; } + /** + * On any edit, evict the document's authoritative diagnostics so the stale + * (last-saved) findings stop being merged the instant the buffer diverges + * from disk. The engine re-publishes the changed document on this same event, + * so the squiggle clears immediately; the next save re-checks and, if the + * error is still there, re-publishes it. + */ + public function onChange(TextDocumentUpdated $event): void + { + $this->store->evict($event->identifier()->uri); + } + public function onSave(TextDocumentSaved $event): void { // The saved file anchors per-document manifest discovery in the runner. diff --git a/src/Diagnostics/AuthoritativeDiagnosticsStore.php b/src/Diagnostics/AuthoritativeDiagnosticsStore.php index e211ff4..35a9f51 100644 --- a/src/Diagnostics/AuthoritativeDiagnosticsStore.php +++ b/src/Diagnostics/AuthoritativeDiagnosticsStore.php @@ -49,4 +49,16 @@ public function get(string $uri): array { return $this->byUri[$uri] ?? []; } + + /** + * Drop a file's authoritative diagnostics. Called when the document is edited: + * the stored findings were computed against the on-disk state, so any edit + * makes them stale (e.g. commenting out the offending line). They must stop + * being merged immediately and only return after the next save re-checks — + * otherwise a stale error lingers on a line the user has already changed. + */ + public function evict(string $uri): void + { + unset($this->byUri[$uri]); + } } diff --git a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php index 8beb159..778f74f 100644 --- a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php +++ b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php @@ -123,7 +123,7 @@ function (string $uri, ?int $version, array $diagnostics) use (&$published): voi self::assertCount(1, $store->get(self::URI), 'but the store still holds them for the merge'); } - public function testOnlyReactsToSaveEvents(): void + public function testReactsToSaveAndChangeEventsOnly(): void { $listener = new AuthoritativeDiagnosticsListener( $this->source([]), @@ -137,14 +137,39 @@ static function (): void { new \Phpactor\LanguageServerProtocol\TextDocumentIdentifier(self::URI), null, ); - self::assertEquals( - [[$listener, 'onSave']], - [...$listener->getListenersForEvent($saved)], - 'a save event binds the onSave handler', + $changed = new \Phpactor\LanguageServer\Event\TextDocumentUpdated( + new \Phpactor\LanguageServerProtocol\VersionedTextDocumentIdentifier(2, self::URI), + 'text', ); + self::assertEquals([[$listener, 'onSave']], [...$listener->getListenersForEvent($saved)], 'save binds onSave'); + self::assertEquals([[$listener, 'onChange']], [...$listener->getListenersForEvent($changed)], 'change binds onChange'); self::assertSame([], [...$listener->getListenersForEvent(new \stdClass())], 'an unrelated event yields nothing'); } + public function testEditingAFileEvictsItsStaleAuthoritativeDiagnostics(): void + { + // A file had an authoritative error at last save; editing it (e.g. + // commenting out the line) must drop that stale finding immediately so it + // isn't merged onto a line the user has already changed. + $store = new AuthoritativeDiagnosticsStore(); + $store->replaceAll([self::URI => [$this->diag()]]); + $listener = new AuthoritativeDiagnosticsListener( + $this->source([]), + $store, + static function (): void { + }, + null, + ); + self::assertCount(1, $store->get(self::URI)); + + $listener->onChange(new \Phpactor\LanguageServer\Event\TextDocumentUpdated( + new \Phpactor\LanguageServerProtocol\VersionedTextDocumentIdentifier(2, self::URI), + 'edited text', + )); + + self::assertSame([], $store->get(self::URI), 'editing evicts the stale authoritative diagnostics'); + } + public function testNonOpenFileIsPublishedEvenAfterAnOpenOne(): void { // The publish loop must SKIP open docs and keep going — not stop — so a From cae55a778b54a1b09928db9ef7d92c78047e5f91 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 19:45:35 +0000 Subject: [PATCH 19/26] feat(diagnostics): run the authoritative check on didOpen too Opening a document now runs the on-save authoritative check for its project, so a file's deep diagnostics (closure conformance, missing_type_argument, cross-file generics) appear immediately on open -- from its last-saved state, which equals the buffer at open time -- without needing a redundant save. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../AuthoritativeDiagnosticsListener.php | 24 +++++++++++++++++-- .../AuthoritativeDiagnosticsListenerTest.php | 4 ++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/Diagnostics/AuthoritativeDiagnosticsListener.php b/src/Diagnostics/AuthoritativeDiagnosticsListener.php index 8cbea9b..0584d47 100644 --- a/src/Diagnostics/AuthoritativeDiagnosticsListener.php +++ b/src/Diagnostics/AuthoritativeDiagnosticsListener.php @@ -6,6 +6,7 @@ use Closure; use Phpactor\LanguageServer\Core\Workspace\Workspace as PhpactorWorkspace; +use Phpactor\LanguageServer\Event\TextDocumentOpened; use Phpactor\LanguageServer\Event\TextDocumentSaved; use Phpactor\LanguageServer\Event\TextDocumentUpdated; use Phpactor\LanguageServerProtocol\Diagnostic as LspDiagnostic; @@ -64,12 +65,27 @@ public function getListenersForEvent(object $event): iterable if ($event instanceof TextDocumentSaved) { return [[$this, 'onSave']]; } + if ($event instanceof TextDocumentOpened) { + return [[$this, 'onOpen']]; + } if ($event instanceof TextDocumentUpdated) { return [[$this, 'onChange']]; } return []; } + /** + * Opening a document runs the check for its project too, so a file's deep + * diagnostics appear immediately on open (from its last-saved state) without + * needing a redundant save. The buffer equals disk at open time, so the + * on-disk analysis is accurate. The engine publishes the opened document on + * this same event, merging the freshly-populated store. + */ + public function onOpen(TextDocumentOpened $event): void + { + $this->scheduleRun(self::uriToPath($event->textDocument()->uri)); + } + /** * On any edit, evict the document's authoritative diagnostics so the stale * (last-saved) findings stop being merged the instant the buffer diverges @@ -85,10 +101,14 @@ public function onChange(TextDocumentUpdated $event): void public function onSave(TextDocumentSaved $event): void { // The saved file anchors per-document manifest discovery in the runner. - $fromPath = self::uriToPath($event->identifier()->uri); + $this->scheduleRun(self::uriToPath($event->identifier()->uri)); + } + + private function scheduleRun(?string $fromPath): void + { $generation = ++$this->generation; asyncCall(function () use ($generation, $fromPath): void { - // A newer save arrived before this tick ran — let that one do the work. + // A newer trigger arrived before this tick ran — let that one do it. if ($generation !== $this->generation) { return; } diff --git a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php index 778f74f..4ab46a7 100644 --- a/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php +++ b/test/Diagnostics/AuthoritativeDiagnosticsListenerTest.php @@ -141,7 +141,11 @@ static function (): void { new \Phpactor\LanguageServerProtocol\VersionedTextDocumentIdentifier(2, self::URI), 'text', ); + $opened = new \Phpactor\LanguageServer\Event\TextDocumentOpened( + new TextDocumentItem(self::URI, 'xphp', 1, 'getListenersForEvent($saved)], 'save binds onSave'); + self::assertEquals([[$listener, 'onOpen']], [...$listener->getListenersForEvent($opened)], 'open binds onOpen'); self::assertEquals([[$listener, 'onChange']], [...$listener->getListenersForEvent($changed)], 'change binds onChange'); self::assertSame([], [...$listener->getListenersForEvent(new \stdClass())], 'an unrelated event yields nothing'); } From 22ad6eb018abf5071ff808f3bdc9584e4bda6875 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 20:41:02 +0000 Subject: [PATCH 20/26] feat(index): fold opened sibling projects into the FQN index The server tracks a single workspace rootPath and does not read workspaceFolders, so opening a file from a project OUTSIDE that root left navigation broken -- its sources were never indexed. On didOpen, discover the opened file's own project (nearest xphp.json, walking up) and register its declared source roots on the existing FqnIndex, which already supports multiple roots and routes duplicate FQNs by proximity. Go-to-definition, references, completion and rename now resolve symbols declared in the opened project even when the workspace is rooted elsewhere. Keys purely off the opened file's path -- no editor-specific signal. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/LspDispatcherFactory.php | 5 + src/Reflection/FqnIndex.php | 58 ++++++++- src/Reflection/OpenedProjectIndexer.php | 76 +++++++++++ test/Reflection/FqnIndexMultiRootTest.php | 54 ++++++++ test/Reflection/OpenedProjectIndexerTest.php | 129 +++++++++++++++++++ 5 files changed, 319 insertions(+), 3 deletions(-) create mode 100644 src/Reflection/OpenedProjectIndexer.php create mode 100644 test/Reflection/OpenedProjectIndexerTest.php diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index 2e5a596..a4aed0f 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -248,6 +248,11 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // pay the ~500ms filesystem-walk cost in-band. Async via // Amp\asyncCall -- doesn't block the initialize handshake. new \XPHP\Lsp\Reflection\FqnIndexWarmer($fqnIndex), + // Multi-root (Track A): when a file from a project OUTSIDE the + // workspace root is opened, fold that project's source roots into the + // FQN index so navigation resolves its symbols. Keys purely off the + // opened file's path -- no editor-specific signal. + new \XPHP\Lsp\Reflection\OpenedProjectIndexer($fqnIndex), // Perf #1: warm ParsedDocumentCache with every filesystem- // indexed file so the cold first `textDocument/references` // (codeLens click, Alt+F7) skips the per-file parse step -- diff --git a/src/Reflection/FqnIndex.php b/src/Reflection/FqnIndex.php index e4a16a1..73fb26e 100644 --- a/src/Reflection/FqnIndex.php +++ b/src/Reflection/FqnIndex.php @@ -159,11 +159,15 @@ final class FqnIndex */ private ?string $currentOrigin = null; - /** @var list deduped, existing source-root dirs to walk (realpath'd). */ - private readonly array $sourceRoots; + /** + * @var list deduped source-root dirs to walk (realpath'd). Not readonly: + * {@see registerSourceRoots} appends the source roots of a sibling project as + * its files are opened, so navigation resolves symbols declared there. + */ + private array $sourceRoots; /** @var array realpath'd dirs to prune from the walk (manifest output/cache). */ - private readonly array $excludedRealDirs; + private array $excludedRealDirs; /** * @param string $rootPath the primary workspace root (InitializeParams). An @@ -484,6 +488,54 @@ public function iterGenericFunctionsAndMethods(): iterable * (~100ms across the playground), and the next FqnIndex query is * usually one keystroke away anyway. */ + /** + * Add source-root dirs (and dirs to exclude) to the walk after construction — + * used to fold a sibling project into the index when one of its files is + * opened, so go-to-definition / references / completion resolve symbols + * declared there even though the workspace is rooted elsewhere. Dirs already + * indexed (by realpath) are ignored. Returns whether anything new was added; + * when it did, the filesystem snapshot is invalidated so the next query + * re-walks (the caller may warm that off-thread). + * + * @param list $dirs absolute source-root dirs to walk + * @param list $excludedDirs absolute dirs to prune (build-output / cache) + */ + public function registerSourceRoots(array $dirs, array $excludedDirs = []): bool + { + $changed = false; + $seen = array_fill_keys($this->sourceRoots, true); + foreach ($dirs as $dir) { + if ($dir === '' || !is_dir($dir)) { + continue; + } + $real = realpath($dir); + // @infection-ignore-all Ternary -- the `: $dir` branch is unreachable: + // realpath() only fails on a missing path, and !is_dir already skipped + // those above. + $key = $real !== false ? $real : $dir; + if (isset($seen[$key])) { + continue; + } + // @infection-ignore-all TrueValue -- $seen is read only via isset(), so + // the stored value is irrelevant. + $seen[$key] = true; + $this->sourceRoots[] = $key; + $changed = true; + } + foreach (self::normalizeExcludedDirs($excludedDirs) as $real => $_) { + if (!isset($this->excludedRealDirs[$real])) { + // @infection-ignore-all TrueValue -- excludedRealDirs is a set read + // via isset(); the sentinel value is irrelevant. + $this->excludedRealDirs[$real] = true; + $changed = true; + } + } + if ($changed) { + $this->invalidateFilesystem(); + } + return $changed; + } + public function invalidateFilesystem(): void { $this->filesystemMap = null; diff --git a/src/Reflection/OpenedProjectIndexer.php b/src/Reflection/OpenedProjectIndexer.php new file mode 100644 index 0000000..19ae165 --- /dev/null +++ b/src/Reflection/OpenedProjectIndexer.php @@ -0,0 +1,76 @@ +register(self::uriToPath($event->textDocument()->uri)); + } + + /** + * Discover the file's project and register its source roots. Returns whether + * any new root was added (so a caller can warm the walk off-thread). + * Synchronous and defensive — the unit-test entry point. + */ + public function register(?string $path): bool + { + if ($path === null) { + return false; + } + $dir = \dirname($path); + if (!is_dir($dir)) { + return false; + } + $manifest = XphpManifest::discover($dir); + if ($manifest === null) { + return false; + } + // @infection-ignore-all UnwrapArrayValues -- registerSourceRoots iterates + // the excluded dirs by value, so the array_values reindex is cosmetic. + $excluded = array_values(array_filter([$manifest->outputDir(), $manifest->cacheDir()])); + + return $this->fqnIndex->registerSourceRoots($manifest->sourceRoots(), $excluded); + } + + private static function uriToPath(string $uri): ?string + { + // @infection-ignore-all DecrementInteger -- 7 == strlen('file://'); an + // off-by-one leaves a leading `//path`, which POSIX collapses to `/path`, + // so the resolved path is unchanged. + return str_starts_with($uri, 'file://') ? substr($uri, 7) : null; + } +} diff --git a/test/Reflection/FqnIndexMultiRootTest.php b/test/Reflection/FqnIndexMultiRootTest.php index 43140c7..69e0079 100644 --- a/test/Reflection/FqnIndexMultiRootTest.php +++ b/test/Reflection/FqnIndexMultiRootTest.php @@ -90,6 +90,60 @@ public function testEmptyRootPathWithNoExtraRootsIndexesNothing(): void self::assertSame([], $index->indexedFilesystemPaths()); } + public function testRegisterSourceRootsFoldsInASiblingProject(): void + { + $rootA = $this->makeDir('pkgA'); + $rootB = $this->makeDir('pkgB'); + $this->write($rootA . '/A.xphp', "write($rootB . '/B.xphp', "index($rootA, []); + self::assertNull($index->pathFor('App\\Beta')); + + $before = $index->filesystemVersion(); + self::assertTrue($index->registerSourceRoots([$rootB]), 'a new root was added'); + self::assertGreaterThan($before, $index->filesystemVersion(), 'adding a root invalidates the snapshot'); + + // Now both projects resolve. + self::assertSame(realpath($rootA . '/A.xphp'), $index->pathFor('App\\Alpha')); + self::assertSame(realpath($rootB . '/B.xphp'), $index->pathFor('App\\Beta')); + } + + public function testRegisterSourceRootsProcessesEveryEntry(): void + { + // A skipped entry (non-dir) and a duplicate in the MIDDLE must not stop + // the loop — the valid root after them still gets registered. + $rootB = $this->makeDir('pkgB'); + $rootC = $this->makeDir('pkgC'); + $this->write($rootB . '/B.xphp', "write($rootC . '/C.xphp', "index('', []); + self::assertTrue($index->registerSourceRoots([$rootB, '/no/such/dir', $rootB, $rootC])); + + self::assertSame(realpath($rootB . '/B.xphp'), $index->pathFor('App\\Beta')); + self::assertSame(realpath($rootC . '/C.xphp'), $index->pathFor('App\\Gamma'), 'the root after a skip/dup is still registered'); + } + + public function testRegisterSourceRootsIsIdempotentAndPrunesExcluded(): void + { + $root = $this->makeDir('app'); + $this->write($root . '/src/Widget.xphp', "write($root . '/build/Widget_x.php', "index('', []); + self::assertTrue($index->registerSourceRoots([$root . '/src'], [$root . '/build'])); + + // Re-registering the same root (and a non-existent one) adds nothing. + $version = $index->filesystemVersion(); + self::assertFalse($index->registerSourceRoots([$root . '/src', $root . '/does-not-exist'])); + self::assertSame($version, $index->filesystemVersion(), 'no change means no invalidation'); + + self::assertSame(realpath($root . '/src/Widget.xphp'), $index->pathFor('App\\Widget')); + self::assertNull($index->pathFor('XPHP\\Generated\\Widget_x'), 'excluded build dir is pruned'); + } + /** * @param list $extraRoots * @param list $excludedDirs diff --git a/test/Reflection/OpenedProjectIndexerTest.php b/test/Reflection/OpenedProjectIndexerTest.php new file mode 100644 index 0000000..8bc069e --- /dev/null +++ b/test/Reflection/OpenedProjectIndexerTest.php @@ -0,0 +1,129 @@ +base = sys_get_temp_dir() . '/xphp-opened-' . bin2hex(random_bytes(6)); + mkdir($this->base, 0o755, true); + } + + protected function tearDown(): void + { + $this->rmrf($this->base); + } + + public function testOpeningASiblingProjectFileRegistersItsSourceRoots(): void + { + // Source root is the project dir itself ("."), so the manifest's output + // and cache dirs sit INSIDE the walk and the exclusion is load-bearing + // (dir names chosen to avoid FqnIndex::SKIP_DIRS like `build`). + $proj = $this->base . '/collections'; + $this->write($proj . '/xphp.json', '{"sources": ["."], "target": "gen", "cache": "cch"}'); + $this->write($proj . '/ImmutableList.xphp', "write($proj . '/gen/Gen.xphp', "write($proj . '/cch/Cached.xphp', "emptyIndex(); + self::assertNull($index->pathFor('Coll\\ImmutableList'), 'not indexed before open'); + + $added = (new OpenedProjectIndexer($index))->register($proj . '/ImmutableList.xphp'); + + self::assertTrue($added, 'the sibling project was registered'); + self::assertSame( + realpath($proj . '/ImmutableList.xphp'), + $index->pathFor('Coll\\ImmutableList'), + 'a symbol declared in the opened project now resolves', + ); + self::assertNull($index->pathFor('Coll\\Gen\\Gen'), 'the manifest output dir is excluded'); + self::assertNull($index->pathFor('Coll\\Cch\\Cached'), 'the manifest cache dir is excluded'); + } + + public function testOnOpenDelegatesToRegister(): void + { + $this->write($this->base . '/proj/xphp.json', '{"sources": ["src"]}'); + $this->write($this->base . '/proj/src/Thing.xphp', "emptyIndex(); + $indexer = new OpenedProjectIndexer($index); + $uri = 'file://' . $this->base . '/proj/src/Thing.xphp'; + $indexer->onOpen(new TextDocumentOpened(new TextDocumentItem($uri, 'xphp', 1, 'base . '/proj/src/Thing.xphp'), $index->pathFor('P\\Thing')); + } + + public function testNoManifestAboveTheFileIsANoOp(): void + { + $this->write($this->base . '/loose/Free.xphp', "emptyIndex(); + + self::assertFalse((new OpenedProjectIndexer($index))->register($this->base . '/loose/Free.xphp')); + } + + public function testNonFileUriIsANoOp(): void + { + self::assertFalse((new OpenedProjectIndexer($this->emptyIndex()))->register(null)); + } + + public function testSubscribesToOpenEventsOnly(): void + { + $indexer = new OpenedProjectIndexer($this->emptyIndex()); + $opened = new TextDocumentOpened(new TextDocumentItem('file:///x.xphp', 'xphp', 1, 'getListenersForEvent($opened)]); + self::assertSame([], [...$indexer->getListenersForEvent(new \stdClass())]); + } + + private function emptyIndex(): FqnIndex + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + + return new FqnIndex(new PhpactorWorkspace(), $cache, $parser, ''); + } + + private function write(string $path, string $contents): void + { + $dir = \dirname($path); + if (!is_dir($dir)) { + mkdir($dir, 0o755, true); + } + file_put_contents($path, $contents); + } + + private function rmrf(string $dir): void + { + if (!is_dir($dir)) { + return; + } + foreach (scandir($dir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $path = $dir . '/' . $entry; + is_dir($path) ? $this->rmrf($path) : unlink($path); + } + rmdir($dir); + } +} From d7564b2af35a17d65e2c956dc1dabb655b96b607 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 20:45:02 +0000 Subject: [PATCH 21/26] perf(index): warm a newly-registered project off-thread Registering an opened project's source roots invalidates the FQN index's filesystem snapshot, so the next query re-walks in-band (~500ms for a large project). Warm that walk via Amp\asyncCall right after registration, mirroring FqnIndexWarmer, so the first navigation into a sibling project isn't cold. The warm is guarded (only when roots were actually added) and wrapped in a try/catch so a failure warming a less-trusted sibling path can't reach the loop error handler -- the roots stay registered and the next query re-walks in-band. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Reflection/OpenedProjectIndexer.php | 26 ++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Reflection/OpenedProjectIndexer.php b/src/Reflection/OpenedProjectIndexer.php index 19ae165..49e7927 100644 --- a/src/Reflection/OpenedProjectIndexer.php +++ b/src/Reflection/OpenedProjectIndexer.php @@ -7,6 +7,9 @@ use Phpactor\LanguageServer\Event\TextDocumentOpened; use Psr\EventDispatcher\ListenerProviderInterface; use XPHP\Lsp\Project\XphpManifest; +use XPHP\Lsp\Stderr; + +use function Amp\asyncCall; /** * Folds a sibling project into the FQN index as its files are opened. @@ -38,7 +41,28 @@ public function getListenersForEvent(object $event): iterable public function onOpen(TextDocumentOpened $event): void { - $this->register(self::uriToPath($event->textDocument()->uri)); + $registered = $this->register(self::uriToPath($event->textDocument()->uri)); + + // Registering new roots invalidated the filesystem snapshot; warm the + // re-walk off-thread so the first navigation into the project doesn't pay + // the ~500ms cost in-band (mirrors FqnIndexWarmer). Pure latency + // optimization — register() above already made the roots resolvable, so + // this branch has no observable behaviour to assert. + // @infection-ignore-all + if ($registered) { + asyncCall(function (): void { + try { + $count = count($this->fqnIndex->allClassFqns()); + Stderr::write(sprintf("[xphp-lsp warmer] opened-project index warmed (%d FQNs)\n", $count)); + } catch (\Throwable $e) { + // Warming a sibling project from an arbitrary didOpen touches + // less-trusted paths; a failure must not reach the loop error + // handler. The roots are still registered — the next query + // re-walks in-band and surfaces any real problem there. + Stderr::write(sprintf("[xphp-lsp warmer] opened-project warm failed: %s\n", $e->getMessage())); + } + }); + } } /** From c7b6701565011bbe0b6466992e17b24b9c9246b9 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 20:48:42 +0000 Subject: [PATCH 22/26] test(index): end-to-end multi-root navigation across sibling projects Add durable committed fixtures (two sibling projects, app + lib, each with its own xphp.json) and an end-to-end test asserting the full navigation query surface -- definition (pathFor), go-to-definition (locationForFqn) and completion/import (fqnsByShortName) -- resolves the sibling's symbols only after its file is opened. Complements the temp-dir OpenedProjectIndexerTest by exercising the wider surface against a realistic rooted-at-A / open-from-B layout. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Reflection/MultiRootNavigationTest.php | 58 ++++++++++++++++++++ test/fixture/multiroot/app/src/Consumer.xphp | 13 +++++ test/fixture/multiroot/app/xphp.json | 3 + test/fixture/multiroot/lib/src/Widget.xphp | 11 ++++ test/fixture/multiroot/lib/xphp.json | 3 + 5 files changed, 88 insertions(+) create mode 100644 test/Reflection/MultiRootNavigationTest.php create mode 100644 test/fixture/multiroot/app/src/Consumer.xphp create mode 100644 test/fixture/multiroot/app/xphp.json create mode 100644 test/fixture/multiroot/lib/src/Widget.xphp create mode 100644 test/fixture/multiroot/lib/xphp.json diff --git a/test/Reflection/MultiRootNavigationTest.php b/test/Reflection/MultiRootNavigationTest.php new file mode 100644 index 0000000..6a0e1df --- /dev/null +++ b/test/Reflection/MultiRootNavigationTest.php @@ -0,0 +1,58 @@ +indexRootedAt($this->fixtureRoot('app/src')); + + self::assertNotNull($index->pathFor('App\\Consumer'), 'the rooted project resolves'); + self::assertNull($index->pathFor('Lib\\Widget'), 'the sibling is unresolved before its file is opened'); + self::assertNull($index->locationForFqn('Lib\\Widget'), 'GTD finds nothing before open'); + self::assertNotContains('Lib\\Widget', $index->fqnsByShortName('Widget'), 'completion offers nothing before open'); + + // Open a file from the `lib` sibling project. + $opened = $this->fixtureRoot('lib/src/Widget.xphp'); + self::assertTrue((new OpenedProjectIndexer($index))->register($opened), 'the sibling project is registered'); + + // Now the full navigation surface resolves Lib\Widget. + self::assertSame(realpath($opened), $index->pathFor('Lib\\Widget'), 'definition resolves to the sibling file'); + self::assertNotNull($index->locationForFqn('Lib\\Widget'), 'go-to-definition resolves the sibling symbol'); + self::assertContains('Lib\\Widget', $index->fqnsByShortName('Widget'), 'completion/import offers the sibling symbol'); + } + + private function indexRootedAt(string $rootPath): FqnIndex + { + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $cache = new ParsedDocumentCache(new Analyzer($parser)); + + return new FqnIndex(new PhpactorWorkspace(), $cache, $parser, $rootPath); + } +} diff --git a/test/fixture/multiroot/app/src/Consumer.xphp b/test/fixture/multiroot/app/src/Consumer.xphp new file mode 100644 index 0000000..64293bc --- /dev/null +++ b/test/fixture/multiroot/app/src/Consumer.xphp @@ -0,0 +1,13 @@ + Date: Sun, 19 Jul 2026 22:44:54 +0000 Subject: [PATCH 23/26] fix(resolver): resolve reassigned variables flow-sensitively Hovering a variable reassigned to a different generic type showed the LAST assignment's type regardless of position -- e.g. hovering `$x = $list->map::(...)` read as `List` because a later `$x = $list->map::(...)` overwrote the single per-scope binding. (Surfaced by multi-root: the sibling collection type was only resolvable at all once its project was indexed.) Record each assignment's LHS position in a per-scope history and, at query time, resolve each variable to the binding from the nearest assignment at or before the queried offset. The flat per-scope map is retained for during-walk RHS resolution (accumulated state); only the query path becomes flow-sensitive. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Resolver/GenericResolver.php | 82 +++++++++++++++++++++------ test/Resolver/GenericResolverTest.php | 46 +++++++++++++++ 2 files changed, 112 insertions(+), 16 deletions(-) diff --git a/src/Resolver/GenericResolver.php b/src/Resolver/GenericResolver.php index 5744ce4..dd0da2b 100644 --- a/src/Resolver/GenericResolver.php +++ b/src/Resolver/GenericResolver.php @@ -92,7 +92,7 @@ final class GenericResolver { /** - * @var array}>}> + * @var array, history: array>}>}> */ private array $cache = []; @@ -933,7 +933,7 @@ private static function buildSubstitutionFromMap( } /** - * @return list}> + * @return list, history: array>}> */ private function scopesFor(string $uri, int $version, string $text): array { @@ -962,11 +962,11 @@ private function scopesFor(string $uri, int $version, string $text): array } /** - * @return list}> + * @return list, history: array>}> */ private static function emptyScopes(): array { - return [['start' => 0, 'end' => PHP_INT_MAX, 'bindings' => []]]; + return [['start' => 0, 'end' => PHP_INT_MAX, 'bindings' => [], 'history' => []]]; } /** @@ -975,7 +975,7 @@ private static function emptyScopes(): array * matches and has the widest range, so any nested function scope * wins on overlap. * - * @param list}> $scopes + * @param list, history: array>}> $scopes * @return array */ public static function bindingsAt(array $scopes, int $offset): array @@ -989,7 +989,29 @@ public static function bindingsAt(array $scopes, int $offset): array $best = $scope; } } - return $best === null ? [] : $best['bindings']; + if ($best === null) { + return []; + } + // Flow-sensitive: for each variable, return the binding from the nearest + // assignment AT OR BEFORE $offset, so a later reassignment to a different + // type doesn't leak backwards onto an earlier hover (a var reassigned + // `$x = ...::()` then `$x = ...::()` reads as the right type + // at each site). Seeded params/uses carry the scope-start position. + $out = []; + foreach ($best['history'] as $name => $entries) { + $chosen = null; + foreach ($entries as $entry) { + // @infection-ignore-all GreaterThanOrEqualTo -- distinct assignments + // have distinct positions, so `>=` vs `>` never differ (no ties). + if ($entry['pos'] <= $offset && ($chosen === null || $entry['pos'] >= $chosen['pos'])) { + $chosen = $entry; + } + } + if ($chosen !== null) { + $out[$name] = $chosen['binding']; + } + } + return $out; } /** @@ -997,7 +1019,7 @@ public static function bindingsAt(array $scopes, int $offset): array * one (top-level) scope. * * @param list $ast - * @return list}> + * @return list, history: array>}> */ private function build(array $ast): array { @@ -1019,7 +1041,7 @@ private function build(array $ast): array $visitor = new class($scopes, $stack, $useMap, $currentNamespace, $classes, $fqnIndex) extends NodeVisitorAbstract { /** - * @param list}> $scopes + * @param list, history: array>}> $scopes * @param list $stack * @param array $useMap */ @@ -1060,10 +1082,12 @@ public function enterNode(Node $node): null if ($start < 0 || $end < 0) { return null; } + $seeded = self::seedFromParams($node->params, $this->classes); $this->scopes[] = [ 'start' => $start, 'end' => $end, - 'bindings' => self::seedFromParams($node->params, $this->classes), + 'bindings' => $seeded, + 'history' => self::seedHistory($seeded, $start), ]; $this->stack[] = count($this->scopes) - 1; return null; @@ -1095,6 +1119,7 @@ public function enterNode(Node $node): null 'start' => $start, 'end' => $end, 'bindings' => $closureBindings, + 'history' => self::seedHistory($closureBindings, $start), ]; $this->stack[] = count($this->scopes) - 1; return null; @@ -1127,6 +1152,10 @@ private function handleAssign(Assign $node): void return; } $name = $lhs->name; + // Position of this assignment's LHS: the binding it produces is + // in effect for hovers AT or AFTER this point, until the next + // reassignment. Enables flow-sensitive lookup (see bindingsAt). + $pos = $lhs->getStartFilePos(); $rhs = $node->expr; if ($rhs instanceof New_) { @@ -1141,7 +1170,7 @@ private function handleAssign(Assign $node): void $this->classes, ); if ($binding !== null) { - $this->writeBinding($name, $binding); + $this->writeBinding($name, $binding, $pos); } return; } @@ -1155,7 +1184,7 @@ private function handleAssign(Assign $node): void $this->currentNamespace, ); if ($resolved !== null) { - $this->writeBinding($name, $resolved); + $this->writeBinding($name, $resolved, $pos); } return; } @@ -1167,14 +1196,14 @@ private function handleAssign(Assign $node): void $this->currentNamespace, ); if ($resolved !== null) { - $this->writeBinding($name, $resolved); + $this->writeBinding($name, $resolved, $pos); } return; } if ($rhs instanceof FuncCall) { $resolved = GenericResolver::resolveFuncCall($rhs, $this->fqnIndex); if ($resolved !== null) { - $this->writeBinding($name, $resolved); + $this->writeBinding($name, $resolved, $pos); } return; } @@ -1188,7 +1217,7 @@ private function handleAssign(Assign $node): void $this->currentNamespace, ); if ($resolved !== null) { - $this->writeBinding($name, $resolved); + $this->writeBinding($name, $resolved, $pos); } } } @@ -1202,10 +1231,31 @@ private function currentBindings(): array return $this->scopes[$idx]['bindings']; } - private function writeBinding(string $name, VarBinding|ResolvedType $binding): void + private function writeBinding(string $name, VarBinding|ResolvedType $binding, int $pos): void { $idx = $this->stack[count($this->stack) - 1]; + // `bindings` is the accumulated (last-wins) map used to resolve + // later RHS expressions DURING the walk. `history` additionally + // records each assignment's position so a QUERY can pick the + // binding in effect at a given offset (flow-sensitive). $this->scopes[$idx]['bindings'][$name] = $binding; + $this->scopes[$idx]['history'][$name][] = ['pos' => $pos, 'binding' => $binding]; + } + + /** + * Seed flow-history from an initial binding map (function params, + * closure uses) so those bindings are visible from the scope's start. + * + * @param array $bindings + * @return array> + */ + private static function seedHistory(array $bindings, int $pos): array + { + $history = []; + foreach ($bindings as $name => $binding) { + $history[$name] = [['pos' => $pos, 'binding' => $binding]]; + } + return $history; } /** @@ -1430,7 +1480,7 @@ public function findNullDerefSites(string $uri, int $version, string $text): arr ) extends NodeVisitorAbstract { /** * @param list $sites - * @param list}> $scopes + * @param list, history: array>}> $scopes * @param array $useMap */ public function __construct( diff --git a/test/Resolver/GenericResolverTest.php b/test/Resolver/GenericResolverTest.php index c1780e2..3609920 100644 --- a/test/Resolver/GenericResolverTest.php +++ b/test/Resolver/GenericResolverTest.php @@ -1138,6 +1138,17 @@ public function testClosureParamSeededAlongsideCapture(): void '?App\\Models\\User', $resolver->resolveVariable('/Use.xphp', 'b', $bOffset), ); + // Directly hovering the seeded bindings inside the closure resolves both. + // The CAPTURE ($outer) is the SECOND seeded entry, so this guards against + // the seed-history dropping later bindings. + self::assertSame( + 'App\\Containers\\Collection', + $resolver->resolveVariable('/Use.xphp', 'param', strpos($source, '$param->first')), + ); + self::assertSame( + 'App\\Containers\\Collection', + $resolver->resolveVariable('/Use.xphp', 'outer', strpos($source, '$outer->first')), + ); } public function testRebuildsBindingsOnDocumentVersionBump(): void @@ -1220,6 +1231,41 @@ private function rmrf(string $dir): void rmdir($dir); } + public function testReassignedVariableResolvesFlowSensitively(): void + { + // A variable reassigned to a different method-turbofish result must read + // as the type in effect AT each site: a later reassignment must not leak + // its type backwards onto an earlier hover. + $workspace = $this->workspace(); + $this->open($workspace, '/Use.xphp', <<<'XPHP' + + { + public function __construct(E ...$e) {} + public function map(Closure(E $x): R $fn): Box { return new Box::(); } + } + $b = new Box::(new Book()); + $x = $b->map::(fn (Book $bk): string => 'x'); + $mid = $x; + $x = $b->map::(fn (Book $bk): int => 1); + XPHP); + + $resolver = $this->resolver($workspace); + $src = $workspace->get('/Use.xphp')->text; + // The `$` of each `$x =` assignment is exactly its binding position. + $firstAssign = (int) strpos($src, '$x = $b->map::'); + $secondAssign = (int) strpos($src, '$x = $b->map::'); + + // AT the exact first-assignment offset the binding is already in effect + // (boundary: pos <= offset), so it reads Box, not the later int. + self::assertSame('App\\Box', $resolver->resolveVariable('/Use.xphp', 'x', $firstAssign)); + self::assertSame('App\\Box', $resolver->resolveVariable('/Use.xphp', 'x', $firstAssign + 5)); + self::assertSame('App\\Box', $resolver->resolveVariable('/Use.xphp', 'x', $secondAssign)); + self::assertSame('App\\Box', $resolver->resolveVariable('/Use.xphp', 'x', PHP_INT_MAX)); + } + private function workspace(): PhpactorWorkspace { return new PhpactorWorkspace(); From 06fce096847a0f898ffa6f49b3b6e9823031136b Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Sun, 19 Jul 2026 23:56:46 +0000 Subject: [PATCH 24/26] fix(hover): render generic method signatures from the xphp declaration Hovering a generic method showed the generics-ERASED worse-reflection view -- `map(Closure $fn): ImmutableList` -- dropping the `` method-generic clause and erasing the `Closure(E $x): R` parameter to bare `Closure`, leaving `R` orphaned in the return type. Recover the original xphp method AST (via FqnIndex::classLikeAstFor on the DECLARING class, so inherited methods resolve) and render its `` clause plus the `Closure(...)` signature params (via ClosureSignatureView). Falls back to the erased worse-reflection view when no index/declaration is available; non-generic method hovers are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- features/understand/hover.feature | 6 +- src/LspDispatcherFactory.php | 2 +- src/Resolver/PhpHoverResolver.php | 95 +++++++++++++++++++++++++- test/Resolver/PhpHoverResolverTest.php | 82 +++++++++++++++++++++- 4 files changed, 177 insertions(+), 8 deletions(-) diff --git a/features/understand/hover.feature b/features/understand/hover.feature index 00b707b..ef91d68 100644 --- a/features/understand/hover.feature +++ b/features/understand/hover.feature @@ -80,7 +80,7 @@ Feature: Hover """ And the FQN index has been warmed on initialize When I request "textDocument/hover" on "identity" at line 3 of "/Use.xphp" - Then the hover contents contain "identity(string $x): string" + Then the hover contents contain "identity(string $x): string" Scenario: Hover over an inherited generic method turbofish resolves through the base class Given the file at "/Base.xphp" contains the following lines: @@ -107,7 +107,7 @@ Feature: Hover """ And the FQN index has been warmed on initialize When I request "textDocument/hover" on "identity" at line 3 of "/Use.xphp" - Then the hover contents contain "identity(string $x): string" + Then the hover contents contain "identity(string $x): string" Scenario: Hover over a method returning static resolves to the receiver's concrete type Given the file at "/Builder.xphp" contains the following lines: @@ -359,7 +359,7 @@ Feature: Hover """ And the FQN index has been warmed on initialize When I request "textDocument/hover" on "list" at line 3 of "/Use.xphp" - Then the hover contents contain "list(string $x): string" + Then the hover contents contain "list(string $x): string" Scenario: Hover over a method type parameter bounded by the enclosing class param Given the file at "/Box.xphp" contains the following lines: diff --git a/src/LspDispatcherFactory.php b/src/LspDispatcherFactory.php index a4aed0f..06c57dd 100644 --- a/src/LspDispatcherFactory.php +++ b/src/LspDispatcherFactory.php @@ -189,7 +189,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia // on property access through a generic method's return type can // resolve via the substituted receiver class. $phpDefinitionResolver = new PhpDefinitionResolver($workspace, $xphpParser, $reflector, $cache, $genericResolver); - $phpHoverResolver = new PhpHoverResolver($workspace, $xphpParser, $reflector, $genericParams, $genericResolver); + $phpHoverResolver = new PhpHoverResolver($workspace, $xphpParser, $reflector, $genericParams, $genericResolver, $fqnIndex); // Authoritative (on-save) diagnostics tier: the compiler's own // whole-project `check()` produces the grounded, call-argument closure diff --git a/src/Resolver/PhpHoverResolver.php b/src/Resolver/PhpHoverResolver.php index 9c4aaff..4b67823 100644 --- a/src/Resolver/PhpHoverResolver.php +++ b/src/Resolver/PhpHoverResolver.php @@ -6,8 +6,14 @@ use Amp\CancellationToken; use PhpParser\Node; +use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Name; +use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; +use XPHP\Lsp\Reflection\FqnIndex; +use XPHP\Transpiler\Monomorphize\ClosureSignature; +use XPHP\Transpiler\Monomorphize\TypeParam; use Phpactor\LanguageServer\Core\Workspace\Workspace as PhpactorWorkspace; use Phpactor\LanguageServerProtocol\Hover; use Phpactor\LanguageServerProtocol\MarkupContent; @@ -50,6 +56,12 @@ public function __construct( private readonly Reflector $reflector, private readonly GenericParamRegistry $genericParams, private readonly GenericResolver $genericResolver, + // Used to recover the ORIGINAL xphp method declaration (worse-reflection + // only sees the generics-erased, stripped form) so a generic method's + // hover shows its `` clause and `Closure(...)` signature params rather + // than a bare `Closure` with an orphaned `R` in the return. Optional: + // when absent, method hovers fall back to the erased worse-reflection view. + private readonly ?FqnIndex $fqnIndex = null, ) { } @@ -330,9 +342,25 @@ private function renderMethod(string $classFqn, string $methodName, ?MethodCallS // available; fall back to prettify (drops the namespace from // placeholder names) for params/return the substitution doesn't // cover (e.g. parameters with union types). + // Recover the original xphp declaration: its method-generic clause (``) + // and any `Closure(...)`-signature parameter types, both erased in the + // worse-reflection view. Look it up on the DECLARING class (not the + // receiver) so a method inherited from a base class still resolves. Null + // for non-xphp / non-generic methods. + $xphpMethod = $this->xphpMethodNode((string) $method->declaringClass()->name(), $methodName); + $clause = $xphpMethod !== null ? self::methodGenericClause($xphpMethod) : ''; + $closureSigParams = $xphpMethod !== null ? self::closureSigParams($xphpMethod) : []; + $params = []; foreach ($method->parameters() as $param) { $paramName = $param->name(); + if (isset($closureSigParams[$paramName])) { + // Restore the erased `Closure(E $x): R` signature type. + // @infection-ignore-all UnwrapTrim -- the rendered closure sig + " $name" + // never carries surrounding whitespace, so trim() is a no-op here. + $params[] = trim($closureSigParams[$paramName] . ' $' . $paramName); + continue; + } $type = $substitution !== null && isset($substitution->paramTypes[$paramName]) ? $substitution->paramTypes[$paramName] : $this->genericParams->prettify((string) $param->inferredType()); @@ -342,10 +370,11 @@ private function renderMethod(string $classFqn, string $methodName, ?MethodCallS ? $substitution->returnType : $this->genericParams->prettify((string) $method->returnType()); $signature = sprintf( - '%s %sfunction %s(%s)%s', + '%s %sfunction %s%s(%s)%s', $visibility, $static, $method->name(), + $clause, implode(', ', $params), $return !== '' && $return !== '' ? ': ' . $return : '', ); @@ -354,6 +383,70 @@ private function renderMethod(string $classFqn, string $methodName, ?MethodCallS return self::format($signature, $docblock); } + /** + * The original xphp {@see ClassMethod} node for `$classFqn::$methodName`, or + * null (no index, unresolved class, or method not found). Used to recover the + * generic clause + closure-signature params the reflected view erases. + */ + private function xphpMethodNode(string $classFqn, string $methodName): ?ClassMethod + { + $hit = $this->fqnIndex?->classLikeAstFor($classFqn); + if ($hit === null) { + return null; + } + foreach ($hit['classLike']->getMethods() as $method) { + if (strcasecmp($method->name->toString(), $methodName) === 0) { + return $method; + } + } + return null; + } + + /** + * Render a method's own generic clause, e.g. `` or ``, from its + * `ATTR_METHOD_GENERIC_PARAMS`. Empty string for a non-generic method. + */ + private static function methodGenericClause(ClassMethod $method): string + { + $params = $method->getAttribute(XphpSourceParser::ATTR_METHOD_GENERIC_PARAMS); + if (!is_array($params) || $params === []) { + return ''; + } + $names = []; + foreach ($params as $param) { + if ($param instanceof TypeParam) { + $names[] = $param->name; + } + } + return $names === [] ? '' : '<' . implode(', ', $names) . '>'; + } + + /** + * Map of paramName -> rendered `Closure(...)` signature, for parameters whose + * declared type carries an `ATTR_CLOSURE_SIG` (erased to bare `\Closure` in + * the reflected view). + * + * @return array + */ + private static function closureSigParams(ClassMethod $method): array + { + $out = []; + foreach ($method->params as $param) { + // @infection-ignore-all LogicalOr -- defensive: a method Param's var is + // always a string-named Variable in parsed source, so the guard's two + // arms never both matter; it can't be exercised to distinguish ||/&&. + if (!$param->var instanceof Variable || !is_string($param->var->name)) { + continue; + } + $type = $param->type; + $sig = $type instanceof Name ? $type->getAttribute(XphpSourceParser::ATTR_CLOSURE_SIG) : null; + if ($sig instanceof ClosureSignature) { + $out[$param->var->name] = ClosureSignatureView::render($sig); + } + } + return $out; + } + private function renderProperty(?string $classFqn, string $propertyName, ?string $substitutedType = null): ?string { if ($classFqn === null) { diff --git a/test/Resolver/PhpHoverResolverTest.php b/test/Resolver/PhpHoverResolverTest.php index 98068cb..6c895df 100644 --- a/test/Resolver/PhpHoverResolverTest.php +++ b/test/Resolver/PhpHoverResolverTest.php @@ -313,7 +313,7 @@ public static function make(T $seed): T { return $seed; } $hover = $this->hoverAt($workspace, '/Use.xphp', $useSource, 'Factory::make', strlen('Factory::make')); self::assertSame( - "```php\n// App\\Containers\\Factory\npublic static function make(App\\Models\\User \$seed): App\\Models\\User\n```", + "```php\n// App\\Containers\\Factory\npublic static function make(App\\Models\\User \$seed): App\\Models\\User\n```", $this->markdown($hover), ); } @@ -385,7 +385,7 @@ public static function first(array $items): ?T { return $items[0] ?? null; } $hover = $this->hoverAt($workspace, '/Use.xphp', $useSource, 'Util::first', strlen('Util::first')); self::assertSame( - "```php\n// App\\Containers\\Util\npublic static function first(array \$items): ?T\n```", + "```php\n// App\\Containers\\Util\npublic static function first(array \$items): ?T\n```", $this->markdown($hover), ); } @@ -1106,7 +1106,7 @@ private function resolver(PhpactorWorkspace $workspace): PhpHoverResolver return $this->resolverWithRoot($workspace, ''); } - private function resolverWithRoot(PhpactorWorkspace $workspace, string $rootPath): PhpHoverResolver + private function resolverWithRoot(PhpactorWorkspace $workspace, string $rootPath, bool $withFqnIndex = true): PhpHoverResolver { $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); $cache = new ParsedDocumentCache(new Analyzer($parser)); @@ -1130,9 +1130,85 @@ private function resolverWithRoot(PhpactorWorkspace $workspace, string $rootPath $reflector, new GenericParamRegistry($fqnIndex), $generic, + $withFqnIndex ? $fqnIndex : null, ); } + public function testGenericMethodHoverRestoresClauseAndClosureSignature(): void + { + // Regression: a generic method with a Closure(...)-signature parameter + // must hover with its `` clause and the closure signature restored -- + // not the generics-erased worse-reflection view (`map(Closure $fn): + // ImmutableList`, where R came from nowhere). + $workspace = $this->workspace(); + $this->open($workspace, '/List.xphp', <<<'XPHP' + + { + public function __construct(E ...$e) {} + public function map(Closure(E $x): R $fn): ImmutableList { return new ImmutableList::(); } + } + XPHP); + $useSource = "(1);\n\$l->map::(fn (int \$x): string => 'x');\n"; + $this->open($workspace, '/Use.xphp', $useSource); + + $hover = $this->hoverAt($workspace, '/Use.xphp', $useSource, '->map', strlen('->')); + + self::assertSame( + "```php\n// App\\Coll\\ImmutableList\npublic function map(Closure(E): R \$fn): App\\Coll\\ImmutableList\n```", + $this->markdown($hover), + ); + } + + public function testMethodHoverRendersEveryClosureSignatureParam(): void + { + // A method with TWO Closure(...)-signature params must render BOTH -- guards + // the per-param loop and the closure-sig map against dropping a later one. + $workspace = $this->workspace(); + $this->open($workspace, '/Z.xphp', <<<'XPHP' + + { + public function zip(Closure(E $x): R $f, Closure(R $y): E $g): E {} + } + XPHP); + $useSource = "();\n\$z->zip::(fn (int \$x): string => 'x', fn (string \$y): int => 1);\n"; + $this->open($workspace, '/Use.xphp', $useSource); + + $md = $this->markdown($this->hoverAt($workspace, '/Use.xphp', $useSource, '->zip', strlen('->'))); + + self::assertStringContainsString('zip(', $md); + self::assertStringContainsString('Closure(E): R $f', $md); + self::assertStringContainsString('Closure(R): E $g', $md); + } + + public function testMethodHoverFallsBackWithoutFqnIndex(): void + { + // Without an FqnIndex the resolver can't recover the xphp declaration, so it + // falls back to the erased worse-reflection signature -- no crash, no clause. + $workspace = $this->workspace(); + $this->open($workspace, '/List.xphp', <<<'XPHP' + + { + public function map(Closure(E $x): R $fn): ImmutableList {} + } + XPHP); + $useSource = "();\n\$l->map::(fn (int \$x): string => 'x');\n"; + $this->open($workspace, '/Use.xphp', $useSource); + + $resolver = $this->resolverWithRoot($workspace, '', withFqnIndex: false); + $byte = strpos($useSource, '->map') + strlen('->'); + [$line, $char] = (new PositionMap($useSource))->offsetToPosition($byte); + $md = $this->markdown($resolver->resolve('/Use.xphp', $line, $char)); + + self::assertStringContainsString('function map(', $md, 'no clause without the index'); + self::assertStringNotContainsString('map', $md); + } + private function workspace(): PhpactorWorkspace { return new PhpactorWorkspace(); From dd2881bde51035ef93202a00cba89e27ba64193f Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Mon, 20 Jul 2026 11:50:57 +0000 Subject: [PATCH 25/26] chore(deps): pin xphp-lang/xphp to the released v0.3.0 Upstream tagged v0.3.0; move off the 0.3.x-dev branch onto the release. Full unit (1219) + behat (111 scenarios / 689 steps) suites green against the tagged version, including the Compiler::check()-driven authoritative-diagnostics tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- composer.json | 2 +- composer.lock | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/composer.json b/composer.json index bdc7f9d..07938ad 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "phpactor/language-server": "^6.0", "phpactor/language-server-protocol": "^3.5", "phpactor/worse-reflection": "^0.6.0", - "xphp-lang/xphp": "0.3.x-dev" + "xphp-lang/xphp": "^v0.3.0" }, "require-dev": { "phpunit/phpunit": "^13.0" diff --git a/composer.lock b/composer.lock index 61a4eb8..e3eac1f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "4d60984500e0e8de35fd334981745ac4", + "content-hash": "ac3197c500e94ee803e89faabffbaee3", "packages": [ { "name": "amphp/amp", @@ -2824,16 +2824,16 @@ }, { "name": "xphp-lang/xphp", - "version": "0.3.x-dev", + "version": "v0.3.0", "source": { "type": "git", "url": "https://github.com/xphp-lang/xphp.git", - "reference": "db66de1f4ff73e728c1a8cd708a1762cfec04d4e" + "reference": "9bc1ff4adf6cbede40ff5f7621ac9442a5eeb24d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/xphp-lang/xphp/zipball/db66de1f4ff73e728c1a8cd708a1762cfec04d4e", - "reference": "db66de1f4ff73e728c1a8cd708a1762cfec04d4e", + "url": "https://api.github.com/repos/xphp-lang/xphp/zipball/9bc1ff4adf6cbede40ff5f7621ac9442a5eeb24d", + "reference": "9bc1ff4adf6cbede40ff5f7621ac9442a5eeb24d", "shasum": "" }, "require": { @@ -2880,7 +2880,7 @@ "issues": "https://github.com/xphp-lang/xphp/issues", "source": "https://github.com/xphp-lang/xphp" }, - "time": "2026-07-14T20:20:15+00:00" + "time": "2026-07-20T11:18:35+00:00" } ], "packages-dev": [ @@ -4679,9 +4679,7 @@ ], "aliases": [], "minimum-stability": "dev", - "stability-flags": { - "xphp-lang/xphp": 20 - }, + "stability-flags": {}, "prefer-stable": true, "prefer-lowest": false, "platform": { From 6c63296a524011b57b70e4e8e4f6edc4a060fad8 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Mon, 20 Jul 2026 11:54:37 +0000 Subject: [PATCH 26/26] docs(features): document open-sibling-project indexing + generic method hover Catch the feature reference up to recently-shipped behavior: - opening a file from a sibling project (outside the workspace root) folds that project's xphp.json source roots into the FQN index on didOpen, so its symbols become navigable (go-to-definition / references / completion / rename); - generic method hovers now render the type-parameter clause + Closure(...) signature params, resolving inherited methods against the declaring class; - variable-type hover is flow-sensitive across reassignments. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/index.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/features/index.md b/docs/features/index.md index 00d4eeb..bcfaa45 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -212,6 +212,14 @@ property / native function info, hover renders: return, or property position, rendered as its structured form (`Closure(int, string): bool`) -- params without names, nullable and composite (union / intersection) members preserved. +- A generic **method's** own signature with its type-parameter clause + and `Closure(...)` signature params restored -- hovering a call to + `map(Closure(E $x): R $fn): List` renders `map(Closure(E): R + $fn): ...` rather than the generics-erased `map(Closure $fn)` form. + Inherited generic methods resolve against the declaring base class. +- A local variable's type is resolved **flow-sensitively**: a variable + reassigned to different generic types reads as the type in effect at + the cursor, not the last assignment in the file. ### Signature Help @@ -442,6 +450,18 @@ than one root is indexed once. An absent or malformed manifest falls back to the single workspace root -- the server never hard-fails on a bad manifest. +Opening a file from a **sibling project outside the workspace root** +extends this on the fly: on `textDocument/didOpen`, the server walks up +from the opened file to its own nearest `xphp.json` and folds that +project's source roots into the index. So with the workspace rooted at +one project, opening a file from a neighbouring package makes its +symbols resolve for go-to-definition, find-references, completion and +rename -- even though the server tracks only a single legacy `rootPath` +and does not read `workspaceFolders`. The newly-registered roots are +warmed off-thread so the first navigation into the sibling isn't cold. +Duplicate FQNs across projects blend by proximity (nearest declaration +to the working file), not hard per-project isolation. + ### AST cache (warmed on Initialize) On the LSP `initialize` handshake, a background warmer parses every