sveneld opened a new pull request, #3483: URL: https://github.com/apache/thrift/pull/3483
Update the PHP generator to emit native parameter and return types on every generated service-level method. Completes the typing pass on the generator side after [THRIFT-5986](https://issues.apache.org/jira/browse/THRIFT-5986) (declare strict_types), [THRIFT-5990](https://issues.apache.org/jira/browse/THRIFT-5990) (struct return types), and [THRIFT-5991](https://issues.apache.org/jira/browse/THRIFT-5991) (struct property types). ## Generated signatures (before → after) ```diff -public function testString($thing) // Interface +public function testString(?string $thing): ?string -public function __construct($input, $output = null) // Client ctor +public function __construct(TProtocol $input, ?TProtocol $output = null) -public function send_testString($thing) // Client +public function send_testString(?string $thing): void -public function recv_testString() // Client +public function recv_testString(): ?string -public function __construct($handler) // Processor ctor +public function __construct(object $handler) -public function process($input, $output) // Processor +public function process(TProtocol $input, TProtocol $output): bool -protected function process_testString($seqid, $input, $output) +protected function process_testString(int $seqid, TProtocol $input, TProtocol $output): void -public function __construct($impl) // Rest ctor +public function __construct(object $impl) -public function testString($request) // Rest method +public function testString(array $request): ?string ``` ## Implementation - New helper `type_to_return(t_type*)` returning `: void` or `: ?T`. Shares the existing `type_to_native()` mapping introduced by THRIFT-5991. - `function_signature()` now appends `type_to_return()` for the return clause. - `argument_list()` now emits `?type_to_native()` on every parameter — previously only struct/container parameters were typed. - `send_*` constructs a synthetic `t_function(g_type_void, ...)` so `function_signature()` produces the correct `: void` for the writer half. - Processor `process()` early "method not found" path now `return false;` instead of bare `return;` to satisfy `: bool`. - Rest method emission gates `return ` on whether the underlying impl is void. ## Drive-by cleanup `ThriftClassLoader::findFileInApcu()` (merged in [THRIFT-6001](https://issues.apache.org/jira/browse/THRIFT-6001)) was flagged by PHPStan: the trailing `$file !== false ? $file : null` was unreachable because the if-branch reassigns `$file` to `findFile()` (`?string`). Rewrote the body to a clear two-step fetch-or-recompute and narrowed the return via `is_string()`. ## Validation (Docker `thrift-php-dev:local` + `thrift/thrift-build:ubuntu-focal`) - Built compiler with the patched generator. - Regenerated all 6 fixture variants under `lib/php/test/Resources/packages/` (gitignored — regen happens in CI). - `phpcs` — 0 errors - `phpstan` (level 5) — 0 errors - `phpunit` Unit Suite — 637 tests, 0 failures - `phpunit` Integration Suite — 108 tests, 0 failures Part of umbrella [THRIFT-5960](https://issues.apache.org/jira/browse/THRIFT-5960). - [x] Apache Jira ticket — [THRIFT-6004](https://issues.apache.org/jira/browse/THRIFT-6004) - [x] PR title follows the pattern "THRIFT-NNNN: …" - [x] Squashed to a single commit - [ ] Did you do your best to avoid breaking changes? — third-party user code calling generated services with the wrong types will now fail loudly under strict types. Required regen of generated code against the new compiler. Generated-by: Claude Opus 4.7 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
