sveneld opened a new pull request, #3462:
URL: https://github.com/apache/thrift/pull/3462
Completes the typing trajectory started by THRIFT-5976/5977/5978/5979/5980
by migrating the public methods on `lib/php/lib/Protocol/` from PHPDoc-only
annotations to native PHP parameter and return types. This closes the last of
the three core hierarchies (Transport done in 5980, Server/Factory in 5979,
Protocol now).
## Library
- **`TProtocol` abstract** — all `write*: int` methods typed with scalar
params; all `read*` typed with nullable by-ref params (e.g.
`readMessageBegin(?string &$name, ?int &$type, ?int &$seqid): int`). `skip(int
$type): int` and static `skipBinary(TTransport $itrans, int $type): int` typed.
- **All 7 concrete protocols** — `TBinaryProtocol`,
`TBinaryProtocolAccelerated`, `TCompactProtocol`, `TJSONProtocol`,
`TSimpleJSONProtocol`, `TMultiplexedProtocol`, `TProtocolDecorator` — overrides
typed to match the abstract.
- **`TJSONProtocol` return values** — write methods return `0` (previously
implicit null) and read methods return `1` to match the previous `bool true`
cast that generated callers accumulate as a byte count.
- **`TCompactProtocol`** — `$boolFid` and `$lastFid` switched from `?int` to
`int` (defaults `0`) so the typed `writeFieldHeader` call chain works without
null-handling. `$this->trans->write(...)` calls drop the vestigial second-arg
byte-count (`TTransport::write` is single-arg post-5980).
- **`TBinaryProtocol`** — same `trans->write(..., len)` cleanup;
reads/writes match the abstract verbatim.
- **`StoredMessageProtocol::readMessageBegin`** matches
`TProtocolDecorator`'s new typed signature.
- **`phpstan-baseline.neon`** — removed four "return statement is missing"
suppressions that are now resolved.
## Tests
- **`TBinaryProtocolTest` / `TCompactProtocolTest`** — dropped the vestigial
byte-count second element from `->with(buf, N)` mock argument matchers and from
`expectedWriteArgs` / `expectedWriteCallParam` data providers, because
`TTransport::write` is single-arg.
- **`TCompactProtocolTest::testWriteByte` / `testWriteUByte`** — removed
`'lowercase'` / `'upercase'` cases that documented PHP's pre-strict silent
string→int coercion (`'a'` → `0`); `writeByte`/`writeUByte` now require `int`
caller-side.
- **`TCompactProtocolTest::testWriteFieldBegin`** — `'list'` case
`expectedBoolFid` updated `null` → `0` to match the typed `int` default.
- **`TProtocolDecoratorTest` rewritten** — write decoration retains the
data-provider style with type-matched argument values; read methods split into
individual tests that pass typed `null` variables by-ref.
- **`TSimpleJSONProtocolTest::testRead` rewritten** — dispatches by shape
(`none` / `name` / `message` / `field` / `map` / `collection` / `scalar-*`) so
it can pass typed by-ref nulls to the strict-typed read signatures and still
assert `TException("Not implemented")` propagates.
## Stats
14 files, +758 / −623 (net −123 LOC of PHPDoc), single squashed commit.
Part of [THRIFT-5960](https://issues.apache.org/jira/browse/THRIFT-5960) —
modernizing PHP runtime library typing.
- [x] Did you create an [Apache
Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? —
[THRIFT-5981](https://issues.apache.org/jira/browse/THRIFT-5981)
- [x] If a ticket exists: Does your pull request title follow the pattern
"THRIFT-NNNN: describe my issue"? — yes
- [x] Did you squash your changes to a single commit? — yes
- [x] Did you do your best to avoid breaking changes? — yes; signatures
match the existing semantics. Two minor test-shape rewrites are necessary
because PHPUnit can't spread strings into typed by-ref params, and
writeByte/writeUByte no longer accept char strings (caller must `ord()` them).
- [ ] If your change does not involve any code, include `[skip ci]` anywhere
in the commit message to free up build resources. — N/A (code change)
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]
