From 5d04c867c28d1912fa4cc70d507a28f721ff502f Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 20:42:28 +0000 Subject: [PATCH 01/16] feat(diagnostics): add Diagnostic value-object model for the check gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the XPHP\Diagnostics namespace: a string-backed Severity enum (Error/Warning/Notice with isFailing()), a DiagnosticSource enum (xphp/phpstan), a SourceLocation (file/line/optional column), the immutable Diagnostic, and a mutable DiagnosticCollector (add/all/hasErrors/count). Pure value objects with no pipeline wiring yet — the foundation for the forthcoming `xphp check` command's structured, collect-all diagnostics. Unit tests cover severity gating, collector ordering/error detection, and defaults (100% mutation score over the new files). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Diagnostics/Diagnostic.php | 31 ++++++++++++ src/Diagnostics/DiagnosticCollector.php | 51 ++++++++++++++++++++ src/Diagnostics/DiagnosticSource.php | 16 ++++++ src/Diagnostics/Severity.php | 26 ++++++++++ src/Diagnostics/SourceLocation.php | 23 +++++++++ test/Diagnostics/DiagnosticCollectorTest.php | 49 +++++++++++++++++++ test/Diagnostics/DiagnosticTest.php | 46 ++++++++++++++++++ test/Diagnostics/SeverityTest.php | 32 ++++++++++++ test/Diagnostics/SourceLocationTest.php | 26 ++++++++++ 9 files changed, 300 insertions(+) create mode 100644 src/Diagnostics/Diagnostic.php create mode 100644 src/Diagnostics/DiagnosticCollector.php create mode 100644 src/Diagnostics/DiagnosticSource.php create mode 100644 src/Diagnostics/Severity.php create mode 100644 src/Diagnostics/SourceLocation.php create mode 100644 test/Diagnostics/DiagnosticCollectorTest.php create mode 100644 test/Diagnostics/DiagnosticTest.php create mode 100644 test/Diagnostics/SeverityTest.php create mode 100644 test/Diagnostics/SourceLocationTest.php diff --git a/src/Diagnostics/Diagnostic.php b/src/Diagnostics/Diagnostic.php new file mode 100644 index 0000000..327ba01 --- /dev/null +++ b/src/Diagnostics/Diagnostic.php @@ -0,0 +1,31 @@ +`) that + * surfaced a finding inside a template body — only set for Phase-2 (PHPStan) + * diagnostics mapped back to a declaration. + * + * Immutable. + */ +final readonly class Diagnostic +{ + public function __construct( + public Severity $severity, + public string $code, + public string $message, + public ?SourceLocation $location = null, + public ?string $triggeredBy = null, + public DiagnosticSource $source = DiagnosticSource::Xphp, + ) { + } +} diff --git a/src/Diagnostics/DiagnosticCollector.php b/src/Diagnostics/DiagnosticCollector.php new file mode 100644 index 0000000..72036d9 --- /dev/null +++ b/src/Diagnostics/DiagnosticCollector.php @@ -0,0 +1,51 @@ + */ + private array $diagnostics = []; + + public function add(Diagnostic $diagnostic): void + { + $this->diagnostics[] = $diagnostic; + } + + /** + * @return list In insertion order. + */ + public function all(): array + { + return $this->diagnostics; + } + + /** + * True iff any collected diagnostic fails the gate (Error severity). + */ + public function hasErrors(): bool + { + foreach ($this->diagnostics as $diagnostic) { + if ($diagnostic->severity->isFailing()) { + return true; + } + } + + return false; + } + + public function count(): int + { + return count($this->diagnostics); + } +} diff --git a/src/Diagnostics/DiagnosticSource.php b/src/Diagnostics/DiagnosticSource.php new file mode 100644 index 0000000..3df627e --- /dev/null +++ b/src/Diagnostics/DiagnosticSource.php @@ -0,0 +1,16 @@ +all()); + self::assertFalse($c->hasErrors()); + self::assertSame(0, $c->count()); + } + + public function testAddPreservesInsertionOrder(): void + { + $c = new DiagnosticCollector(); + $first = new Diagnostic(Severity::Notice, 'a', 'first'); + $second = new Diagnostic(Severity::Warning, 'b', 'second'); + $c->add($first); + $c->add($second); + + self::assertSame([$first, $second], $c->all()); + self::assertSame(2, $c->count()); + } + + public function testHasErrorsIsFalseWithoutErrorSeverity(): void + { + $c = new DiagnosticCollector(); + $c->add(new Diagnostic(Severity::Warning, 'a', 'w')); + $c->add(new Diagnostic(Severity::Notice, 'b', 'n')); + + self::assertFalse($c->hasErrors()); + } + + public function testHasErrorsIsTrueWhenAnyErrorPresent(): void + { + $c = new DiagnosticCollector(); + $c->add(new Diagnostic(Severity::Warning, 'a', 'w')); + $c->add(new Diagnostic(Severity::Error, 'b', 'e')); + + self::assertTrue($c->hasErrors()); + } +} diff --git a/test/Diagnostics/DiagnosticTest.php b/test/Diagnostics/DiagnosticTest.php new file mode 100644 index 0000000..bec683c --- /dev/null +++ b/test/Diagnostics/DiagnosticTest.php @@ -0,0 +1,46 @@ +severity); + self::assertSame('xphp.bound_violation', $d->code); + self::assertSame('boom', $d->message); + self::assertNull($d->location); + self::assertNull($d->triggeredBy); + self::assertSame(DiagnosticSource::Xphp, $d->source); + } + + public function testAllFieldsSet(): void + { + $loc = new SourceLocation('/src/Box.xphp', 12, 5); + $d = new Diagnostic( + Severity::Warning, + 'phpstan.return.type', + 'bad return', + $loc, + 'App\\Box', + DiagnosticSource::PhpStan, + ); + + self::assertSame(Severity::Warning, $d->severity); + self::assertSame($loc, $d->location); + self::assertSame('App\\Box', $d->triggeredBy); + self::assertSame(DiagnosticSource::PhpStan, $d->source); + } + + public function testSourceBackingValues(): void + { + self::assertSame('xphp', DiagnosticSource::Xphp->value); + self::assertSame('phpstan', DiagnosticSource::PhpStan->value); + } +} diff --git a/test/Diagnostics/SeverityTest.php b/test/Diagnostics/SeverityTest.php new file mode 100644 index 0000000..f8a2099 --- /dev/null +++ b/test/Diagnostics/SeverityTest.php @@ -0,0 +1,32 @@ +isFailing()); + } + + public function testWarningIsNotFailing(): void + { + self::assertFalse(Severity::Warning->isFailing()); + } + + public function testNoticeIsNotFailing(): void + { + self::assertFalse(Severity::Notice->isFailing()); + } + + public function testBackingValues(): void + { + self::assertSame('error', Severity::Error->value); + self::assertSame('warning', Severity::Warning->value); + self::assertSame('notice', Severity::Notice->value); + } +} diff --git a/test/Diagnostics/SourceLocationTest.php b/test/Diagnostics/SourceLocationTest.php new file mode 100644 index 0000000..3584eb3 --- /dev/null +++ b/test/Diagnostics/SourceLocationTest.php @@ -0,0 +1,26 @@ +file); + self::assertSame(42, $loc->line); + self::assertNull($loc->column); + } + + public function testColumnIsPreservedWhenGiven(): void + { + $loc = new SourceLocation('/src/Box.xphp', 42, 7); + + self::assertSame(7, $loc->column); + } +} From b9e117397e492d11b3c9d8754aee2e48a515d946 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 20:54:05 +0000 Subject: [PATCH 02/16] feat(check): collect generic bound violations via an optional diagnostics sink Thread an optional DiagnosticCollector through the bound-validation path (Registry ctor -> recordInstantiation -> validateBounds -> checkBounds). When absent (xphp compile) violations throw exactly as before, byte-identical; when present each violation is appended as a Diagnostic -- located at the instantiation site captured from the AST node in RegistryCollector -- and recording continues so all violations surface in one run. The user-facing message now comes from a single shared boundViolationMessage() builder so the throw text and the diagnostic text can never drift. Tests cover collect-vs-throw, byte-identical and exact message text, multi-violation collection, and AST-derived source line (100% mutation score over the diff). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Registry.php | 77 +++++++-- .../Monomorphize/RegistryCollector.php | 10 +- .../RegistryBoundsDiagnosticTest.php | 151 ++++++++++++++++++ 3 files changed, 226 insertions(+), 12 deletions(-) create mode 100644 test/Transpiler/Monomorphize/RegistryBoundsDiagnosticTest.php diff --git a/src/Transpiler/Monomorphize/Registry.php b/src/Transpiler/Monomorphize/Registry.php index de6fa00..a90ab7d 100644 --- a/src/Transpiler/Monomorphize/Registry.php +++ b/src/Transpiler/Monomorphize/Registry.php @@ -14,9 +14,16 @@ use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\UnionType; use RuntimeException; +use XPHP\Diagnostics\Diagnostic; +use XPHP\Diagnostics\DiagnosticCollector; +use XPHP\Diagnostics\Severity; +use XPHP\Diagnostics\SourceLocation; final class Registry { + /** Stable diagnostic code for a generic bound violation at an instantiation site. */ + public const CODE_BOUND_VIOLATION = 'xphp.bound_violation'; + /** * All specialized classes live under this namespace prefix; the full target FQCN * mirrors the original template's namespace and ends with a hash-based class name. @@ -39,9 +46,16 @@ final class Registry /** @var array Keyed by full generated FQCN. */ private array $instantiations = []; + /** + * @param ?DiagnosticCollector $diagnostics When null (the default, used by `xphp compile`), + * validation failures throw as before — byte-identical behavior. When provided (by + * `xphp check`), bound violations are appended to the collector and recording continues, + * so every error of the validation phase is reported in one run. + */ public function __construct( private readonly int $hashLength = self::DEFAULT_HASH_HEX_LENGTH, private readonly ?TypeHierarchy $hierarchy = null, + private readonly ?DiagnosticCollector $diagnostics = null, ) { self::validateHashLength($this->hashLength); } @@ -87,18 +101,24 @@ public function recordDefinition( * generated FQCN, throws with a self-contained error message explaining how to raise XPHP_HASH_LENGTH. * * @param list $args + * @param ?SourceLocation $callSite The `.xphp` position of the instantiation site, used to + * locate a bound-violation diagnostic in check-mode. Nested sub-instantiations inherit the + * enclosing site (they have no distinct source token). Ignored in throw-mode. */ - public function recordInstantiation(string $templateFqn, array $args): GenericInstantiation - { + public function recordInstantiation( + string $templateFqn, + array $args, + ?SourceLocation $callSite = null, + ): GenericInstantiation { $args = $this->padWithDefaults($templateFqn, $args); foreach ($args as $arg) { if ($arg->isGeneric()) { - $this->recordInstantiation($arg->name, $arg->args); + $this->recordInstantiation($arg->name, $arg->args, $callSite); } } - $this->validateBounds($templateFqn, $args); + $this->validateBounds($templateFqn, $args, $callSite); $generatedFqn = self::generatedFqn($templateFqn, $args, $this->hashLength); $template = ltrim($templateFqn, '\\'); @@ -609,7 +629,7 @@ private static function assertLeaf( * * @param list $args */ - private function validateBounds(string $templateFqn, array $args): void + private function validateBounds(string $templateFqn, array $args, ?SourceLocation $callSite = null): void { if ($this->hierarchy === null) { return; @@ -623,6 +643,8 @@ private function validateBounds(string $templateFqn, array $args): void $args, $this->hierarchy, self::formatInstantiation(ltrim($templateFqn, '\\'), $args), + $this->diagnostics, + $callSite, ); } @@ -637,6 +659,11 @@ private function validateBounds(string $templateFqn, array $args): void * `$instantiationLabel` is the human-readable context string that opens the error * (e.g. `"App\Box"` or `"App\Util::identity"`). * + * When `$diagnostics` is null (the default — `xphp compile`, and every `GenericMethodCompiler` + * call site) a violation throws as before (byte-identical message). When a collector is + * supplied (`xphp check`), each violation is appended as a `Diagnostic` and the loop continues, + * so all violating parameters of one instantiation are reported in a single run. + * * @param list $typeParams * @param list $args */ @@ -645,6 +672,8 @@ public static function checkBounds( array $args, TypeHierarchy $hierarchy, string $instantiationLabel, + ?DiagnosticCollector $diagnostics = null, + ?SourceLocation $callSite = null, ): void { // Arity mismatch is a different error class (caught upstream); skip silently here // so that the existing pipeline can produce the more specific message. @@ -675,20 +704,46 @@ public static function checkBounds( $concrete->toDisplayString(), $boundDisplay, ); - throw new RuntimeException(sprintf( - "Generic bound violated while instantiating %s.\n" - . " type parameter %s is bounded by %s\n" - . " but the supplied concrete type is %s\n\n" - . " %s", + $message = self::boundViolationMessage( $instantiationLabel, $param->name, $boundDisplay, $concrete->toDisplayString(), $detail, - )); + ); + if ($diagnostics !== null) { + $diagnostics->add(new Diagnostic(Severity::Error, self::CODE_BOUND_VIOLATION, $message, $callSite)); + continue; + } + throw new RuntimeException($message); } } + /** + * Build the user-facing bound-violation message. Single source of truth shared by the + * throw path (`xphp compile`) and the diagnostic path (`xphp check`) so the two can never + * drift — the exact text is pinned by `expectExceptionMessage` assertions. + */ + private static function boundViolationMessage( + string $instantiationLabel, + string $paramName, + string $boundDisplay, + string $concreteDisplay, + string $detail, + ): string { + return sprintf( + "Generic bound violated while instantiating %s.\n" + . " type parameter %s is bounded by %s\n" + . " but the supplied concrete type is %s\n\n" + . " %s", + $instantiationLabel, + $paramName, + $boundDisplay, + $concreteDisplay, + $detail, + ); + } + /** * Three-way verdict (true / false / null) for a bound expression against a * concrete TypeRef. Walks the BoundExpr tree: diff --git a/src/Transpiler/Monomorphize/RegistryCollector.php b/src/Transpiler/Monomorphize/RegistryCollector.php index 006e42a..205730f 100644 --- a/src/Transpiler/Monomorphize/RegistryCollector.php +++ b/src/Transpiler/Monomorphize/RegistryCollector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\Use_; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; +use XPHP\Diagnostics\SourceLocation; /** * Walks an AST and feeds generic definitions and instantiations into a Registry. @@ -133,7 +134,11 @@ public function enterNode(Node $node): null if (is_array($args)) { /** @var list $args — set as a list by XphpSourceParser::resolveAndAttach. */ if (is_string($fqn) && self::allConcrete($args)) { - $this->registry->recordInstantiation($fqn, $args); + $this->registry->recordInstantiation( + $fqn, + $args, + new SourceLocation($this->currentFile, $node->getStartLine()), + ); } } } @@ -178,6 +183,9 @@ private function synthesizeBareNewIfAllDefaults(Name $name): void // instantiation downstream -- only the path differs. The split exists to // record the synthesis at the same time as the attribute attach so that the // fixed-point loop's nested-instantiation walk picks it up in the same pass. + // Call-site location is threaded for the explicit-turbofish path (the bound-violation + // case); the all-defaults bare-`new` path can only fail bounds via a default, which is + // reported at the definition site, so it records without a call-site here. $this->registry->recordInstantiation($resolved, []); } diff --git a/test/Transpiler/Monomorphize/RegistryBoundsDiagnosticTest.php b/test/Transpiler/Monomorphize/RegistryBoundsDiagnosticTest.php new file mode 100644 index 0000000..a7e28d3 --- /dev/null +++ b/test/Transpiler/Monomorphize/RegistryBoundsDiagnosticTest.php @@ -0,0 +1,151 @@ +boxRegistry($collector); + $loc = new SourceLocation('/App/Box.xphp', 7); + + // Must NOT throw. + $registry->recordInstantiation('App\\Box', [new TypeRef('int', isScalar: true)], $loc); + + self::assertTrue($collector->hasErrors()); + self::assertCount(1, $collector->all()); + + $d = $collector->all()[0]; + self::assertSame(Severity::Error, $d->severity); + self::assertSame(Registry::CODE_BOUND_VIOLATION, $d->code); + self::assertSame(DiagnosticSource::Xphp, $d->source); + self::assertSame($loc, $d->location); + self::assertStringContainsString('Generic bound violated', $d->message); + self::assertStringContainsString('"int" does not extend/implement "Stringable"', $d->message); + + // Recording still completed (continue-safely): the instantiation is on file. + self::assertCount(1, $registry->instantiations()); + } + + public function testCollectedMessageHasExactText(): void + { + $collector = new DiagnosticCollector(); + $this->boxRegistry($collector) + ->recordInstantiation('App\\Box', [new TypeRef('int', isScalar: true)]); + + $expected = <<<'TXT' + Generic bound violated while instantiating App\Box. + type parameter T is bounded by Stringable + but the supplied concrete type is int + + "int" does not extend/implement "Stringable". + TXT; + + self::assertSame($expected, $collector->all()[0]->message); + } + + public function testCollectedMessageIsByteIdenticalToThrownMessage(): void + { + $thrown = null; + try { + $this->boxRegistry(null) + ->recordInstantiation('App\\Box', [new TypeRef('int', isScalar: true)]); + self::fail('expected RuntimeException in throw-mode'); + } catch (RuntimeException $e) { + $thrown = $e->getMessage(); + } + + $collector = new DiagnosticCollector(); + $this->boxRegistry($collector) + ->recordInstantiation('App\\Box', [new TypeRef('int', isScalar: true)]); + + self::assertSame($thrown, $collector->all()[0]->message); + } + + public function testMultipleViolationsCollectedInOneRun(): void + { + $collector = new DiagnosticCollector(); + $hierarchy = new TypeHierarchy([]); + $registry = new Registry(hierarchy: $hierarchy, diagnostics: $collector); + $registry->recordDefinition( + 'App\\Pair', + 'Pair', + [ + new TypeParam('A', new BoundLeaf(new TypeRef('Stringable'))), + new TypeParam('B', new BoundLeaf(new TypeRef('Countable'))), + ], + new Class_(new Identifier('Pair')), + '/App/Pair.xphp', + ); + + $registry->recordInstantiation( + 'App\\Pair', + [new TypeRef('int', isScalar: true), new TypeRef('float', isScalar: true)], + ); + + self::assertCount(2, $collector->all()); + } + + public function testNodeLineIsCapturedFromTheInstantiationSite(): void + { + // Line 5 holds the `new Box::(...)` instantiation site. + $source = <<<'XPHP' + { public function __construct(public T $v) {} } + $b = new Box::(5); + XPHP; + + $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); + $ast = $parser->parse($source); + + $collector = new DiagnosticCollector(); + $registry = new Registry( + hierarchy: TypeHierarchy::fromAstPerFile(['/App/Box.xphp' => $ast]), + diagnostics: $collector, + ); + $rc = new RegistryCollector($registry); + $rc->collectDefinitions($ast, '/App/Box.xphp'); + $rc->collectInstantiations($ast, '/App/Box.xphp'); + + self::assertCount(1, $collector->all()); + $loc = $collector->all()[0]->location; + self::assertNotNull($loc); + self::assertSame('/App/Box.xphp', $loc->file); + self::assertSame(5, $loc->line); + } + + private function boxRegistry(?DiagnosticCollector $collector): Registry + { + $registry = new Registry(hierarchy: new TypeHierarchy([]), diagnostics: $collector); + $registry->recordDefinition( + 'App\\Box', + 'Box', + [new TypeParam('T', new BoundLeaf(new TypeRef('Stringable')))], + new Class_(new Identifier('Box')), + '/App/Box.xphp', + ); + + return $registry; + } +} From 598a3fbfdaf1fccc5b52fd5f59ceb3444a29b28c Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 21:34:30 +0000 Subject: [PATCH 03/16] feat(check): add a validate-only pass that collects generic errors Add Compiler::check(): parse, build the type hierarchy, collect definitions, validate defaults-against-bounds, collect instantiations (bounds + missing type arguments), and report instantiations of undefined templates -- gathering every error into a DiagnosticCollector and halting before specialization/emit, so a partially-invalid registry never reaches the fixed-point loop. Extends the optional-collector seam to the padding path (missing required type argument), validateDefaultsAgainstBounds (per-parameter, continue-on-error), and a new collectUndefinedTemplates pass. Each reused message is built by a single shared helper so the throw (compile) and diagnostic (check) text stay byte-identical. The parse loop is factored into parseAll(), reused by compile() and check(); compile()'s undefined-template throw now routes through the shared builder. Duplicate-definition is intentionally not part of the seam: RegistryCollector's already-recorded guard makes the class-template path unreachable and surfacing it would change compile-mode semantics -- deferred. Variance and method-level generic checks remain fail-fast (not yet part of the check pass). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Compiler.php | 61 ++++++-- src/Transpiler/Monomorphize/Registry.php | 138 +++++++++++++++--- .../Monomorphize/CheckPassIntegrationTest.php | 136 +++++++++++++++++ .../RegistryCheckDiagnosticsTest.php | 128 ++++++++++++++++ test/fixture/check/clean/source/Box.xphp | 12 ++ test/fixture/check/clean/source/Use.xphp | 7 + .../check/default_violation/source/Box.xphp | 14 ++ .../check/missing_arg/source/Pair.xphp | 12 ++ .../fixture/check/missing_arg/source/Use.xphp | 8 + .../fixture/check/multi_error/source/Box.xphp | 12 ++ .../fixture/check/multi_error/source/Use.xphp | 10 ++ .../check/undefined_template/source/Use.xphp | 8 + 12 files changed, 516 insertions(+), 30 deletions(-) create mode 100644 test/Transpiler/Monomorphize/CheckPassIntegrationTest.php create mode 100644 test/Transpiler/Monomorphize/RegistryCheckDiagnosticsTest.php create mode 100644 test/fixture/check/clean/source/Box.xphp create mode 100644 test/fixture/check/clean/source/Use.xphp create mode 100644 test/fixture/check/default_violation/source/Box.xphp create mode 100644 test/fixture/check/missing_arg/source/Pair.xphp create mode 100644 test/fixture/check/missing_arg/source/Use.xphp create mode 100644 test/fixture/check/multi_error/source/Box.xphp create mode 100644 test/fixture/check/multi_error/source/Use.xphp create mode 100644 test/fixture/check/undefined_template/source/Use.xphp diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 12868d2..e820290 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -6,6 +6,7 @@ use PhpParser\PrettyPrinter\Standard as StandardPrinter; use RuntimeException; +use XPHP\Diagnostics\DiagnosticCollector; use XPHP\FileSystem\FileReader; use XPHP\FileSystem\FileWriter; use XPHP\FileSystem\FilepathArray; @@ -51,11 +52,7 @@ public function compile( // Phase 0: parse every source up front. The TypeHierarchy (used to validate generic // bounds at recordInstantiation time) needs to see every class/interface/trait // declaration *before* any instantiation is recorded, so parsing has to finish first. - $astPerFile = []; - foreach ($sources->filepaths as $filepath) { - $content = $this->fileReader->read($filepath); - $astPerFile[$filepath] = $this->sourceParser->parse($content); - } + $astPerFile = $this->parseAll($sources); $hierarchy = TypeHierarchy::fromAstPerFile($astPerFile); $registry = new Registry($this->hashLength, $hierarchy); @@ -111,11 +108,9 @@ public function compile( $definition = $registry->definition($instantiation->templateFqn); if ($definition === null) { - throw new RuntimeException(sprintf( - 'Generic template "%s" was instantiated but never defined (generated as: %s).', - $instantiation->templateFqn, - $generatedFqn, - )); + throw new RuntimeException( + Registry::undefinedTemplateMessage($instantiation->templateFqn, $generatedFqn), + ); } $substitution = array_combine($definition->typeParamNames(), $instantiation->concreteTypes); @@ -209,6 +204,52 @@ public function compile( ); } + /** + * Validate-only pass for `xphp check`: parse, build the hierarchy, collect definitions, + * then validate (defaults-vs-bounds) and collect instantiations (bounds, missing args) + * with a DiagnosticCollector so every generic error is gathered instead of throwing on + * the first. Stops after validation — it never specializes or emits, so a partially-invalid + * registry never reaches the fixed-point loop. Returns the collected diagnostics. + * + * Scope note: method/function/closure-level generic checks (GenericMethodCompiler, Phase 1a + * of compile()) are intentionally NOT run here — they remain fail-fast and are not yet part + * of the check gate. Variance checks are wired in a later step. + */ + public function check(FilepathArray $sources): DiagnosticCollector + { + $astPerFile = $this->parseAll($sources); + $hierarchy = TypeHierarchy::fromAstPerFile($astPerFile); + $diagnostics = new DiagnosticCollector(); + $registry = new Registry($this->hashLength, $hierarchy, $diagnostics); + $collector = new RegistryCollector($registry); + + foreach ($astPerFile as $filepath => $ast) { + $collector->collectDefinitions($ast, $filepath); + } + $registry->validateDefaultsAgainstBounds(); + foreach ($astPerFile as $filepath => $ast) { + $collector->collectInstantiations($ast, $filepath); + } + $registry->collectUndefinedTemplates($diagnostics); + + return $diagnostics; + } + + /** + * Parse every source file into an AST keyed by filepath. + * + * @return array> + */ + private function parseAll(FilepathArray $sources): array + { + $astPerFile = []; + foreach ($sources->filepaths as $filepath) { + $astPerFile[$filepath] = $this->sourceParser->parse($this->fileReader->read($filepath)); + } + + return $astPerFile; + } + private static function relativePath(string $base, string $filepath): string { $base = rtrim($base, '/') . '/'; diff --git a/src/Transpiler/Monomorphize/Registry.php b/src/Transpiler/Monomorphize/Registry.php index a90ab7d..d477741 100644 --- a/src/Transpiler/Monomorphize/Registry.php +++ b/src/Transpiler/Monomorphize/Registry.php @@ -21,8 +21,11 @@ final class Registry { - /** Stable diagnostic code for a generic bound violation at an instantiation site. */ + /** Stable diagnostic codes (machine identifiers tooling can match on). */ public const CODE_BOUND_VIOLATION = 'xphp.bound_violation'; + public const CODE_MISSING_TYPE_ARGUMENT = 'xphp.missing_type_argument'; + public const CODE_DEFAULT_BOUND_VIOLATION = 'xphp.default_bound_violation'; + public const CODE_UNDEFINED_TEMPLATE = 'xphp.undefined_template'; /** * All specialized classes live under this namespace prefix; the full target FQCN @@ -71,6 +74,11 @@ public function recordDefinition( string $sourceFile, ): void { if (isset($this->definitions[$templateFqn])) { + // NB: cross-file duplicate class templates are filtered out earlier by + // RegistryCollector's `!isAlreadyRecorded()` guard, so this throw is only + // reachable via the generic-function path. Surfacing duplicate definitions in + // `xphp check` would require reworking that guard (and would change compile-mode + // semantics), so it is intentionally NOT part of the collector seam — deferred. throw new RuntimeException(sprintf( 'Generic template "%s" already declared (in %s); duplicate declaration in %s.', $templateFqn, @@ -110,7 +118,7 @@ public function recordInstantiation( array $args, ?SourceLocation $callSite = null, ): GenericInstantiation { - $args = $this->padWithDefaults($templateFqn, $args); + $args = $this->padWithDefaults($templateFqn, $args, $callSite); foreach ($args as $arg) { if ($arg->isGeneric()) { @@ -157,7 +165,7 @@ public function recordInstantiation( * @param list $args * @return list */ - private function padWithDefaults(string $templateFqn, array $args): array + private function padWithDefaults(string $templateFqn, array $args, ?SourceLocation $callSite = null): array { $definition = $this->definitions[ltrim($templateFqn, '\\')] ?? null; if ($definition === null) { @@ -167,6 +175,8 @@ private function padWithDefaults(string $templateFqn, array $args): array $definition->typeParams, $args, ltrim($templateFqn, '\\'), + $this->diagnostics, + $callSite, ); } @@ -178,9 +188,10 @@ private function padWithDefaults(string $templateFqn, array $args): array * templates) so the padding semantics stay identical regardless of * the call-site shape. * - * Throws when a non-defaulted param is missing and there are fewer - * supplied args than required. Returns `$args` unchanged when the - * supplied count already matches or exceeds the param count. + * When `$diagnostics` is null (compile, and every `GenericMethodCompiler` call) a missing + * non-defaulted param throws as before. With a collector (check) it appends a Diagnostic and + * returns the partial padding gathered so far, so the run continues to surface other errors. + * Returns `$args` unchanged when the supplied count already matches or exceeds the param count. * * @param list $params * @param list $args @@ -190,6 +201,8 @@ public static function padArgsWithDefaults( array $params, array $args, string $templateLabel, + ?DiagnosticCollector $diagnostics = null, + ?SourceLocation $callSite = null, ): array { $supplied = count($args); $needed = count($params); @@ -200,15 +213,18 @@ public static function padArgsWithDefaults( $padded = $args; for ($i = $supplied; $i < $needed; $i++) { if ($params[$i]->default === null) { - throw new RuntimeException(sprintf( - 'Generic template "%s" was instantiated with %d type argument(s) ' - . 'but parameter `%s` (position %d) has no default; supply it ' - . 'explicitly or add defaults to every preceding required parameter.', - $templateLabel, - $supplied, - $params[$i]->name, - $i + 1, - )); + $message = self::missingTypeArgumentMessage($templateLabel, $supplied, $params[$i]->name, $i + 1); + if ($diagnostics !== null) { + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_MISSING_TYPE_ARGUMENT, + $message, + $callSite, + )); + + return $padded; + } + throw new RuntimeException($message); } $subst = []; foreach ($padded as $j => $concrete) { @@ -219,6 +235,26 @@ public static function padArgsWithDefaults( return $padded; } + /** + * Single source of truth for the missing-required-type-argument message. + */ + private static function missingTypeArgumentMessage( + string $templateLabel, + int $supplied, + string $paramName, + int $position, + ): string { + return sprintf( + 'Generic template "%s" was instantiated with %d type argument(s) ' + . 'but parameter `%s` (position %d) has no default; supply it ' + . 'explicitly or add defaults to every preceding required parameter.', + $templateLabel, + $supplied, + $paramName, + $position, + ); + } + /** * Declaration-time bound check on fully-concrete defaults. Defaults that * reference earlier type-params can't be checked here because the bound @@ -261,21 +297,83 @@ public function validateDefaultsAgainstBounds(): void . 'it satisfies "%s".', $boundDisplay, ); - throw new RuntimeException(sprintf( - "Default for generic parameter `%s` of \"%s\" violates the parameter's bound.\n" - . " bound: %s\n" - . " default: %s\n" - . " reason: %s", + $message = self::defaultBoundViolationMessage( $param->name, $definition->templateFqn, $boundDisplay, $defaultDisplay, $reason, + ); + if ($this->diagnostics !== null) { + $this->diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_DEFAULT_BOUND_VIOLATION, + $message, + new SourceLocation($definition->sourceFile, $definition->templateAst->getStartLine()), + )); + continue; + } + throw new RuntimeException($message); + } + } + } + + /** + * Report every recorded instantiation whose template was never defined. In `xphp compile` + * this surfaces as a thrown error inside the specialization loop; `xphp check` doesn't run + * that loop, so it detects the same condition here by comparing recorded instantiations + * against the definition set. No source location is attached — the instantiation does not + * retain its call site — but the message names the template and its generated FQCN. + */ + public function collectUndefinedTemplates(DiagnosticCollector $diagnostics): void + { + foreach ($this->instantiations as $generatedFqn => $instantiation) { + if (!isset($this->definitions[$instantiation->templateFqn])) { + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_UNDEFINED_TEMPLATE, + self::undefinedTemplateMessage($instantiation->templateFqn, $generatedFqn), )); } } } + /** + * Single source of truth for the "instantiated but never defined" message, shared by the + * compile-time throw (Compiler) and the check-time diagnostic. + */ + public static function undefinedTemplateMessage(string $templateFqn, string $generatedFqn): string + { + return sprintf( + 'Generic template "%s" was instantiated but never defined (generated as: %s).', + $templateFqn, + $generatedFqn, + ); + } + + /** + * Single source of truth for the default-violates-bound message. + */ + private static function defaultBoundViolationMessage( + string $paramName, + string $templateFqn, + string $boundDisplay, + string $defaultDisplay, + string $reason, + ): string { + return sprintf( + "Default for generic parameter `%s` of \"%s\" violates the parameter's bound.\n" + . " bound: %s\n" + . " default: %s\n" + . " reason: %s", + $paramName, + $templateFqn, + $boundDisplay, + $defaultDisplay, + $reason, + ); + } + /** * Inner-template variance composition pass. Runs after `collectDefinitions` * but before `collectInstantiations`, so every template's variance markers diff --git a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php new file mode 100644 index 0000000..ceff18d --- /dev/null +++ b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php @@ -0,0 +1,136 @@ +check('multi_error'); + + self::assertTrue($diagnostics->hasErrors()); + self::assertCount(2, $diagnostics->all()); + foreach ($diagnostics->all() as $d) { + self::assertSame(Registry::CODE_BOUND_VIOLATION, $d->code); + self::assertNotNull($d->location); + self::assertStringEndsWith('Use.xphp', $d->location->file); + } + } + + public function testCleanSourcesProduceNoDiagnostics(): void + { + $diagnostics = $this->check('clean'); + + self::assertFalse($diagnostics->hasErrors()); + self::assertSame([], $diagnostics->all()); + } + + public function testDefaultBoundViolationIsCollectedByCheck(): void + { + // Exercises the validateDefaultsAgainstBounds() step of check(). + $diagnostics = $this->check('default_violation'); + + self::assertCount(1, $diagnostics->all()); + self::assertSame(Registry::CODE_DEFAULT_BOUND_VIOLATION, $diagnostics->all()[0]->code); + } + + public function testMissingTypeArgumentIsCollectedByCheck(): void + { + $diagnostics = $this->check('missing_arg'); + + self::assertCount(1, $diagnostics->all()); + self::assertSame(Registry::CODE_MISSING_TYPE_ARGUMENT, $diagnostics->all()[0]->code); + self::assertNotNull($diagnostics->all()[0]->location); + } + + public function testUndefinedTemplateIsCollectedByCheck(): void + { + // Exercises the collectUndefinedTemplates() step of check(). + $diagnostics = $this->check('undefined_template'); + + self::assertCount(1, $diagnostics->all()); + self::assertSame(Registry::CODE_UNDEFINED_TEMPLATE, $diagnostics->all()[0]->code); + } + + public function testCompileStillThrowsOnUndefinedTemplate(): void + { + // The same condition is a hard error in compile-mode (no collector) — confirms the + // shared message builder keeps the throw path intact. + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('was instantiated but never defined'); + + $work = sys_get_temp_dir() . '/xphp-check-compile-' . uniqid('', true); + mkdir($work, 0o755, true); + try { + $this->buildCompiler()->compile($this->sources('undefined_template'), $this->sourceDir('undefined_template'), $work . '/dist', $work . '/cache'); + } finally { + self::rrmdir($work); + } + } + + private function check(string $fixture): DiagnosticCollector + { + return $this->buildCompiler()->check($this->sources($fixture)); + } + + private function sources(string $fixture): FilepathArray + { + return (new NativeFileFinder()) + ->find($this->sourceDir($fixture)) + ->filter(static fn (string $f): bool => str_ends_with($f, '.xphp')); + } + + private function sourceDir(string $fixture): string + { + return realpath(__DIR__ . '/../../fixture/check/' . $fixture . '/source') + ?: throw new RuntimeException("Fixture missing: {$fixture}"); + } + + private static function rrmdir(string $dir): void + { + if (!is_dir($dir)) { + return; + } + foreach (scandir($dir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $path = $dir . '/' . $entry; + is_dir($path) ? self::rrmdir($path) : unlink($path); + } + rmdir($dir); + } + + private function buildCompiler(): Compiler + { + $phpParser = (new ParserFactory())->createForHostVersion(); + $printer = new StandardPrinter(); + $writer = new NativeFileWriter(); + + return new Compiler( + new NativeFileReader(), + $writer, + new XphpSourceParser($phpParser), + new Specializer(), + new SpecializedClassGenerator($printer, $writer), + $printer, + ); + } +} diff --git a/test/Transpiler/Monomorphize/RegistryCheckDiagnosticsTest.php b/test/Transpiler/Monomorphize/RegistryCheckDiagnosticsTest.php new file mode 100644 index 0000000..1fa6514 --- /dev/null +++ b/test/Transpiler/Monomorphize/RegistryCheckDiagnosticsTest.php @@ -0,0 +1,128 @@ +recordDefinition( + 'App\\Pair', + 'Pair', + [new TypeParam('A'), new TypeParam('B')], // B has no default + $this->classAt(1), + '/Pair.xphp', + ); + + $registry->recordInstantiation('App\\Pair', [new TypeRef('int', isScalar: true)], new SourceLocation('/Use.xphp', 8)); + + self::assertCount(1, $collector->all()); + $d = $collector->all()[0]; + self::assertSame(Registry::CODE_MISSING_TYPE_ARGUMENT, $d->code); + self::assertEquals(new SourceLocation('/Use.xphp', 8), $d->location); + self::assertSame( + 'Generic template "App\\Pair" was instantiated with 1 type argument(s) ' + . 'but parameter `B` (position 2) has no default; supply it ' + . 'explicitly or add defaults to every preceding required parameter.', + $d->message, + ); + } + + public function testDefaultBoundViolationIsCollectedAtDeclaration(): void + { + $collector = new DiagnosticCollector(); + $registry = new Registry(hierarchy: new TypeHierarchy([]), diagnostics: $collector); + $registry->recordDefinition( + 'App\\Box', + 'Box', + [new TypeParam('T', new BoundLeaf(new TypeRef('Stringable')), new TypeRef('int', isScalar: true))], + $this->classAt(4), + '/Box.xphp', + ); + + $registry->validateDefaultsAgainstBounds(); + + self::assertCount(1, $collector->all()); + $d = $collector->all()[0]; + self::assertSame(Registry::CODE_DEFAULT_BOUND_VIOLATION, $d->code); + self::assertEquals(new SourceLocation('/Box.xphp', 4), $d->location); + + $expected = <<<'TXT' + Default for generic parameter `T` of "App\Box" violates the parameter's bound. + bound: Stringable + default: int + reason: does not satisfy "Stringable". + TXT; + self::assertSame($expected, $d->message); + } + + public function testMultipleDefaultViolationsCollectedForOneDefinition(): void + { + $collector = new DiagnosticCollector(); + $registry = new Registry(hierarchy: new TypeHierarchy([]), diagnostics: $collector); + $registry->recordDefinition( + 'App\\Pair', + 'Pair', + [ + new TypeParam('A', new BoundLeaf(new TypeRef('Stringable')), new TypeRef('int', isScalar: true)), + new TypeParam('B', new BoundLeaf(new TypeRef('Countable')), new TypeRef('float', isScalar: true)), + ], + $this->classAt(1), + '/Pair.xphp', + ); + + $registry->validateDefaultsAgainstBounds(); + + // Both violating defaults are reported — the per-param loop continues past the first. + self::assertCount(2, $collector->all()); + } + + public function testUndefinedTemplateIsCollected(): void + { + $collector = new DiagnosticCollector(); + $registry = new Registry(diagnostics: $collector); + + // No definition recorded for App\Ghost. + $registry->recordInstantiation('App\\Ghost', [new TypeRef('int', isScalar: true)]); + $registry->collectUndefinedTemplates($collector); + + self::assertCount(1, $collector->all()); + $d = $collector->all()[0]; + self::assertSame(Registry::CODE_UNDEFINED_TEMPLATE, $d->code); + self::assertNull($d->location); + self::assertStringContainsString('Generic template "App\\Ghost" was instantiated but never defined', $d->message); + } + + public function testDefinedTemplatesProduceNoUndefinedDiagnostics(): void + { + $collector = new DiagnosticCollector(); + $registry = new Registry(diagnostics: $collector); + $registry->recordDefinition('App\\Box', 'Box', [new TypeParam('T')], $this->classAt(1), '/Box.xphp'); + $registry->recordInstantiation('App\\Box', [new TypeRef('int', isScalar: true)]); + + $registry->collectUndefinedTemplates($collector); + + self::assertSame([], $collector->all()); + } + + private function classAt(int $line): Class_ + { + return new Class_(new Identifier('C'), [], ['startLine' => $line]); + } +} diff --git a/test/fixture/check/clean/source/Box.xphp b/test/fixture/check/clean/source/Box.xphp new file mode 100644 index 0000000..ae32895 --- /dev/null +++ b/test/fixture/check/clean/source/Box.xphp @@ -0,0 +1,12 @@ + +{ + public function __construct(public T $value) + { + } +} diff --git a/test/fixture/check/clean/source/Use.xphp b/test/fixture/check/clean/source/Use.xphp new file mode 100644 index 0000000..7269797 --- /dev/null +++ b/test/fixture/check/clean/source/Use.xphp @@ -0,0 +1,7 @@ +(1); diff --git a/test/fixture/check/default_violation/source/Box.xphp b/test/fixture/check/default_violation/source/Box.xphp new file mode 100644 index 0000000..675d221 --- /dev/null +++ b/test/fixture/check/default_violation/source/Box.xphp @@ -0,0 +1,14 @@ + +{ + public function __construct(public T $value) + { + } +} diff --git a/test/fixture/check/missing_arg/source/Pair.xphp b/test/fixture/check/missing_arg/source/Pair.xphp new file mode 100644 index 0000000..17248ed --- /dev/null +++ b/test/fixture/check/missing_arg/source/Pair.xphp @@ -0,0 +1,12 @@ + +{ + public function __construct(public A $a, public B $b) + { + } +} diff --git a/test/fixture/check/missing_arg/source/Use.xphp b/test/fixture/check/missing_arg/source/Use.xphp new file mode 100644 index 0000000..b48dc66 --- /dev/null +++ b/test/fixture/check/missing_arg/source/Use.xphp @@ -0,0 +1,8 @@ +(1, 2); diff --git a/test/fixture/check/multi_error/source/Box.xphp b/test/fixture/check/multi_error/source/Box.xphp new file mode 100644 index 0000000..bac8e95 --- /dev/null +++ b/test/fixture/check/multi_error/source/Box.xphp @@ -0,0 +1,12 @@ + +{ + public function __construct(public T $value) + { + } +} diff --git a/test/fixture/check/multi_error/source/Use.xphp b/test/fixture/check/multi_error/source/Use.xphp new file mode 100644 index 0000000..44bcd6e --- /dev/null +++ b/test/fixture/check/multi_error/source/Use.xphp @@ -0,0 +1,10 @@ +(1); +$b = new Box::(2.0); diff --git a/test/fixture/check/undefined_template/source/Use.xphp b/test/fixture/check/undefined_template/source/Use.xphp new file mode 100644 index 0000000..cad0f20 --- /dev/null +++ b/test/fixture/check/undefined_template/source/Use.xphp @@ -0,0 +1,8 @@ +(1); From 6f86e03c14b7213dcc632500cdb49cc4f03265e8 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 21:51:43 +0000 Subject: [PATCH 04/16] refactor(check): move variance-position validation to a collectable phase Move the variance-position check out of the parser into a Registry phase (validateVariancePositions) over collected definitions, wired into compile() (fail-fast, byte-identical first-violation throw) and check() (collects every violation across all definitions, each located at the offending member). VariancePositionValidator now accumulates violations behind a static assertPositions facade that throws the first when no collector is given or emits a diagnostic per violation when one is. The parser-level variance-position tests move to a dedicated VariancePositionPhaseTest (compile-mode throws via data provider + check-mode collect/location), and the check integration suite gains a variance_violation fixture covering the compile-throw and check-collect paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Compiler.php | 9 + src/Transpiler/Monomorphize/Registry.php | 18 ++ .../VariancePositionValidator.php | 211 +++++++++-------- .../Monomorphize/XphpSourceParser.php | 20 +- .../Monomorphize/CheckPassIntegrationTest.php | 23 ++ .../VariancePositionPhaseTest.php | 134 +++++++++++ .../Monomorphize/XphpSourceParserTest.php | 212 ------------------ .../variance_violation/source/Producer.xphp | 13 ++ 8 files changed, 316 insertions(+), 324 deletions(-) create mode 100644 test/Transpiler/Monomorphize/VariancePositionPhaseTest.php create mode 100644 test/fixture/check/variance_violation/source/Producer.xphp diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index e820290..0f5bff8 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -82,6 +82,11 @@ public function compile( // padded instantiation is recorded), then collect instantiations -- including // bare `new Foo;` shapes for templates whose every param has a default. $registry->validateDefaultsAgainstBounds(); + // Variance-position rules (covariant T in input, contravariant T in output, + // bound/default/property/constructor invariance, F-bounded variance). Moved + // here from the parser so all definitions are present and `xphp check` can + // collect across files; compile-mode still throws on the first violation. + $registry->validateVariancePositions(); // Inner-template variance composition: every template's variance // markers are known by now, so cases the parse-time validator // couldn't catch (e.g. `class P<+T> { f(): Container }` where @@ -227,6 +232,10 @@ public function check(FilepathArray $sources): DiagnosticCollector $collector->collectDefinitions($ast, $filepath); } $registry->validateDefaultsAgainstBounds(); + $registry->validateVariancePositions(); + // NB: validateInnerVariance() is not wired in here yet — it still throws + // (not collectable until the inner-variance step), so calling it in check-mode + // would abort on the first violation. Added once it accepts the collector. foreach ($astPerFile as $filepath => $ast) { $collector->collectInstantiations($ast, $filepath); } diff --git a/src/Transpiler/Monomorphize/Registry.php b/src/Transpiler/Monomorphize/Registry.php index d477741..29e7688 100644 --- a/src/Transpiler/Monomorphize/Registry.php +++ b/src/Transpiler/Monomorphize/Registry.php @@ -430,6 +430,24 @@ private static function defaultBoundViolationMessage( * builder -- Invariant declared never reaches the throw (Invariant * passes every allowed-list), so the arm is observably unreachable. */ + /** + * Variance-position check over every collected definition (moved out of the parser so + * `xphp check` can collect all violations across files in one run). Delegates to + * {@see VariancePositionValidator}: with this Registry's collector it gathers diagnostics + * at each offending member; without one (compile) it throws the first violation. + */ + public function validateVariancePositions(): void + { + foreach ($this->definitions as $definition) { + VariancePositionValidator::assertPositions( + $definition->templateAst, + $definition->typeParams, + $this->diagnostics, + $definition->sourceFile, + ); + } + } + public function validateInnerVariance(): void { foreach ($this->definitions as $definition) { diff --git a/src/Transpiler/Monomorphize/VariancePositionValidator.php b/src/Transpiler/Monomorphize/VariancePositionValidator.php index b64788b..6e1abfc 100644 --- a/src/Transpiler/Monomorphize/VariancePositionValidator.php +++ b/src/Transpiler/Monomorphize/VariancePositionValidator.php @@ -18,6 +18,10 @@ use PhpParser\Node\Stmt\Property; use PhpParser\Node\UnionType; use RuntimeException; +use XPHP\Diagnostics\Diagnostic; +use XPHP\Diagnostics\DiagnosticCollector; +use XPHP\Diagnostics\Severity; +use XPHP\Diagnostics\SourceLocation; /** * Declaration-time check that every appearance of a variance-marked type @@ -49,14 +53,40 @@ * * Errors include the param name, variance marker, and the position class * so the user sees what's wrong without reading the implementation. + * + * Runs over collected definitions (`Registry::validateVariancePositions`), not + * in the parser. With a `DiagnosticCollector` it gathers every violation in the + * class (each located at the offending member) and continues; without one it + * throws the first violation — byte-identical to the previous parse-time check. */ final class VariancePositionValidator { + /** Stable diagnostic code for a variance-position violation. */ + public const CODE_VARIANCE_POSITION = 'xphp.variance_position'; + + /** @var array */ + private array $varianceByName; + + /** @var list */ + private array $violations = []; + /** - * @param list $params + * @param array $varianceByName */ - public static function assertPositions(ClassLike $node, array $params): void + private function __construct(array $varianceByName) { + $this->varianceByName = $varianceByName; + } + + /** + * @param list $params + */ + public static function assertPositions( + ClassLike $node, + array $params, + ?DiagnosticCollector $diagnostics = null, + ?string $file = null, + ): void { $varianceByName = []; foreach ($params as $param) { if ($param->variance !== Variance::Invariant) { @@ -67,73 +97,82 @@ public static function assertPositions(ClassLike $node, array $params): void return; } - // 1. Bound and default positions are invariant by RFC. Walk each - // param's bound expression + default TypeRef tree; reject if any - // referenced leaf carries a name whose variance isn't Invariant. + $validator = new self($varianceByName); + $validator->collect($node, $params); + if ($validator->violations === []) { + return; + } + + if ($diagnostics === null) { + // Compile-mode: fail fast on the first violation (byte-identical message). + throw new RuntimeException($validator->violations[0]['message']); + } + + foreach ($validator->violations as $violation) { + $location = ($violation['line'] !== null && $file !== null) + ? new SourceLocation($file, $violation['line']) + : null; + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_VARIANCE_POSITION, + $violation['message'], + $location, + )); + } + } + + /** + * @param list $params + */ + private function collect(ClassLike $node, array $params): void + { + $declarationLine = $node->getStartLine(); + + // 1. Bound and default positions are invariant by RFC. foreach ($params as $param) { if ($param->bound !== null) { - self::checkBoundExpr($param->bound, $varianceByName, $param->name, 'bound'); + $this->checkBoundExpr($param->bound, $param->name, 'bound', $declarationLine); } if ($param->default !== null) { - self::checkTypeRef($param->default, $varianceByName, $param->name, 'default'); + $this->checkTypeRef($param->default, $param->name, 'default', $declarationLine); } } // 2. Class-body positions: properties and methods. foreach ($node->getProperties() as $property) { - self::checkProperty($property, $varianceByName); + $this->checkProperty($property); } foreach ($node->getMethods() as $method) { - self::checkMethod($method, $varianceByName); + $this->checkMethod($method); } } - /** - * @param array $varianceByName - */ - private static function checkBoundExpr( - BoundExpr $bound, - array $varianceByName, - string $hostParam, - string $hostPosition, - ): void { + private function checkBoundExpr(BoundExpr $bound, string $hostParam, string $hostPosition, int $line): void + { if ($bound instanceof BoundLeaf) { - self::checkTypeRef($bound->type, $varianceByName, $hostParam, $hostPosition); + $this->checkTypeRef($bound->type, $hostParam, $hostPosition, $line); return; } assert($bound instanceof BoundIntersection || $bound instanceof BoundUnion); foreach ($bound->operands as $operand) { - self::checkBoundExpr($operand, $varianceByName, $hostParam, $hostPosition); + $this->checkBoundExpr($operand, $hostParam, $hostPosition, $line); } } - /** - * @param array $varianceByName - */ - private static function checkTypeRef( - TypeRef $ref, - array $varianceByName, - string $hostParam, - string $hostPosition, - ): void { - if ($ref->isTypeParam && isset($varianceByName[$ref->name])) { - $variance = $varianceByName[$ref->name]; - throw self::violationError( - paramName: $ref->name, - variance: $variance, - position: $hostPosition, - hostParam: $hostParam, + private function checkTypeRef(TypeRef $ref, string $hostParam, string $hostPosition, int $line): void + { + if ($ref->isTypeParam && isset($this->varianceByName[$ref->name])) { + $this->record( + self::violationMessage($ref->name, $this->varianceByName[$ref->name], $hostPosition, $hostParam), + $line, ); } foreach ($ref->args as $inner) { - self::checkTypeRef($inner, $varianceByName, $hostParam, $hostPosition); + $this->checkTypeRef($inner, $hostParam, $hostPosition, $line); } } - /** - * @param array $varianceByName - */ - private static function checkProperty(Property $property, array $varianceByName): void + private function checkProperty(Property $property): void { $type = $property->type; if ($type === null) { @@ -143,13 +182,10 @@ private static function checkProperty(Property $property, array $varianceByName) // regardless of `readonly`. Even +T on a readonly property would // PHP-fatal at autoload when the variance edge lands. $position = $property->isReadonly() ? 'readonly property' : 'mutable property'; - self::checkPhpType($type, $varianceByName, [Variance::Invariant], $position); + $this->checkPhpType($type, [Variance::Invariant], $position); } - /** - * @param array $varianceByName - */ - private static function checkMethod(ClassMethod $method, array $varianceByName): void + private function checkMethod(ClassMethod $method): void { $name = $method->name->toLowerString(); $isConstructor = $name === '__construct'; @@ -166,16 +202,15 @@ private static function checkMethod(ClassMethod $method, array $varianceByName): continue; } if ($param->type !== null) { - self::checkPhpType($param->type, $varianceByName, $paramAllowed, $paramPosition); + $this->checkPhpType($param->type, $paramAllowed, $paramPosition); } } // Return type. Constructors don't have one; for the rest, invariant // or covariant. if (!$isConstructor && $method->returnType !== null) { - self::checkPhpType( + $this->checkPhpType( $method->returnType, - $varianceByName, [Variance::Invariant, Variance::Covariant], 'method return', ); @@ -190,7 +225,7 @@ private static function checkMethod(ClassMethod $method, array $varianceByName): // then nested closures have no type-params and every name in their // signature is an outer reference. if ($method->stmts !== null) { - self::walkBodyForNestedClosures($method->stmts, $varianceByName); + $this->walkBodyForNestedClosures($method->stmts); } } @@ -201,27 +236,23 @@ private static function checkMethod(ClassMethod $method, array $varianceByName): * * Cheap hand-rolled recursive walk -- avoids spinning up a NodeTraverser * inside the per-class validator hot path. - * - * @param array $varianceByName */ - private static function walkBodyForNestedClosures(mixed $node, array $varianceByName): void + private function walkBodyForNestedClosures(mixed $node): void { if ($node instanceof Closure || $node instanceof ArrowFunction) { foreach ($node->params as $param) { // @phpstan-ignore-next-line instanceof.alwaysTrue — defensive guard against nikic/php-parser PHPDoc-narrowed param collection element. if ($param instanceof Param && $param->type !== null) { - self::checkPhpType( + $this->checkPhpType( $param->type, - $varianceByName, [Variance::Invariant, Variance::Contravariant], 'nested closure/arrow parameter', ); } } if ($node->returnType !== null) { - self::checkPhpType( + $this->checkPhpType( $node->returnType, - $varianceByName, [Variance::Invariant, Variance::Covariant], 'nested closure/arrow return', ); @@ -231,13 +262,13 @@ private static function walkBodyForNestedClosures(mixed $node, array $varianceBy if (is_array($node)) { foreach ($node as $child) { - self::walkBodyForNestedClosures($child, $varianceByName); + $this->walkBodyForNestedClosures($child); } return; } if ($node instanceof Node) { foreach ($node->getSubNodeNames() as $subName) { - self::walkBodyForNestedClosures($node->$subName, $varianceByName); + $this->walkBodyForNestedClosures($node->$subName); } } } @@ -247,15 +278,10 @@ private static function walkBodyForNestedClosures(mixed $node, array $varianceBy * UnionType / IntersectionType), checking every Name's parts against * the variance map. * - * @param array $varianceByName * @param list $allowed */ - private static function checkPhpType( - Node $type, - array $varianceByName, - array $allowed, - string $position, - ): void { + private function checkPhpType(Node $type, array $allowed, string $position): void + { if ($type instanceof Identifier) { return; // scalar / pseudo type; never a type-param ref. } @@ -263,14 +289,12 @@ private static function checkPhpType( $parts = $type->getParts(); if (count($parts) === 1) { $name = $parts[0]; - if (isset($varianceByName[$name])) { - $variance = $varianceByName[$name]; + if (isset($this->varianceByName[$name])) { + $variance = $this->varianceByName[$name]; if (!in_array($variance, $allowed, true)) { - throw self::violationError( - paramName: $name, - variance: $variance, - position: $position, - hostParam: null, + $this->record( + self::violationMessage($name, $variance, $position, null), + $type->getStartLine(), ); } } @@ -282,19 +306,19 @@ private static function checkPhpType( if (is_array($args)) { foreach ($args as $arg) { if ($arg instanceof TypeRef) { - self::checkInnerTypeRef($arg, $varianceByName, $allowed, $position); + $this->checkInnerTypeRef($arg, $allowed, $position, $type->getStartLine()); } } } return; } if ($type instanceof NullableType) { - self::checkPhpType($type->type, $varianceByName, $allowed, $position); + $this->checkPhpType($type->type, $allowed, $position); return; } if ($type instanceof UnionType || $type instanceof IntersectionType) { foreach ($type->types as $inner) { - self::checkPhpType($inner, $varianceByName, $allowed, $position); + $this->checkPhpType($inner, $allowed, $position); } return; } @@ -304,37 +328,32 @@ private static function checkPhpType( } /** - * @param array $varianceByName * @param list $allowed */ - private static function checkInnerTypeRef( - TypeRef $ref, - array $varianceByName, - array $allowed, - string $position, - ): void { - if ($ref->isTypeParam && isset($varianceByName[$ref->name])) { - $variance = $varianceByName[$ref->name]; + private function checkInnerTypeRef(TypeRef $ref, array $allowed, string $position, int $line): void + { + if ($ref->isTypeParam && isset($this->varianceByName[$ref->name])) { + $variance = $this->varianceByName[$ref->name]; if (!in_array($variance, $allowed, true)) { - throw self::violationError( - paramName: $ref->name, - variance: $variance, - position: $position, - hostParam: null, - ); + $this->record(self::violationMessage($ref->name, $variance, $position, null), $line); } } foreach ($ref->args as $inner) { - self::checkInnerTypeRef($inner, $varianceByName, $allowed, $position); + $this->checkInnerTypeRef($inner, $allowed, $position, $line); } } - private static function violationError( + private function record(string $message, ?int $line): void + { + $this->violations[] = ['message' => $message, 'line' => $line]; + } + + private static function violationMessage( string $paramName, Variance $variance, string $position, ?string $hostParam, - ): RuntimeException { + ): string { $marker = match ($variance) { Variance::Covariant => '+', Variance::Contravariant => '-', @@ -343,12 +362,12 @@ private static function violationError( $context = $hostParam !== null ? sprintf(' inside the %s of generic parameter `%s`', $position, $hostParam) : sprintf(' in %s position', $position); - return new RuntimeException(sprintf( + return sprintf( 'Generic parameter `%s%s` appears%s, which is not allowed for %s variance.', $marker, $paramName, $context, $variance->value, - )); + ); } } diff --git a/src/Transpiler/Monomorphize/XphpSourceParser.php b/src/Transpiler/Monomorphize/XphpSourceParser.php index a1d9157..175a463 100644 --- a/src/Transpiler/Monomorphize/XphpSourceParser.php +++ b/src/Transpiler/Monomorphize/XphpSourceParser.php @@ -1770,22 +1770,10 @@ private function buildBoundExprNode(array $node): BoundExpr public function leaveNode(Node $node): null { - // Variance position check fires once per class definition, - // AFTER the body has been fully resolved -- so any Name nodes - // inside the body that carry nested ATTR_GENERIC_ARGS are - // visible to the validator. Rejects covariant T in input - // position, contravariant T in output, either in - // bound/default/property/constructor positions, and - // F-bounded variance (`+T : Box`). - if ($node instanceof ClassLike && $node->name !== null) { - $params = $node->getAttribute(XphpSourceParser::ATTR_GENERIC_PARAMS); - if (is_array($params)) { - /** @var list $params — set as a list by XphpSourceParser::resolveAndAttach. */ - if ($params !== []) { - VariancePositionValidator::assertPositions($node, $params); - } - } - } + // NB: variance-position validation no longer runs here. It moved to a + // Registry validation phase (`validateVariancePositions`) that runs over + // collected definitions, so `xphp check` can collect every variance error + // across all files in one run instead of aborting at the first parse. // @infection-ignore-all -- the instanceof chain mirrors enterNode's push; // restructuring `||` as `&&` produces a leaveNode that no longer pops the // stack for any node, but the test suite's AST shapes never re-use the diff --git a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php index ceff18d..da3f2cc 100644 --- a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php +++ b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php @@ -51,6 +51,29 @@ public function testDefaultBoundViolationIsCollectedByCheck(): void self::assertSame(Registry::CODE_DEFAULT_BOUND_VIOLATION, $diagnostics->all()[0]->code); } + public function testVariancePositionViolationIsCollectedByCheck(): void + { + // Exercises the validateVariancePositions() step of check(). + $diagnostics = $this->check('variance_violation'); + + self::assertCount(1, $diagnostics->all()); + self::assertSame(VariancePositionValidator::CODE_VARIANCE_POSITION, $diagnostics->all()[0]->code); + } + + public function testCompileStillThrowsOnVariancePositionViolation(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('not allowed for covariant variance'); + + $work = sys_get_temp_dir() . '/xphp-check-compile-' . uniqid('', true); + mkdir($work, 0o755, true); + try { + $this->buildCompiler()->compile($this->sources('variance_violation'), $this->sourceDir('variance_violation'), $work . '/dist', $work . '/cache'); + } finally { + self::rrmdir($work); + } + } + public function testMissingTypeArgumentIsCollectedByCheck(): void { $diagnostics = $this->check('missing_arg'); diff --git a/test/Transpiler/Monomorphize/VariancePositionPhaseTest.php b/test/Transpiler/Monomorphize/VariancePositionPhaseTest.php new file mode 100644 index 0000000..f9145d7 --- /dev/null +++ b/test/Transpiler/Monomorphize/VariancePositionPhaseTest.php @@ -0,0 +1,134 @@ +}> + */ + public static function rejectedSources(): iterable + { + yield 'covariant in method parameter' => [ + "\n{\n public function set(T \$x): void {}\n}\n", + ['+T', 'method parameter'], + ]; + yield 'contravariant in method return' => [ + "\n{\n public function get(): T { throw new \\LogicException; }\n}\n", + ['-T', 'method return'], + ]; + yield 'covariant in mutable property' => [ + "\n{\n public T \$item;\n}\n", + ['mutable property'], + ]; + yield 'covariant in readonly property' => [ + "\n{\n public readonly T \$item;\n public function get(): T { return \$this->item; }\n}\n", + ['readonly property'], + ]; + yield 'covariant in bound' => [ + ">\n{\n public function get(): T { throw new \\LogicException; }\n}\n", + ['bound'], + ]; + yield 'covariant in constructor parameter' => [ + "\n{\n public function __construct(T \$item) {}\n public function get(): T { throw new \\LogicException; }\n}\n", + ['constructor parameter'], + ]; + yield 'covariant in nested closure parameter' => [ + "\n{\n public function emit(): array\n {\n \$f = function (T \$x) {};\n return [];\n }\n}\n", + ['nested closure/arrow parameter'], + ]; + yield 'contravariant in nested arrow return' => [ + "\n{\n public function pipe(): array\n {\n \$f = fn (): T => null;\n return [];\n }\n}\n", + ['nested closure/arrow return'], + ]; + yield 'covariant in nested generic input' => [ + "\n{\n public function set(Box \$x): void {}\n}\n", + ['+T', 'method parameter'], + ]; + yield 'contravariant in nested generic return' => [ + "\n{\n public function fetch(): Box { throw new \\LogicException; }\n}\n", + ['-T', 'method return'], + ]; + yield 'interface method signature' => [ + "\n{\n public function feed(T \$x): void;\n}\n", + ['+T'], + ]; + } + + /** + * @param list $fragments + */ + #[DataProvider('rejectedSources')] + public function testVariancePositionIsRejectedInCompileMode(string $source, array $fragments): void + { + $registry = $this->registryFor($source); + + try { + $registry->validateVariancePositions(); + self::fail('expected a variance-position violation'); + } catch (RuntimeException $e) { + foreach ($fragments as $fragment) { + self::assertStringContainsString($fragment, $e->getMessage()); + } + } + } + + public function testViolationIsCollectedWithMemberLineInCheckMode(): void + { + // Line 5 holds `public function set(T $x)`. + $source = "\n{\n public function set(T \$x): void {}\n}\n"; + $collector = new DiagnosticCollector(); + $registry = $this->registryFor($source, $collector); + + $registry->validateVariancePositions(); // must NOT throw + + self::assertCount(1, $collector->all()); + $d = $collector->all()[0]; + self::assertSame(VariancePositionValidator::CODE_VARIANCE_POSITION, $d->code); + self::assertNotNull($d->location); + self::assertSame('/T.xphp', $d->location->file); + self::assertSame(5, $d->location->line); + self::assertStringContainsString('method parameter', $d->message); + } + + public function testAllViolationsAcrossDefinitionsCollectedInOneRun(): void + { + // Two distinct templates, each with a variance violation — both reported. + $source = "\n{\n public function set(T \$x): void {}\n}\n" + . "class Consumer<-T>\n{\n public function get(): T { throw new \\LogicException; }\n}\n"; + $collector = new DiagnosticCollector(); + $registry = $this->registryFor($source, $collector); + + $registry->validateVariancePositions(); + + self::assertCount(2, $collector->all()); + foreach ($collector->all() as $d) { + self::assertSame(VariancePositionValidator::CODE_VARIANCE_POSITION, $d->code); + } + } + + private function registryFor(string $source, ?DiagnosticCollector $collector = null): Registry + { + $ast = (new XphpSourceParser((new ParserFactory())->createForHostVersion()))->parse($source); + $registry = new Registry(diagnostics: $collector); + (new RegistryCollector($registry))->collectDefinitions($ast, '/T.xphp'); + + return $registry; + } +} diff --git a/test/Transpiler/Monomorphize/XphpSourceParserTest.php b/test/Transpiler/Monomorphize/XphpSourceParserTest.php index d3848d2..2c0b42d 100644 --- a/test/Transpiler/Monomorphize/XphpSourceParserTest.php +++ b/test/Transpiler/Monomorphize/XphpSourceParserTest.php @@ -2234,116 +2234,6 @@ function id<-T>(T $x): T { return $x; } $parser->parse($source); } - public function testCovariantInInputPositionIsRejected(): void - { - $source = <<<'PHP' - -{ - public function set(T $x): void {} -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('+T'); - $this->expectExceptionMessage('method parameter'); - $parser->parse($source); - } - - public function testContravariantInOutputPositionIsRejected(): void - { - $source = <<<'PHP' - -{ - public function get(): T { throw new \LogicException; } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('-T'); - $this->expectExceptionMessage('method return'); - $parser->parse($source); - } - - public function testCovariantInMutablePropertyIsRejected(): void - { - $source = <<<'PHP' - -{ - public T $item; -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('mutable property'); - $parser->parse($source); - } - - public function testCovariantInReadonlyPropertyIsAlsoRejected(): void - { - // PHP enforces invariant property types across `extends` chains - // regardless of `readonly`. Even though a readonly property is - // semantically "output-only", PHP's static type system rejects - // covariance on the property declaration -- so we reject it at - // declaration time to avoid an autoload-time fatal. - $source = <<<'PHP' - -{ - public readonly T $item; - public function get(): T { return $this->item; } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('readonly property'); - $parser->parse($source); - } - - public function testCovariantInBoundIsRejected(): void - { - // F-bounded with variance: `+T : Box` rejected because T appears - // inside its own bound (an invariant position). - $source = <<<'PHP' -> -{ - public function get(): T { throw new \LogicException; } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('bound'); - $parser->parse($source); - } - - public function testCovariantInConstructorParamIsRejected(): void - { - // xphp deviates from RFC: constructor params are invariant because - // PHP's autoload-time signature compatibility check applies to - // __construct on `Producer_Banana implements Producer_Fruit`. - $source = <<<'PHP' - -{ - public function __construct(T $item) {} - public function get(): T { throw new \LogicException; } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('constructor parameter'); - $parser->parse($source); - } - public function testContravariantInInputPositionIsAccepted(): void { $source = <<<'PHP' @@ -2652,108 +2542,6 @@ public function testGenericClosureVarianceIsRejected(): void $parser->parse($source); } - public function testCovariantInNestedClosureParameterIsRejected(): void - { - // `+T` of the OUTER class appears in the parameter type of a nested - // CLOSURE -- the variance validator must recurse into method bodies. - // Without the recursion the inner closure's param `T $x` slips - // through, and at PHP autoload time the variance edge produces a - // signature-compat fatal. - $source = <<<'PHP' - -{ - public function emit(): array - { - $f = function (T $x) {}; - return []; - } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('nested closure/arrow parameter'); - $parser->parse($source); - } - - public function testContravariantInNestedArrowReturnIsRejected(): void - { - // `-T` in the return position of a nested ARROW FUNCTION inside a - // contravariant Consumer's method body. - $source = <<<'PHP' - -{ - public function pipe(): array - { - $f = fn (): T => null; - return []; - } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('nested closure/arrow return'); - $parser->parse($source); - } - - public function testCovariantInNestedGenericInputPositionIsRejected(): void - { - // `+T` inside `Box` in a method parameter position. The validator - // walks into the inner generic args attached via xphp:genericArgs; - // the +T leaf is rejected just as if it appeared directly. - $source = <<<'PHP' - -{ - public function set(Box $x): void {} -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('+T'); - $this->expectExceptionMessage('method parameter'); - $parser->parse($source); - } - - public function testContravariantInNestedGenericReturnPositionIsRejected(): void - { - // Symmetric case: `-T` inside `Box` in a method return type. - $source = <<<'PHP' - -{ - public function fetch(): Box { throw new \LogicException; } -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('-T'); - $this->expectExceptionMessage('method return'); - $parser->parse($source); - } - - public function testInterfaceMethodSignatureIsValidatedForVariance(): void - { - // Variance rules apply to interface methods too. - $source = <<<'PHP' - -{ - public function feed(T $x): void; -} -PHP; - $parser = new XphpSourceParser((new ParserFactory())->createForHostVersion()); - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('+T'); - $parser->parse($source); - } - /** * @template TNode of Node * @param array $ast diff --git a/test/fixture/check/variance_violation/source/Producer.xphp b/test/fixture/check/variance_violation/source/Producer.xphp new file mode 100644 index 0000000..fa98c77 --- /dev/null +++ b/test/fixture/check/variance_violation/source/Producer.xphp @@ -0,0 +1,13 @@ + +{ + public function set(T $value): void + { + } +} From 90450e85cc00ce72ff00cfde91d4a28370e9a9c7 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:13:34 +0000 Subject: [PATCH 05/16] refactor(check): make inner-variance composition collectable Extract the inner-variance composition walk out of Registry into a dedicated InnerVarianceValidator (mirroring VariancePositionValidator): it accumulates violations behind a static assertComposition facade that throws the first when no collector is given (compile, byte-identical) or emits a diagnostic per violation (check), each located at the offending member. Registry's validateInnerVariance is now a thin delegate. To avoid double-reporting a direct +T/-T misuse, the position check now returns which definitions it flagged and the inner-variance pass skips them -- matching compile-mode, where the position check fails fast before inner-variance runs. Both passes are wired into Compiler::check(); compile() is unchanged. Adds inner-variance check fixture + collect-mode, gating, and null-file tests (100% mutation score over the new validator and the diff). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Compiler.php | 10 +- .../Monomorphize/InnerVarianceValidator.php | 309 +++++++++++++++ src/Transpiler/Monomorphize/Registry.php | 356 ++---------------- .../VariancePositionValidator.php | 15 +- .../Monomorphize/CheckPassIntegrationTest.php | 15 + .../RegistryInnerVarianceTest.php | 124 +++++- .../inner_variance/source/Container.xphp | 13 + .../check/inner_variance/source/P.xphp | 16 + 8 files changed, 519 insertions(+), 339 deletions(-) create mode 100644 src/Transpiler/Monomorphize/InnerVarianceValidator.php create mode 100644 test/fixture/check/inner_variance/source/Container.xphp create mode 100644 test/fixture/check/inner_variance/source/P.xphp diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 0f5bff8..59396af 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -86,13 +86,13 @@ public function compile( // bound/default/property/constructor invariance, F-bounded variance). Moved // here from the parser so all definitions are present and `xphp check` can // collect across files; compile-mode still throws on the first violation. - $registry->validateVariancePositions(); + $variancePositionFlagged = $registry->validateVariancePositions(); // Inner-template variance composition: every template's variance // markers are known by now, so cases the parse-time validator // couldn't catch (e.g. `class P<+T> { f(): Container }` where // Container's slot is invariant) fail here BEFORE instantiations // amplify the error. - $registry->validateInnerVariance(); + $registry->validateInnerVariance($variancePositionFlagged); foreach ($astPerFile as $filepath => $ast) { $collector->collectInstantiations($ast, $filepath); } @@ -232,10 +232,8 @@ public function check(FilepathArray $sources): DiagnosticCollector $collector->collectDefinitions($ast, $filepath); } $registry->validateDefaultsAgainstBounds(); - $registry->validateVariancePositions(); - // NB: validateInnerVariance() is not wired in here yet — it still throws - // (not collectable until the inner-variance step), so calling it in check-mode - // would abort on the first violation. Added once it accepts the collector. + $variancePositionFlagged = $registry->validateVariancePositions(); + $registry->validateInnerVariance($variancePositionFlagged); foreach ($astPerFile as $filepath => $ast) { $collector->collectInstantiations($ast, $filepath); } diff --git a/src/Transpiler/Monomorphize/InnerVarianceValidator.php b/src/Transpiler/Monomorphize/InnerVarianceValidator.php new file mode 100644 index 0000000..19d4e24 --- /dev/null +++ b/src/Transpiler/Monomorphize/InnerVarianceValidator.php @@ -0,0 +1,309 @@ + { f(): Container }` where `Container`'s slot is + * invariant), the effective variance at that position is the composition of the + * outer position and the inner slot's variance. This catches cases the + * position validator can't, because the effective variance only resolves once + * the inner template's slot variances are known. + * + * Runs over collected definitions (`Registry::validateInnerVariance`). With a + * DiagnosticCollector it gathers every violation in the definition (each located + * at the offending member) and continues; without one it throws the first — + * byte-identical to the previous behavior. + */ +final class InnerVarianceValidator +{ + /** Stable diagnostic code for an inner-variance composition violation. */ + public const CODE_INNER_VARIANCE = 'xphp.inner_variance'; + + /** @var array */ + private array $definitions; + + /** @var array */ + private array $varianceMap; + + /** @var list */ + private array $violations = []; + + /** + * @param array $definitions + * @param array $varianceMap + */ + private function __construct(array $definitions, array $varianceMap) + { + $this->definitions = $definitions; + $this->varianceMap = $varianceMap; + } + + /** + * @param array $definitions All definitions, for inner-slot lookup. + */ + public static function assertComposition( + GenericDefinition $definition, + array $definitions, + ?DiagnosticCollector $diagnostics = null, + ?string $file = null, + ): void { + $varianceMap = self::buildVarianceMap($definition->typeParams); + // @infection-ignore-all -- optimization only: with an empty variance map every leaf + // check is a no-op (no name is in the map), so removing this guard walks the body + // pointlessly but produces the same (empty) result. + if ($varianceMap === []) { + return; + } + + $validator = new self($definitions, $varianceMap); + $validator->collect($definition); + if ($validator->violations === []) { + return; + } + + if ($diagnostics === null) { + throw new RuntimeException($validator->violations[0]['message']); + } + + foreach ($validator->violations as $violation) { + $location = ($violation['line'] !== null && $file !== null) + ? new SourceLocation($file, $violation['line']) + : null; + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_INNER_VARIANCE, + $violation['message'], + $location, + )); + } + } + + private function collect(GenericDefinition $definition): void + { + $label = $definition->templateShortName; + $declarationLine = $definition->templateAst->getStartLine(); + + foreach ($definition->templateAst->getMethods() as $method) { + $isCtor = $method->name->toLowerString() === '__construct'; + foreach ($method->params as $param) { + // Constructor params (promoted or not) get Invariant outer + // position -- PHP's class-compat rules enforce invariance on + // ctor signatures regardless of param flavor. `getProperties()` + // below skips promoted ones (they're `Param`, not `Property`), + // so each promoted property is walked exactly once. + $outerPos = $isCtor ? Variance::Invariant : Variance::Contravariant; + if ($param->type !== null) { + $this->walkPhpType($param->type, $outerPos, $label, null, null); + } + } + if ($method->returnType !== null) { + $this->walkPhpType($method->returnType, Variance::Covariant, $label, null, null); + } + } + foreach ($definition->templateAst->getProperties() as $prop) { + if ($prop->type !== null) { + $this->walkPhpType($prop->type, Variance::Invariant, $label, null, null); + } + } + foreach ($definition->typeParams as $typeParam) { + if ($typeParam->bound !== null) { + $this->walkBoundExpr($typeParam->bound, $label, $declarationLine); + } + if ($typeParam->default !== null) { + $this->walkTypeRef($typeParam->default, Variance::Invariant, $label, null, null, $declarationLine); + } + } + } + + /** + * @infection-ignore-all -- semantic-equivalent mutants in this walker: + * fall-through returns for unhandled node kinds, `??`-suppressed null-safe + * slot lookups, and Union/Intersection branch swaps (both recurse the same). + */ + private function walkPhpType( + Node $type, + Variance $position, + string $outerLabel, + ?string $innerLabel, + ?int $innerSlot, + ): void { + if ($type instanceof Identifier) { + return; + } + if ($type instanceof Name) { + $parts = $type->getParts(); + if (count($parts) === 1 && isset($this->varianceMap[$parts[0]])) { + $this->assertLeaf($parts[0], $this->varianceMap[$parts[0]], $position, $outerLabel, $innerLabel, $innerSlot, $type->getStartLine()); + } + $args = $type->getAttribute(XphpSourceParser::ATTR_GENERIC_ARGS); + if (is_array($args)) { + $innerFqn = $type->getAttribute(XphpSourceParser::ATTR_TEMPLATE_FQN); + $innerDef = is_string($innerFqn) + ? ($this->definitions[ltrim($innerFqn, '\\')] ?? null) + : null; + $nextInnerLabel = $innerDef !== null ? $innerDef->templateShortName : $type->toString(); + foreach ($args as $i => $arg) { + if (!$arg instanceof TypeRef) { + continue; + } + $slotVariance = $innerDef?->typeParams[$i]->variance ?? Variance::Invariant; + $this->walkTypeRef($arg, self::compose($position, $slotVariance), $outerLabel, $nextInnerLabel, $i, $type->getStartLine()); + } + } + return; + } + if ($type instanceof NullableType) { + $this->walkPhpType($type->type, $position, $outerLabel, $innerLabel, $innerSlot); + return; + } + if ($type instanceof UnionType || $type instanceof IntersectionType) { + foreach ($type->types as $sub) { + $this->walkPhpType($sub, $position, $outerLabel, $innerLabel, $innerSlot); + } + return; + } + if ($type instanceof ComplexType) { + return; + } + } + + /** + * @infection-ignore-all -- same equivalence rationale as `walkPhpType`. + */ + private function walkTypeRef( + TypeRef $ref, + Variance $position, + string $outerLabel, + ?string $innerLabel, + ?int $innerSlot, + ?int $line, + ): void { + if ($ref->isScalar) { + return; + } + if ($ref->isTypeParam && isset($this->varianceMap[$ref->name])) { + $this->assertLeaf($ref->name, $this->varianceMap[$ref->name], $position, $outerLabel, $innerLabel, $innerSlot, $line); + } + if ($ref->args === []) { + return; + } + $innerDef = $this->definitions[ltrim($ref->name, '\\')] ?? null; + $nextInnerLabel = $innerDef !== null ? $innerDef->templateShortName : $ref->name; + foreach ($ref->args as $i => $sub) { + $slotVariance = $innerDef?->typeParams[$i]->variance ?? Variance::Invariant; + $this->walkTypeRef($sub, self::compose($position, $slotVariance), $outerLabel, $nextInnerLabel, $i, $line); + } + } + + /** + * @infection-ignore-all -- BoundUnion / BoundIntersection share the same + * `operands` walk; the InstanceOf_ / LogicalOr mutants on the discriminator + * are observably identical for any non-Leaf bound expression. + */ + private function walkBoundExpr(BoundExpr $expr, string $outerLabel, ?int $line): void + { + if ($expr instanceof BoundLeaf) { + $this->walkTypeRef($expr->type, Variance::Invariant, $outerLabel, null, null, $line); + return; + } + if ($expr instanceof BoundUnion || $expr instanceof BoundIntersection) { + foreach ($expr->operands as $operand) { + $this->walkBoundExpr($operand, $outerLabel, $line); + } + } + } + + /** + * @infection-ignore-all -- the `Variance::Invariant => ''` arm of the + * sigil-builder `match` is unreachable: Invariant declared variance passes + * every allowed-list, so this records nothing before the sigil is built. + */ + private function assertLeaf( + string $paramName, + Variance $declared, + Variance $effective, + string $outerLabel, + ?string $innerLabel, + ?int $innerSlot, + ?int $line, + ): void { + $allowed = match ($effective) { + Variance::Invariant => [Variance::Invariant], + Variance::Covariant => [Variance::Invariant, Variance::Covariant], + Variance::Contravariant => [Variance::Invariant, Variance::Contravariant], + }; + if (in_array($declared, $allowed, true)) { + return; + } + // $declared is Covariant or Contravariant at this point — the Invariant + // case passes every allowed-list and early-returns above. + $sigil = $declared === Variance::Covariant ? '+' : '-'; + $where = $innerLabel !== null + ? sprintf(' (via slot %d of %s)', $innerSlot, $innerLabel) + : ''; + $this->violations[] = [ + 'message' => sprintf( + 'Variance violation in template %s: type-parameter %s%s appears in %s-only position%s.', + $outerLabel, + $sigil, + $paramName, + $effective->value, + $where, + ), + 'line' => $line, + ]; + } + + private static function compose(Variance $position, Variance $innerSlot): Variance + { + return match ($innerSlot) { + Variance::Invariant => Variance::Invariant, + Variance::Covariant => $position, + Variance::Contravariant => match ($position) { + Variance::Covariant => Variance::Contravariant, + Variance::Contravariant => Variance::Covariant, + Variance::Invariant => Variance::Invariant, + }, + }; + } + + /** + * @param list $typeParams + * @return array + * + * @infection-ignore-all -- FalseValue mutator on `$hasVariance = false` is + * observably identical: for all-Invariant templates, walking is a no-op + * (Invariant is allowed at every effective position), so the "skip the + * walk" optimization isn't testable. + */ + private static function buildVarianceMap(array $typeParams): array + { + $hasVariance = false; + $map = []; + foreach ($typeParams as $tp) { + $map[$tp->name] = $tp->variance; + if ($tp->variance !== Variance::Invariant) { + $hasVariance = true; + } + } + return $hasVariance ? $map : []; + } +} diff --git a/src/Transpiler/Monomorphize/Registry.php b/src/Transpiler/Monomorphize/Registry.php index 29e7688..daa028d 100644 --- a/src/Transpiler/Monomorphize/Registry.php +++ b/src/Transpiler/Monomorphize/Registry.php @@ -374,357 +374,57 @@ private static function defaultBoundViolationMessage( ); } - /** - * Inner-template variance composition pass. Runs after `collectDefinitions` - * but before `collectInstantiations`, so every template's variance markers - * are known and a bad declaration fails before any padded instantiation - * amplifies the error. - * - * The parse-time validator at `VariancePositionValidator` already rejects - * direct misuses like `class P<+T> { function f(T $x): void }` (T as param - * with covariance). It also recurses into `xphp:genericArgs` but propagates - * the SAME outer allowed-list -- which is wrong: when T appears as the i-th - * arg of an inner template `Container` whose X is invariant, T's - * effective position is *invariant* regardless of the outer position. - * - * This pass tightens the verdict whenever the inner template is in the - * registry, applying the composition rule: - * - * compose(V_outer_position, V_inner_slot): - * V_inner == Invariant -> Invariant (inner forces strict) - * V_inner == Covariant -> V_outer (transparent) - * V_inner == Contravariant -> flip(V_outer) (covariant <-> contravariant) - * - * The leaf check is "T's declared variance must be in the allowed-list for - * the effective position": - * - * allowed_for(Invariant) = {Invariant} - * allowed_for(Covariant) = {Invariant, Covariant} - * allowed_for(Contravariant) = {Invariant, Contravariant} - * - * Conservative-unknown: when the inner template isn't in the registry - * (vendor classes, in-progress files), treat its slots as Invariant. - * Sound (rejects more than necessary); users with vendor templates can - * either register them or remove variance markers on the outer template. - * - * @infection-ignore-all -- surviving mutants in this method, the walkers, - * and assertLeaf are all semantic equivalents: - * - `strtolower((string) $method->name)` -- PHP method names are - * case-insensitive at dispatch, but PhpParser stores them as - * written. No fixture uses an uppercased `__CONSTRUCT`, so the - * `strtolower` mutator survives without observable difference. - * - Fall-through `return` removals on `Identifier`, post-`Name`, - * post-`NullableType` -- the next branch checks `instanceof X` and - * fails for the prior type, so removing the `return` is a no-op. - * - `$x?->prop ?? $default` -- PHP 8.4's `??` suppresses property-on-null - * errors, so the NullSafe mutator (`?->` -> `->`) is observably - * identical to the original. - * - InstanceOf_ / LogicalOr swaps on `Union||Intersection` -- both - * `->types`/`->operands` branches walk the same way; for inputs - * that aren't either, the prior `Name`/`BoundLeaf` branches already - * returned. - * - LogicalAnd in `$ref->isTypeParam && isset($map[$ref->name])` -- - * no fixture creates a stray type-param ref outside the variance - * map, so the OR variant produces the same accept/reject decision. - * - MatchArmRemoval on `Variance::Invariant => ''` in the sigil - * builder -- Invariant declared never reaches the throw (Invariant - * passes every allowed-list), so the arm is observably unreachable. - */ /** * Variance-position check over every collected definition (moved out of the parser so * `xphp check` can collect all violations across files in one run). Delegates to * {@see VariancePositionValidator}: with this Registry's collector it gathers diagnostics * at each offending member; without one (compile) it throws the first violation. */ - public function validateVariancePositions(): void + /** + * @return list Template FQNs that had at least one position violation, so the + * inner-variance pass can skip them (the issue is already reported) — mirroring + * compile-mode, where the position check fails fast before inner-variance runs. + */ + public function validateVariancePositions(): array { - foreach ($this->definitions as $definition) { - VariancePositionValidator::assertPositions( + $flagged = []; + foreach ($this->definitions as $templateFqn => $definition) { + $hadViolation = VariancePositionValidator::assertPositions( $definition->templateAst, $definition->typeParams, $this->diagnostics, $definition->sourceFile, ); - } - } - - public function validateInnerVariance(): void - { - foreach ($this->definitions as $definition) { - $varianceMap = self::buildVarianceMap($definition->typeParams); - if ($varianceMap === []) { - continue; - } - $label = $definition->templateShortName; - foreach ($definition->templateAst->getMethods() as $method) { - $isCtor = strtolower((string) $method->name) === '__construct'; - foreach ($method->params as $param) { - // Constructor params (promoted or not) get Invariant outer - // position -- PHP's class-compat rules enforce invariance on - // ctor signatures regardless of param flavor. `getProperties()` - // below skips promoted ones (they're `Param`, not `Property`), - // so each promoted property is walked exactly once. - $outerPos = $isCtor ? Variance::Invariant : Variance::Contravariant; - if ($param->type !== null) { - $this->walkPhpType($param->type, $varianceMap, $outerPos, $label, null, null); - } - } - if ($method->returnType !== null) { - $this->walkPhpType( - $method->returnType, - $varianceMap, - Variance::Covariant, - $label, - null, - null, - ); - } - } - foreach ($definition->templateAst->getProperties() as $prop) { - if ($prop->type !== null) { - $this->walkPhpType( - $prop->type, - $varianceMap, - Variance::Invariant, - $label, - null, - null, - ); - } - } - foreach ($definition->typeParams as $typeParam) { - if ($typeParam->bound !== null) { - $this->walkBoundExpr($typeParam->bound, $varianceMap, $label); - } - if ($typeParam->default !== null) { - $this->walkTypeRef( - $typeParam->default, - $varianceMap, - Variance::Invariant, - $label, - null, - null, - ); - } + if ($hadViolation) { + $flagged[] = $templateFqn; } } - } - /** - * @param list $typeParams - * @return array - * - * @infection-ignore-all -- FalseValue mutator on `$hasVariance = false` - * is observably identical: for all-Invariant templates, walking is a - * no-op (Invariant is allowed at every effective position), so the - * "skip the walk" optimization isn't testable. - */ - private static function buildVarianceMap(array $typeParams): array - { - $hasVariance = false; - $map = []; - foreach ($typeParams as $tp) { - $map[$tp->name] = $tp->variance; - if ($tp->variance !== Variance::Invariant) { - $hasVariance = true; - } - } - return $hasVariance ? $map : []; + return $flagged; } /** - * @param array $varianceMap - * - * @infection-ignore-all -- see `validateInnerVariance` docblock for the - * catalog of semantic-equivalent mutants in this walker (fall-through - * returns, `??`-suppressed null-safe ops, Union/Intersection swaps). + * Inner-template variance composition check over every collected definition (delegates to + * {@see InnerVarianceValidator}). With this Registry's collector it gathers every violation + * (each located at the offending member); without one (compile) it throws the first. */ - private function walkPhpType( - Node $type, - array $varianceMap, - Variance $position, - string $outerLabel, - ?string $innerLabel, - ?int $innerSlot, - ): void { - if ($type instanceof Identifier) { - return; - } - if ($type instanceof Name) { - $parts = $type->getParts(); - if (count($parts) === 1 && isset($varianceMap[$parts[0]])) { - self::assertLeaf( - $parts[0], - $varianceMap[$parts[0]], - $position, - $outerLabel, - $innerLabel, - $innerSlot, - ); - } - $args = $type->getAttribute(XphpSourceParser::ATTR_GENERIC_ARGS); - if (is_array($args)) { - $innerFqn = $type->getAttribute(XphpSourceParser::ATTR_TEMPLATE_FQN); - $innerDef = is_string($innerFqn) - ? ($this->definitions[ltrim($innerFqn, '\\')] ?? null) - : null; - $nextInnerLabel = $innerDef !== null ? $innerDef->templateShortName : $type->toString(); - foreach ($args as $i => $arg) { - if (!$arg instanceof TypeRef) { - continue; - } - $slotVariance = $innerDef?->typeParams[$i]->variance ?? Variance::Invariant; - $this->walkTypeRef( - $arg, - $varianceMap, - self::compose($position, $slotVariance), - $outerLabel, - $nextInnerLabel, - $i, - ); - } - } - return; - } - if ($type instanceof NullableType) { - $this->walkPhpType($type->type, $varianceMap, $position, $outerLabel, $innerLabel, $innerSlot); - return; - } - if ($type instanceof UnionType || $type instanceof IntersectionType) { - foreach ($type->types as $sub) { - $this->walkPhpType($sub, $varianceMap, $position, $outerLabel, $innerLabel, $innerSlot); - } - return; - } - if ($type instanceof ComplexType) { - return; - } - } - - /** - * @param array $varianceMap - * - * @infection-ignore-all -- same equivalence rationale as `walkPhpType`. - */ - private function walkTypeRef( - TypeRef $ref, - array $varianceMap, - Variance $position, - string $outerLabel, - ?string $innerLabel, - ?int $innerSlot, - ): void { - if ($ref->isScalar) { - return; - } - if ($ref->isTypeParam && isset($varianceMap[$ref->name])) { - self::assertLeaf( - $ref->name, - $varianceMap[$ref->name], - $position, - $outerLabel, - $innerLabel, - $innerSlot, - ); - } - if ($ref->args === []) { - return; - } - $innerDef = $this->definitions[ltrim($ref->name, '\\')] ?? null; - $nextInnerLabel = $innerDef !== null ? $innerDef->templateShortName : $ref->name; - foreach ($ref->args as $i => $sub) { - $slotVariance = $innerDef?->typeParams[$i]->variance ?? Variance::Invariant; - $this->walkTypeRef( - $sub, - $varianceMap, - self::compose($position, $slotVariance), - $outerLabel, - $nextInnerLabel, - $i, - ); - } - } - /** - * @param array $varianceMap - * - * @infection-ignore-all -- BoundUnion / BoundIntersection share the same - * `operands` walk; the InstanceOf_ / LogicalOr mutants on the discriminator - * are observably identical for any non-Leaf bound expression. + * @param list $skipTemplateFqns Definitions already flagged by the position check; + * skipped here so the same `+T`/`-T` misuse isn't reported by both passes. */ - private function walkBoundExpr( - BoundExpr $expr, - array $varianceMap, - string $outerLabel, - ): void { - if ($expr instanceof BoundLeaf) { - $this->walkTypeRef( - $expr->type, - $varianceMap, - Variance::Invariant, - $outerLabel, - null, - null, - ); - return; - } - if ($expr instanceof BoundUnion || $expr instanceof BoundIntersection) { - foreach ($expr->operands as $operand) { - $this->walkBoundExpr($operand, $varianceMap, $outerLabel); - } - } - } - - private static function compose(Variance $position, Variance $innerSlot): Variance + public function validateInnerVariance(array $skipTemplateFqns = []): void { - return match ($innerSlot) { - Variance::Invariant => Variance::Invariant, - Variance::Covariant => $position, - Variance::Contravariant => match ($position) { - Variance::Covariant => Variance::Contravariant, - Variance::Contravariant => Variance::Covariant, - Variance::Invariant => Variance::Invariant, - }, - }; - } - - /** - * @infection-ignore-all -- the `Variance::Invariant => ''` arm of the - * sigil-builder `match` is unreachable: Invariant declared variance - * passes every allowed-list, so this method early-returns before the - * sigil construction. MatchArmRemoval on that arm is observably - * identical. - */ - private static function assertLeaf( - string $paramName, - Variance $declared, - Variance $effective, - string $outerLabel, - ?string $innerLabel, - ?int $innerSlot, - ): void { - $allowed = match ($effective) { - Variance::Invariant => [Variance::Invariant], - Variance::Covariant => [Variance::Invariant, Variance::Covariant], - Variance::Contravariant => [Variance::Invariant, Variance::Contravariant], - }; - if (in_array($declared, $allowed, true)) { - return; + foreach ($this->definitions as $templateFqn => $definition) { + if (in_array($templateFqn, $skipTemplateFqns, true)) { + continue; + } + InnerVarianceValidator::assertComposition( + $definition, + $this->definitions, + $this->diagnostics, + $definition->sourceFile, + ); } - // $declared is Covariant or Contravariant at this point — the Invariant - // case passes every allowed-list and early-returns above. - $sigil = $declared === Variance::Covariant ? '+' : '-'; - $where = $innerLabel !== null - ? sprintf(' (via slot %d of %s)', $innerSlot, $innerLabel) - : ''; - throw new RuntimeException(sprintf( - 'Variance violation in template %s: type-parameter %s%s appears in %s-only position%s.', - $outerLabel, - $sigil, - $paramName, - $effective->value, - $where, - )); } /** diff --git a/src/Transpiler/Monomorphize/VariancePositionValidator.php b/src/Transpiler/Monomorphize/VariancePositionValidator.php index 6e1abfc..5d62c9a 100644 --- a/src/Transpiler/Monomorphize/VariancePositionValidator.php +++ b/src/Transpiler/Monomorphize/VariancePositionValidator.php @@ -80,27 +80,34 @@ private function __construct(array $varianceByName) /** * @param list $params + * @return bool True iff at least one violation was found (in check-mode; compile-mode + * throws before returning). Lets the caller skip the inner-variance pass for a + * definition already flagged here, avoiding a double report of the same issue. */ public static function assertPositions( ClassLike $node, array $params, ?DiagnosticCollector $diagnostics = null, ?string $file = null, - ): void { + ): bool { $varianceByName = []; foreach ($params as $param) { if ($param->variance !== Variance::Invariant) { $varianceByName[$param->name] = $param->variance; } } + // @infection-ignore-all FalseValue -- returning true here (no variance markers) would + // only add this definition to the caller's "flagged" set, which merely skips the + // inner-variance pass for it; but a no-variance definition is already a no-op in + // inner-variance (empty variance map), so flagging it changes nothing observable. if ($varianceByName === []) { - return; + return false; } $validator = new self($varianceByName); $validator->collect($node, $params); if ($validator->violations === []) { - return; + return false; } if ($diagnostics === null) { @@ -119,6 +126,8 @@ public static function assertPositions( $location, )); } + + return true; } /** diff --git a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php index da3f2cc..711b9ac 100644 --- a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php +++ b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php @@ -74,6 +74,21 @@ public function testCompileStillThrowsOnVariancePositionViolation(): void } } + public function testInnerVarianceViolationIsCollectedByCheck(): void + { + // Composition case the position check misses → only inner-variance reports it, + // and the position check does NOT also flag it (no double report). + $diagnostics = $this->check('inner_variance'); + + self::assertCount(1, $diagnostics->all()); + $d = $diagnostics->all()[0]; + self::assertSame(InnerVarianceValidator::CODE_INNER_VARIANCE, $d->code); + // Located at the `Container` return type in P.xphp (line 12). + self::assertNotNull($d->location); + self::assertStringEndsWith('P.xphp', $d->location->file); + self::assertSame(12, $d->location->line); + } + public function testMissingTypeArgumentIsCollectedByCheck(): void { $diagnostics = $this->check('missing_arg'); diff --git a/test/Transpiler/Monomorphize/RegistryInnerVarianceTest.php b/test/Transpiler/Monomorphize/RegistryInnerVarianceTest.php index f4ef36b..ae56f61 100644 --- a/test/Transpiler/Monomorphize/RegistryInnerVarianceTest.php +++ b/test/Transpiler/Monomorphize/RegistryInnerVarianceTest.php @@ -18,6 +18,7 @@ use PhpParser\Node\VarLikeIdentifier; use PHPUnit\Framework\TestCase; use RuntimeException; +use XPHP\Diagnostics\DiagnosticCollector; /** * Tests `Registry::validateInnerVariance` -- the pass that composes outer @@ -31,6 +32,125 @@ */ final class RegistryInnerVarianceTest extends TestCase { + public function testPositionFlaggedDefinitionIsSkippedButLaterDefinitionsStillRun(): void + { + // P (direct +T-in-param) is flagged by the position check and recorded FIRST; + // Q (composition violation) is recorded AFTER. Inner-variance must skip P (already + // reported) yet still report Q — i.e. it must `continue` past P, not `break`. + $collector = new DiagnosticCollector(); + $registry = $this->registryWith([ + $this->makeDefinition( + 'App\\P', + 'P', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod('P', 'set', [new Param(new \PhpParser\Node\Expr\Variable('x'), type: new Name(['T']))], new Identifier('void')), + ), + $this->makeDefinition('App\\Container', 'Container', [new TypeParam('X')], new Class_(new Identifier('Container'))), + $this->makeDefinition( + 'App\\Q', + 'Q', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod('Q', 'f', [], $this->genericName('App\\Container', [new TypeRef('T', isTypeParam: true)])), + ), + ], $collector); + + $flagged = $registry->validateVariancePositions(); + $registry->validateInnerVariance($flagged); + + self::assertSame(['App\\P'], $flagged); + self::assertCount(2, $collector->all()); + $codes = array_map(static fn ($d): string => $d->code, $collector->all()); + self::assertContains(VariancePositionValidator::CODE_VARIANCE_POSITION, $codes); + self::assertContains(InnerVarianceValidator::CODE_INNER_VARIANCE, $codes); + } + + public function testAllPositionFlaggedDefinitionsAreSkippedByInnerVariance(): void + { + // Two direct +T-in-param violations: both flagged by the position check, so the + // inner-variance pass must skip BOTH (the full flagged list, not a truncation). + $collector = new DiagnosticCollector(); + $registry = $this->registryWith([ + $this->makeDefinition( + 'App\\P', + 'P', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod('P', 'set', [new Param(new \PhpParser\Node\Expr\Variable('x'), type: new Name(['T']))], new Identifier('void')), + ), + $this->makeDefinition( + 'App\\R', + 'R', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod('R', 'set', [new Param(new \PhpParser\Node\Expr\Variable('x'), type: new Name(['T']))], new Identifier('void')), + ), + ], $collector); + + $flagged = $registry->validateVariancePositions(); + $registry->validateInnerVariance($flagged); + + self::assertSame(['App\\P', 'App\\R'], $flagged); + self::assertCount(2, $collector->all()); + foreach ($collector->all() as $d) { + self::assertSame(VariancePositionValidator::CODE_VARIANCE_POSITION, $d->code); + } + } + + public function testNullFileProducesNullLocationWithoutError(): void + { + // Defensive: with no file, the diagnostic still emits (null location) and must not + // attempt to build a SourceLocation from a null file. + $registry = $this->registryWith([ + $this->makeDefinition('App\\Container', 'Container', [new TypeParam('X')], new Class_(new Identifier('Container'))), + $this->makeDefinition( + 'App\\P', + 'P', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod( + 'P', + 'f', + [], + $this->genericName('App\\Container', [new TypeRef('T', isTypeParam: true)]), + ), + ), + ]); + $container = $registry->definition('App\\Container'); + $p = $registry->definition('App\\P'); + self::assertNotNull($container); + self::assertNotNull($p); + + $collector = new DiagnosticCollector(); + InnerVarianceValidator::assertComposition($p, ['App\\Container' => $container, 'App\\P' => $p], $collector, null); + + self::assertCount(1, $collector->all()); + self::assertNull($collector->all()[0]->location); + } + + public function testCollectModeGathersInnerVarianceDiagnosticInsteadOfThrowing(): void + { + // Same composition violation as the throw-mode test, but with a collector: + // it must be reported as a Diagnostic (not thrown), so `xphp check` continues. + $collector = new DiagnosticCollector(); + $registry = $this->registryWith([ + $this->makeDefinition('App\\Container', 'Container', [new TypeParam('X')], new Class_(new Identifier('Container'))), + $this->makeDefinition( + 'App\\P', + 'P', + [new TypeParam('T', variance: Variance::Covariant)], + $this->classWithMethod( + 'P', + 'f', + [], + $this->genericName('App\\Container', [new TypeRef('T', isTypeParam: true)]), + ), + ), + ], $collector); + + $registry->validateInnerVariance(); + + self::assertCount(1, $collector->all()); + self::assertSame(InnerVarianceValidator::CODE_INNER_VARIANCE, $collector->all()[0]->code); + self::assertStringContainsString('Variance violation in template P', $collector->all()[0]->message); + } + public function testCovariantOuterInInvariantInnerSlotIsRejected(): void { // class Container {} // X is Invariant @@ -904,9 +1024,9 @@ public function testPropertyInvariantPositionRejectsCovariantOuter(): void /** * @param list $defs */ - private function registryWith(array $defs): Registry + private function registryWith(array $defs, ?DiagnosticCollector $collector = null): Registry { - $registry = new Registry(); + $registry = new Registry(diagnostics: $collector); foreach ($defs as $def) { $registry->recordDefinition( $def->fqn, diff --git a/test/fixture/check/inner_variance/source/Container.xphp b/test/fixture/check/inner_variance/source/Container.xphp new file mode 100644 index 0000000..886c9c3 --- /dev/null +++ b/test/fixture/check/inner_variance/source/Container.xphp @@ -0,0 +1,13 @@ + +{ + public function __construct(public T $value) + { + } +} diff --git a/test/fixture/check/inner_variance/source/P.xphp b/test/fixture/check/inner_variance/source/P.xphp new file mode 100644 index 0000000..74b4a61 --- /dev/null +++ b/test/fixture/check/inner_variance/source/P.xphp @@ -0,0 +1,16 @@ + +{ + public function f(): Container + { + throw new \LogicException(); + } +} From c4e68156c04c0ce6be9162a3469c273edc6d2d57 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:19:38 +0000 Subject: [PATCH 06/16] refactor(check): run variance-position before defaults; tidy docblocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run the variance-position check before the defaults-vs-bounds check so a class with both surfaces the variance error first in compile-mode — the order it surfaced when the check lived in the parser. Merge the stacked docblocks on the two variance delegate methods. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Compiler.php | 6 ++++-- src/Transpiler/Monomorphize/Registry.php | 6 ++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 59396af..051a876 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -81,12 +81,14 @@ public function compile( // bad declaration like `class Box` fails BEFORE any // padded instantiation is recorded), then collect instantiations -- including // bare `new Foo;` shapes for templates whose every param has a default. - $registry->validateDefaultsAgainstBounds(); // Variance-position rules (covariant T in input, contravariant T in output, // bound/default/property/constructor invariance, F-bounded variance). Moved // here from the parser so all definitions are present and `xphp check` can // collect across files; compile-mode still throws on the first violation. + // Runs BEFORE the defaults-vs-bounds check so that, when a class has both, + // the variance error surfaces first (the order it surfaced at parse time). $variancePositionFlagged = $registry->validateVariancePositions(); + $registry->validateDefaultsAgainstBounds(); // Inner-template variance composition: every template's variance // markers are known by now, so cases the parse-time validator // couldn't catch (e.g. `class P<+T> { f(): Container }` where @@ -231,8 +233,8 @@ public function check(FilepathArray $sources): DiagnosticCollector foreach ($astPerFile as $filepath => $ast) { $collector->collectDefinitions($ast, $filepath); } - $registry->validateDefaultsAgainstBounds(); $variancePositionFlagged = $registry->validateVariancePositions(); + $registry->validateDefaultsAgainstBounds(); $registry->validateInnerVariance($variancePositionFlagged); foreach ($astPerFile as $filepath => $ast) { $collector->collectInstantiations($ast, $filepath); diff --git a/src/Transpiler/Monomorphize/Registry.php b/src/Transpiler/Monomorphize/Registry.php index daa028d..6db6e53 100644 --- a/src/Transpiler/Monomorphize/Registry.php +++ b/src/Transpiler/Monomorphize/Registry.php @@ -379,8 +379,7 @@ private static function defaultBoundViolationMessage( * `xphp check` can collect all violations across files in one run). Delegates to * {@see VariancePositionValidator}: with this Registry's collector it gathers diagnostics * at each offending member; without one (compile) it throws the first violation. - */ - /** + * * @return list Template FQNs that had at least one position violation, so the * inner-variance pass can skip them (the issue is already reported) — mirroring * compile-mode, where the position check fails fast before inner-variance runs. @@ -407,8 +406,7 @@ public function validateVariancePositions(): array * Inner-template variance composition check over every collected definition (delegates to * {@see InnerVarianceValidator}). With this Registry's collector it gathers every violation * (each located at the offending member); without one (compile) it throws the first. - */ - /** + * * @param list $skipTemplateFqns Definitions already flagged by the position check; * skipped here so the same `+T`/`-T` misuse isn't reported by both passes. */ From 7e856870ce4c8863b522733beb81a690fb2563e3 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:23:02 +0000 Subject: [PATCH 07/16] feat(check): add Text, JSON, and GitHub diagnostic renderers Add a DiagnosticRenderer interface and three implementations for `xphp check` output: TextRenderer (human-readable blocks), JsonRenderer (a stable documented JSON contract), and GithubRenderer (Actions workflow-command annotations with proper escaping). Unit tests pin each format exactly, including the JSON shape and GitHub escaping (100% mutation score over the renderers). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Renderer/DiagnosticRenderer.php | 19 +++ src/Diagnostics/Renderer/GithubRenderer.php | 56 ++++++++ src/Diagnostics/Renderer/JsonRenderer.php | 50 +++++++ src/Diagnostics/Renderer/TextRenderer.php | 53 +++++++ test/Diagnostics/Renderer/RendererTest.php | 129 ++++++++++++++++++ 5 files changed, 307 insertions(+) create mode 100644 src/Diagnostics/Renderer/DiagnosticRenderer.php create mode 100644 src/Diagnostics/Renderer/GithubRenderer.php create mode 100644 src/Diagnostics/Renderer/JsonRenderer.php create mode 100644 src/Diagnostics/Renderer/TextRenderer.php create mode 100644 test/Diagnostics/Renderer/RendererTest.php diff --git a/src/Diagnostics/Renderer/DiagnosticRenderer.php b/src/Diagnostics/Renderer/DiagnosticRenderer.php new file mode 100644 index 0000000..7732f7d --- /dev/null +++ b/src/Diagnostics/Renderer/DiagnosticRenderer.php @@ -0,0 +1,19 @@ + $diagnostics + */ + public function render(array $diagnostics): string; +} diff --git a/src/Diagnostics/Renderer/GithubRenderer.php b/src/Diagnostics/Renderer/GithubRenderer.php new file mode 100644 index 0000000..76b9151 --- /dev/null +++ b/src/Diagnostics/Renderer/GithubRenderer.php @@ -0,0 +1,56 @@ + `::warning`, `Notice` -> `::notice`. Message and property values + * are escaped per GitHub's workflow-command rules (newlines/commas/colons). + */ +final class GithubRenderer implements DiagnosticRenderer +{ + public function render(array $diagnostics): string + { + $lines = []; + foreach ($diagnostics as $d) { + $command = match ($d->severity) { + Severity::Error => 'error', + Severity::Warning => 'warning', + Severity::Notice => 'notice', + }; + + $props = []; + if ($d->location !== null) { + $props[] = 'file=' . self::escapeProperty($d->location->file); + $props[] = 'line=' . $d->location->line; + if ($d->location->column !== null) { + $props[] = 'col=' . $d->location->column; + } + } + + $prefix = $props === [] ? '::' . $command : '::' . $command . ' ' . implode(',', $props); + $lines[] = $prefix . '::' . self::escapeData($d->message); + } + + return $lines === [] ? '' : implode(PHP_EOL, $lines) . PHP_EOL; + } + + private static function escapeData(string $value): string + { + return str_replace(['%', "\r", "\n"], ['%25', '%0D', '%0A'], $value); + } + + private static function escapeProperty(string $value): string + { + return str_replace(['%', "\r", "\n", ':', ','], ['%25', '%0D', '%0A', '%3A', '%2C'], $value); + } +} diff --git a/src/Diagnostics/Renderer/JsonRenderer.php b/src/Diagnostics/Renderer/JsonRenderer.php new file mode 100644 index 0000000..34fb44f --- /dev/null +++ b/src/Diagnostics/Renderer/JsonRenderer.php @@ -0,0 +1,50 @@ + $d->severity->value, + 'code' => $d->code, + 'message' => $d->message, + 'source' => $d->source->value, + 'triggeredBy' => $d->triggeredBy, + 'file' => $d->location?->file, + 'line' => $d->location?->line, + 'column' => $d->location?->column, + ]; + } + + return json_encode( + ['diagnostics' => $items], + JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR, + ) . PHP_EOL; + } +} diff --git a/src/Diagnostics/Renderer/TextRenderer.php b/src/Diagnostics/Renderer/TextRenderer.php new file mode 100644 index 0000000..ac81b8b --- /dev/null +++ b/src/Diagnostics/Renderer/TextRenderer.php @@ -0,0 +1,53 @@ + + * + * The message may be multi-line; the location/code sit on their own line so + * multi-line messages stay readable. + */ +final class TextRenderer implements DiagnosticRenderer +{ + public function render(array $diagnostics): string + { + if ($diagnostics === []) { + return 'No problems found.' . PHP_EOL; + } + + $blocks = []; + foreach ($diagnostics as $d) { + $lines = [sprintf('%s: %s', $d->severity->value, $d->message)]; + $lines[] = ' ' . $this->locationSuffix($d); + if ($d->triggeredBy !== null) { + $lines[] = ' triggered by ' . $d->triggeredBy; + } + $blocks[] = implode(PHP_EOL, $lines); + } + + return implode(PHP_EOL . PHP_EOL, $blocks) . PHP_EOL; + } + + private function locationSuffix(Diagnostic $d): string + { + $code = '[' . $d->code . ']'; + if ($d->location === null) { + return $code; + } + $at = $d->location->file . ':' . $d->location->line; + if ($d->location->column !== null) { + $at .= ':' . $d->location->column; + } + + return 'at ' . $at . ' ' . $code; + } +} diff --git a/test/Diagnostics/Renderer/RendererTest.php b/test/Diagnostics/Renderer/RendererTest.php new file mode 100644 index 0000000..30a359a --- /dev/null +++ b/test/Diagnostics/Renderer/RendererTest.php @@ -0,0 +1,129 @@ + */ + private function sample(): array + { + return [ + new Diagnostic( + Severity::Error, + 'xphp.bound_violation', + 'bad bound', + new SourceLocation('/src/Box.xphp', 7, 3), + ), + new Diagnostic( + Severity::Warning, + 'phpstan.return', + 'maybe', + null, + 'App\\Box', + DiagnosticSource::PhpStan, + ), + ]; + } + + public function testTextEmpty(): void + { + self::assertSame('No problems found.' . PHP_EOL, (new TextRenderer())->render([])); + } + + public function testTextRender(): void + { + $expected = implode(PHP_EOL, [ + 'error: bad bound', + ' at /src/Box.xphp:7:3 [xphp.bound_violation]', + '', + 'warning: maybe', + ' [phpstan.return]', + ' triggered by App\\Box', + ]) . PHP_EOL; + + self::assertSame($expected, (new TextRenderer())->render($this->sample())); + } + + public function testTextOmitsColumnWhenAbsent(): void + { + $d = [new Diagnostic(Severity::Error, 'c', 'm', new SourceLocation('/a.xphp', 4))]; + self::assertStringContainsString('at /a.xphp:4 [c]', (new TextRenderer())->render($d)); + } + + public function testJsonContract(): void + { + $out = (new JsonRenderer())->render($this->sample()); + // Pin the raw formatting: pretty-printed (space after key) and slashes unescaped. + self::assertStringContainsString('"file": "/src/Box.xphp"', $out); + /** @var array{diagnostics: list>} $decoded */ + $decoded = json_decode($out, true, flags: JSON_THROW_ON_ERROR); + + self::assertSame([ + 'diagnostics' => [ + [ + 'severity' => 'error', + 'code' => 'xphp.bound_violation', + 'message' => 'bad bound', + 'source' => 'xphp', + 'triggeredBy' => null, + 'file' => '/src/Box.xphp', + 'line' => 7, + 'column' => 3, + ], + [ + 'severity' => 'warning', + 'code' => 'phpstan.return', + 'message' => 'maybe', + 'source' => 'phpstan', + 'triggeredBy' => 'App\\Box', + 'file' => null, + 'line' => null, + 'column' => null, + ], + ], + ], $decoded); + } + + public function testJsonEmpty(): void + { + self::assertSame('{' . PHP_EOL . ' "diagnostics": []' . PHP_EOL . '}' . PHP_EOL, (new JsonRenderer())->render([])); + } + + public function testGithubRender(): void + { + $expected = implode(PHP_EOL, [ + '::error file=/src/Box.xphp,line=7,col=3::bad bound', + '::warning::maybe', + ]) . PHP_EOL; + + self::assertSame($expected, (new GithubRenderer())->render($this->sample())); + } + + public function testGithubEscapesMessageAndProperties(): void + { + $d = [new Diagnostic( + Severity::Notice, + 'c', + "line one\nline two", + new SourceLocation('/weird,name:x.xphp', 1), + )]; + + self::assertSame( + '::notice file=/weird%2Cname%3Ax.xphp,line=1::line one%0Aline two' . PHP_EOL, + (new GithubRenderer())->render($d), + ); + } + + public function testGithubEmpty(): void + { + self::assertSame('', (new GithubRenderer())->render([])); + } +} From b077a4cf787b340004d3d19105af44470bad03b6 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:25:20 +0000 Subject: [PATCH 08/16] test(check): pin GitHub renderer percent/carriage-return escaping Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Diagnostics/Renderer/RendererTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/Diagnostics/Renderer/RendererTest.php b/test/Diagnostics/Renderer/RendererTest.php index 30a359a..18cbb97 100644 --- a/test/Diagnostics/Renderer/RendererTest.php +++ b/test/Diagnostics/Renderer/RendererTest.php @@ -126,4 +126,12 @@ public function testGithubEmpty(): void { self::assertSame('', (new GithubRenderer())->render([])); } + + public function testGithubEscapesPercentAndCarriageReturnInMessage(): void + { + $d = [new Diagnostic(Severity::Error, 'c', "50%\rdone")]; + + // `%` must be escaped first (to %25), then `\r` to %0D — no double-escaping. + self::assertSame('::error::50%25%0Ddone' . PHP_EOL, (new GithubRenderer())->render($d)); + } } From 3034f64b0d89a46cedb6e67c977ae1d4daffcba4 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:35:27 +0000 Subject: [PATCH 09/16] feat(check): add the `xphp check` command with per-file parse resilience Wire a CheckCommand (`xphp check [--format=text|json|github]`) into the console alongside compile, sharing one Compiler. It runs the validate-only pass, renders diagnostics in the chosen format, and exits 0 (clean) / 1 (errors) / 2 (bad source dir or unknown format). Compiler::check() now parses each file in its own try/catch: a file that fails to parse (PHP syntax error or an xphp-specific parse rejection) is reported as a diagnostic and skipped, so the remaining files are still checked. Tests drive the command via CommandTester across all formats/exit codes, and a parse_error fixture proves a valid file's bound violation is still reported alongside two unparseable files. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Console/ApplicationConsole.php | 25 ++-- src/Console/Command/CheckCommand.php | 82 +++++++++++++ src/Transpiler/Monomorphize/Compiler.php | 43 ++++++- test/Console/CheckCommandTest.php | 112 ++++++++++++++++++ .../fixture/check/parse_error/source/Bag.xphp | 12 ++ .../check/parse_error/source/Broken.xphp | 11 ++ .../parse_error/source/ClosureVariance.xphp | 11 ++ .../fixture/check/parse_error/source/Use.xphp | 8 ++ 8 files changed, 289 insertions(+), 15 deletions(-) create mode 100644 src/Console/Command/CheckCommand.php create mode 100644 test/Console/CheckCommandTest.php create mode 100644 test/fixture/check/parse_error/source/Bag.xphp create mode 100644 test/fixture/check/parse_error/source/Broken.xphp create mode 100644 test/fixture/check/parse_error/source/ClosureVariance.xphp create mode 100644 test/fixture/check/parse_error/source/Use.xphp diff --git a/src/Console/ApplicationConsole.php b/src/Console/ApplicationConsole.php index bed4708..9daf74f 100644 --- a/src/Console/ApplicationConsole.php +++ b/src/Console/ApplicationConsole.php @@ -7,6 +7,7 @@ use PhpParser\ParserFactory; use PhpParser\PrettyPrinter\Standard as StandardPrinter; use Symfony\Component\Console\Application; +use XPHP\Console\Command\CheckCommand; use XPHP\Console\Command\CompileCommand; use XPHP\FileSystem\FileFinder; use XPHP\FileSystem\FileReader; @@ -35,17 +36,17 @@ public function __construct( $phpParser = (new ParserFactory())->createForHostVersion(); $printer = new StandardPrinter(); - $this->addCommand(new CompileCommand( - $fileFinder, - new Compiler( - $fileReader, - $fileWriter, - new XphpSourceParser($phpParser), - new Specializer(), - new SpecializedClassGenerator($printer, $fileWriter), - $printer, - $hashLength, - ), - )); + $compiler = new Compiler( + $fileReader, + $fileWriter, + new XphpSourceParser($phpParser), + new Specializer(), + new SpecializedClassGenerator($printer, $fileWriter), + $printer, + $hashLength, + ); + + $this->addCommand(new CompileCommand($fileFinder, $compiler)); + $this->addCommand(new CheckCommand($fileFinder, $compiler)); } } diff --git a/src/Console/Command/CheckCommand.php b/src/Console/Command/CheckCommand.php new file mode 100644 index 0000000..a88e8b7 --- /dev/null +++ b/src/Console/Command/CheckCommand.php @@ -0,0 +1,82 @@ + [--format=text|json|github]` + * + * Validates the generic code without emitting any output, reporting every + * diagnostic in one run. Exit codes: 0 = clean, 1 = at least one error-severity + * diagnostic, 2 = operational failure (bad source dir or unknown format). + */ +#[AsCommand('check', 'Validate generic .xphp code and report diagnostics without emitting output')] +final class CheckCommand extends Command +{ + public function __construct( + private readonly FileFinder $fileFinder, + private readonly Compiler $compiler, + ) { + parent::__construct(); + } + + protected function configure(): void + { + $this + ->addArgument('source', InputArgument::REQUIRED, 'Directory containing .xphp source files') + ->addOption('format', null, InputOption::VALUE_REQUIRED, 'Output format: text, json, or github', 'text'); + } + + protected function execute( + InputInterface $input, + OutputInterface $output, + ): int { + // @infection-ignore-all CastString -- a REQUIRED argument is always a string; + // the cast is defensive for getArgument()'s mixed return, so removing it is equivalent. + $sourceDir = (string) $input->getArgument('source'); + if (!is_dir($sourceDir)) { + $output->writeln("Source directory not found: {$sourceDir}"); + return self::INVALID; + } + + $renderer = $this->rendererFor((string) $input->getOption('format')); + if ($renderer === null) { + $output->writeln('Unknown format (expected: text, json, github)'); + return self::INVALID; + } + + $sources = $this->fileFinder + ->find($sourceDir) + ->filter(static fn (string $filepath): bool => str_ends_with($filepath, '.xphp')); + + $diagnostics = $this->compiler->check($sources); + + $output->write($renderer->render($diagnostics->all())); + + return $diagnostics->hasErrors() ? self::FAILURE : self::SUCCESS; + } + + private function rendererFor(string $format): ?DiagnosticRenderer + { + return match ($format) { + 'text' => new TextRenderer(), + 'json' => new JsonRenderer(), + 'github' => new GithubRenderer(), + default => null, + }; + } +} diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 051a876..8681290 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -4,9 +4,13 @@ namespace XPHP\Transpiler\Monomorphize; +use PhpParser\Error as PhpParserError; use PhpParser\PrettyPrinter\Standard as StandardPrinter; use RuntimeException; +use XPHP\Diagnostics\Diagnostic; use XPHP\Diagnostics\DiagnosticCollector; +use XPHP\Diagnostics\Severity; +use XPHP\Diagnostics\SourceLocation; use XPHP\FileSystem\FileReader; use XPHP\FileSystem\FileWriter; use XPHP\FileSystem\FilepathArray; @@ -32,6 +36,9 @@ { public const MAX_SPECIALIZATION_DEPTH = 16; + /** Diagnostic code for a file that failed to parse during `xphp check`. */ + public const CODE_PARSE_ERROR = 'xphp.parse_error'; + public function __construct( private FileReader $fileReader, private FileWriter $fileWriter, @@ -220,13 +227,43 @@ public function compile( * * Scope note: method/function/closure-level generic checks (GenericMethodCompiler, Phase 1a * of compile()) are intentionally NOT run here — they remain fail-fast and are not yet part - * of the check gate. Variance checks are wired in a later step. + * of the check gate. + * + * Per-file resilience: a file that fails to parse is reported as a diagnostic and skipped, + * so the remaining files are still checked (unlike compile(), which fails fast). */ public function check(FilepathArray $sources): DiagnosticCollector { - $astPerFile = $this->parseAll($sources); - $hierarchy = TypeHierarchy::fromAstPerFile($astPerFile); $diagnostics = new DiagnosticCollector(); + $astPerFile = []; + foreach ($sources->filepaths as $filepath) { + try { + $astPerFile[$filepath] = $this->sourceParser->parse($this->fileReader->read($filepath)); + } catch (PhpParserError $e) { + $line = $e->getStartLine(); + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_PARSE_ERROR, + $e->getMessage(), + // @infection-ignore-all GreaterThan/IncrementInteger/DecrementInteger -- a real + // PHP syntax error always reports a line >= 1, so the `> 0` boundary (and its + // `?: 1` fallback) is defensive and unobservable; the happy-path line is pinned + // by CheckCommandTest (Broken.xphp -> line 11). + new SourceLocation($filepath, $line > 0 ? $line : 1), + )); + } catch (RuntimeException $e) { + // xphp-specific parse-time rejections (e.g. variance markers on methods) — these + // carry no line, so the diagnostic points at the file (line 1). + $diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_PARSE_ERROR, + $e->getMessage(), + new SourceLocation($filepath, 1), + )); + } + } + + $hierarchy = TypeHierarchy::fromAstPerFile($astPerFile); $registry = new Registry($this->hashLength, $hierarchy, $diagnostics); $collector = new RegistryCollector($registry); diff --git a/test/Console/CheckCommandTest.php b/test/Console/CheckCommandTest.php new file mode 100644 index 0000000..b047e5a --- /dev/null +++ b/test/Console/CheckCommandTest.php @@ -0,0 +1,112 @@ +tester(); + $exit = $tester->execute(['source' => $this->fixtureDir('clean')]); + + self::assertSame(0, $exit); + self::assertStringContainsString('No problems found', $tester->getDisplay()); + } + + public function testGenericErrorsExitOne(): void + { + $tester = $this->tester(); + $exit = $tester->execute(['source' => $this->fixtureDir('multi_error')]); + + self::assertSame(1, $exit); + self::assertStringContainsString('bound violated', $tester->getDisplay()); + } + + public function testParseErrorIsReportedButOtherFilesStillChecked(): void + { + $tester = $this->tester(); + $exit = $tester->execute(['source' => $this->fixtureDir('parse_error'), '--format' => 'json']); + + self::assertSame(1, $exit); + /** @var array{diagnostics: list} $decoded */ + $decoded = json_decode($tester->getDisplay(), true, flags: JSON_THROW_ON_ERROR); + + $byFile = []; + foreach ($decoded['diagnostics'] as $d) { + $byFile[basename($d['file'])] = $d; + } + + // A PHP syntax error: reported with its real line. + self::assertSame(Compiler::CODE_PARSE_ERROR, $byFile['Broken.xphp']['code']); + self::assertSame(11, $byFile['Broken.xphp']['line']); + // An xphp-specific parse rejection (no line): reported at line 1. + self::assertSame(Compiler::CODE_PARSE_ERROR, $byFile['ClosureVariance.xphp']['code']); + self::assertSame(1, $byFile['ClosureVariance.xphp']['line']); + // A VALID file is still checked despite the two unparseable files. + self::assertSame('xphp.bound_violation', $byFile['Use.xphp']['code']); + } + + public function testMissingSourceDirectoryExitsTwoWithMessage(): void + { + $tester = $this->tester(); + $exit = $tester->execute(['source' => sys_get_temp_dir() . '/xphp-does-not-exist-' . uniqid('', true)]); + + self::assertSame(2, $exit); + self::assertStringContainsString('Source directory not found', $tester->getDisplay()); + } + + public function testUnknownFormatExitsTwoWithMessage(): void + { + $tester = $this->tester(); + $exit = $tester->execute(['source' => $this->fixtureDir('clean'), '--format' => 'xml']); + + self::assertSame(2, $exit); + self::assertStringContainsString('Unknown format', $tester->getDisplay()); + } + + public function testGithubFormatEmitsAnnotations(): void + { + $tester = $this->tester(); + $tester->execute(['source' => $this->fixtureDir('multi_error'), '--format' => 'github']); + + self::assertStringContainsString('::error file=', $tester->getDisplay()); + } + + private function tester(): CommandTester + { + $phpParser = (new ParserFactory())->createForHostVersion(); + $printer = new StandardPrinter(); + $writer = new NativeFileWriter(); + $compiler = new Compiler( + new NativeFileReader(), + $writer, + new XphpSourceParser($phpParser), + new Specializer(), + new SpecializedClassGenerator($printer, $writer), + $printer, + ); + + return new CommandTester(new CheckCommand(new NativeFileFinder(), $compiler)); + } + + private function fixtureDir(string $fixture): string + { + return realpath(__DIR__ . '/../fixture/check/' . $fixture . '/source') + ?: throw new RuntimeException("Fixture missing: {$fixture}"); + } +} diff --git a/test/fixture/check/parse_error/source/Bag.xphp b/test/fixture/check/parse_error/source/Bag.xphp new file mode 100644 index 0000000..c50ddb6 --- /dev/null +++ b/test/fixture/check/parse_error/source/Bag.xphp @@ -0,0 +1,12 @@ + +{ + public function __construct(public T $value) + { + } +} diff --git a/test/fixture/check/parse_error/source/Broken.xphp b/test/fixture/check/parse_error/source/Broken.xphp new file mode 100644 index 0000000..43ddf26 --- /dev/null +++ b/test/fixture/check/parse_error/source/Broken.xphp @@ -0,0 +1,11 @@ +(T $x): T { + return $x; +}; diff --git a/test/fixture/check/parse_error/source/Use.xphp b/test/fixture/check/parse_error/source/Use.xphp new file mode 100644 index 0000000..1534b1b --- /dev/null +++ b/test/fixture/check/parse_error/source/Use.xphp @@ -0,0 +1,8 @@ +(1); From 21f855b2600ad75c0b43e1a815ac45bdf8094709 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:40:18 +0000 Subject: [PATCH 10/16] refactor(check): read source files outside the parse try/catch Move the file read out of check()'s per-file try so an I/O failure surfaces as itself rather than being mislabeled xphp.parse_error; only parsing is treated as a recoverable per-file diagnostic. Clarify the parse-error line comment re nikic's -1 sentinel. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Transpiler/Monomorphize/Compiler.php | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 8681290..153a8f0 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -237,23 +237,27 @@ public function check(FilepathArray $sources): DiagnosticCollector $diagnostics = new DiagnosticCollector(); $astPerFile = []; foreach ($sources->filepaths as $filepath) { + // Read OUTSIDE the try so an I/O failure surfaces as itself, not a mislabeled + // "parse error" — only parsing is treated as a per-file, recoverable diagnostic. + $content = $this->fileReader->read($filepath); try { - $astPerFile[$filepath] = $this->sourceParser->parse($this->fileReader->read($filepath)); + $astPerFile[$filepath] = $this->sourceParser->parse($content); } catch (PhpParserError $e) { $line = $e->getStartLine(); $diagnostics->add(new Diagnostic( Severity::Error, self::CODE_PARSE_ERROR, $e->getMessage(), - // @infection-ignore-all GreaterThan/IncrementInteger/DecrementInteger -- a real - // PHP syntax error always reports a line >= 1, so the `> 0` boundary (and its - // `?: 1` fallback) is defensive and unobservable; the happy-path line is pinned - // by CheckCommandTest (Broken.xphp -> line 11). + // @infection-ignore-all GreaterThan/IncrementInteger/DecrementInteger -- nikic + // emits either a real line (>= 1) or the sentinel -1 for position-less errors; + // every `> 0` boundary variant routes -1 to the same `?: 1` fallback, so the + // mutants are equivalent. The real-line path is pinned by CheckCommandTest + // (Broken.xphp -> line 11). new SourceLocation($filepath, $line > 0 ? $line : 1), )); } catch (RuntimeException $e) { - // xphp-specific parse-time rejections (e.g. variance markers on methods) — these - // carry no line, so the diagnostic points at the file (line 1). + // xphp-specific parse-time rejections from the parser (e.g. variance markers on + // methods) — these carry no line, so the diagnostic points at the file (line 1). $diagnostics->add(new Diagnostic( Severity::Error, self::CODE_PARSE_ERROR, From d552742723160facb3c857b4c4391bfc9e901953 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Wed, 17 Jun 2026 22:43:02 +0000 Subject: [PATCH 11/16] docs: document the `xphp check` diagnostics gate Add an `xphp check` section to the errors reference (formats, exit codes, per-file parse resilience, and the stable diagnostic codes for the json/github formats) and a short pointer from the README quick start. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 9 +++++++++ docs/errors.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/README.md b/README.md index 1d4ee90..2f0f90c 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,15 @@ compile. `dist/` holds your rewritten code; `.xphp-cache/Generated/` holds the specialized classes. Both can be gitignored and rebuilt in CI. +To validate generics without emitting anything — a CI gate that reports +every bound/variance/etc. problem with a `file:line`: + +```bash +vendor/bin/xphp check src # exit 1 if any error; --format=text|json|github +``` + +See [Errors and diagnostics](docs/errors.md#xphp-check--validate-without-emitting). + ## See also - [Getting started](docs/getting-started.md) -- full walkthrough including PSR-4 details, runtime semantics, and what the generated PHP looks like diff --git a/docs/errors.md b/docs/errors.md index 1e14d3c..8bf0982 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -4,6 +4,55 @@ Every compile-time rejection from xphp lists the error message verbatim below, paired with the docs section that explains the constraint. Search this page for the text your compile output shows. +## `xphp check` — validate without emitting + +`vendor/bin/xphp check ` validates your generic `.xphp` +code and reports **every** problem below as a structured diagnostic — +each with a `file:line` location — instead of aborting on the first. +It writes no output (no `dist/`, no cache); it's a pure gate, ideal +for CI. + +```bash +vendor/bin/xphp check src +``` + +Output formats via `--format`: + +| Format | Use | +|--------|-----| +| `text` (default) | human-readable, one block per problem | +| `json` | machine-readable; stable `{ "diagnostics": [ … ] }` shape for tooling | +| `github` | GitHub Actions annotations (`::error file=…,line=…::…`) so problems show inline on a PR | + +Exit codes: **0** (clean), **1** (at least one error), **2** (bad +source directory or unknown `--format`). A file that fails to parse is +reported and skipped, so the rest of the tree is still checked in the +same run. + +### Diagnostic codes + +The `json` and `github` formats tag each diagnostic with a stable code: + +| Code | Meaning | +|------|---------| +| `xphp.bound_violation` | a concrete type argument doesn't satisfy its parameter's bound | +| `xphp.default_bound_violation` | a parameter's default doesn't satisfy its own bound | +| `xphp.missing_type_argument` | a required type argument was omitted and has no default | +| `xphp.variance_position` | a `+T`/`-T` parameter appears in a position its variance forbids | +| `xphp.inner_variance` | variance is violated through another generic's slot (composition) | +| `xphp.undefined_template` | a generic was instantiated but never declared | +| `xphp.parse_error` | the file isn't valid PHP after the generic strip pass | + +> Scope: `xphp check` covers the class/interface/trait-level generic +> checks on this page. Method-, function-, and closure-level generic +> errors remain `xphp compile` failures for now. + +In CI (GitHub Actions), one step gates the build and annotates the diff: + +```yaml +- run: vendor/bin/xphp check src --format=github +``` + ## Quick index | If the message contains... | Read | From c34b0a1b17e3b3d6a0514f63a4e7d4c7fa4368ec Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Thu, 18 Jun 2026 06:11:56 +0000 Subject: [PATCH 12/16] docs: clarify that `xphp check` is not yet a substitute for `xphp compile` Spell out the scope consequence: a clean `check` does not guarantee a clean `compile`, because method/function/closure-level generic checks (and the specialization-loop guards, by design) are not run by `check` yet. Advise keeping `compile` in the build pipeline. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 5 ++++- docs/errors.md | 11 ++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2f0f90c..7f5c728 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,10 @@ every bound/variance/etc. problem with a `file:line`: vendor/bin/xphp check src # exit 1 if any error; --format=text|json|github ``` -See [Errors and diagnostics](docs/errors.md#xphp-check--validate-without-emitting). +`check` is a fast pre-flight gate, not yet a full substitute for `compile` +(method/function/closure-level generic errors surface only at compile) — keep +`compile` in your build. See +[Errors and diagnostics](docs/errors.md#xphp-check--validate-without-emitting). ## See also diff --git a/docs/errors.md b/docs/errors.md index 8bf0982..9ad6558 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -43,9 +43,14 @@ The `json` and `github` formats tag each diagnostic with a stable code: | `xphp.undefined_template` | a generic was instantiated but never declared | | `xphp.parse_error` | the file isn't valid PHP after the generic strip pass | -> Scope: `xphp check` covers the class/interface/trait-level generic -> checks on this page. Method-, function-, and closure-level generic -> errors remain `xphp compile` failures for now. +> **Scope — `xphp check` is not yet a substitute for `xphp compile`.** +> `check` covers the class/interface/trait-level generic checks on this +> page. It does **not** yet run the method-, function-, and +> closure-level generic checks (those still surface only as `xphp +> compile` failures), and by design it never runs the specialization +> loop, so the depth-cap and hash-collision guards are out of scope too. +> A clean `check` therefore does **not** guarantee a clean `compile` — +> keep `xphp compile` in your build pipeline. In CI (GitHub Actions), one step gates the build and annotates the diff: From 078f9d98171b8edc91806171d98cc4aa58a4e3de Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Thu, 18 Jun 2026 09:30:13 +0000 Subject: [PATCH 13/16] feat(check): collect method/function/closure-level generic errors Run GenericMethodCompiler in a new validate-only mode from Compiler::check(): process(emit: false) walks the call sites for their bound/missing-arg checks and the duplicate-function / $this-capture / static-closure rejections, threading the optional DiagnosticCollector + source locations through the (already collector-aware) Registry::checkBounds/padArgsWithDefaults and the in-process throws, while suppressing the specialize/strip/finalize side-effects. xphp compile is unchanged (default emit: true, no collector -> byte-identical fail-fast). This makes `xphp check` a validation-superset of `xphp compile`: a class-level and a method-level generic error are now both reported in one run. New diagnostic codes xphp.duplicate_generic_function / xphp.closure_this_capture / xphp.static_closure; bound + missing-arg reuse the existing codes. Fixtures + CheckPassIntegrationTest cover each new collected diagnostic (with file:line), the both-passes-in-one-run guarantee, and byte-identical-compile guards. Docs updated: check now covers all generic validation; only the specialization-loop guards (depth cap, hash collision) remain compile-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 5 +- docs/errors.md | 16 +-- src/Transpiler/Monomorphize/Compiler.php | 16 ++- .../Monomorphize/GenericMethodCompiler.php | 109 +++++++++++++++--- .../Monomorphize/CheckPassIntegrationTest.php | 98 ++++++++++++++++ .../check/closure_this_capture/source/C.xphp | 19 +++ .../duplicate_generic_function/source/a.xphp | 15 +++ .../duplicate_generic_function/source/b.xphp | 15 +++ .../generic_function_bound/source/Use.xphp | 8 ++ .../generic_function_bound/source/funcs.xphp | 10 ++ .../source/Use.xphp | 9 ++ .../source/Util.xphp | 12 ++ .../source/Producer.xphp | 13 +++ .../method_and_class_errors/source/Use.xphp | 8 ++ .../method_and_class_errors/source/funcs.xphp | 10 ++ 15 files changed, 335 insertions(+), 28 deletions(-) create mode 100644 test/fixture/check/closure_this_capture/source/C.xphp create mode 100644 test/fixture/check/duplicate_generic_function/source/a.xphp create mode 100644 test/fixture/check/duplicate_generic_function/source/b.xphp create mode 100644 test/fixture/check/generic_function_bound/source/Use.xphp create mode 100644 test/fixture/check/generic_function_bound/source/funcs.xphp create mode 100644 test/fixture/check/generic_method_missing_arg/source/Use.xphp create mode 100644 test/fixture/check/generic_method_missing_arg/source/Util.xphp create mode 100644 test/fixture/check/method_and_class_errors/source/Producer.xphp create mode 100644 test/fixture/check/method_and_class_errors/source/Use.xphp create mode 100644 test/fixture/check/method_and_class_errors/source/funcs.xphp diff --git a/README.md b/README.md index 7f5c728..a01c61f 100644 --- a/README.md +++ b/README.md @@ -150,9 +150,8 @@ every bound/variance/etc. problem with a `file:line`: vendor/bin/xphp check src # exit 1 if any error; --format=text|json|github ``` -`check` is a fast pre-flight gate, not yet a full substitute for `compile` -(method/function/closure-level generic errors surface only at compile) — keep -`compile` in your build. See +`check` runs all of xphp's generic validation (the specialization-loop guards +aside); you still run `compile` to emit the PHP. See [Errors and diagnostics](docs/errors.md#xphp-check--validate-without-emitting). ## See also diff --git a/docs/errors.md b/docs/errors.md index 9ad6558..790a8a5 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -43,14 +43,14 @@ The `json` and `github` formats tag each diagnostic with a stable code: | `xphp.undefined_template` | a generic was instantiated but never declared | | `xphp.parse_error` | the file isn't valid PHP after the generic strip pass | -> **Scope — `xphp check` is not yet a substitute for `xphp compile`.** -> `check` covers the class/interface/trait-level generic checks on this -> page. It does **not** yet run the method-, function-, and -> closure-level generic checks (those still surface only as `xphp -> compile` failures), and by design it never runs the specialization -> loop, so the depth-cap and hash-collision guards are out of scope too. -> A clean `check` therefore does **not** guarantee a clean `compile` — -> keep `xphp compile` in your build pipeline. +> **Scope.** `xphp check` runs every generic *validation* check `xphp compile` +> does — class/interface/trait-level **and** method/function/closure-level +> (bounds, variance, defaults, missing/duplicate generics, unsupported closures). +> By design it does not run the specialization loop or emit code, so the two +> runaway/config guards — the nested-specialization **depth cap** and the +> **hash-collision** check — surface only at `xphp compile` (they aren't type +> errors). You still run `xphp compile` to produce the PHP; `check` is the fast +> validation gate in front of it. In CI (GitHub Actions), one step gates the build and annotates the diff: diff --git a/src/Transpiler/Monomorphize/Compiler.php b/src/Transpiler/Monomorphize/Compiler.php index 153a8f0..d3d1005 100644 --- a/src/Transpiler/Monomorphize/Compiler.php +++ b/src/Transpiler/Monomorphize/Compiler.php @@ -225,9 +225,9 @@ public function compile( * the first. Stops after validation — it never specializes or emits, so a partially-invalid * registry never reaches the fixed-point loop. Returns the collected diagnostics. * - * Scope note: method/function/closure-level generic checks (GenericMethodCompiler, Phase 1a - * of compile()) are intentionally NOT run here — they remain fail-fast and are not yet part - * of the check gate. + * Includes method/function/closure-level generic checks: GenericMethodCompiler runs in + * validate-only mode (`emit: false`) so it collects bound / missing-arg / duplicate-function / + * closure-rejection diagnostics without specializing or emitting. * * Per-file resilience: a file that fails to parse is reported as a diagnostic and skipped, * so the remaining files are still checked (unlike compile(), which fails fast). @@ -282,6 +282,16 @@ public function check(FilepathArray $sources): DiagnosticCollector } $registry->collectUndefinedTemplates($diagnostics); + // Method/function/closure-level generic checks: run GenericMethodCompiler in validate-only + // mode (emit: false) so it collects bound / missing-arg / duplicate-function / closure-rejection + // diagnostics without specializing or mutating the (discarded) AST. + // @infection-ignore-all FalseValue -- `emit: true` is observably equivalent here: the + // validation calls (which produce the diagnostics) run in BOTH modes; `emit` only governs + // append/strip/finalize side-effects on `$astPerFile`, which is local and discarded. So + // flipping it changes only wasted work, not the collected diagnostics. `emit: false` is the + // correct (no-wasted-work, no-mutation) choice. + (new GenericMethodCompiler($this->hashLength, $hierarchy, $diagnostics))->process($astPerFile, emit: false); + return $diagnostics; } diff --git a/src/Transpiler/Monomorphize/GenericMethodCompiler.php b/src/Transpiler/Monomorphize/GenericMethodCompiler.php index 124411e..3e3d459 100644 --- a/src/Transpiler/Monomorphize/GenericMethodCompiler.php +++ b/src/Transpiler/Monomorphize/GenericMethodCompiler.php @@ -45,6 +45,10 @@ use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; use RuntimeException; +use XPHP\Diagnostics\Diagnostic; +use XPHP\Diagnostics\DiagnosticCollector; +use XPHP\Diagnostics\Severity; +use XPHP\Diagnostics\SourceLocation; /** * Specializes method-scoped generics: `function NAME(...)` inside a class body, called via @@ -72,9 +76,21 @@ */ final class GenericMethodCompiler { + /** Stable diagnostic codes for method/function/closure-level generic errors collected by `check`. */ + public const CODE_DUPLICATE_GENERIC_FUNCTION = 'xphp.duplicate_generic_function'; + public const CODE_UNSUPPORTED_THIS_CAPTURE = 'xphp.closure_this_capture'; + public const CODE_UNSUPPORTED_STATIC_CLOSURE = 'xphp.static_closure'; + + /** + * @param ?DiagnosticCollector $diagnostics When null (the default — `xphp compile`), every + * method/function/closure-level generic error throws as before, byte-identical. When provided + * (by `xphp check` with `process(..., emit: false)`), each is appended as a Diagnostic and the + * pass continues, so all are reported in one run. + */ public function __construct( private readonly int $hashLength = Registry::DEFAULT_HASH_HEX_LENGTH, private readonly ?TypeHierarchy $hierarchy = null, + private readonly ?DiagnosticCollector $diagnostics = null, ) { } @@ -85,8 +101,12 @@ public function __construct( * * @param array> $astSet keyed by an arbitrary string id (filepath * or ""). The values are the top-level statements of each AST. + * @param bool $emit When true (default, compile) the pass specializes, appends, and strips + * templates as before. When false (`xphp check`) it walks for VALIDATION only — no append-flush, + * no strip, no closure-dispatcher finalize — so the (discarded) AST is left untouched and only + * diagnostics are produced. */ - public function process(array &$astSet): void + public function process(array &$astSet, bool $emit = true): void { /** @var array $methodTemplates keyed by "classFqn::methodName" */ $methodTemplates = []; @@ -112,14 +132,24 @@ public function process(array &$astSet): void // files) with both paths, matching the shape `Registry::recordDefinition` // uses for generic classes. Silently overwriting the first body — which is // the prior behavior here — costs a real refactoring footgun. - foreach ($perFileFns as $fqn => $_template) { + foreach ($perFileFns as $fqn => $duplicate) { if (isset($functionTemplates[$fqn])) { - throw new RuntimeException(sprintf( + $message = sprintf( 'Generic function template "%s" already declared (in %s); duplicate declaration in %s.', $fqn, $functionSourceByFqn[$fqn], (string) $astKey, - )); + ); + if ($this->diagnostics !== null) { + $this->diagnostics->add(new Diagnostic( + Severity::Error, + self::CODE_DUPLICATE_GENERIC_FUNCTION, + $message, + new SourceLocation((string) $astKey, $duplicate->getStartLine()), + )); + continue; + } + throw new RuntimeException($message); } } @@ -166,13 +196,22 @@ public function process(array &$astSet): void $functionNamespaceByFqn, $alreadyGenerated, $topLevelAppends, + (string) $astKey, + $emit, ); - foreach ($topLevelAppends as $specialized) { - $ast[] = $specialized; + if ($emit) { + foreach ($topLevelAppends as $specialized) { + $ast[] = $specialized; + } } } unset($ast); + // Validate-only (check) stops here: no template stripping, so the AST is left intact. + if (!$emit) { + return; + } + // Strip the original method templates from their owning classes. foreach ($methodTemplates as $key => $template) { // @infection-ignore-all — explode limit 2 vs 3: the key never contains @@ -314,13 +353,16 @@ private function rewriteCallSites( array $functionNamespaceByFqn, array &$alreadyGenerated, array &$topLevelAppends, + string $currentFile, + bool $emit, ): void { $hashLength = $this->hashLength; $hierarchy = $this->hierarchy; + $diagnostics = $this->diagnostics; // @infection-ignore-all — see rationale above the indexTemplates visitor: defensive // guards and call-shape mutations are masked by the surrounding pipeline's // type-strict invariants. End-to-end coverage from GenericMethodIntegrationTest. - $visitor = new class($methodTemplates, $classByFqn, $functionTemplates, $functionNamespaceByFqn, $alreadyGenerated, $hashLength, $hierarchy, $topLevelAppends) extends NodeVisitorAbstract { + $visitor = new class($methodTemplates, $classByFqn, $functionTemplates, $functionNamespaceByFqn, $alreadyGenerated, $hashLength, $hierarchy, $topLevelAppends, $diagnostics, $currentFile) extends NodeVisitorAbstract { private string $currentNamespace = ''; private ?Namespace_ $currentNamespaceNode = null; /** @var array alias => fqn */ @@ -460,6 +502,8 @@ public function __construct( private int $hashLength, private ?TypeHierarchy $hierarchy, public array &$topLevelAppends, + private readonly ?DiagnosticCollector $diagnostics, + private readonly string $currentFile, ) { } @@ -895,7 +939,8 @@ private function rewriteStaticCall(StaticCall $node): ?Node $args = []; } /** @var list $args — set as a list by XphpSourceParser::resolveAndAttach (or empty after the all-defaults branch above). */ - $padded = Registry::padArgsWithDefaults($params, $args, $key); + $location = new SourceLocation($this->currentFile, $node->getStartLine()); + $padded = Registry::padArgsWithDefaults($params, $args, $key, $this->diagnostics, $location); if (!self::allConcrete($padded) || count($params) !== count($padded)) { return null; } @@ -907,6 +952,8 @@ private function rewriteStaticCall(StaticCall $node): ?Node $args, $this->hierarchy, $classFqn . '::' . $methodName . '<' . self::formatArgList($args) . '>', + $this->diagnostics, + $location, ); } @@ -982,7 +1029,8 @@ private function rewriteInstanceMethodCall(MethodCall|NullsafeMethodCall $node): $args = []; } /** @var list $args — set as a list by XphpSourceParser::resolveAndAttach (or empty after the all-defaults branch above). */ - $padded = Registry::padArgsWithDefaults($params, $args, $key); + $location = new SourceLocation($this->currentFile, $node->getStartLine()); + $padded = Registry::padArgsWithDefaults($params, $args, $key, $this->diagnostics, $location); if (!self::allConcrete($padded) || count($params) !== count($padded)) { return null; } @@ -994,6 +1042,8 @@ private function rewriteInstanceMethodCall(MethodCall|NullsafeMethodCall $node): $args, $this->hierarchy, $classFqn . '::' . $methodName . '<' . self::formatArgList($args) . '>', + $this->diagnostics, + $location, ); } @@ -1121,6 +1171,8 @@ private function rewriteFuncCall(FuncCall $node): ?Node $args, $this->hierarchy, $fqn . '<' . self::formatArgList($args) . '>', + $this->diagnostics, + new SourceLocation($this->currentFile, $node->getStartLine()), ); } @@ -1204,7 +1256,7 @@ private function rewriteVariableTurbofishCall(FuncCall $node, array $args): null // A future commit can rewrite `$this->v` to a lifted // param. $flavor = $template instanceof ArrowFunction ? 'arrow' : 'closure'; - throw new RuntimeException(sprintf( + $message = sprintf( 'Generic %s `$%s::<...>(...)` captures `$this`, ' . 'which is not yet supported. Rewrite as a method ' . 'on the enclosing class, or extract the value of ' @@ -1213,15 +1265,35 @@ private function rewriteVariableTurbofishCall(FuncCall $node, array $args): null $flavor, $varName, $flavor, - )); + ); + if ($this->diagnostics !== null) { + $this->diagnostics->add(new Diagnostic( + Severity::Error, + GenericMethodCompiler::CODE_UNSUPPORTED_THIS_CAPTURE, + $message, + new SourceLocation($this->currentFile, $node->getStartLine()), + )); + return null; + } + throw new RuntimeException($message); } if ($template instanceof Closure && $template->static) { - throw new RuntimeException(sprintf( + $message = sprintf( 'Generic static closures cannot yet be specialized at ' . 'call sites. Rewrite the call site for `$%s::<...>(...)` ' . 'to use a named generic function at file scope.', $varName, - )); + ); + if ($this->diagnostics !== null) { + $this->diagnostics->add(new Diagnostic( + Severity::Error, + GenericMethodCompiler::CODE_UNSUPPORTED_STATIC_CLOSURE, + $message, + new SourceLocation($this->currentFile, $node->getStartLine()), + )); + return null; + } + throw new RuntimeException($message); } $params = $template->getAttribute(XphpSourceParser::ATTR_METHOD_GENERIC_PARAMS); @@ -1234,7 +1306,8 @@ private function rewriteVariableTurbofishCall(FuncCall $node, array $args): null // generic closure / arrow. Padding throws when leading // required params are missing -- the throw surfaces with // a clear `Registry::padArgsWithDefaults` message. - $args = Registry::padArgsWithDefaults($params, $args, 'closure<' . $varName . '>'); + $location = new SourceLocation($this->currentFile, $node->getStartLine()); + $args = Registry::padArgsWithDefaults($params, $args, 'closure<' . $varName . '>', $this->diagnostics, $location); if (count($params) !== count($args)) { return null; } @@ -1244,6 +1317,8 @@ private function rewriteVariableTurbofishCall(FuncCall $node, array $args): null $args, $this->hierarchy, 'closure<' . self::formatArgList($args) . '>', + $this->diagnostics, + $location, ); } $context = $this->currentScopeClosureContexts[$varName] ?? null; @@ -1391,6 +1466,12 @@ private static function lastSegment(string $name): string $traverser->addVisitor($visitor); $traverser->traverse($ast); + // Validate-only (check) skips all emission: no dispatcher materialization, no buffered + // appends. The traversal above already produced the diagnostics via the call-site checks. + if (!$emit) { + return; + } + // Pass 2 of the closure-dispatcher pipeline: materialize a dispatcher // closure per recorded template, replace the original Assign's RHS, // append specialized declarations, and rewrite each collected call diff --git a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php index 711b9ac..2c1c9e9 100644 --- a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php +++ b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php @@ -107,6 +107,93 @@ public function testUndefinedTemplateIsCollectedByCheck(): void self::assertSame(Registry::CODE_UNDEFINED_TEMPLATE, $diagnostics->all()[0]->code); } + public function testGenericFunctionBoundViolationIsCollectedByCheck(): void + { + $diagnostics = $this->check('generic_function_bound'); + + self::assertCount(1, $diagnostics->all()); + $d = $diagnostics->all()[0]; + self::assertSame(Registry::CODE_BOUND_VIOLATION, $d->code); + self::assertNotNull($d->location); + self::assertStringEndsWith('Use.xphp', $d->location->file); + self::assertSame(8, $d->location->line); + } + + public function testGenericMethodMissingArgumentIsCollectedByCheck(): void + { + $diagnostics = $this->check('generic_method_missing_arg'); + + self::assertCount(1, $diagnostics->all()); + $d = $diagnostics->all()[0]; + self::assertSame(Registry::CODE_MISSING_TYPE_ARGUMENT, $d->code); + self::assertNotNull($d->location); + self::assertSame(9, $d->location->line); + } + + public function testDuplicateGenericFunctionIsCollectedByCheck(): void + { + // Two functions re-declared in the second file → BOTH duplicates collected in one run + // (the loop must `continue`, not `break`, after the first). + $diagnostics = $this->check('duplicate_generic_function'); + + self::assertCount(2, $diagnostics->all()); + foreach ($diagnostics->all() as $d) { + self::assertSame(GenericMethodCompiler::CODE_DUPLICATE_GENERIC_FUNCTION, $d->code); + self::assertNotNull($d->location); + self::assertStringEndsWith('b.xphp', $d->location->file); + } + } + + public function testThisCapturingGenericClosureIsCollectedByCheck(): void + { + $diagnostics = $this->check('closure_this_capture'); + + self::assertCount(1, $diagnostics->all()); + $d = $diagnostics->all()[0]; + self::assertSame(GenericMethodCompiler::CODE_UNSUPPORTED_THIS_CAPTURE, $d->code); + self::assertNotNull($d->location); + self::assertSame(17, $d->location->line); + } + + public function testClassAndMethodLevelErrorsAreBothCollectedInOneRun(): void + { + // Proves the validation-superset guarantee: a class-level check (variance) AND a + // method-level check (generic-function bound) both report in a single `check` run. + $diagnostics = $this->check('method_and_class_errors'); + + $codes = array_map(static fn ($d): string => $d->code, $diagnostics->all()); + self::assertContains(VariancePositionValidator::CODE_VARIANCE_POSITION, $codes); + self::assertContains(Registry::CODE_BOUND_VIOLATION, $codes); + } + + public function testCompileStillThrowsOnGenericFunctionBound(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Generic bound violated'); + $this->compileFixture('generic_function_bound'); + } + + public function testCompileStillThrowsOnGenericMethodMissingArgument(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('has no default'); + $this->compileFixture('generic_method_missing_arg'); + } + + public function testCompileStillThrowsOnDuplicateGenericFunction(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('already declared'); + $this->compileFixture('duplicate_generic_function'); + } + + public function testCompileStillThrowsOnThisCapturingGenericClosure(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('captures `$this`'); + $this->compileFixture('closure_this_capture'); + } + public function testCompileStillThrowsOnUndefinedTemplate(): void { // The same condition is a hard error in compile-mode (no collector) — confirms the @@ -123,6 +210,17 @@ public function testCompileStillThrowsOnUndefinedTemplate(): void } } + private function compileFixture(string $fixture): void + { + $work = sys_get_temp_dir() . '/xphp-check-compile-' . uniqid('', true); + mkdir($work, 0o755, true); + try { + $this->buildCompiler()->compile($this->sources($fixture), $this->sourceDir($fixture), $work . '/dist', $work . '/cache'); + } finally { + self::rrmdir($work); + } + } + private function check(string $fixture): DiagnosticCollector { return $this->buildCompiler()->check($this->sources($fixture)); diff --git a/test/fixture/check/closure_this_capture/source/C.xphp b/test/fixture/check/closure_this_capture/source/C.xphp new file mode 100644 index 0000000..d01714e --- /dev/null +++ b/test/fixture/check/closure_this_capture/source/C.xphp @@ -0,0 +1,19 @@ +(T $x): string { + return $this->name; + }; + + return $f::(1); + } +} diff --git a/test/fixture/check/duplicate_generic_function/source/a.xphp b/test/fixture/check/duplicate_generic_function/source/a.xphp new file mode 100644 index 0000000..930d23b --- /dev/null +++ b/test/fixture/check/duplicate_generic_function/source/a.xphp @@ -0,0 +1,15 @@ +(T $x): T +{ + return $x; +} + +function dup2(T $x): T +{ + return $x; +} diff --git a/test/fixture/check/duplicate_generic_function/source/b.xphp b/test/fixture/check/duplicate_generic_function/source/b.xphp new file mode 100644 index 0000000..930d23b --- /dev/null +++ b/test/fixture/check/duplicate_generic_function/source/b.xphp @@ -0,0 +1,15 @@ +(T $x): T +{ + return $x; +} + +function dup2(T $x): T +{ + return $x; +} diff --git a/test/fixture/check/generic_function_bound/source/Use.xphp b/test/fixture/check/generic_function_bound/source/Use.xphp new file mode 100644 index 0000000..738aae8 --- /dev/null +++ b/test/fixture/check/generic_function_bound/source/Use.xphp @@ -0,0 +1,8 @@ +(1); diff --git a/test/fixture/check/generic_function_bound/source/funcs.xphp b/test/fixture/check/generic_function_bound/source/funcs.xphp new file mode 100644 index 0000000..bb72049 --- /dev/null +++ b/test/fixture/check/generic_function_bound/source/funcs.xphp @@ -0,0 +1,10 @@ +(T $x): T +{ + return $x; +} diff --git a/test/fixture/check/generic_method_missing_arg/source/Use.xphp b/test/fixture/check/generic_method_missing_arg/source/Use.xphp new file mode 100644 index 0000000..cdf2ae8 --- /dev/null +++ b/test/fixture/check/generic_method_missing_arg/source/Use.xphp @@ -0,0 +1,9 @@ +pair::(1, 'x'); diff --git a/test/fixture/check/generic_method_missing_arg/source/Util.xphp b/test/fixture/check/generic_method_missing_arg/source/Util.xphp new file mode 100644 index 0000000..b498d80 --- /dev/null +++ b/test/fixture/check/generic_method_missing_arg/source/Util.xphp @@ -0,0 +1,12 @@ +(A $a, B $b): void + { + } +} diff --git a/test/fixture/check/method_and_class_errors/source/Producer.xphp b/test/fixture/check/method_and_class_errors/source/Producer.xphp new file mode 100644 index 0000000..8276ea2 --- /dev/null +++ b/test/fixture/check/method_and_class_errors/source/Producer.xphp @@ -0,0 +1,13 @@ + +{ + public function set(T $value): void + { + } +} diff --git a/test/fixture/check/method_and_class_errors/source/Use.xphp b/test/fixture/check/method_and_class_errors/source/Use.xphp new file mode 100644 index 0000000..4c54820 --- /dev/null +++ b/test/fixture/check/method_and_class_errors/source/Use.xphp @@ -0,0 +1,8 @@ +(1); diff --git a/test/fixture/check/method_and_class_errors/source/funcs.xphp b/test/fixture/check/method_and_class_errors/source/funcs.xphp new file mode 100644 index 0000000..73cafa3 --- /dev/null +++ b/test/fixture/check/method_and_class_errors/source/funcs.xphp @@ -0,0 +1,10 @@ +(T $x): T +{ + return $x; +} From 1892b73bb324cc948333cf9fff8d0cfcb6081cbd Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Thu, 18 Jun 2026 09:36:16 +0000 Subject: [PATCH 14/16] test(check): cover the static-closure collect path; document new codes Add a closure_static fixture + check-collect and compile-throws tests for the generic static-closure rejection (xphp.static_closure), matching the symmetry of the other method-level checks (the collect path was previously untested). Add the three new method-level codes to the errors-doc table, and correct the validate-only comments (the discarded per-file AST may carry in-place call-site rewrites; templates are deep-cloned so nothing shared is mutated). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/errors.md | 3 +++ .../Monomorphize/GenericMethodCompiler.php | 6 ++++-- .../Monomorphize/CheckPassIntegrationTest.php | 18 ++++++++++++++++++ .../check/closure_static/source/Use.xphp | 12 ++++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 test/fixture/check/closure_static/source/Use.xphp diff --git a/docs/errors.md b/docs/errors.md index 790a8a5..6cd1122 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -41,6 +41,9 @@ The `json` and `github` formats tag each diagnostic with a stable code: | `xphp.variance_position` | a `+T`/`-T` parameter appears in a position its variance forbids | | `xphp.inner_variance` | variance is violated through another generic's slot (composition) | | `xphp.undefined_template` | a generic was instantiated but never declared | +| `xphp.duplicate_generic_function` | the same generic function is declared in two files | +| `xphp.closure_this_capture` | a generic closure/arrow used via turbofish captures `$this` (unsupported) | +| `xphp.static_closure` | a generic `static` closure used via turbofish (unsupported) | | `xphp.parse_error` | the file isn't valid PHP after the generic strip pass | > **Scope.** `xphp check` runs every generic *validation* check `xphp compile` diff --git a/src/Transpiler/Monomorphize/GenericMethodCompiler.php b/src/Transpiler/Monomorphize/GenericMethodCompiler.php index 3e3d459..c9f3903 100644 --- a/src/Transpiler/Monomorphize/GenericMethodCompiler.php +++ b/src/Transpiler/Monomorphize/GenericMethodCompiler.php @@ -103,7 +103,8 @@ public function __construct( * or ""). The values are the top-level statements of each AST. * @param bool $emit When true (default, compile) the pass specializes, appends, and strips * templates as before. When false (`xphp check`) it walks for VALIDATION only — no append-flush, - * no strip, no closure-dispatcher finalize — so the (discarded) AST is left untouched and only + * no strip, no closure-dispatcher finalize. The traversal still rewrites call-site nodes on the + * (discarded) per-file AST, but templates are deep-cloned so nothing shared is mutated; only * diagnostics are produced. */ public function process(array &$astSet, bool $emit = true): void @@ -207,7 +208,8 @@ public function process(array &$astSet, bool $emit = true): void } unset($ast); - // Validate-only (check) stops here: no template stripping, so the AST is left intact. + // Validate-only (check) stops here: no template stripping or append-flush. The discarded + // per-file AST may carry the traversal's in-place call-site rewrites, but nothing shared is. if (!$emit) { return; } diff --git a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php index 2c1c9e9..8822852 100644 --- a/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php +++ b/test/Transpiler/Monomorphize/CheckPassIntegrationTest.php @@ -155,6 +155,24 @@ public function testThisCapturingGenericClosureIsCollectedByCheck(): void self::assertSame(17, $d->location->line); } + public function testStaticGenericClosureIsCollectedByCheck(): void + { + $diagnostics = $this->check('closure_static'); + + self::assertCount(1, $diagnostics->all()); + $d = $diagnostics->all()[0]; + self::assertSame(GenericMethodCompiler::CODE_UNSUPPORTED_STATIC_CLOSURE, $d->code); + self::assertNotNull($d->location); + self::assertSame(12, $d->location->line); + } + + public function testCompileStillThrowsOnStaticGenericClosure(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('static closures cannot yet be specialized'); + $this->compileFixture('closure_static'); + } + public function testClassAndMethodLevelErrorsAreBothCollectedInOneRun(): void { // Proves the validation-superset guarantee: a class-level check (variance) AND a diff --git a/test/fixture/check/closure_static/source/Use.xphp b/test/fixture/check/closure_static/source/Use.xphp new file mode 100644 index 0000000..ac05560 --- /dev/null +++ b/test/fixture/check/closure_static/source/Use.xphp @@ -0,0 +1,12 @@ +(T $x): T { + return $x; +}; + +$r = $f::(1); From d008c05e13031d364d4bc3fb1d3fbe46842d9531 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Thu, 18 Jun 2026 11:11:25 +0000 Subject: [PATCH 15/16] build(phpstan): raise analysis to level 9 and fix the surfaced findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bump phpstan.neon from level 7 to level 9 and make src/ clean at it: - CompileCommand / CheckCommand: narrow getArgument()/getOption() (typed mixed) to string via is_string() instead of a blind (string) cast — the inputs are always strings (required argument / option with a string default), so behavior is unchanged; level 9 just rejects casting mixed. - Specializer: annotate the ATTR_GENERIC_ARGS array as list so array_map infers the callback's parameter type (level-9 callable-variance check). Full suite green; src/ clean at level 9. Co-Authored-By: Claude Opus 4.8 (1M context) --- phpstan.neon | 2 +- src/Console/Command/CheckCommand.php | 11 +++++++---- src/Console/Command/CompileCommand.php | 11 ++++++++--- src/Transpiler/Monomorphize/Specializer.php | 1 + 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index eab1b77..c158993 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,4 +1,4 @@ parameters: - level: 7 + level: 9 paths: - src diff --git a/src/Console/Command/CheckCommand.php b/src/Console/Command/CheckCommand.php index a88e8b7..308339e 100644 --- a/src/Console/Command/CheckCommand.php +++ b/src/Console/Command/CheckCommand.php @@ -45,15 +45,18 @@ protected function execute( InputInterface $input, OutputInterface $output, ): int { - // @infection-ignore-all CastString -- a REQUIRED argument is always a string; - // the cast is defensive for getArgument()'s mixed return, so removing it is equivalent. - $sourceDir = (string) $input->getArgument('source'); + // getArgument()/getOption() are typed `mixed`; these are scalar inputs (a required + // argument and an option with a string default), so they are always strings — narrow + // rather than blind-cast (PHPStan level 9 rejects casting mixed). + $sourceArg = $input->getArgument('source'); + $sourceDir = is_string($sourceArg) ? $sourceArg : ''; if (!is_dir($sourceDir)) { $output->writeln("Source directory not found: {$sourceDir}"); return self::INVALID; } - $renderer = $this->rendererFor((string) $input->getOption('format')); + $formatOption = $input->getOption('format'); + $renderer = $this->rendererFor(is_string($formatOption) ? $formatOption : ''); if ($renderer === null) { $output->writeln('Unknown format (expected: text, json, github)'); return self::INVALID; diff --git a/src/Console/Command/CompileCommand.php b/src/Console/Command/CompileCommand.php index 67bdb42..366bb5f 100644 --- a/src/Console/Command/CompileCommand.php +++ b/src/Console/Command/CompileCommand.php @@ -34,9 +34,14 @@ public function execute( InputInterface $input, OutputInterface $output, ): int { - $sourceDir = (string) $input->getArgument('source'); - $targetDir = (string) $input->getArgument('target'); - $cacheDir = (string) $input->getArgument('cache'); + // getArgument() is typed `mixed`; these are scalar args (a required one and two with + // string defaults), so they are always strings — narrow rather than blind-cast. + $sourceArg = $input->getArgument('source'); + $targetArg = $input->getArgument('target'); + $cacheArg = $input->getArgument('cache'); + $sourceDir = is_string($sourceArg) ? $sourceArg : ''; + $targetDir = is_string($targetArg) ? $targetArg : 'dist'; + $cacheDir = is_string($cacheArg) ? $cacheArg : '.xphp-cache'; if (!is_dir($sourceDir)) { $output->writeln("Source directory not found: {$sourceDir}"); diff --git a/src/Transpiler/Monomorphize/Specializer.php b/src/Transpiler/Monomorphize/Specializer.php index 1fe295f..51a9081 100644 --- a/src/Transpiler/Monomorphize/Specializer.php +++ b/src/Transpiler/Monomorphize/Specializer.php @@ -155,6 +155,7 @@ public function leaveNode(Node $node): ?Node $args = $node->getAttribute(XphpSourceParser::ATTR_GENERIC_ARGS); if (is_array($args) && $args !== []) { + /** @var list $args — ATTR_GENERIC_ARGS is a TypeRef list (set by XphpSourceParser); the type-hint lets array_map infer the callback's parameter as TypeRef. */ $substituted = array_map( fn (TypeRef $a): TypeRef => Specializer::substituteTypeRef($a, $this->substitution), $args, From 984c2fda8c304a4f04ebe64299e592fea48f18f0 Mon Sep 17 00:00:00 2001 From: Matheus Martins Date: Thu, 18 Jun 2026 11:35:19 +0000 Subject: [PATCH 16/16] ci: self-test the `xphp check` gate end-to-end (+ PHAR smoke) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check command is unit-tested in-process via CommandTester, but nothing exercised the real binary: its autoload wiring, the process exit code the shell sees, or the rendered github/json/text output on stdout. The released PHAR was only smoke-tested with `list`, never `check`. Add test/smoke/check.sh — a parameterized POSIX script (XPHP_BIN selects the binary) that runs `check` against the clean and multi_error fixtures and asserts the 0/1/2 exit contract plus that every renderer emits and json stays well-formed. Wire it in: - Makefile: `test/check` target. - ci-core.yml: a dedicated `xphp check (self-test)` job running `make test/check` against bin/xphp. - release.yml: a post-build step running the same script against dist/xphp.phar, so a packaged binary that can't gate fails the release before upload. No src/ changes. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci-core.yml | 24 ++++++++++++++++ .github/workflows/release.yml | 7 +++++ Makefile | 9 ++++++ test/smoke/check.sh | 54 +++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+) create mode 100755 test/smoke/check.sh diff --git a/.github/workflows/ci-core.yml b/.github/workflows/ci-core.yml index 5115e79..46bc492 100644 --- a/.github/workflows/ci-core.yml +++ b/.github/workflows/ci-core.yml @@ -66,6 +66,30 @@ jobs: - name: Run PHPStan run: make lint/phpstan + check: + # End-to-end self-test of the `xphp check` gate: runs the real bin/xphp + # binary against the check fixtures and asserts the 0/1/2 exit contract. + # The in-process CheckCommandTest can't observe the shipped binary's + # process exit code or its rendered stdout, so this covers that gap. + name: xphp check (self-test) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup PHP 8.4 + uses: shivammathur/setup-php@v2 + with: + php-version: '8.4' + extensions: dom, json, mbstring, tokenizer + coverage: none + tools: composer:v2 + + - name: Install dependencies + uses: ramsey/composer-install@v3 + + - name: Run xphp check self-test + run: make test/check + infection: name: Mutation testing runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 03d8c72..c8b15af 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -36,6 +36,13 @@ jobs: # release fails BEFORE the asset is uploaded. run: php dist/xphp.phar list + - name: Smoke-test `check` on the PHAR + # Exercise the released artifact as a real gate: same 0/1/2 + # exit-contract assertions as CI, but against dist/xphp.phar + # instead of bin/xphp. Fails the release before upload if the + # packaged binary can't validate sources. + run: make test/check XPHP_BIN="php dist/xphp.phar" + - name: Compute SHA256 sum # `sha256sum` outputs ` `; running it from # the dist/ directory keeps the filename relative so users diff --git a/Makefile b/Makefile index 3288b02..aad524a 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,15 @@ lint/phpstan: test/mutation: php -d memory_limit=-1 vendor/bin/infection --show-mutations=max --threads=max --min-covered-msi=95 +.PHONY: test/check +# End-to-end self-test of the `check` gate: runs the real bin/xphp binary +# against the check fixtures and asserts the 0/1/2 exit contract plus that the +# text/json/github renderers all emit. Reused by release.yml against the built +# PHAR (override XPHP_BIN="php dist/xphp.phar"). Complements the in-process +# CheckCommandTest, which can't observe the shipped binary's process exit code. +test/check: + sh test/smoke/check.sh + # Humbug Box is the standard tool for compiling a Composer-managed # PHP project into a single self-contained PHAR. Pinned to a known- # good release (Box 4.6.6 supports PHP 8.4) so a new Box version diff --git a/test/smoke/check.sh b/test/smoke/check.sh new file mode 100755 index 0000000..f2fe831 --- /dev/null +++ b/test/smoke/check.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env sh +# +# End-to-end self-test of the `xphp check` gate. +# +# Runs the REAL binary (not the in-process CommandTester) against the check +# fixtures and asserts the 0/1/2 exit contract plus that every renderer emits: +# 0 = clean 1 = at least one error 2 = operational failure +# +# Parameterized by the binary under test so the same assertions cover both the +# dev entrypoint and the released PHAR: +# XPHP_BIN="php bin/xphp" (default, CI self-test) +# XPHP_BIN="php dist/xphp.phar" (release.yml, post-build) +set -eu + +XPHP="${XPHP_BIN:-php bin/xphp}" +FIX="test/fixture/check" + +# check_exit +# Captures stdout+stderr into the global OUT; fails loudly on a code mismatch. +check_exit() { + set +e + OUT=$($XPHP check "$2" --format="$3" 2>&1) + code=$? + set -e + if [ "$code" != "$1" ]; then + echo "FAIL: '$XPHP check $2 --format=$3' expected exit $1, got $code" + echo "$OUT" + exit 1 + fi +} + +# Assert the captured OUT is well-formed JSON (php is always present; avoids a jq dep). +valid_json() { + printf '%s' "$OUT" | php -r 'json_decode(stream_get_contents(STDIN), false, 512, JSON_THROW_ON_ERROR);' \ + || { echo "FAIL: output was not valid JSON"; echo "$OUT"; exit 1; } +} + +# clean sources → exit 0, and all three renderers emit without error. +check_exit 0 "$FIX/clean/source" text +check_exit 0 "$FIX/clean/source" github +check_exit 0 "$FIX/clean/source" json; valid_json +echo "[ok] clean passes (exit 0) in text/json/github" + +# known-bad sources → exit 1, a GitHub annotation is emitted, json stays well-formed. +check_exit 1 "$FIX/multi_error/source" github +printf '%s' "$OUT" | grep -q '^::error file=' || { echo "FAIL: no ::error annotation"; echo "$OUT"; exit 1; } +check_exit 1 "$FIX/multi_error/source" json; valid_json +echo "[ok] multi_error fails (exit 1) with ::error annotation + valid json" + +# operational failure: a missing source dir is exit 2, distinct from a found-errors exit 1. +check_exit 2 "$FIX/does-not-exist" text +echo "[ok] missing source dir is operational failure (exit 2)" + +echo "check smoke: PASS ($XPHP)"