Skip to content

Commit 9a31538

Browse files
mglamanmglaman
authored
Fixes for InspectorTypeExtension (#853)
* test for #852 * Update InspectorTypeExtension - assertAll accepts any callable and does not use the callable's return type but uses it like array_filter - assertAllHaveKey does not specify an array, just iterable - never type caused by intersect vs union but we lose offset exists * fix usage of HasOffsetType HasOffsetType is a compound type which represents an accessible object that has a key, not the constant string key offset itself * add exact test for project_browser * do not mark mixed as explicitly mixed
1 parent be02a3c commit 9a31538

File tree

4 files changed

+73
-42
lines changed

4 files changed

+73
-42
lines changed

‎src/Type/InspectorTypeExtension.php‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use PHPStan\Type\Accessory\NonEmptyArrayType;
1818
use PHPStan\Type\ArrayType;
1919
use PHPStan\Type\CallableType;
20-
use PHPStan\Type\ClosureType;
2120
use PHPStan\Type\Constant\ConstantStringType;
2221
use PHPStan\Type\FloatType;
2322
use PHPStan\Type\IntegerRangeType;
@@ -104,13 +103,13 @@ private function specifyAssertAll(MethodReflection $staticMethodReflection, Stat
104103
$callable = $node->getArgs()[0]->value;
105104
$callableInfo = $scope->getType($callable);
106105

107-
if (!$callableInfo instanceof ClosureType) {
106+
if (!$callableInfo->isCallable()->yes()) {
108107
return new SpecifiedTypes();
109108
}
110109

111110
return $this->typeSpecifier->create(
112111
$node->getArgs()[1]->value,
113-
new IterableType(new MixedType(true), $callableInfo->getReturnType()),
112+
new IterableType(new MixedType(), new MixedType()),
114113
TypeSpecifierContext::createTruthy(),
115114
$scope,
116115
);
@@ -123,7 +122,7 @@ private function specifyAssertAllStrings(MethodReflection $staticMethodReflectio
123122
{
124123
return $this->typeSpecifier->create(
125124
$node->getArgs()[0]->value,
126-
new IterableType(new MixedType(true), new StringType()),
125+
new IterableType(new MixedType(), new StringType()),
127126
TypeSpecifierContext::createTruthy(),
128127
$scope,
129128
);
@@ -136,7 +135,7 @@ private function specifyAssertAllStringable(MethodReflection $staticMethodReflec
136135
{
137136
// Drupal considers string as part of "stringable" as well.
138137
$stringable = TypeCombinator::union(new ObjectType(Stringable::class), new StringType());
139-
$newType = new IterableType(new MixedType(true), $stringable);
138+
$newType = new IterableType(new MixedType(), $stringable);
140139

141140
return $this->typeSpecifier->create(
142141
$node->getArgs()[0]->value,
@@ -151,8 +150,8 @@ private function specifyAssertAllStringable(MethodReflection $staticMethodReflec
151150
*/
152151
private function specifyAssertAllArrays(MethodReflection $staticMethodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
153152
{
154-
$arrayType = new ArrayType(new MixedType(true), new MixedType(true));
155-
$newType = new IterableType(new MixedType(true), $arrayType);
153+
$arrayType = new ArrayType(new MixedType(), new MixedType());
154+
$newType = new IterableType(new MixedType(), $arrayType);
156155

157156
return $this->typeSpecifier->create(
158157
$node->getArgs()[0]->value,
@@ -171,7 +170,7 @@ private function specifyAssertStrictArray(MethodReflection $staticMethodReflecti
171170
// In Drupal, 'strict arrays' are defined as arrays whose indexes
172171
// consist of integers that are equal to or greater than 0.
173172
IntegerRangeType::createAllGreaterThanOrEqualTo(0),
174-
new MixedType(true),
173+
new MixedType(),
175174
);
176175

177176
return $this->typeSpecifier->create(
@@ -188,10 +187,10 @@ private function specifyAssertStrictArray(MethodReflection $staticMethodReflecti
188187
private function specifyAssertAllStrictArrays(MethodReflection $staticMethodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
189188
{
190189
$newType = new IterableType(
191-
new MixedType(true),
190+
new MixedType(),
192191
new ArrayType(
193192
IntegerRangeType::createAllGreaterThanOrEqualTo(0),
194-
new MixedType(true),
193+
new MixedType(),
195194
),
196195
);
197196

@@ -229,17 +228,20 @@ private function specifyAssertAllHaveKey(MethodReflection $staticMethodReflectio
229228
}
230229
}
231230

232-
$keyTypes = [];
231+
// @see ArrayKeyExistsFunctionTypeSpecifyingExtension.
232+
$possibleTypes = [
233+
new ArrayType(new MixedType(), new MixedType())
234+
];
233235
foreach ($keys as $key) {
234-
$keyTypes[] = new HasOffsetType(new ConstantStringType($key));
236+
$possibleTypes[] = new HasOffsetType(new ConstantStringType($key));
235237
}
236238

237-
$newArrayType = new ArrayType(
238-
new MixedType(true),
239-
new ArrayType(TypeCombinator::intersect(new MixedType(), ...$keyTypes), new MixedType(true)),
239+
$newType = new IterableType(
240+
new MixedType(),
241+
TypeCombinator::intersect(...$possibleTypes),
240242
);
241243

242-
return $this->typeSpecifier->create($traversableArg, $newArrayType, TypeSpecifierContext::createTruthy(), $scope);
244+
return $this->typeSpecifier->create($traversableArg, $newType, TypeSpecifierContext::createTruthy(), $scope);
243245
}
244246

245247
/**
@@ -249,7 +251,7 @@ private function specifyAssertAllIntegers(MethodReflection $staticMethodReflecti
249251
{
250252
return $this->typeSpecifier->create(
251253
$node->getArgs()[0]->value,
252-
new IterableType(new MixedType(true), new IntegerType()),
254+
new IterableType(new MixedType(), new IntegerType()),
253255
TypeSpecifierContext::createTruthy(),
254256
$scope,
255257
);
@@ -262,7 +264,7 @@ private function specifyAssertAllFloat(MethodReflection $staticMethodReflection,
262264
{
263265
return $this->typeSpecifier->create(
264266
$node->getArgs()[0]->value,
265-
new IterableType(new MixedType(true), new FloatType()),
267+
new IterableType(new MixedType(), new FloatType()),
266268
TypeSpecifierContext::createTruthy(),
267269
$scope,
268270
);
@@ -275,7 +277,7 @@ private function specifyAssertAllCallable(MethodReflection $staticMethodReflecti
275277
{
276278
return $this->typeSpecifier->create(
277279
$node->getArgs()[0]->value,
278-
new IterableType(new MixedType(true), new CallableType()),
280+
new IterableType(new MixedType(), new CallableType()),
279281
TypeSpecifierContext::createTruthy(),
280282
$scope,
281283
);
@@ -295,7 +297,7 @@ private function specifyAssertAllNotEmpty(MethodReflection $staticMethodReflecti
295297
new FloatType(),
296298
new ResourceType(),
297299
];
298-
$newType = new IterableType(new MixedType(true), new UnionType($non_empty_types));
300+
$newType = new IterableType(new MixedType(), new UnionType($non_empty_types));
299301

300302
return $this->typeSpecifier->create(
301303
$node->getArgs()[0]->value,
@@ -312,7 +314,7 @@ private function specifyAssertAllNumeric(MethodReflection $staticMethodReflectio
312314
{
313315
return $this->typeSpecifier->create(
314316
$node->getArgs()[0]->value,
315-
new IterableType(new MixedType(true), new UnionType([new IntegerType(), new FloatType()])),
317+
new IterableType(new MixedType(), new UnionType([new IntegerType(), new FloatType()])),
316318
TypeSpecifierContext::createTruthy(),
317319
$scope,
318320
);
@@ -325,7 +327,7 @@ private function specifyAssertAllMatch(MethodReflection $staticMethodReflection,
325327
{
326328
return $this->typeSpecifier->create(
327329
$node->getArgs()[1]->value,
328-
new IterableType(new MixedType(true), new StringType()),
330+
new IterableType(new MixedType(), new StringType()),
329331
TypeSpecifierContext::createTruthy(),
330332
$scope,
331333
);
@@ -340,7 +342,7 @@ private function specifyAssertAllRegularExpressionMatch(MethodReflection $static
340342
$node->getArgs()[1]->value,
341343
// Drupal treats any non-string input in traversable as invalid
342344
// value, so it is possible to narrow type here.
343-
new IterableType(new MixedType(true), new StringType()),
345+
new IterableType(new MixedType(), new StringType()),
344346
TypeSpecifierContext::createTruthy(),
345347
$scope,
346348
);
@@ -373,7 +375,7 @@ private function specifyAssertAllObjects(MethodReflection $staticMethodReflectio
373375

374376
return $this->typeSpecifier->create(
375377
$node->getArgs()[0]->value,
376-
new IterableType(new MixedType(true), TypeCombinator::union(...$objectTypes)),
378+
new IterableType(new MixedType(), TypeCombinator::union(...$objectTypes)),
377379
TypeSpecifierContext::createTruthy(),
378380
$scope,
379381
);

‎tests/src/Type/InspectorTypeExtensionTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ final class InspectorTypeExtensionTest extends TypeInferenceTestCase {
1313

1414
public static function dataFileAsserts(): iterable {
1515
yield from self::gatherAssertTypes(__DIR__ . '/data/inspector.php');
16+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-852.php');
1617
}
1718

1819
/**
@@ -28,5 +29,5 @@ public function testFileAsserts(
2829
): void {
2930
$this->assertFileAsserts($assertType, $file, ...$args);
3031
}
31-
32+
3233
}

‎tests/src/Type/data/bug-852.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
4+
use Drupal\Component\Assertion\Inspector;
5+
use Drupal\Core\Url;
6+
use function PHPStan\Testing\assertType;
7+
8+
function foo(array $baz): void {
9+
assert(Inspector::assertAllArrays($baz));
10+
assertType('array<array>', $baz);
11+
12+
assert(Inspector::assertAll(fn (array $i): bool => $i['file'] instanceof Url, $baz));
13+
assertType('array<array>', $baz);
14+
15+
assert(Inspector::assertAllHaveKey($baz, 'alt'));
16+
assertType("array<non-empty-array&hasOffset('alt')>", $baz);
17+
}
18+
19+
function bar(array $zed): void {
20+
assert(Inspector::assertAll(fn (string $value) => $value === 'foo', $zed));
21+
assertType('array', $zed);
22+
23+
}
24+
25+
/**
26+
* @param array $images
27+
* Images of the project. Each item needs to be an array with two elements:
28+
* `file`, which is a \Drupal\Core\Url object pointing to the image, and
29+
* `alt`, which is the alt text.
30+
*/
31+
function project_browser_example(array $images) {
32+
assert(
33+
Inspector::assertAllArrays($images) &&
34+
Inspector::assertAllHaveKey($images, 'file') &&
35+
Inspector::assertAll(fn (array $i): bool => $i['file'] instanceof Url, $images) &&
36+
Inspector::assertAllHaveKey($images, 'alt')
37+
) or throw new \InvalidArgumentException('The project images must be arrays with `file` and `alt` elements.');
38+
assertType("array<non-empty-array&hasOffset('alt')&hasOffset('file')>", $images);
39+
}

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

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,16 @@ function mixed_function(): mixed {
1010
}
1111

1212
// Inspector::assertAll()
13-
$callable = fn (): string => 'foo';
13+
$callable = fn (string $value): bool => $value === 'foo';
1414
$input = mixed_function();
1515
assert(Inspector::assertAll($callable, $input));
16-
assertType("iterable<'foo'>", $input);
16+
assertType("iterable", $input);
1717

1818
$input = mixed_function();
19-
$callable = static function (): int {
20-
return rand(0, 1000);
21-
};
19+
$callable = is_string(...);
2220
assert(Inspector::assertAll($callable, $input));
23-
assertType('iterable<int<0, 1000>>', $input);
21+
assertType('iterable', $input);
2422

25-
$input = mixed_function();
26-
$callable = Closure::fromCallable('is_string');
27-
assert(Inspector::assertAll($callable, $input));
28-
assertType('iterable<bool>', $input);
29-
30-
$callable = fn(mixed $arg): string => (string) $arg;
31-
$input = mixed_function();
32-
assert(Inspector::assertAll($callable, $input));
33-
assertType('iterable<string>', $input);
3423

3524
// Inspector::assertAllStrings()
3625
$input = mixed_function();
@@ -45,7 +34,7 @@ function mixed_function(): mixed {
4534
// Inspector::assertAllArrays()
4635
$input = mixed_function();
4736
\assert(Inspector::assertAllArrays($input));
48-
assertType('iterable<array<mixed, mixed>>', $input);
37+
assertType('iterable<array>', $input);
4938

5039
// Inspector::assertStrictArray()
5140
$input = mixed_function();
@@ -60,7 +49,7 @@ function mixed_function(): mixed {
6049
// Inspector::assertAllHaveKey()
6150
$input = mixed_function();
6251
assert(Inspector::assertAllHaveKey($input, 'foo', 'baz'));
63-
assertType("array<mixed, array<hasOffset('baz')&hasOffset('foo'), mixed>>", $input);
52+
assertType("iterable<non-empty-array&hasOffset('baz')&hasOffset('foo')>", $input);
6453

6554
// Inspector::assertAllIntegers()
6655
$input = mixed_function();

0 commit comments

Comments
 (0)