sveneld opened a new pull request, #3456:
URL: https://github.com/apache/thrift/pull/3456

   ## Summary
   
   Follow-up to 
[THRIFT-5976](https://issues.apache.org/jira/browse/THRIFT-5976) (typed 
properties). After that work, many constructors in `lib/php/lib/` were pure 1:1 
argument→property assignments — exactly what PHP 8 constructor property 
promotion replaces. Apply promotion across the library: cleaner declarations, 
no behavioural change, no LSP impact.
   
   JIRA: https://issues.apache.org/jira/browse/THRIFT-5977 (related to 
[THRIFT-5960](https://issues.apache.org/jira/browse/THRIFT-5960)).
   
   ## Scope
   
   ### Full promote (ctor body becomes empty or a single parent call)
   
   * `Factory/TBinaryProtocolFactory` — `strictRead`, `strictWrite`.
   * `Server/TServer` (6 properties) — `processor`, `transport`, four factory 
parameters.
   * `Server/TServerSocket` — `host`, `port` (`acceptTimeout` retained as a 
regular property, mutated via `setAcceptTimeout()`).
   * `Transport/TMemoryBuffer` — `buf`.
   * `Transport/TBufferedTransport` — `transport`, `rBufSize`, `wBufSize`.
   * `Transport/TFramedTransport` — `transport`, `read`, `write`.
   * `ClassLoader/ThriftClassLoader` — `apcu`, `apcu_prefix`.
   
   ### Partial promote (body keeps `parent::__construct(...)` or one 
non-trivial line)
   
   * `Protocol/TBinaryProtocol` — `strictRead`, `strictWrite`; `$trans` 
forwarded to parent.
   * `Protocol/TMultiplexedProtocol` — `serviceName`; `$protocol` forwarded.
   * `StoredMessageProtocol` — `fname`, `mtype`, `rseqid`; `$protocol` 
forwarded.
   * `Transport/TSocket` — `host`, `port`, `persist`; `debugHandler` retains 
the `?: 'error_log'` fallback in the body.
   * `Transport/THttpClient` — `host`, `port`, `scheme`, `context`; `uri` keeps 
its prefix-normalisation in the body.
   * `Transport/TCurlClient` — `host`, `port`, `scheme`; same `uri` logic.
   
   ### Bonus fixes surfaced while promoting
   
   * `Transport/TSSLSocket` was bypassing `parent::__construct` entirely. Now 
properly delegates `parent::__construct(self::getSSLHost($host), $port, false, 
$debugHandler)`, ensuring TSocket's promoted properties get initialised. 
`getSSLHost` made `private static` since it doesn't use `$this`.
   * `Server/TSSLServerSocket` — same pattern: 
`parent::__construct(self::getSSLHost($host), $port)`; `getSSLHost` made 
`public static`.
   
   ## Out of scope (intentionally skipped)
   
   * `TPhpStream` — bitwise (`$mode & MODE_R/MODE_W`) ctor logic, not 1:1.
   * Exception hierarchy (`TException`, `TApplicationException`, 
`TProtocolException`, `TTransportException`) — dual-mode bridge constructor.
   * `TBase`, `TSimpleJSONProtocol`, `TJSONProtocol` — non-trivial init bodies 
(`$context = new Context()`, `$reader = new LookaheadReader()`, conditional 
`spec/vals` processing).
   * `TSocketPool` — ctor builds `$servers` and normalises `$ports`; not 1:1.
   * `TBinaryProtocolAccelerated` — passes through to parent unchanged after 
the parent promotion.
   
   ## Verification (PHP 8.4 / composer 2 / phpstan 1.12 / phpunit 13 in docker)
   
   ```
   composer phpstan         OK (level 1, baseline unchanged)
   vendor/bin/phpcs         OK
   phpunit Unit suite       629 tests, 1907 assertions, 0 errors
   phpunit Integration      106 tests,  166 assertions, 0 errors
   ```
   
   ## Checklist
   
   - [x] Did you create an [Apache 
Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? — 
THRIFT-5977
   - [x] If a ticket exists: Does your pull request title follow the pattern 
"THRIFT-NNNN: describe my issue"?
   - [x] Did you squash your changes to a single commit?
   - [x] Did you do your best to avoid breaking changes? — yes; promotion is 
purely syntactic. The only observable diff is that `TSSLSocket` and 
`TSSLServerSocket` now correctly call `parent::__construct`, which initialises 
previously-uninitialised inherited typed properties (no functional behaviour 
change for callers since the SSL classes overrode every method that read those 
properties).
   - [ ] If your change does not involve any code, include `[skip ci]` anywhere 
in the commit message to free up build resources.
   


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

Reply via email to