From 28b03e36714a559ee526d2b247c36f47cbe3af3f Mon Sep 17 00:00:00 2001 From: "Reymart A. Calicdan" Date: Mon, 10 Aug 2026 19:45:23 +0800 Subject: [PATCH] Fix critical edgecases on tyephp eagerly validating unknown type annotation and causing it to fallback to is_object check --- src/Validator/GenericValidator.php | 36 +++++++++------- src/Validator/IdentifierValidator.php | 16 ++++++- tests/Fixtures/Types/DatabaseDriverMap.php | 6 +-- .../IgnoreUnrecognizeDoctypeTest.php | 39 +++++++++++++++++ tests/TypeChecking/KeyOfValueOfTest.php | 42 ++++++++++++------- 5 files changed, 104 insertions(+), 35 deletions(-) create mode 100644 tests/TypeChecking/IgnoreUnrecognizeDoctypeTest.php diff --git a/src/Validator/GenericValidator.php b/src/Validator/GenericValidator.php index 0825d7a..5ef417f 100644 --- a/src/Validator/GenericValidator.php +++ b/src/Validator/GenericValidator.php @@ -4,10 +4,10 @@ namespace TypePHP\Validator; +use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprIntegerNode; +use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstFetchNode; use PHPStan\PhpDocParser\Ast\Type\ArrayShapeNode; -use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode; -use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprIntegerNode; use PHPStan\PhpDocParser\Ast\Type\ConstTypeNode; use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode; use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode; @@ -29,7 +29,7 @@ final class GenericValidator implements TypeValidatorInterface private static array $constantCache = []; /** - * @var array> + * @var array> */ private static array $enumKeyCache = []; @@ -78,7 +78,7 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont $constName = $constExpr->name; $cacheKey = $fqcn !== '' ? "$fqcn::$constName" : $constName; - if (!\array_key_exists($cacheKey, self::$constantCache)) { + if (! \array_key_exists($cacheKey, self::$constantCache)) { $constValue = false; if ($fqcn !== '') { if (class_exists($fqcn) || interface_exists($fqcn)) { @@ -101,21 +101,23 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont $constValue = self::$constantCache[$cacheKey]; if (\is_array($constValue)) { - if ((!\is_int($value) && !\is_string($value)) || !\array_key_exists($value, $constValue)) { + if ((! \is_int($value) && ! \is_string($value)) || ! \array_key_exists($value, $constValue)) { return ErrorFactory::createError($context . " must be a key of $cacheKey, " . TypeFormatter::formatGivenValue($value) . ' given'); } + return null; } } elseif ($targetType instanceof IdentifierTypeNode) { $enumClass = $targetType->name; if (ClassNameValidator::isValid($enumClass) && enum_exists($enumClass)) { - if (!isset(self::$enumKeyCache[$enumClass])) { - self::$enumKeyCache[$enumClass] = array_map(fn($case) => $case->name, $enumClass::cases()); + if (! isset(self::$enumKeyCache[$enumClass])) { + self::$enumKeyCache[$enumClass] = array_map(fn ($case) => $case->name, $enumClass::cases()); } - if (!\in_array($value, self::$enumKeyCache[$enumClass], true)) { + if (! \in_array($value, self::$enumKeyCache[$enumClass], true)) { return ErrorFactory::createError($context . " must be a key of enum $enumClass, " . TypeFormatter::formatGivenValue($value) . ' given'); } + return null; } } elseif ($targetType instanceof ArrayShapeNode) { @@ -130,9 +132,10 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont } } - if (!\in_array($value, $validKeys, true)) { + if (! \in_array($value, $validKeys, true)) { return ErrorFactory::createError($context . ' must be a key of the specified array shape, ' . TypeFormatter::formatGivenValue($value) . ' given'); } + return null; } @@ -146,7 +149,7 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont * 1. Array Constants: If T is a class constant (e.g., self::DRIVER_MAP), it safely reflects the * target class to bypass visibility restrictions (private/protected), caches the array in memory, * and verifies that the provided value exists as a value in that array. - * 2. Enums: If T is a Backed Enum identifier, it extracts and caches the enum case backing values, + * 2. Enums: If T is a Backed Enum identifier, it extracts and caches the enum case backing values, * then verifies that the provided value matches a valid case value. * 3. Fallback: Returns null gracefully for unresolvable or unsupported structures. */ @@ -160,7 +163,7 @@ private function validateValueOf(mixed $value, GenericTypeNode $node, string $co $constName = $constExpr->name; $cacheKey = $fqcn !== '' ? "$fqcn::$constName" : $constName; - if (!\array_key_exists($cacheKey, self::$constantCache)) { + if (! \array_key_exists($cacheKey, self::$constantCache)) { $constValue = false; if ($fqcn !== '') { if (class_exists($fqcn) || interface_exists($fqcn)) { @@ -183,27 +186,30 @@ private function validateValueOf(mixed $value, GenericTypeNode $node, string $co $constValue = self::$constantCache[$cacheKey]; if (\is_array($constValue)) { - if (!\in_array($value, $constValue, true)) { + if (! \in_array($value, $constValue, true)) { return ErrorFactory::createError($context . " must be a value of $cacheKey, " . TypeFormatter::formatGivenValue($value) . ' given'); } + return null; } } elseif ($targetType instanceof IdentifierTypeNode) { $enumClass = $targetType->name; if (ClassNameValidator::isValid($enumClass) && enum_exists($enumClass) && is_subclass_of($enumClass, \BackedEnum::class)) { - if (!isset(self::$enumValueCache[$enumClass])) { - self::$enumValueCache[$enumClass] = array_map(fn($case) => $case->value, $enumClass::cases()); + if (! isset(self::$enumValueCache[$enumClass])) { + self::$enumValueCache[$enumClass] = array_map(fn ($case) => $case->value, $enumClass::cases()); } - if (!\in_array($value, self::$enumValueCache[$enumClass], true)) { + if (! \in_array($value, self::$enumValueCache[$enumClass], true)) { return ErrorFactory::createError($context . " must be a value of enum $enumClass, " . TypeFormatter::formatGivenValue($value) . ' given'); } + return null; } } return null; } + /** * Validates integer ranges (e.g. int<1, 100> or int). */ diff --git a/src/Validator/IdentifierValidator.php b/src/Validator/IdentifierValidator.php index bab3ba0..c820172 100644 --- a/src/Validator/IdentifierValidator.php +++ b/src/Validator/IdentifierValidator.php @@ -6,6 +6,7 @@ use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode; use PHPStan\PhpDocParser\Ast\Type\TypeNode; +use TypePHP\Internal\ClassNameValidator; use TypePHP\Internal\ErrorFactory; use TypePHP\Internal\ErrorMessage; use TypePHP\Internal\TypeFormatter; @@ -71,7 +72,7 @@ public function validate(mixed $value, TypeNode $node, string $context, TypeVali 'open-resource' => \is_resource($value), 'closed-resource' => ! \is_resource($value) && get_debug_type($value) === 'resource (closed)', - default => \is_object($value) && is_a($value, $identifierNode->name), + default => $this->validateClassOrIgnore($value, $identifierNode->name), }; if (! $ok) { @@ -80,4 +81,17 @@ public function validate(mixed $value, TypeNode $node, string $context, TypeVali return null; } + + /** + * Enforces strict object/class checks for valid PHP class identifiers (e.g. User, NonExistentClass), + * but gracefully ignores invalid class syntax (e.g. madeup-type, custom-tag-name). + */ + private function validateClassOrIgnore(mixed $value, string $name): bool + { + if (! ClassNameValidator::isValid($name)) { + return true; + } + + return \is_object($value) && is_a($value, $name); + } } diff --git a/tests/Fixtures/Types/DatabaseDriverMap.php b/tests/Fixtures/Types/DatabaseDriverMap.php index d7fa025..0da64b9 100644 --- a/tests/Fixtures/Types/DatabaseDriverMap.php +++ b/tests/Fixtures/Types/DatabaseDriverMap.php @@ -7,12 +7,12 @@ class DatabaseDriverMap { private const DRIVER_MAP = [ - 'pdo_mysql' => 'PDO\MySQL\Driver', + 'pdo_mysql' => 'PDO\MySQL\Driver', 'pdo_sqlite' => 'PDO\SQLite\Driver', ]; public const PUBLIC_MAP = [ - 'read' => 1, + 'read' => 1, 'write' => 2, ]; @@ -60,4 +60,4 @@ public static function checkArrayShapeKey(string $key): string { return $key; } -} \ No newline at end of file +} diff --git a/tests/TypeChecking/IgnoreUnrecognizeDoctypeTest.php b/tests/TypeChecking/IgnoreUnrecognizeDoctypeTest.php new file mode 100644 index 0000000..0cb2978 --- /dev/null +++ b/tests/TypeChecking/IgnoreUnrecognizeDoctypeTest.php @@ -0,0 +1,39 @@ +toBe('special-type'); +}); + +test('strictly validates valid class syntax even if class does not exist at runtime', function () { + expect(fn () => testNonExistentClassType('hello')) + ->toThrow(TypeError::class, 'must be of type NonExistentClass, string \'hello\' given') + ; + + expect(fn () => testNonExistentClassType(new stdClass())) + ->toThrow(TypeError::class, 'must be of type NonExistentClass, stdClass given') + ; +}); diff --git a/tests/TypeChecking/KeyOfValueOfTest.php b/tests/TypeChecking/KeyOfValueOfTest.php index caf6ceb..18a0502 100644 --- a/tests/TypeChecking/KeyOfValueOfTest.php +++ b/tests/TypeChecking/KeyOfValueOfTest.php @@ -43,7 +43,8 @@ function testEnumValueOf(string $statusValue): string test('throws TypeError on invalid key-of on a private constant from a static method', function () { expect(fn () => DatabaseDriverMap::checkStaticDriverKey('pdo_pgsql')) - ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP'); + ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP') + ; }); test('accepts valid key-of on a private constant from an instance method', function () { @@ -53,18 +54,20 @@ function testEnumValueOf(string $statusValue): string test('throws TypeError on invalid key-of on a public constant from a private method', function () { $dbMap = new DatabaseDriverMap(); - + expect($dbMap->proxyPrivateMethod('read'))->toBe('read'); expect(fn () => $dbMap->proxyPrivateMethod('delete')) - ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP'); + ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP') + ; }); test('accepts valid key-of on a public constant from an external function', function () { expect(testExternalPublicConstKey('write'))->toBe('write'); expect(fn () => testExternalPublicConstKey('execute')) - ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP'); + ->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP') + ; }); test('accepts valid value-of on a private constant array', function () { @@ -73,7 +76,8 @@ function testEnumValueOf(string $statusValue): string test('throws TypeError on invalid value-of on a private constant array', function () { expect(fn () => DatabaseDriverMap::checkStaticDriverValue('PDO\PgSQL\Driver')) - ->toThrow(TypeError::class, 'must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP'); + ->toThrow(TypeError::class, 'must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP') + ; }); }); @@ -85,10 +89,12 @@ function testEnumValueOf(string $statusValue): string expect(testEnumKeyOf('Pending'))->toBe('Pending'); expect(fn () => testEnumKeyOf('Archived')) - ->toThrow(TypeError::class, 'must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum'); - + ->toThrow(TypeError::class, 'must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum') + ; + expect(fn () => testEnumKeyOf('active')) - ->toThrow(TypeError::class, "must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'active' given"); + ->toThrow(TypeError::class, "must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'active' given") + ; }); test('value-of strictly checks against the Enum BACKING VALUES', function () { @@ -96,10 +102,12 @@ function testEnumValueOf(string $statusValue): string expect(testEnumValueOf('pending'))->toBe('pending'); expect(fn () => testEnumValueOf('archived')) - ->toThrow(TypeError::class, 'must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum'); - + ->toThrow(TypeError::class, 'must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum') + ; + expect(fn () => testEnumValueOf('Active')) - ->toThrow(TypeError::class, "must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'Active' given"); + ->toThrow(TypeError::class, "must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'Active' given") + ; }); }); @@ -113,7 +121,8 @@ function testEnumValueOf(string $statusValue): string test('throws TypeError on invalid string key of an inline array shape', function () { expect(fn () => DatabaseDriverMap::checkArrayShapeKey('invalid_key')) - ->toThrow(TypeError::class, 'must be a key of the specified array shape'); + ->toThrow(TypeError::class, 'must be a key of the specified array shape') + ; }); }); @@ -125,15 +134,16 @@ function testEnumValueOf(string $statusValue): string expect($conn->connect([ 'driver' => 'pdo_mysql', - 'driverClass' => 'PDO\MySQL\Driver' + 'driverClass' => 'PDO\MySQL\Driver', ]))->toBeTrue(); expect(fn () => $conn->connect(['driver' => 'pdo_pgsql'])) - ->toThrow(TypeError::class, "['driver'] must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP"); + ->toThrow(TypeError::class, "['driver'] must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP") + ; expect(fn () => $conn->connect([ 'driver' => 'pdo_mysql', - 'driverClass' => 'PDO\PgSQL\Driver' + 'driverClass' => 'PDO\PgSQL\Driver', ]))->toThrow(TypeError::class, "['driverClass'] must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP"); }); @@ -147,4 +157,4 @@ function testEnumValueOf(string $statusValue): string }); }); -}); \ No newline at end of file +});