sveneld opened a new pull request, #3472: URL: https://github.com/apache/thrift/pull/3472
Phase **G2** of the [THRIFT-5960](https://issues.apache.org/jira/browse/THRIFT-5960) PHP modernization. The PHP code generator now emits native return types on the public struct surface. **Generated method signatures:** | Method | Standard | Binary-inline mode | |---|---|---| | `getName()` | `: string` | `: string` | | `read()` | `(TProtocol $input): int` | `($input): int` (1) | | `write()` | `(TProtocol $output): int` | `(string &$output): int` | | `validateForRead/Write()` | `: void` | `: void` | | `jsonSerialize()` | `: mixed` (2) | n/a | (1) Binary-inline mode reads from a raw `TTransport` buffer, not a `TProtocol`, so the param remains untyped to avoid pulling another import into every generated file in this PR. (Can be typed in a follow-up.) (2) The previously-emitted `#[\ReturnTypeWillChange]` attribute is dropped since the project targets PHP 8.1+, which natively supports the `JsonSerializable::jsonSerialize(): mixed` interface. **Library change: tighten `TBase` abstracts in lock-step.** To make `TProtocol $input` / `TProtocol $output` in the generated subclass methods compatible with PHP's LSP rules, the abstract methods on `Thrift\Base\TBase` are tightened: ```diff - abstract public function read($input); - abstract public function write($output); + abstract public function read(TProtocol $input): int; + abstract public function write(TProtocol $output): int; ``` Any third-party generated PHP code from an older Thrift compiler that extends `TBase` will need to be regenerated against this compiler. **Hand-written test fixtures updated:** - `lib/php/test/Unit/Lib/Fixture/TestSerializerStruct.php` - `lib/php/test/Unit/Lib/Base/Fixture/ComplexStruct.php` - `lib/php/test/Unit/Lib/Base/Fixture/NestedStruct.php` These extend TBase and were updated to match the new abstract signatures. **Generator fixtures regeneration:** `lib/php/test/Resources/packages/` is git-ignored and is regenerated by `make stubs` in CI before tests run. No fixture files appear in this diff. **Validation (Docker `thrift/thrift-build:ubuntu-focal` + `thrift-php-dev:local`):** - Built compiler with the patched generator. - Regenerated all 6 fixture variants (435 PHP files). - `phpcs` — 0 errors. - `phpstan` (level 1) — 0 errors. - `phpunit` Unit Suite — 635 tests, 0 failures. - `phpunit` Integration Suite — 108 tests, 0 failures. **Scope:** This is phase G2. The remaining phase **G3** will emit native types on generated property declarations and the `__construct(?array $vals = null)` parameter, completing the generator side of THRIFT-5960. - [x] Did you create an Apache Jira ticket? — [THRIFT-5990](https://issues.apache.org/jira/browse/THRIFT-5990) - [x] PR title follows the pattern "THRIFT-NNNN: describe my issue" - [x] Squashed to a single commit - [ ] Did you do your best to avoid breaking changes? — `TBase` abstract signature change is a deliberate, scoped break that affects only direct extenders of `TBase`. In practice that's only Thrift-generated code, which is regenerated. 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]
