Skip to content

Commit 7b02221

Browse files
NiklanNiklan
authored
2.x: Fix negation types for Inspector (#860)
* Fix negation types for Inspector * Improve type for negated '::assertAll()'
1 parent 9a31538 commit 7b02221

File tree

2 files changed

+124
-47
lines changed

2 files changed

+124
-47
lines changed

‎src/Type/InspectorTypeExtension.php

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,23 @@ private function specifyAssertAll(MethodReflection $staticMethodReflection, Stat
107107
return new SpecifiedTypes();
108108
}
109109

110+
// In a negation context, we cannot precisely narrow types because we do
111+
// not know the exact logic of the callable function. This means we
112+
// cannot safely return 'mixed~iterable' since the value might still be
113+
// a valid iterable.
114+
//
115+
// For example, a negation context with an 'is_string(...)' callable
116+
// does not necessarily mean that the value cannot be an
117+
// 'iterable<int>'. In such cases, it is safer to skip type narrowing
118+
// altogether to prevent introducing new bugs into the code.
119+
if ($context->false()) {
120+
return new SpecifiedTypes();
121+
}
122+
110123
return $this->typeSpecifier->create(
111124
$node->getArgs()[1]->value,
112125
new IterableType(new MixedType(), new MixedType()),
113-
TypeSpecifierContext::createTruthy(),
126+
$context,
114127
$scope,
115128
);
116129
}
@@ -123,7 +136,7 @@ private function specifyAssertAllStrings(MethodReflection $staticMethodReflectio
123136
return $this->typeSpecifier->create(
124137
$node->getArgs()[0]->value,
125138
new IterableType(new MixedType(), new StringType()),
126-
TypeSpecifierContext::createTruthy(),
139+
$context,
127140
$scope,
128141
);
129142
}
@@ -140,7 +153,7 @@ private function specifyAssertAllStringable(MethodReflection $staticMethodReflec
140153
return $this->typeSpecifier->create(
141154
$node->getArgs()[0]->value,
142155
$newType,
143-
TypeSpecifierContext::createTruthy(),
156+
$context,
144157
$scope,
145158
);
146159
}
@@ -156,7 +169,7 @@ private function specifyAssertAllArrays(MethodReflection $staticMethodReflection
156169
return $this->typeSpecifier->create(
157170
$node->getArgs()[0]->value,
158171
$newType,
159-
TypeSpecifierContext::createTruthy(),
172+
$context,
160173
$scope,
161174
);
162175
}
@@ -176,7 +189,7 @@ private function specifyAssertStrictArray(MethodReflection $staticMethodReflecti
176189
return $this->typeSpecifier->create(
177190
$node->getArgs()[0]->value,
178191
$newType,
179-
TypeSpecifierContext::createTruthy(),
192+
$context,
180193
$scope,
181194
);
182195
}
@@ -197,7 +210,7 @@ private function specifyAssertAllStrictArrays(MethodReflection $staticMethodRefl
197210
return $this->typeSpecifier->create(
198211
$node->getArgs()[0]->value,
199212
$newType,
200-
TypeSpecifierContext::createTruthy(),
213+
$context,
201214
$scope,
202215
);
203216
}
@@ -241,7 +254,7 @@ private function specifyAssertAllHaveKey(MethodReflection $staticMethodReflectio
241254
TypeCombinator::intersect(...$possibleTypes),
242255
);
243256

244-
return $this->typeSpecifier->create($traversableArg, $newType, TypeSpecifierContext::createTruthy(), $scope);
257+
return $this->typeSpecifier->create($traversableArg, $newType, $context, $scope);
245258
}
246259

247260
/**
@@ -252,7 +265,7 @@ private function specifyAssertAllIntegers(MethodReflection $staticMethodReflecti
252265
return $this->typeSpecifier->create(
253266
$node->getArgs()[0]->value,
254267
new IterableType(new MixedType(), new IntegerType()),
255-
TypeSpecifierContext::createTruthy(),
268+
$context,
256269
$scope,
257270
);
258271
}
@@ -265,7 +278,7 @@ private function specifyAssertAllFloat(MethodReflection $staticMethodReflection,
265278
return $this->typeSpecifier->create(
266279
$node->getArgs()[0]->value,
267280
new IterableType(new MixedType(), new FloatType()),
268-
TypeSpecifierContext::createTruthy(),
281+
$context,
269282
$scope,
270283
);
271284
}
@@ -278,7 +291,7 @@ private function specifyAssertAllCallable(MethodReflection $staticMethodReflecti
278291
return $this->typeSpecifier->create(
279292
$node->getArgs()[0]->value,
280293
new IterableType(new MixedType(), new CallableType()),
281-
TypeSpecifierContext::createTruthy(),
294+
$context,
282295
$scope,
283296
);
284297
}
@@ -302,7 +315,7 @@ private function specifyAssertAllNotEmpty(MethodReflection $staticMethodReflecti
302315
return $this->typeSpecifier->create(
303316
$node->getArgs()[0]->value,
304317
$newType,
305-
TypeSpecifierContext::createTruthy(),
318+
$context,
306319
$scope,
307320
);
308321
}
@@ -315,7 +328,7 @@ private function specifyAssertAllNumeric(MethodReflection $staticMethodReflectio
315328
return $this->typeSpecifier->create(
316329
$node->getArgs()[0]->value,
317330
new IterableType(new MixedType(), new UnionType([new IntegerType(), new FloatType()])),
318-
TypeSpecifierContext::createTruthy(),
331+
$context,
319332
$scope,
320333
);
321334
}
@@ -328,7 +341,7 @@ private function specifyAssertAllMatch(MethodReflection $staticMethodReflection,
328341
return $this->typeSpecifier->create(
329342
$node->getArgs()[1]->value,
330343
new IterableType(new MixedType(), new StringType()),
331-
TypeSpecifierContext::createTruthy(),
344+
$context,
332345
$scope,
333346
);
334347
}
@@ -343,7 +356,7 @@ private function specifyAssertAllRegularExpressionMatch(MethodReflection $static
343356
// Drupal treats any non-string input in traversable as invalid
344357
// value, so it is possible to narrow type here.
345358
new IterableType(new MixedType(), new StringType()),
346-
TypeSpecifierContext::createTruthy(),
359+
$context,
347360
$scope,
348361
);
349362
}
@@ -376,7 +389,7 @@ private function specifyAssertAllObjects(MethodReflection $staticMethodReflectio
376389
return $this->typeSpecifier->create(
377390
$node->getArgs()[0]->value,
378391
new IterableType(new MixedType(), TypeCombinator::union(...$objectTypes)),
379-
TypeSpecifierContext::createTruthy(),
392+
$context,
380393
$scope,
381394
);
382395
}

‎tests/src/Type/data/inspector.php

Lines changed: 96 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,82 +12,146 @@ function mixed_function(): mixed {
1212
// Inspector::assertAll()
1313
$callable = fn (string $value): bool => $value === 'foo';
1414
$input = mixed_function();
15-
assert(Inspector::assertAll($callable, $input));
16-
assertType("iterable", $input);
15+
if (Inspector::assertAll($callable, $input)) {
16+
assertType("iterable", $input);
17+
}
18+
else {
19+
assertType('mixed', $input);
20+
}
1721

1822
$input = mixed_function();
1923
$callable = is_string(...);
20-
assert(Inspector::assertAll($callable, $input));
21-
assertType('iterable', $input);
24+
if (Inspector::assertAll($callable, $input)) {
25+
assertType('iterable', $input);
26+
}
27+
else {
28+
assertType('mixed', $input);
29+
}
2230

2331

2432
// Inspector::assertAllStrings()
2533
$input = mixed_function();
26-
assert(Inspector::assertAllStrings($input));
27-
assertType('iterable<string>', $input);
34+
if (Inspector::assertAllStrings($input)) {
35+
assertType('iterable<string>', $input);
36+
}
37+
else {
38+
assertType('mixed~iterable<string>', $input);
39+
}
2840

2941
// Inspector::assertAllStringable()
3042
$input = mixed_function();
31-
assert(Inspector::assertAllStringable($input));
32-
assertType('iterable<string|Stringable>', $input);
43+
if (Inspector::assertAllStringable($input)) {
44+
assertType('iterable<string|Stringable>', $input);
45+
}
46+
else {
47+
assertType('mixed~iterable<string|Stringable>', $input);
48+
}
3349

3450
// Inspector::assertAllArrays()
3551
$input = mixed_function();
36-
\assert(Inspector::assertAllArrays($input));
37-
assertType('iterable<array>', $input);
52+
if (Inspector::assertAllArrays($input)) {
53+
assertType('iterable<array>', $input);
54+
}
55+
else {
56+
assertType('mixed~iterable<array>', $input);
57+
}
3858

3959
// Inspector::assertStrictArray()
4060
$input = mixed_function();
41-
assert(Inspector::assertStrictArray($input));
42-
assertType('array<int<0, max>, mixed>', $input);
61+
if (Inspector::assertStrictArray($input)) {
62+
assertType('array<int<0, max>, mixed>', $input);
63+
}
64+
else {
65+
assertType('mixed~array<int<0, max>, mixed>', $input);
66+
}
4367

4468
// Inspector::assertAllStrictArrays()
4569
$input = mixed_function();
46-
assert(Inspector::assertAllStrictArrays($input));
47-
assertType('iterable<array<int<0, max>, mixed>>', $input);
70+
if (Inspector::assertAllStrictArrays($input)) {
71+
assertType('iterable<array<int<0, max>, mixed>>', $input);
72+
}
73+
else {
74+
assertType('mixed~iterable<array<int<0, max>, mixed>>', $input);
75+
}
4876

4977
// Inspector::assertAllHaveKey()
5078
$input = mixed_function();
51-
assert(Inspector::assertAllHaveKey($input, 'foo', 'baz'));
52-
assertType("iterable<non-empty-array&hasOffset('baz')&hasOffset('foo')>", $input);
79+
if (Inspector::assertAllHaveKey($input, 'foo', 'baz')) {
80+
assertType("iterable<non-empty-array&hasOffset('baz')&hasOffset('foo')>", $input);
81+
}
82+
else {
83+
assertType("mixed~iterable<non-empty-array&hasOffset('baz')&hasOffset('foo')>", $input);
84+
}
5385

5486
// Inspector::assertAllIntegers()
5587
$input = mixed_function();
56-
assert(Inspector::assertAllIntegers($input));
57-
assertType('iterable<int>', $input);
88+
if (Inspector::assertAllIntegers($input)) {
89+
assertType('iterable<int>', $input);
90+
}
91+
else {
92+
assertType('mixed~iterable<int>', $input);
93+
}
5894

5995
// Inspector::assertAllFloat()
6096
$input = mixed_function();
61-
assert(Inspector::assertAllFloat($input));
62-
assertType('iterable<float>', $input);
97+
if (Inspector::assertAllFloat($input)) {
98+
assertType('iterable<float>', $input);
99+
}
100+
else {
101+
assertType('mixed~iterable<float>', $input);
102+
}
63103

64104
// Inspector::assertAllCallable()
65105
$input = mixed_function();
66-
assert(Inspector::assertAllCallable($input));
67-
assertType('iterable<callable(): mixed>', $input);
106+
if (Inspector::assertAllCallable($input)) {
107+
assertType('iterable<callable(): mixed>', $input);
108+
}
109+
else {
110+
assertType('mixed~iterable<callable(): mixed>', $input);
111+
}
68112

69113
// Inspector::assertAllNotEmpty()
70114
$input = mixed_function();
71-
assert(Inspector::assertAllNotEmpty($input));
72-
assertType('iterable<float|int<min, -1>|int<1, max>|object|resource|non-empty-string|non-empty-array>', $input);
115+
if (Inspector::assertAllNotEmpty($input)) {
116+
assertType('iterable<float|int<min, -1>|int<1, max>|object|resource|non-empty-string|non-empty-array>', $input);
117+
}
118+
else {
119+
assertType('mixed~iterable<float|int<min, -1>|int<1, max>|object|resource|non-empty-string|non-empty-array>', $input);
120+
}
73121

74122
// Inspector::assertAllNumeric()
75123
$input = mixed_function();
76-
assert(Inspector::assertAllNumeric($input));
77-
assertType('iterable<float|int>', $input);
124+
if (Inspector::assertAllNumeric($input)) {
125+
assertType('iterable<float|int>', $input);
126+
}
127+
else {
128+
assertType('mixed~iterable<float|int>', $input);
129+
}
78130

79131
// Inspector::assertAllMatch()
80132
$pattern = 'foo';
81133
$input = mixed_function();
82-
assert(Inspector::assertAllMatch($pattern, $input, false));
83-
assertType('iterable<string>', $input);
134+
if (Inspector::assertAllMatch($pattern, $input, false)) {
135+
assertType('iterable<string>', $input);
136+
}
137+
else {
138+
assertType('mixed~iterable<string>', $input);
139+
}
84140

85141
// Inspector::assertAllRegularExpressionMatch()
86142
$input = mixed_function();
87-
assert(Inspector::assertAllRegularExpressionMatch($pattern, $input));
88-
assertType('iterable<string>', $input);
143+
if (Inspector::assertAllRegularExpressionMatch($pattern, $input)) {
144+
assertType('iterable<string>', $input);
145+
}
146+
else {
147+
assertType('mixed~iterable<string>', $input);
148+
}
89149

90150
// Inspector::assertAllObjects()
91151
$input = mixed_function();
92-
assert(Inspector::assertAllObjects($input, TranslatableMarkup::class, '\\Stringable', '\\Drupal\\jsonapi\\JsonApiResource\\ResourceIdentifier'));
93-
assertType('iterable<\Drupal\jsonapi\JsonApiResource\ResourceIdentifier|\Stringable>', $input);
152+
if (Inspector::assertAllObjects($input, TranslatableMarkup::class, '\\Stringable', '\\Drupal\\jsonapi\\JsonApiResource\\ResourceIdentifier')) {
153+
assertType('iterable<\Drupal\jsonapi\JsonApiResource\ResourceIdentifier|\Stringable>', $input);
154+
}
155+
else {
156+
assertType('mixed~iterable<\Drupal\jsonapi\JsonApiResource\ResourceIdentifier|\Stringable>', $input);
157+
}

0 commit comments

Comments
 (0)