From 48d2245b17efd5ac6af58e56342f34f6baae7ece Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 15 May 2026 08:12:01 +0200 Subject: [PATCH] THRIFT-6005: Raise PHPStan level from 5 to 6 on PHP library Client: php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step lib/php/phpstan.neon level from 5 to 6 and address every issue PHPStan surfaces along the way. Library typing fixes -------------------- * TBinarySerializer::serialize(object $object): string * TBinarySerializer::deserialize(string $string_object, class-string $class_name, int $buffer_size = 8192): object * TServer::__construct(object $processor, ...) * TSocket / TSSLSocket __construct: documented $debugHandler as callable|string|null via PHPDoc. * TSocketPool::__construct: tightened to (array $hosts, int|array $ports, bool $persist, ...) with list shape PHPDoc. * TJSONProtocol static array properties get a native `array` declaration plus `@var` element-type docblocks. * TBase / TException internal helpers — readMap, readList, readStruct, writeMap, writeList, writeStruct, __construct, plus TException bridge ctor — gain `@param array<...>` shape documentation so PHPStan can reason about $spec / $vals / $var element types. Bug fixes --------- * TBase::__wakeup() previously round-tripped through $this->__construct(get_object_vars($this)). That call short-circuited inside the body (second parameter was null) but tripped strict-mode type checks under tighter array-shape PHPDoc. PHP restores object state automatically on unserialize, so the body is now empty with a comment. * ThriftClassLoader::findFileInApcu() now uses a clear two-step fetch-or-recompute pattern and narrows the return via is_string(), eliminating the unreachable else-arm flagged by PHPStan. Baseline regex updated to match the new "expects array, int|string given" wording for the four unreachable dead-code arms in TBase / TException writeMap that mirror Thrift IDL `map` semantics PHP arrays cannot represent. Final phase of the umbrella THRIFT-5960 PHP modernization. Generated-by: Claude Opus 4.7 --- lib/php/lib/Base/TBase.php | 29 ++++++++++++++++++- lib/php/lib/ClassLoader/ThriftClassLoader.php | 8 +++-- lib/php/lib/Exception/TException.php | 24 +++++++++++++++ lib/php/lib/Protocol/TJSONProtocol.php | 9 ++++-- lib/php/lib/Serializer/TBinarySerializer.php | 7 +++-- lib/php/lib/Server/TServer.php | 2 +- lib/php/lib/Transport/TSSLSocket.php | 3 +- lib/php/lib/Transport/TSocket.php | 2 ++ lib/php/lib/Transport/TSocketPool.php | 13 ++++----- lib/php/phpstan-baseline.neon | 4 +-- lib/php/phpstan.neon | 2 +- 11 files changed, 82 insertions(+), 21 deletions(-) diff --git a/lib/php/lib/Base/TBase.php b/lib/php/lib/Base/TBase.php index 0232fcae2b..ae575a1461 100644 --- a/lib/php/lib/Base/TBase.php +++ b/lib/php/lib/Base/TBase.php @@ -54,6 +54,10 @@ abstract public function read(TProtocol $input): int; abstract public function write(TProtocol $output): int; + /** + * @param array>|null $spec + * @param array|null $vals + */ public function __construct(?array $spec = null, ?array $vals = null) { if (is_array($spec) && is_array($vals)) { @@ -68,9 +72,15 @@ public function __construct(?array $spec = null, ?array $vals = null) public function __wakeup(): void { - $this->__construct(get_object_vars($this)); + // PHP restores instance state automatically on unserialize(); + // re-invoking __construct() with get_object_vars() was a no-op + // (it would short-circuit because the second parameter is null) + // and tripped strict-mode type checks on the first parameter. } + /** + * @param array $spec + */ private function readMap(mixed &$var, array $spec, TProtocol $input): int { $xfer = 0; @@ -139,6 +149,9 @@ private function readMap(mixed &$var, array $spec, TProtocol $input): int return $xfer; } + /** + * @param array $spec + */ private function readList(mixed &$var, array $spec, TProtocol $input, bool $set = false): int { $xfer = 0; @@ -194,6 +207,9 @@ private function readList(mixed &$var, array $spec, TProtocol $input, bool $set return $xfer; } + /** + * @param array> $spec + */ protected function readStruct(string $class, array $spec, TProtocol $input): int { $xfer = 0; @@ -245,6 +261,10 @@ protected function readStruct(string $class, array $spec, TProtocol $input): int return $xfer; } + /** + * @param array $var + * @param array $spec + */ private function writeMap(array $var, array $spec, TProtocol $output): int { $xfer = 0; @@ -305,6 +325,10 @@ private function writeMap(array $var, array $spec, TProtocol $output): int return $xfer; } + /** + * @param array $var + * @param array $spec + */ private function writeList(array $var, array $spec, TProtocol $output, bool $set = false): int { $xfer = 0; @@ -350,6 +374,9 @@ private function writeList(array $var, array $spec, TProtocol $output, bool $set return $xfer; } + /** + * @param array> $spec + */ protected function writeStruct(string $class, array $spec, TProtocol $output): int { $xfer = 0; diff --git a/lib/php/lib/ClassLoader/ThriftClassLoader.php b/lib/php/lib/ClassLoader/ThriftClassLoader.php index 13e501bd12..4cde1c764e 100644 --- a/lib/php/lib/ClassLoader/ThriftClassLoader.php +++ b/lib/php/lib/ClassLoader/ThriftClassLoader.php @@ -99,11 +99,13 @@ public function loadClass(string $class): void */ protected function findFileInApcu(string $class): ?string { - if (false === $file = apcu_fetch($this->apcu_prefix . $class)) { - apcu_store($this->apcu_prefix . $class, $file = $this->findFile($class)); + $file = apcu_fetch($this->apcu_prefix . $class); + if ($file === false) { + $file = $this->findFile($class); + apcu_store($this->apcu_prefix . $class, $file); } - return $file !== false ? $file : null; + return is_string($file) ? $file : null; } /** diff --git a/lib/php/lib/Exception/TException.php b/lib/php/lib/Exception/TException.php index f655bf38bf..9c188f2aa5 100644 --- a/lib/php/lib/Exception/TException.php +++ b/lib/php/lib/Exception/TException.php @@ -45,6 +45,10 @@ #[\AllowDynamicProperties] class TException extends \Exception { + /** + * @param string|array>|null $p1 + * @param int|array $p2 + */ public function __construct(string|array|null $p1 = null, int|array $p2 = 0) { if (is_array($p1) && is_array($p2)) { @@ -73,6 +77,9 @@ public function __construct(string|array|null $p1 = null, int|array $p2 = 0) TType::UUID => 'Uuid', ]; + /** + * @param array $spec + */ private function readMap(mixed &$var, array $spec, TProtocol $input): int { $xfer = 0; @@ -141,6 +148,9 @@ private function readMap(mixed &$var, array $spec, TProtocol $input): int return $xfer; } + /** + * @param array $spec + */ private function readList(mixed &$var, array $spec, TProtocol $input, bool $set = false): int { $xfer = 0; @@ -196,6 +206,9 @@ private function readList(mixed &$var, array $spec, TProtocol $input, bool $set return $xfer; } + /** + * @param array> $spec + */ protected function readStruct(string $class, array $spec, TProtocol $input): int { $xfer = 0; @@ -247,6 +260,10 @@ protected function readStruct(string $class, array $spec, TProtocol $input): int return $xfer; } + /** + * @param array $var + * @param array $spec + */ private function writeMap(array $var, array $spec, TProtocol $output): int { $xfer = 0; @@ -307,6 +324,10 @@ private function writeMap(array $var, array $spec, TProtocol $output): int return $xfer; } + /** + * @param array $var + * @param array $spec + */ private function writeList(array $var, array $spec, TProtocol $output, bool $set = false): int { $xfer = 0; @@ -352,6 +373,9 @@ private function writeList(array $var, array $spec, TProtocol $output, bool $set return $xfer; } + /** + * @param array> $spec + */ protected function writeStruct(string $class, array $spec, TProtocol $output): int { $xfer = 0; diff --git a/lib/php/lib/Protocol/TJSONProtocol.php b/lib/php/lib/Protocol/TJSONProtocol.php index 516c1ae045..8404758c4f 100644 --- a/lib/php/lib/Protocol/TJSONProtocol.php +++ b/lib/php/lib/Protocol/TJSONProtocol.php @@ -53,16 +53,19 @@ class TJSONProtocol extends TProtocol public const VERSION = 1; - public static $JSON_CHAR_TABLE = [ + /** @var array */ + public static array $JSON_CHAR_TABLE = [ /* 0 1 2 3 4 5 6 7 8 9 A B C D E F */ 0, 0, 0, 0, 0, 0, 0, 0, 'b', 't', 'n', 0, 'f', 'r', 0, 0, // 0 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 1 1, 1, '"', 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 2 ]; - public static $ESCAPE_CHARS = ['"', '\\', '/', "b", "f", "n", "r", "t"]; + /** @var list */ + public static array $ESCAPE_CHARS = ['"', '\\', '/', "b", "f", "n", "r", "t"]; - public static $ESCAPE_CHAR_VALS = [ + /** @var list */ + public static array $ESCAPE_CHAR_VALS = [ '"', '\\', '/', "\x08", "\f", "\n", "\r", "\t", ]; diff --git a/lib/php/lib/Serializer/TBinarySerializer.php b/lib/php/lib/Serializer/TBinarySerializer.php index 6f659d2dc2..d3849db53c 100644 --- a/lib/php/lib/Serializer/TBinarySerializer.php +++ b/lib/php/lib/Serializer/TBinarySerializer.php @@ -43,7 +43,7 @@ class TBinarySerializer // a transport in which to serialize an object. It has to // be a string. Otherwise we will break the compatibility with // normal deserialization. - public static function serialize($object) + public static function serialize(object $object): string { $transport = new TMemoryBuffer(); $protocol = new TBinaryProtocolAccelerated($transport); @@ -66,7 +66,10 @@ public static function serialize($object) return $transport->getBuffer(); } - public static function deserialize($string_object, $class_name, $buffer_size = 8192) + /** + * @param class-string $class_name + */ + public static function deserialize(string $string_object, string $class_name, int $buffer_size = 8192): object { $transport = new TMemoryBuffer(); $protocol = new TBinaryProtocolAccelerated($transport); diff --git a/lib/php/lib/Server/TServer.php b/lib/php/lib/Server/TServer.php index 219ef7f362..580091c842 100644 --- a/lib/php/lib/Server/TServer.php +++ b/lib/php/lib/Server/TServer.php @@ -15,7 +15,7 @@ abstract class TServer { public function __construct( - protected $processor, + protected object $processor, protected TServerTransport $transport, protected TTransportFactoryInterface $inputTransportFactory, protected TTransportFactoryInterface $outputTransportFactory, diff --git a/lib/php/lib/Transport/TSSLSocket.php b/lib/php/lib/Transport/TSSLSocket.php index 74ea6f68c9..a999bc9e6f 100644 --- a/lib/php/lib/Transport/TSSLSocket.php +++ b/lib/php/lib/Transport/TSSLSocket.php @@ -45,7 +45,8 @@ class TSSLSocket extends TSocket /** * Socket constructor * - * @param resource|null $context Stream context + * @param resource|null $context Stream context + * @param callable|string|null $debugHandler */ public function __construct( string $host = 'localhost', diff --git a/lib/php/lib/Transport/TSocket.php b/lib/php/lib/Transport/TSocket.php index 9fe3bc270b..79ec8c40a6 100644 --- a/lib/php/lib/Transport/TSocket.php +++ b/lib/php/lib/Transport/TSocket.php @@ -89,6 +89,8 @@ class TSocket extends TTransport /** * Socket constructor + * + * @param callable|string|null $debugHandler */ public function __construct( protected string $host = 'localhost', diff --git a/lib/php/lib/Transport/TSocketPool.php b/lib/php/lib/Transport/TSocketPool.php index 2c65174f7e..32ea291822 100644 --- a/lib/php/lib/Transport/TSocketPool.php +++ b/lib/php/lib/Transport/TSocketPool.php @@ -73,15 +73,14 @@ class TSocketPool extends TSocket /** * Socket pool constructor * - * @param array $hosts List of remote hostnames - * @param mixed $ports Array of remote ports, or a single common port - * @param bool $persist Whether to use a persistent socket - * @param mixed $debugHandler Function for error logging + * @param list $hosts List of remote hostnames + * @param int|list $ports Array of remote ports, or a single common port + * @param callable|string|null $debugHandler Function for error logging */ public function __construct( - $hosts = ['localhost'], - $ports = [9090], - $persist = false, + array $hosts = ['localhost'], + int|array $ports = [9090], + bool $persist = false, $debugHandler = null ) { parent::__construct('', 0, $persist, $debugHandler); diff --git a/lib/php/phpstan-baseline.neon b/lib/php/phpstan-baseline.neon index 0ec3c97f41..3445c6e3fc 100644 --- a/lib/php/phpstan-baseline.neon +++ b/lib/php/phpstan-baseline.neon @@ -5,10 +5,10 @@ parameters: # admit int|string keys so this arm is defensive dead code and # unreachable at runtime. - - message: "#^(Cannot call method write\\(\\) on \\(int\\|string\\)|Parameter \\#1 \\$var of method Thrift\\\\(Base\\\\TBase|Exception\\\\TException)::write(Map|List)\\(\\) expects array, \\(int\\|string\\) given)\\.$#" + message: "#^(Cannot call method write\\(\\) on int\\|string|Parameter \\#1 \\$var of method Thrift\\\\(Base\\\\TBase|Exception\\\\TException)::write(Map|List)\\(\\) expects array, int\\|string given)\\.$#" count: 4 path: lib/Base/TBase.php - - message: "#^(Cannot call method write\\(\\) on \\(int\\|string\\)|Parameter \\#1 \\$var of method Thrift\\\\(Base\\\\TBase|Exception\\\\TException)::write(Map|List)\\(\\) expects array, \\(int\\|string\\) given)\\.$#" + message: "#^(Cannot call method write\\(\\) on int\\|string|Parameter \\#1 \\$var of method Thrift\\\\(Base\\\\TBase|Exception\\\\TException)::write(Map|List)\\(\\) expects array, int\\|string given)\\.$#" count: 4 path: lib/Exception/TException.php diff --git a/lib/php/phpstan.neon b/lib/php/phpstan.neon index 4572deac0f..d7408de130 100644 --- a/lib/php/phpstan.neon +++ b/lib/php/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 5 + level: 6 paths: - lib bootstrapFiles: