From b056da2a69b6859b14cd035b2814833671f4176e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 10:00:22 +0000 Subject: [PATCH 1/3] test: document PFA parse behavior while blocked on PHP-Parser PHP 8.6 introduces Partial Function Application (PFA): `foo(1, ?)` turns a call into a Closure. nikic/php-parser 5.8.0 (latest, 2026-06-04) has no grammar for the `?` argument placeholder, so such sources cannot be analyzed at all. Pin down the interim contract of issue #224: - `tests/Stub/FileWithPartialFunctionApplication86.php` - PFA placeholders in function, method and closure bodies. Valid PHP 8.6, invalid PHP 8.5, therefore never included and deliberately kept out of `AbstractTestCase::getFilesToAnalyze()`, mirroring how `FileWithFunctionsFcc.php` is handled. - `tests/Stub/FileWithFccInBodies.php` - the PFA-adjacent syntax that is already parseable: `foo(...)` inside function-like bodies. - `tests/Php86PartialFunctionApplicationTest.php` - asserts that PFA sources surface a catchable `PhpParser\Error` (the engine does not wrap parser errors) instead of a truncated AST, that a failed parse does not poison the engine cache, that first-class callables in bodies still reflect cleanly with their `VariadicPlaceholder` node intact, and that `NodeExpressionResolver` degrades into `ReflectionException` for placeholder arguments and unhandled node types. Refs #224 Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Sy8BcM8ivUEpu8uVm7wADn --- tests/Php86PartialFunctionApplicationTest.php | 275 ++++++++++++++++++ tests/Stub/FileWithFccInBodies.php | 57 ++++ .../FileWithPartialFunctionApplication86.php | 77 +++++ 3 files changed, 409 insertions(+) create mode 100644 tests/Php86PartialFunctionApplicationTest.php create mode 100644 tests/Stub/FileWithFccInBodies.php create mode 100644 tests/Stub/FileWithPartialFunctionApplication86.php diff --git a/tests/Php86PartialFunctionApplicationTest.php b/tests/Php86PartialFunctionApplicationTest.php new file mode 100644 index 0000000..ee4981a --- /dev/null +++ b/tests/Php86PartialFunctionApplicationTest.php @@ -0,0 +1,275 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Go\ParserReflection; + +use Go\ParserReflection\Locator\ComposerLocator; +use Go\ParserReflection\Resolver\NodeExpressionResolver; +use PhpParser\Node; +use PhpParser\Node\Expr; +use PhpParser\Node\VariadicPlaceholder; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; + +/** + * Documents the current behavior of the engine for PHP 8.6 Partial Function Application (PFA). + * + * PHP 8.6 lets any call use the `?` placeholder for a single open argument, turning the call into + * a Closure: + * + * ```php + * $makeSlug = str_replace(' ', '-', ?); + * ``` + * + * The required nikic/php-parser (5.8.0, released 2026-06-04) has no grammar for the `?` argument + * placeholder yet, so such sources simply can not be analyzed. This test pins down the *interim* + * contract of issue #224: the engine must surface a clear, catchable parse error rather than + * silently returning a truncated or corrupted AST. + * + * The related-but-already-supported first-class callable syntax `foo(...)` is asserted to keep + * working, so that the follow-up work on PFA can be recognized as a real change of behavior. + * + * @see https://github.com/goaop/parser-reflection/issues/224 + */ +class Php86PartialFunctionApplicationTest extends TestCase +{ + /** + * Stub with PFA placeholders. It is not valid PHP 8.5 source, therefore it is never included + * and it is deliberately kept out of AbstractTestCase::getFilesToAnalyze(). + */ + public const PFA_STUB_FILE = '/Stub/FileWithPartialFunctionApplication86.php'; + + /** + * Stub with first-class callables inside function-like bodies, which is parseable today. + */ + public const FCC_STUB_FILE = '/Stub/FileWithFccInBodies.php'; + + protected function tearDown(): void + { + // Some tests below replace the engine state, restore the default locator for the rest + ReflectionEngine::init(new ComposerLocator()); + } + + /** + * The PFA stub must never be part of the general parity data providers, as those parse + * (and include) every listed file eagerly. + */ + public function testPfaStubIsExcludedFromGeneralAnalysis(): void + { + $analyzedFiles = []; + foreach (AbstractTestCase::getFilesToAnalyze() as $fileList) { + foreach ($fileList as $fileName) { + $analyzedFiles[] = basename($fileName); + } + } + + $this->assertNotContains(basename(self::PFA_STUB_FILE), $analyzedFiles); + } + + /** + * The stub really does contain PFA syntax, otherwise the assertions below would be vacuous. + */ + public function testPfaStubContainsPlaceholderSyntax(): void + { + $stubContent = file_get_contents(__DIR__ . self::PFA_STUB_FILE); + + $this->assertIsString($stubContent); + $this->assertStringContainsString("str_replace(' ', '-', ?)", $stubContent); + } + + /** + * Parsing a file with PFA placeholders fails loudly with a PhpParser\Error. + * + * Note that ReflectionEngine does not wrap parser errors, so PhpParser\Error is what actually + * surfaces through ReflectionEngine::parseFile() and, transitively, through ReflectionFile. + */ + public function testParsingStubWithPartialFunctionApplicationRaisesParseError(): void + { + $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); + $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); + + $this->expectException(\PhpParser\Error::class); + $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + + ReflectionEngine::parseFile($resolvedFileName); + } + + /** + * The very same error must reach the user through the public ReflectionFile entry point, + * i.e. it is not swallowed or converted into an empty list of namespaces. + */ + public function testReflectionFileOnPartialFunctionApplicationRaisesParseError(): void + { + $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); + $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); + + $this->expectException(\PhpParser\Error::class); + $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + + new ReflectionFile($resolvedFileName); + } + + /** + * A failed parse must not poison the engine cache: nothing is stored for that file name, so a + * later attempt (e.g. after the php-parser constraint is bumped) re-parses from scratch. + */ + public function testFailedParseIsNotCached(): void + { + $virtualFileName = __DIR__ . '/Stub/VirtualPfaFile.php'; + + try { + ReflectionEngine::parseFile($virtualFileName, 'fail('Parsing partial function application was expected to fail'); + } catch (\PhpParser\Error) { + // expected + } + + // The same virtual name now parses fine with valid content, which proves nothing was cached + $nodes = ReflectionEngine::parseFile($virtualFileName, 'assertCount(1, $nodes); + } + + /** + * Every PFA placeholder position currently produces a syntax error mentioning the `?` token. + * + * @param string $source PHP source code using a partial function application + */ + #[DataProvider('partialFunctionApplicationSourceProvider')] + public function testEveryPlaceholderPositionRaisesParseError(string $source): void + { + $this->expectException(\PhpParser\Error::class); + $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + + ReflectionEngine::parseFile(__DIR__ . '/Stub/VirtualPfaSnippet.php', $source); + } + + /** + * @return \Generator + */ + public static function partialFunctionApplicationSourceProvider(): \Generator + { + yield 'trailing placeholder' => [' [' [' ['run(?); } }']; + yield 'placeholder in static' => [' ['assertIsString($resolvedFileName, 'FCC stub file should be available'); + + $reflectionFile = new ReflectionFile($resolvedFileName); + $reflectionNamespace = $reflectionFile->getFileNamespace('Go\ParserReflection\Stub'); + + $this->assertTrue($reflectionNamespace->hasFunction('functionWithFccInBody')); + + $parsedFunction = $reflectionNamespace->getFunction('functionWithFccInBody'); + $this->assertSame('Go\ParserReflection\Stub\functionWithFccInBody', $parsedFunction->getName()); + $this->assertSame(0, $parsedFunction->getNumberOfParameters()); + + $parsedClass = $reflectionNamespace->getClass('Go\ParserReflection\Stub\ClassWithFccInBodies'); + $this->assertTrue($parsedClass->hasMethod('methodWithFccInBody')); + + $parsedMethod = $parsedClass->getMethod('methodWithFccInBody'); + $this->assertSame(2, $parsedMethod->getNumberOfParameters()); + $this->assertSame(1, $parsedMethod->getNumberOfRequiredParameters()); + $this->assertSame('separator', $parsedMethod->getParameters()[0]->getName()); + $this->assertSame(2, $parsedMethod->getParameters()[1]->getDefaultValue()); + + foreach (['methodWithFccInClosureBody', 'methodWithStaticFccInBody', 'helper'] as $methodName) { + $this->assertTrue($parsedClass->hasMethod($methodName)); + } + } + + /** + * The body of an FCC-containing method is still a well-formed AST that can be walked, which is + * exactly what the future PFA support has to preserve. + */ + public function testFirstClassCallableBodyKeepsVariadicPlaceholderNode(): void + { + $resolvedFileName = stream_resolve_include_path(__DIR__ . self::FCC_STUB_FILE); + $this->assertIsString($resolvedFileName, 'FCC stub file should be available'); + + $reflectionFile = new ReflectionFile($resolvedFileName); + $parsedClass = $reflectionFile + ->getFileNamespace('Go\ParserReflection\Stub') + ->getClass('Go\ParserReflection\Stub\ClassWithFccInBodies'); + + $methodNode = $parsedClass->getMethod('methodWithFccInBody')->getNode(); + $statements = $methodNode->stmts ?? []; + $this->assertCount(1, $statements); + + $returnStatement = $statements[0]; + $this->assertInstanceOf(Node\Stmt\Return_::class, $returnStatement); + $this->assertInstanceOf(Expr\FuncCall::class, $returnStatement->expr); + $this->assertTrue($returnStatement->expr->isFirstClassCallable()); + $this->assertInstanceOf(VariadicPlaceholder::class, $returnStatement->expr->args[0]); + } + + /** + * A placeholder argument that the resolver can not evaluate has to degrade into a regular + * ReflectionException, never into a fatal error or a silently wrong value. + * + * The node built here is the closest available stand-in for a future PFA argument: a call that + * is *not* a first-class callable but still carries a non-Arg placeholder argument. + */ + public function testResolverFailsGracefullyOnPlaceholderArgument(): void + { + $funcCallNode = new Expr\FuncCall( + new Node\Name\FullyQualified('str_replace'), + [ + new Node\Arg(new Node\Scalar\String_(' ')), + new Node\Arg(new Node\Scalar\String_('-')), + new VariadicPlaceholder(), + ] + ); + + $this->expectException(ReflectionException::class); + $this->expectExceptionMessage('Cannot statically resolve a variadic placeholder argument in a function call'); + + (new NodeExpressionResolver(null))->process($funcCallNode); + } + + /** + * The same graceful degradation is required for constructor calls, which PFA also covers. + */ + public function testResolverFailsGracefullyOnPlaceholderArgumentInNewExpression(): void + { + $newNode = new Expr\New_( + new Node\Name\FullyQualified('DateTimeImmutable'), + [new VariadicPlaceholder()] + ); + + $this->expectException(ReflectionException::class); + $this->expectExceptionMessage('Cannot statically resolve a variadic placeholder argument in a constructor call'); + + (new NodeExpressionResolver(null))->process($newNode); + } + + /** + * Any node type the resolver has no handler for (which is what an eventual PFA placeholder node + * would be, before explicit support is added) must produce a ReflectionException as well. + */ + public function testResolverFailsGracefullyOnUnknownNodeType(): void + { + $this->expectException(ReflectionException::class); + $this->expectExceptionMessageMatches('/Could not find handler for the .*NodeExpressionResolver::resolveExpr\w+ method/'); + + (new NodeExpressionResolver(null))->process(new Expr\Variable('placeholder')); + } +} diff --git a/tests/Stub/FileWithFccInBodies.php b/tests/Stub/FileWithFccInBodies.php new file mode 100644 index 0000000..c8d3d19 --- /dev/null +++ b/tests/Stub/FileWithFccInBodies.php @@ -0,0 +1,57 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +/** + * Stub file containing first-class callable syntax (FCC) inside function/method/closure bodies. + * + * Unlike FileWithFunctionsFcc.php (which puts FCC into constant-expression positions and thus + * can not be loaded), this file is perfectly valid runtime PHP and may be included. + * + * It is the "PFA-adjacent but already parseable" counterpart of + * FileWithPartialFunctionApplication86.php: `foo(...)` is represented by PHP-Parser as a call + * whose single argument is a VariadicPlaceholder, and that representation is intended to be + * forward-compatible with Partial Function Application. Reflecting function-like bodies that + * contain it must keep working. + * + * @see https://github.com/goaop/parser-reflection/issues/224 + */ + +namespace Go\ParserReflection\Stub; + +function functionWithFccInBody(): \Closure +{ + return strlen(...); +} + +class ClassWithFccInBodies +{ + public function methodWithFccInBody(string $separator, int $limit = 2): \Closure + { + return str_replace(...); + } + + public function methodWithFccInClosureBody(): \Closure + { + return function (): \Closure { + return trim(...); + }; + } + + public function methodWithStaticFccInBody(): \Closure + { + return self::helper(...); + } + + public static function helper(string $value): string + { + return $value; + } +} diff --git a/tests/Stub/FileWithPartialFunctionApplication86.php b/tests/Stub/FileWithPartialFunctionApplication86.php new file mode 100644 index 0000000..40c2999 --- /dev/null +++ b/tests/Stub/FileWithPartialFunctionApplication86.php @@ -0,0 +1,77 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ + +/** + * Stub file containing PHP 8.6 Partial Function Application (PFA) placeholders. + * + * WARNING: this file is intentionally NOT valid PHP 8.5 source and it can NOT be parsed by the + * currently required nikic/php-parser (5.8.0 has no grammar for the `?` argument placeholder). + * + * Therefore this file: + * - must NEVER be included/required (it would be a fatal parse error on a PHP 8.5 runtime); + * - must NEVER be listed in AbstractTestCase::getFilesToAnalyze(), because every parity data + * provider parses those files eagerly; + * - is only ever read as raw text by Php86PartialFunctionApplicationTest, which asserts that + * the engine reports a clear parse error instead of silently producing a corrupted AST. + * + * Once nikic/php-parser gains PFA support, this stub becomes the positive fixture for + * issue #224: the constraint gets bumped, the engine picks the grammar up automatically via + * ParserFactory::createForNewestSupportedVersion(), and the assertions here flip from + * "raises a parse error" to "reflects cleanly". + * + * @see https://github.com/goaop/parser-reflection/issues/224 + */ + +namespace Go\ParserReflection\Stub; + +/** + * Function whose body builds a partial application of an internal function. + */ +function functionWithPartialApplicationInBody(): \Closure +{ + return str_replace(' ', '-', ?); +} + +/** + * Function that mixes a bound argument with the "all remaining arguments" placeholder. + */ +function functionWithTrailingVariadicPlaceholder(): \Closure +{ + return str_replace(' ', '-', ...); +} + +/** + * Class with methods and closures using PFA placeholders inside their bodies. + */ +class ClassWithPartialFunctionApplication +{ + public function methodWithPartialApplication(): \Closure + { + return str_pad(?, 10, '.'); + } + + public function closureWithPartialApplication(): \Closure + { + return function (): \Closure { + return implode(', ', ?); + }; + } + + public function staticCallWithPartialApplication(): \Closure + { + return self::helper(?, 1); + } + + public static function helper(string $value, int $times): string + { + return str_repeat($value, $times); + } +} From 33d0d4c16d83d84c5b4d677d18d218e5e809b15f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 10:17:56 +0000 Subject: [PATCH 2/3] feat: full PHP 8.6 partial function application support via php-parser ^5.9 nikic/php-parser 5.9.0 parses the PFA `?` placeholder into ArgPlaceholder nodes, so PFA-containing sources now reflect cleanly on every supported host runtime: - bump the nikic/php-parser constraint to ^5.9 - flip the interim parse-error tests to positive reflection assertions: engine and ReflectionFile reflect the PFA stub, every placeholder position (incl. named placeholders) parses, method bodies keep their ArgPlaceholder nodes and isPartialFunctionApplication() flag - keep the resolver contract: a placeholder argument in a constant-expression position degrades into ReflectionException (message generalized to cover ArgPlaceholder and VariadicPlaceholder alike) - pin cache hygiene with a genuinely broken source now that PFA parses - guarded PHP 8.6 test executes the stub and checks native behavior Note: a first-class callable reports isPartialFunctionApplication() true in php-parser 5.9 by design; the FCC regression test documents that. Refs #224 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Sy8BcM8ivUEpu8uVm7wADn --- composer.json | 2 +- src/Resolver/NodeExpressionResolver.php | 4 +- tests/Php86PartialFunctionApplicationTest.php | 237 ++++++++++++------ .../FileWithPartialFunctionApplication86.php | 19 +- 4 files changed, 175 insertions(+), 87 deletions(-) diff --git a/composer.json b/composer.json index e51cd77..57b039b 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ }, "require": { "php": ">=8.5", - "nikic/php-parser": "^5.4" + "nikic/php-parser": "^5.9" }, "require-dev": { "phpunit/phpunit": "^13.3.1", diff --git a/src/Resolver/NodeExpressionResolver.php b/src/Resolver/NodeExpressionResolver.php index c27b23a..c5da10c 100644 --- a/src/Resolver/NodeExpressionResolver.php +++ b/src/Resolver/NodeExpressionResolver.php @@ -257,7 +257,7 @@ protected function resolveExprFuncCall(Expr\FuncCall $node): mixed $resolvedArgs = []; foreach ($node->args as $argumentNode) { if (!$argumentNode instanceof Node\Arg) { - throw new ReflectionException('Cannot statically resolve a variadic placeholder argument in a function call'); + throw new ReflectionException('Cannot statically resolve a placeholder argument in a function call'); } $value = $this->resolve($argumentNode->value); // if function uses named arguments, then unpack argument name first @@ -385,7 +385,7 @@ protected function resolveExprNew(Expr\New_ $node): object $resolvedArgs = []; foreach ($node->args as $argumentNode) { if (!$argumentNode instanceof Node\Arg) { - throw new ReflectionException('Cannot statically resolve a variadic placeholder argument in a constructor call'); + throw new ReflectionException('Cannot statically resolve a placeholder argument in a constructor call'); } $value = $this->resolve($argumentNode->value); // if constructor uses named arguments, then unpack argument name first diff --git a/tests/Php86PartialFunctionApplicationTest.php b/tests/Php86PartialFunctionApplicationTest.php index ee4981a..a31083d 100644 --- a/tests/Php86PartialFunctionApplicationTest.php +++ b/tests/Php86PartialFunctionApplicationTest.php @@ -14,13 +14,15 @@ use Go\ParserReflection\Locator\ComposerLocator; use Go\ParserReflection\Resolver\NodeExpressionResolver; use PhpParser\Node; +use PhpParser\Node\ArgPlaceholder; use PhpParser\Node\Expr; use PhpParser\Node\VariadicPlaceholder; +use PhpParser\NodeFinder; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; /** - * Documents the current behavior of the engine for PHP 8.6 Partial Function Application (PFA). + * Covers reflection of PHP 8.6 Partial Function Application (PFA). * * PHP 8.6 lets any call use the `?` placeholder for a single open argument, turning the call into * a Closure: @@ -29,29 +31,33 @@ * $makeSlug = str_replace(' ', '-', ?); * ``` * - * The required nikic/php-parser (5.8.0, released 2026-06-04) has no grammar for the `?` argument - * placeholder yet, so such sources simply can not be analyzed. This test pins down the *interim* - * contract of issue #224: the engine must surface a clear, catchable parse error rather than - * silently returning a truncated or corrupted AST. + * Since nikic/php-parser 5.9.0 the `?` placeholder is parsed into a `PhpParser\Node\ArgPlaceholder` + * node (the "all remaining arguments" form `foo(1, ...)` reuses `VariadicPlaceholder`), so sources + * containing PFA now reflect cleanly on every supported host runtime — the engine parses with the + * newest supported grammar regardless of the PHP version it runs on. * - * The related-but-already-supported first-class callable syntax `foo(...)` is asserted to keep - * working, so that the follow-up work on PFA can be recognized as a real change of behavior. + * A PFA expression in a constant-expression position still degrades into a ReflectionException, + * the same contract user-defined first-class callables already have: a Closure cannot be + * represented statically. * * @see https://github.com/goaop/parser-reflection/issues/224 */ class Php86PartialFunctionApplicationTest extends TestCase { /** - * Stub with PFA placeholders. It is not valid PHP 8.5 source, therefore it is never included - * and it is deliberately kept out of AbstractTestCase::getFilesToAnalyze(). + * Stub with PFA placeholders. It parses on every runtime since php-parser 5.9, but it can only + * be *included* by a PHP 8.6+ runtime, therefore it is kept out of the general parity data + * providers, which include every listed file eagerly. */ public const PFA_STUB_FILE = '/Stub/FileWithPartialFunctionApplication86.php'; /** - * Stub with first-class callables inside function-like bodies, which is parseable today. + * Stub with first-class callables inside function-like bodies. */ public const FCC_STUB_FILE = '/Stub/FileWithFccInBodies.php'; + public const STUB_NAMESPACE = 'Go\ParserReflection\Stub'; + protected function tearDown(): void { // Some tests below replace the engine state, restore the default locator for the rest @@ -59,8 +65,8 @@ protected function tearDown(): void } /** - * The PFA stub must never be part of the general parity data providers, as those parse - * (and include) every listed file eagerly. + * The PFA stub must not be part of the general parity data providers: those include every + * listed file eagerly, and PFA syntax is a compile error on a PHP 8.5 runtime. */ public function testPfaStubIsExcludedFromGeneralAnalysis(): void { @@ -86,82 +92,121 @@ public function testPfaStubContainsPlaceholderSyntax(): void } /** - * Parsing a file with PFA placeholders fails loudly with a PhpParser\Error. - * - * Note that ReflectionEngine does not wrap parser errors, so PhpParser\Error is what actually - * surfaces through ReflectionEngine::parseFile() and, transitively, through ReflectionFile. + * A file with PFA placeholders parses cleanly through the engine, even on a PHP 8.5 host: + * ReflectionEngine relies on the newest grammar supported by php-parser, not on the host + * runtime, so the 5.9 grammar is picked up without any engine change. */ - public function testParsingStubWithPartialFunctionApplicationRaisesParseError(): void + public function testStubWithPartialFunctionApplicationIsParsed(): void { $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); - $this->expectException(\PhpParser\Error::class); - $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + $fileNodes = ReflectionEngine::parseFile($resolvedFileName); - ReflectionEngine::parseFile($resolvedFileName); + $this->assertNotEmpty($fileNodes); + $placeholders = (new NodeFinder())->findInstanceOf($fileNodes, ArgPlaceholder::class); + $this->assertNotEmpty($placeholders, 'The parsed AST should contain ArgPlaceholder nodes'); } /** - * The very same error must reach the user through the public ReflectionFile entry point, - * i.e. it is not swallowed or converted into an empty list of namespaces. + * The public ReflectionFile entry point reflects a PFA-containing source without errors: + * namespaces, functions, classes and their signatures are all available. */ - public function testReflectionFileOnPartialFunctionApplicationRaisesParseError(): void + public function testReflectionFileReflectsPartialFunctionApplicationStub(): void { $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); - $this->expectException(\PhpParser\Error::class); - $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + $reflectionFile = new ReflectionFile($resolvedFileName); + $reflectionNamespace = $reflectionFile->getFileNamespace(self::STUB_NAMESPACE); + + $this->assertTrue($reflectionNamespace->hasFunction('functionWithPartialApplicationInBody')); + $this->assertTrue($reflectionNamespace->hasFunction('functionWithTrailingVariadicPlaceholder')); + + $parsedFunction = $reflectionNamespace->getFunction('functionWithPartialApplicationInBody'); + $this->assertSame('Closure', (string) $parsedFunction->getReturnType()); + $this->assertSame(0, $parsedFunction->getNumberOfParameters()); - new ReflectionFile($resolvedFileName); + $parsedClass = $reflectionNamespace->getClass(self::STUB_NAMESPACE . '\ClassWithPartialFunctionApplication'); + foreach (['methodWithPartialApplication', 'closureWithPartialApplication', 'staticCallWithPartialApplication', 'helper'] as $methodName) { + $this->assertTrue($parsedClass->hasMethod($methodName)); + } + $this->assertSame(2, $parsedClass->getMethod('helper')->getNumberOfParameters()); } /** - * A failed parse must not poison the engine cache: nothing is stored for that file name, so a - * later attempt (e.g. after the php-parser constraint is bumped) re-parses from scratch. + * The body of a PFA-containing method is a well-formed AST: the call carries an ArgPlaceholder + * argument and reports itself as a partial function application, distinct from a first-class + * callable. */ - public function testFailedParseIsNotCached(): void + public function testMethodBodyKeepsArgPlaceholderNode(): void { - $virtualFileName = __DIR__ . '/Stub/VirtualPfaFile.php'; + $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); + $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); - try { - ReflectionEngine::parseFile($virtualFileName, 'fail('Parsing partial function application was expected to fail'); - } catch (\PhpParser\Error) { - // expected - } + $parsedClass = (new ReflectionFile($resolvedFileName)) + ->getFileNamespace(self::STUB_NAMESPACE) + ->getClass(self::STUB_NAMESPACE . '\ClassWithPartialFunctionApplication'); - // The same virtual name now parses fine with valid content, which proves nothing was cached - $nodes = ReflectionEngine::parseFile($virtualFileName, 'assertCount(1, $nodes); + $methodNode = $parsedClass->getMethod('methodWithPartialApplication')->getNode(); + $statements = $methodNode->stmts ?? []; + $this->assertCount(1, $statements); + + $returnStatement = $statements[0]; + $this->assertInstanceOf(Node\Stmt\Return_::class, $returnStatement); + $this->assertInstanceOf(Expr\FuncCall::class, $returnStatement->expr); + $this->assertTrue($returnStatement->expr->isPartialFunctionApplication()); + $this->assertFalse($returnStatement->expr->isFirstClassCallable()); + $this->assertInstanceOf(ArgPlaceholder::class, $returnStatement->expr->args[0]); } /** - * Every PFA placeholder position currently produces a syntax error mentioning the `?` token. + * Every PFA placeholder position parses into the expected number of ArgPlaceholder nodes. * * @param string $source PHP source code using a partial function application */ #[DataProvider('partialFunctionApplicationSourceProvider')] - public function testEveryPlaceholderPositionRaisesParseError(string $source): void + public function testEveryPlaceholderPositionIsParsed(string $source, int $expectedPlaceholders): void { - $this->expectException(\PhpParser\Error::class); - $this->expectExceptionMessageMatches('/Syntax error, unexpected \'\?\'/'); + $fileNodes = ReflectionEngine::parseFile(__DIR__ . '/Stub/VirtualPfaSnippet.php', $source); - ReflectionEngine::parseFile(__DIR__ . '/Stub/VirtualPfaSnippet.php', $source); + $placeholders = (new NodeFinder())->findInstanceOf($fileNodes, ArgPlaceholder::class); + $this->assertCount($expectedPlaceholders, $placeholders); } /** - * @return \Generator + * @return \Generator */ public static function partialFunctionApplicationSourceProvider(): \Generator { - yield 'trailing placeholder' => [' [' [' ['run(?); } }']; - yield 'placeholder in static' => [' [' [' [' [' [' ['run(?); } }', 1]; + yield 'placeholder in static' => [' ['fail('Parsing a broken source was expected to fail'); + } catch (\PhpParser\Error) { + // expected + } + + // The same virtual name now parses fine with valid content, which proves nothing was cached + $nodes = ReflectionEngine::parseFile($virtualFileName, 'assertCount(1, $nodes); } /** @@ -174,15 +219,15 @@ public function testFirstClassCallableInsideBodiesIsStillReflected(): void $this->assertIsString($resolvedFileName, 'FCC stub file should be available'); $reflectionFile = new ReflectionFile($resolvedFileName); - $reflectionNamespace = $reflectionFile->getFileNamespace('Go\ParserReflection\Stub'); + $reflectionNamespace = $reflectionFile->getFileNamespace(self::STUB_NAMESPACE); $this->assertTrue($reflectionNamespace->hasFunction('functionWithFccInBody')); $parsedFunction = $reflectionNamespace->getFunction('functionWithFccInBody'); - $this->assertSame('Go\ParserReflection\Stub\functionWithFccInBody', $parsedFunction->getName()); + $this->assertSame(self::STUB_NAMESPACE . '\functionWithFccInBody', $parsedFunction->getName()); $this->assertSame(0, $parsedFunction->getNumberOfParameters()); - $parsedClass = $reflectionNamespace->getClass('Go\ParserReflection\Stub\ClassWithFccInBodies'); + $parsedClass = $reflectionNamespace->getClass(self::STUB_NAMESPACE . '\ClassWithFccInBodies'); $this->assertTrue($parsedClass->hasMethod('methodWithFccInBody')); $parsedMethod = $parsedClass->getMethod('methodWithFccInBody'); @@ -197,8 +242,10 @@ public function testFirstClassCallableInsideBodiesIsStillReflected(): void } /** - * The body of an FCC-containing method is still a well-formed AST that can be walked, which is - * exactly what the future PFA support has to preserve. + * The body of an FCC-containing method still parses into a first-class callable with its + * established VariadicPlaceholder representation. Note that php-parser 5.9 deliberately + * reports a first-class callable as a special case of partial function application, so + * isPartialFunctionApplication() is true for it as well. */ public function testFirstClassCallableBodyKeepsVariadicPlaceholderNode(): void { @@ -207,8 +254,8 @@ public function testFirstClassCallableBodyKeepsVariadicPlaceholderNode(): void $reflectionFile = new ReflectionFile($resolvedFileName); $parsedClass = $reflectionFile - ->getFileNamespace('Go\ParserReflection\Stub') - ->getClass('Go\ParserReflection\Stub\ClassWithFccInBodies'); + ->getFileNamespace(self::STUB_NAMESPACE) + ->getClass(self::STUB_NAMESPACE . '\ClassWithFccInBodies'); $methodNode = $parsedClass->getMethod('methodWithFccInBody')->getNode(); $statements = $methodNode->stmts ?? []; @@ -218,29 +265,29 @@ public function testFirstClassCallableBodyKeepsVariadicPlaceholderNode(): void $this->assertInstanceOf(Node\Stmt\Return_::class, $returnStatement); $this->assertInstanceOf(Expr\FuncCall::class, $returnStatement->expr); $this->assertTrue($returnStatement->expr->isFirstClassCallable()); + // A first-class callable counts as a partial function application in php-parser 5.9+ + $this->assertTrue($returnStatement->expr->isPartialFunctionApplication()); $this->assertInstanceOf(VariadicPlaceholder::class, $returnStatement->expr->args[0]); } /** - * A placeholder argument that the resolver can not evaluate has to degrade into a regular - * ReflectionException, never into a fatal error or a silently wrong value. - * - * The node built here is the closest available stand-in for a future PFA argument: a call that - * is *not* a first-class callable but still carries a non-Arg placeholder argument. + * A PFA placeholder argument in a constant-expression position degrades into a regular + * ReflectionException, never into a fatal error or a silently wrong value — the same contract + * user-defined first-class callables already have. */ - public function testResolverFailsGracefullyOnPlaceholderArgument(): void + public function testResolverFailsGracefullyOnArgPlaceholderInFunctionCall(): void { $funcCallNode = new Expr\FuncCall( new Node\Name\FullyQualified('str_replace'), [ new Node\Arg(new Node\Scalar\String_(' ')), new Node\Arg(new Node\Scalar\String_('-')), - new VariadicPlaceholder(), + new ArgPlaceholder(), ] ); $this->expectException(ReflectionException::class); - $this->expectExceptionMessage('Cannot statically resolve a variadic placeholder argument in a function call'); + $this->expectExceptionMessage('Cannot statically resolve a placeholder argument in a function call'); (new NodeExpressionResolver(null))->process($funcCallNode); } @@ -248,22 +295,41 @@ public function testResolverFailsGracefullyOnPlaceholderArgument(): void /** * The same graceful degradation is required for constructor calls, which PFA also covers. */ - public function testResolverFailsGracefullyOnPlaceholderArgumentInNewExpression(): void + public function testResolverFailsGracefullyOnArgPlaceholderInNewExpression(): void { $newNode = new Expr\New_( new Node\Name\FullyQualified('DateTimeImmutable'), - [new VariadicPlaceholder()] + [new ArgPlaceholder()] ); $this->expectException(ReflectionException::class); - $this->expectExceptionMessage('Cannot statically resolve a variadic placeholder argument in a constructor call'); + $this->expectExceptionMessage('Cannot statically resolve a placeholder argument in a constructor call'); (new NodeExpressionResolver(null))->process($newNode); } /** - * Any node type the resolver has no handler for (which is what an eventual PFA placeholder node - * would be, before explicit support is added) must produce a ReflectionException as well. + * The "all remaining arguments" placeholder keeps the same resolver contract. + */ + public function testResolverFailsGracefullyOnVariadicPlaceholderArgument(): void + { + $funcCallNode = new Expr\FuncCall( + new Node\Name\FullyQualified('str_replace'), + [ + new Node\Arg(new Node\Scalar\String_(' ')), + new Node\Arg(new Node\Scalar\String_('-')), + new VariadicPlaceholder(), + ] + ); + + $this->expectException(ReflectionException::class); + $this->expectExceptionMessage('Cannot statically resolve a placeholder argument in a function call'); + + (new NodeExpressionResolver(null))->process($funcCallNode); + } + + /** + * Any node type the resolver has no handler for must produce a ReflectionException as well. */ public function testResolverFailsGracefullyOnUnknownNodeType(): void { @@ -272,4 +338,33 @@ public function testResolverFailsGracefullyOnUnknownNodeType(): void (new NodeExpressionResolver(null))->process(new Expr\Variable('placeholder')); } + + /** + * On a PHP 8.6 runtime the stub is genuinely loadable and behaves as reflected: the parsed + * signatures match native reflection, and the partial applications evaluate to Closures. + */ + public function testNativeBehaviorOnPhp86(): void + { + if (PHP_VERSION_ID < 80600) { + $this->markTestSkipped('Executing partial function application requires a PHP 8.6 runtime'); + } + + $resolvedFileName = stream_resolve_include_path(__DIR__ . self::PFA_STUB_FILE); + $this->assertIsString($resolvedFileName, 'PFA stub file should be available'); + + include_once $resolvedFileName; + + $functionName = self::STUB_NAMESPACE . '\functionWithPartialApplicationInBody'; + $nativeFunction = new \ReflectionFunction($functionName); + $parsedFunction = (new ReflectionFile($resolvedFileName)) + ->getFileNamespace(self::STUB_NAMESPACE) + ->getFunction('functionWithPartialApplicationInBody'); + + $this->assertSame((string) $nativeFunction->getReturnType(), (string) $parsedFunction->getReturnType()); + $this->assertSame($nativeFunction->getNumberOfParameters(), $parsedFunction->getNumberOfParameters()); + + $partialApplication = $functionName(); + $this->assertInstanceOf(\Closure::class, $partialApplication); + $this->assertSame('a-b', $partialApplication('a b')); + } } diff --git a/tests/Stub/FileWithPartialFunctionApplication86.php b/tests/Stub/FileWithPartialFunctionApplication86.php index 40c2999..d7e3945 100644 --- a/tests/Stub/FileWithPartialFunctionApplication86.php +++ b/tests/Stub/FileWithPartialFunctionApplication86.php @@ -12,20 +12,13 @@ /** * Stub file containing PHP 8.6 Partial Function Application (PFA) placeholders. * - * WARNING: this file is intentionally NOT valid PHP 8.5 source and it can NOT be parsed by the - * currently required nikic/php-parser (5.8.0 has no grammar for the `?` argument placeholder). + * Since nikic/php-parser 5.9 the `?` placeholder parses into an ArgPlaceholder node, so this file + * is the positive fixture for issue #224 and reflects cleanly on every supported host runtime. * - * Therefore this file: - * - must NEVER be included/required (it would be a fatal parse error on a PHP 8.5 runtime); - * - must NEVER be listed in AbstractTestCase::getFilesToAnalyze(), because every parity data - * provider parses those files eagerly; - * - is only ever read as raw text by Php86PartialFunctionApplicationTest, which asserts that - * the engine reports a clear parse error instead of silently producing a corrupted AST. - * - * Once nikic/php-parser gains PFA support, this stub becomes the positive fixture for - * issue #224: the constraint gets bumped, the engine picks the grammar up automatically via - * ParserFactory::createForNewestSupportedVersion(), and the assertions here flip from - * "raises a parse error" to "reflects cleanly". + * It is still NOT valid PHP 8.5 at runtime (PFA is a compile error before PHP 8.6), therefore: + * - it must only be included/required behind a PHP_VERSION_ID >= 80600 guard; + * - it must NOT be listed in AbstractTestCase::getFilesToAnalyze(), because every parity data + * provider includes those files eagerly. * * @see https://github.com/goaop/parser-reflection/issues/224 */ From b2ef5430ad2592f71bbe17e134a31d548528e8b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 10:18:20 +0000 Subject: [PATCH 3/3] build: require rector ^2.6.7 so its preloaded php-parser bundle is >= 5.9 Rector's bootstrap preloads its own unprefixed php-parser copy whenever PHPUnit >= 12 is running, shadowing the project's php-parser. Rector 2.6.7 is the first release bundling php-parser ^5.9, so older versions would break the PFA tests in the lowest-dependencies CI job. Refs #224 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Sy8BcM8ivUEpu8uVm7wADn --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 57b039b..71d3cef 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ "phpunit/phpunit": "^13.3.1", "phpstan/phpstan": "^2.0", "tracy/tracy": "^2.10", - "rector/rector": "^2.0" + "rector/rector": "^2.6.7" }, "extra": { "branch-alias": {