Skip to content

Commit a3e76a3

Browse files
rcalicdanrcalicdan
authored
Fix critical edgecases on tyephp eagerly validating unknown type annotation and causing it to fallback to is_object check (#7)
1 parent 7ceb241 commit a3e76a3

5 files changed

Lines changed: 104 additions & 35 deletions

File tree

‎src/Validator/GenericValidator.php‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44

55
namespace TypePHP\Validator;
66

7+
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprIntegerNode;
8+
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode;
79
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstFetchNode;
810
use PHPStan\PhpDocParser\Ast\Type\ArrayShapeNode;
9-
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode;
10-
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprIntegerNode;
1111
use PHPStan\PhpDocParser\Ast\Type\ConstTypeNode;
1212
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
1313
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
@@ -29,7 +29,7 @@ final class GenericValidator implements TypeValidatorInterface
2929
private static array $constantCache = [];
3030

3131
/**
32-
* @var array<string, array<int, string>>
32+
* @var array<string, array<int, string>>
3333
*/
3434
private static array $enumKeyCache = [];
3535

@@ -78,7 +78,7 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont
7878
$constName = $constExpr->name;
7979
$cacheKey = $fqcn !== '' ? "$fqcn::$constName" : $constName;
8080

81-
if (!\array_key_exists($cacheKey, self::$constantCache)) {
81+
if (! \array_key_exists($cacheKey, self::$constantCache)) {
8282
$constValue = false;
8383
if ($fqcn !== '') {
8484
if (class_exists($fqcn) || interface_exists($fqcn)) {
@@ -101,21 +101,23 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont
101101
$constValue = self::$constantCache[$cacheKey];
102102

103103
if (\is_array($constValue)) {
104-
if ((!\is_int($value) && !\is_string($value)) || !\array_key_exists($value, $constValue)) {
104+
if ((! \is_int($value) && ! \is_string($value)) || ! \array_key_exists($value, $constValue)) {
105105
return ErrorFactory::createError($context . " must be a key of $cacheKey, " . TypeFormatter::formatGivenValue($value) . ' given');
106106
}
107+
107108
return null;
108109
}
109110
} elseif ($targetType instanceof IdentifierTypeNode) {
110111
$enumClass = $targetType->name;
111112
if (ClassNameValidator::isValid($enumClass) && enum_exists($enumClass)) {
112-
if (!isset(self::$enumKeyCache[$enumClass])) {
113-
self::$enumKeyCache[$enumClass] = array_map(fn($case) => $case->name, $enumClass::cases());
113+
if (! isset(self::$enumKeyCache[$enumClass])) {
114+
self::$enumKeyCache[$enumClass] = array_map(fn ($case) => $case->name, $enumClass::cases());
114115
}
115116

116-
if (!\in_array($value, self::$enumKeyCache[$enumClass], true)) {
117+
if (! \in_array($value, self::$enumKeyCache[$enumClass], true)) {
117118
return ErrorFactory::createError($context . " must be a key of enum $enumClass, " . TypeFormatter::formatGivenValue($value) . ' given');
118119
}
120+
119121
return null;
120122
}
121123
} elseif ($targetType instanceof ArrayShapeNode) {
@@ -130,9 +132,10 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont
130132
}
131133
}
132134

133-
if (!\in_array($value, $validKeys, true)) {
135+
if (! \in_array($value, $validKeys, true)) {
134136
return ErrorFactory::createError($context . ' must be a key of the specified array shape, ' . TypeFormatter::formatGivenValue($value) . ' given');
135137
}
138+
136139
return null;
137140
}
138141

@@ -146,7 +149,7 @@ private function validateKeyOf(mixed $value, GenericTypeNode $node, string $cont
146149
* 1. Array Constants: If T is a class constant (e.g., self::DRIVER_MAP), it safely reflects the
147150
* target class to bypass visibility restrictions (private/protected), caches the array in memory,
148151
* and verifies that the provided value exists as a value in that array.
149-
* 2. Enums: If T is a Backed Enum identifier, it extracts and caches the enum case backing values,
152+
* 2. Enums: If T is a Backed Enum identifier, it extracts and caches the enum case backing values,
150153
* then verifies that the provided value matches a valid case value.
151154
* 3. Fallback: Returns null gracefully for unresolvable or unsupported structures.
152155
*/
@@ -160,7 +163,7 @@ private function validateValueOf(mixed $value, GenericTypeNode $node, string $co
160163
$constName = $constExpr->name;
161164
$cacheKey = $fqcn !== '' ? "$fqcn::$constName" : $constName;
162165

163-
if (!\array_key_exists($cacheKey, self::$constantCache)) {
166+
if (! \array_key_exists($cacheKey, self::$constantCache)) {
164167
$constValue = false;
165168
if ($fqcn !== '') {
166169
if (class_exists($fqcn) || interface_exists($fqcn)) {
@@ -183,27 +186,30 @@ private function validateValueOf(mixed $value, GenericTypeNode $node, string $co
183186
$constValue = self::$constantCache[$cacheKey];
184187

185188
if (\is_array($constValue)) {
186-
if (!\in_array($value, $constValue, true)) {
189+
if (! \in_array($value, $constValue, true)) {
187190
return ErrorFactory::createError($context . " must be a value of $cacheKey, " . TypeFormatter::formatGivenValue($value) . ' given');
188191
}
192+
189193
return null;
190194
}
191195
} elseif ($targetType instanceof IdentifierTypeNode) {
192196
$enumClass = $targetType->name;
193197
if (ClassNameValidator::isValid($enumClass) && enum_exists($enumClass) && is_subclass_of($enumClass, \BackedEnum::class)) {
194-
if (!isset(self::$enumValueCache[$enumClass])) {
195-
self::$enumValueCache[$enumClass] = array_map(fn($case) => $case->value, $enumClass::cases());
198+
if (! isset(self::$enumValueCache[$enumClass])) {
199+
self::$enumValueCache[$enumClass] = array_map(fn ($case) => $case->value, $enumClass::cases());
196200
}
197201

198-
if (!\in_array($value, self::$enumValueCache[$enumClass], true)) {
202+
if (! \in_array($value, self::$enumValueCache[$enumClass], true)) {
199203
return ErrorFactory::createError($context . " must be a value of enum $enumClass, " . TypeFormatter::formatGivenValue($value) . ' given');
200204
}
205+
201206
return null;
202207
}
203208
}
204209

205210
return null;
206211
}
212+
207213
/**
208214
* Validates integer ranges (e.g. int<1, 100> or int<min, max>).
209215
*/

‎src/Validator/IdentifierValidator.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
88
use PHPStan\PhpDocParser\Ast\Type\TypeNode;
9+
use TypePHP\Internal\ClassNameValidator;
910
use TypePHP\Internal\ErrorFactory;
1011
use TypePHP\Internal\ErrorMessage;
1112
use TypePHP\Internal\TypeFormatter;
@@ -71,7 +72,7 @@ public function validate(mixed $value, TypeNode $node, string $context, TypeVali
7172
'open-resource' => \is_resource($value),
7273
'closed-resource' => ! \is_resource($value) && get_debug_type($value) === 'resource (closed)',
7374

74-
default => \is_object($value) && is_a($value, $identifierNode->name),
75+
default => $this->validateClassOrIgnore($value, $identifierNode->name),
7576
};
7677

7778
if (! $ok) {
@@ -80,4 +81,17 @@ public function validate(mixed $value, TypeNode $node, string $context, TypeVali
8081

8182
return null;
8283
}
84+
85+
/**
86+
* Enforces strict object/class checks for valid PHP class identifiers (e.g. User, NonExistentClass),
87+
* but gracefully ignores invalid class syntax (e.g. madeup-type, custom-tag-name).
88+
*/
89+
private function validateClassOrIgnore(mixed $value, string $name): bool
90+
{
91+
if (! ClassNameValidator::isValid($name)) {
92+
return true;
93+
}
94+
95+
return \is_object($value) && is_a($value, $name);
96+
}
8397
}

‎tests/Fixtures/Types/DatabaseDriverMap.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
class DatabaseDriverMap
88
{
99
private const DRIVER_MAP = [
10-
'pdo_mysql' => 'PDO\MySQL\Driver',
10+
'pdo_mysql' => 'PDO\MySQL\Driver',
1111
'pdo_sqlite' => 'PDO\SQLite\Driver',
1212
];
1313

1414
public const PUBLIC_MAP = [
15-
'read' => 1,
15+
'read' => 1,
1616
'write' => 2,
1717
];
1818

@@ -60,4 +60,4 @@ public static function checkArrayShapeKey(string $key): string
6060
{
6161
return $key;
6262
}
63-
}
63+
}

‎tests/TypeChecking/IgnoreUnrecognizeDoctypeTest.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* Custom annotation with invalid class syntax (contains hyphens)
7+
*
8+
* @param madeup-type $specialType
9+
*/
10+
function testUnsupportedTypeSyntax(string $specialType): string
11+
{
12+
return $specialType;
13+
}
14+
15+
/**
16+
* Valid PHP class name syntax for a class that does not exist at runtime
17+
*
18+
* @param NonExistentClass $param
19+
*/
20+
function testNonExistentClassType(mixed $param): mixed
21+
{
22+
return $param;
23+
}
24+
25+
test('ignores custom unsupported type syntax with hyphens gracefully', function () {
26+
$result = testUnsupportedTypeSyntax('special-type');
27+
28+
expect($result)->toBe('special-type');
29+
});
30+
31+
test('strictly validates valid class syntax even if class does not exist at runtime', function () {
32+
expect(fn () => testNonExistentClassType('hello'))
33+
->toThrow(TypeError::class, 'must be of type NonExistentClass, string \'hello\' given')
34+
;
35+
36+
expect(fn () => testNonExistentClassType(new stdClass()))
37+
->toThrow(TypeError::class, 'must be of type NonExistentClass, stdClass given')
38+
;
39+
});

‎tests/TypeChecking/KeyOfValueOfTest.php‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ function testEnumValueOf(string $statusValue): string
4343

4444
test('throws TypeError on invalid key-of on a private constant from a static method', function () {
4545
expect(fn () => DatabaseDriverMap::checkStaticDriverKey('pdo_pgsql'))
46-
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP');
46+
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP')
47+
;
4748
});
4849

4950
test('accepts valid key-of on a private constant from an instance method', function () {
@@ -53,18 +54,20 @@ function testEnumValueOf(string $statusValue): string
5354

5455
test('throws TypeError on invalid key-of on a public constant from a private method', function () {
5556
$dbMap = new DatabaseDriverMap();
56-
57+
5758
expect($dbMap->proxyPrivateMethod('read'))->toBe('read');
5859

5960
expect(fn () => $dbMap->proxyPrivateMethod('delete'))
60-
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP');
61+
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP')
62+
;
6163
});
6264

6365
test('accepts valid key-of on a public constant from an external function', function () {
6466
expect(testExternalPublicConstKey('write'))->toBe('write');
6567

6668
expect(fn () => testExternalPublicConstKey('execute'))
67-
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP');
69+
->toThrow(TypeError::class, 'must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::PUBLIC_MAP')
70+
;
6871
});
6972

7073
test('accepts valid value-of on a private constant array', function () {
@@ -73,7 +76,8 @@ function testEnumValueOf(string $statusValue): string
7376

7477
test('throws TypeError on invalid value-of on a private constant array', function () {
7578
expect(fn () => DatabaseDriverMap::checkStaticDriverValue('PDO\PgSQL\Driver'))
76-
->toThrow(TypeError::class, 'must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP');
79+
->toThrow(TypeError::class, 'must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP')
80+
;
7781
});
7882

7983
});
@@ -85,21 +89,25 @@ function testEnumValueOf(string $statusValue): string
8589
expect(testEnumKeyOf('Pending'))->toBe('Pending');
8690

8791
expect(fn () => testEnumKeyOf('Archived'))
88-
->toThrow(TypeError::class, 'must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum');
89-
92+
->toThrow(TypeError::class, 'must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum')
93+
;
94+
9095
expect(fn () => testEnumKeyOf('active'))
91-
->toThrow(TypeError::class, "must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'active' given");
96+
->toThrow(TypeError::class, "must be a key of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'active' given")
97+
;
9298
});
9399

94100
test('value-of<Enum> strictly checks against the Enum BACKING VALUES', function () {
95101
expect(testEnumValueOf('active'))->toBe('active');
96102
expect(testEnumValueOf('pending'))->toBe('pending');
97103

98104
expect(fn () => testEnumValueOf('archived'))
99-
->toThrow(TypeError::class, 'must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum');
100-
105+
->toThrow(TypeError::class, 'must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum')
106+
;
107+
101108
expect(fn () => testEnumValueOf('Active'))
102-
->toThrow(TypeError::class, "must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'Active' given");
109+
->toThrow(TypeError::class, "must be a value of enum TypePHP\Tests\Fixtures\Types\StatusEnum, string 'Active' given")
110+
;
103111
});
104112

105113
});
@@ -113,7 +121,8 @@ function testEnumValueOf(string $statusValue): string
113121

114122
test('throws TypeError on invalid string key of an inline array shape', function () {
115123
expect(fn () => DatabaseDriverMap::checkArrayShapeKey('invalid_key'))
116-
->toThrow(TypeError::class, 'must be a key of the specified array shape');
124+
->toThrow(TypeError::class, 'must be a key of the specified array shape')
125+
;
117126
});
118127

119128
});
@@ -125,15 +134,16 @@ function testEnumValueOf(string $statusValue): string
125134

126135
expect($conn->connect([
127136
'driver' => 'pdo_mysql',
128-
'driverClass' => 'PDO\MySQL\Driver'
137+
'driverClass' => 'PDO\MySQL\Driver',
129138
]))->toBeTrue();
130139

131140
expect(fn () => $conn->connect(['driver' => 'pdo_pgsql']))
132-
->toThrow(TypeError::class, "['driver'] must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP");
141+
->toThrow(TypeError::class, "['driver'] must be a key of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP")
142+
;
133143

134144
expect(fn () => $conn->connect([
135145
'driver' => 'pdo_mysql',
136-
'driverClass' => 'PDO\PgSQL\Driver'
146+
'driverClass' => 'PDO\PgSQL\Driver',
137147
]))->toThrow(TypeError::class, "['driverClass'] must be a value of TypePHP\Tests\Fixtures\Types\DatabaseDriverMap::DRIVER_MAP");
138148
});
139149

@@ -147,4 +157,4 @@ function testEnumValueOf(string $statusValue): string
147157
});
148158

149159
});
150-
});
160+
});

0 commit comments

Comments
 (0)