Skip to content

THRIFT-5990: Emit native return types on generated PHP struct methods#3472

Merged
sveneld merged 1 commit into
apache:masterfrom
sveneld:THRIFT-5990
May 14, 2026
Merged

THRIFT-5990: Emit native return types on generated PHP struct methods#3472
sveneld merged 1 commit into
apache:masterfrom
sveneld:THRIFT-5990

Conversation

@sveneld
Copy link
Copy Markdown
Contributor

@sveneld sveneld commented May 13, 2026

Phase G2 of the 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:

- 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.

  • Did you create an Apache Jira ticket? — THRIFT-5990
  • PR title follows the pattern "THRIFT-NNNN: describe my issue"
  • 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

Client: php

Phase G2 of THRIFT-5960 PHP modernization. The PHP code generator
now emits native return types on the public struct surface:

- getName(): string
- read(TProtocol $input): int / read($input): int for binary_inline
  mode (which reads from a raw TTransport buffer, not a TProtocol)
- write(TProtocol $output): int / write(string &$output): int for
  binary_inline mode
- validateForRead(): void / validateForWrite(): void
- jsonSerialize(): mixed (drops the now-unnecessary
  #[ReturnTypeWillChange] attribute since the project targets PHP 8.1+)

To make the TProtocol parameter type on read/write compatible with
LSP for generated subclasses extending TBase, the abstract signatures
on `TBase::read` / `TBase::write` are tightened in lock-step:

  abstract public function read(TProtocol $input): int;
  abstract public function write(TProtocol $output): int;

Three hand-written test fixtures extending TBase
(TestSerializerStruct, ComplexStruct, NestedStruct) are updated to
match the new TBase signatures. Generator fixtures under
lib/php/test/Resources/packages/ are git-ignored and regenerated by
`make stubs` in CI.

Generated-by: Claude Opus 4.7
@sveneld sveneld marked this pull request as ready for review May 14, 2026 05:18
@sveneld sveneld merged commit 1936349 into apache:master May 14, 2026
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant