Skip to content

Commit 27d712a

Browse files
NiklanNiklan
committed
Merge branch 'main' into 2.x-formatter-generic
2 parents e7f3e9d + c50f146 commit 27d712a

File tree

8 files changed

+286
-85
lines changed

8 files changed

+286
-85
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ those namespaces so that PHPStan can properly discover symbols in your Drupal co
1010

1111
> [!NOTE]
1212
> With Drupal 11.2, Drupal core is now using PHPStan 2.0. The 1.x branch of phpstan-drupal will be supported until
13-
> 11.1 loses security support when 11.3 is released.
13+
> Drupal 10 loses security support when Drupal 12 is released.
1414
1515
## Sponsors
1616

‎src/Type/InspectorTypeExtension.php

Lines changed: 63 additions & 38 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,14 +103,37 @@ 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()) {
107+
return new SpecifiedTypes();
108+
}
109+
110+
$traversable = $node->getArgs()[1]->value;
111+
$traversableInfo = $scope->getType($traversable);
112+
113+
// If it is already not mixed (narrowed by other code, like
114+
// '::assertAllArray()'), we could not provide any additional
115+
// information. We can only narrow this method to 'array<mixed, mixed>'.
116+
if (!$traversableInfo->equals(new MixedType())) {
117+
return new SpecifiedTypes();
118+
}
119+
120+
// In a negation context, we cannot precisely narrow types because we do
121+
// not know the exact logic of the callable function. This means we
122+
// cannot safely return 'mixed~iterable' since the value might still be
123+
// a valid iterable.
124+
//
125+
// For example, a negation context with an 'is_string(...)' callable
126+
// does not necessarily mean that the value cannot be an
127+
// 'iterable<int>'. In such cases, it is safer to skip type narrowing
128+
// altogether to prevent introducing new bugs into the code.
129+
if ($context->false()) {
108130
return new SpecifiedTypes();
109131
}
110132

111133
return $this->typeSpecifier->create(
112134
$node->getArgs()[1]->value,
113-
new IterableType(new MixedType(true), $callableInfo->getReturnType()),
114-
TypeSpecifierContext::createTruthy(),
135+
new IterableType(new MixedType(), new MixedType()),
136+
$context,
115137
$scope,
116138
);
117139
}
@@ -123,8 +145,8 @@ private function specifyAssertAllStrings(MethodReflection $staticMethodReflectio
123145
{
124146
return $this->typeSpecifier->create(
125147
$node->getArgs()[0]->value,
126-
new IterableType(new MixedType(true), new StringType()),
127-
TypeSpecifierContext::createTruthy(),
148+
new IterableType(new MixedType(), new StringType()),
149+
$context,
128150
$scope,
129151
);
130152
}
@@ -136,12 +158,12 @@ private function specifyAssertAllStringable(MethodReflection $staticMethodReflec
136158
{
137159
// Drupal considers string as part of "stringable" as well.
138160
$stringable = TypeCombinator::union(new ObjectType(Stringable::class), new StringType());
139-
$newType = new IterableType(new MixedType(true), $stringable);
161+
$newType = new IterableType(new MixedType(), $stringable);
140162

141163
return $this->typeSpecifier->create(
142164
$node->getArgs()[0]->value,
143165
$newType,
144-
TypeSpecifierContext::createTruthy(),
166+
$context,
145167
$scope,
146168
);
147169
}
@@ -151,13 +173,13 @@ private function specifyAssertAllStringable(MethodReflection $staticMethodReflec
151173
*/
152174
private function specifyAssertAllArrays(MethodReflection $staticMethodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
153175
{
154-
$arrayType = new ArrayType(new MixedType(true), new MixedType(true));
155-
$newType = new IterableType(new MixedType(true), $arrayType);
176+
$arrayType = new ArrayType(new MixedType(), new MixedType());
177+
$newType = new IterableType(new MixedType(), $arrayType);
156178

157179
return $this->typeSpecifier->create(
158180
$node->getArgs()[0]->value,
159181
$newType,
160-
TypeSpecifierContext::createTruthy(),
182+
$context,
161183
$scope,
162184
);
163185
}
@@ -171,13 +193,13 @@ private function specifyAssertStrictArray(MethodReflection $staticMethodReflecti
171193
// In Drupal, 'strict arrays' are defined as arrays whose indexes
172194
// consist of integers that are equal to or greater than 0.
173195
IntegerRangeType::createAllGreaterThanOrEqualTo(0),
174-
new MixedType(true),
196+
new MixedType(),
175197
);
176198

177199
return $this->typeSpecifier->create(
178200
$node->getArgs()[0]->value,
179201
$newType,
180-
TypeSpecifierContext::createTruthy(),
202+
$context,
181203
$scope,
182204
);
183205
}
@@ -188,17 +210,17 @@ private function specifyAssertStrictArray(MethodReflection $staticMethodReflecti
188210
private function specifyAssertAllStrictArrays(MethodReflection $staticMethodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
189211
{
190212
$newType = new IterableType(
191-
new MixedType(true),
213+
new MixedType(),
192214
new ArrayType(
193215
IntegerRangeType::createAllGreaterThanOrEqualTo(0),
194-
new MixedType(true),
216+
new MixedType(),
195217
),
196218
);
197219

198220
return $this->typeSpecifier->create(
199221
$node->getArgs()[0]->value,
200222
$newType,
201-
TypeSpecifierContext::createTruthy(),
223+
$context,
202224
$scope,
203225
);
204226
}
@@ -229,17 +251,20 @@ private function specifyAssertAllHaveKey(MethodReflection $staticMethodReflectio
229251
}
230252
}
231253

232-
$keyTypes = [];
254+
// @see ArrayKeyExistsFunctionTypeSpecifyingExtension.
255+
$possibleTypes = [
256+
new ArrayType(new MixedType(), new MixedType())
257+
];
233258
foreach ($keys as $key) {
234-
$keyTypes[] = new HasOffsetType(new ConstantStringType($key));
259+
$possibleTypes[] = new HasOffsetType(new ConstantStringType($key));
235260
}
236261

237-
$newArrayType = new ArrayType(
238-
new MixedType(true),
239-
new ArrayType(TypeCombinator::intersect(new MixedType(), ...$keyTypes), new MixedType(true)),
262+
$newType = new IterableType(
263+
new MixedType(),
264+
TypeCombinator::intersect(...$possibleTypes),
240265
);
241266

242-
return $this->typeSpecifier->create($traversableArg, $newArrayType, TypeSpecifierContext::createTruthy(), $scope);
267+
return $this->typeSpecifier->create($traversableArg, $newType, $context, $scope);
243268
}
244269

245270
/**
@@ -249,8 +274,8 @@ private function specifyAssertAllIntegers(MethodReflection $staticMethodReflecti
249274
{
250275
return $this->typeSpecifier->create(
251276
$node->getArgs()[0]->value,
252-
new IterableType(new MixedType(true), new IntegerType()),
253-
TypeSpecifierContext::createTruthy(),
277+
new IterableType(new MixedType(), new IntegerType()),
278+
$context,
254279
$scope,
255280
);
256281
}
@@ -262,8 +287,8 @@ private function specifyAssertAllFloat(MethodReflection $staticMethodReflection,
262287
{
263288
return $this->typeSpecifier->create(
264289
$node->getArgs()[0]->value,
265-
new IterableType(new MixedType(true), new FloatType()),
266-
TypeSpecifierContext::createTruthy(),
290+
new IterableType(new MixedType(), new FloatType()),
291+
$context,
267292
$scope,
268293
);
269294
}
@@ -275,8 +300,8 @@ private function specifyAssertAllCallable(MethodReflection $staticMethodReflecti
275300
{
276301
return $this->typeSpecifier->create(
277302
$node->getArgs()[0]->value,
278-
new IterableType(new MixedType(true), new CallableType()),
279-
TypeSpecifierContext::createTruthy(),
303+
new IterableType(new MixedType(), new CallableType()),
304+
$context,
280305
$scope,
281306
);
282307
}
@@ -295,12 +320,12 @@ private function specifyAssertAllNotEmpty(MethodReflection $staticMethodReflecti
295320
new FloatType(),
296321
new ResourceType(),
297322
];
298-
$newType = new IterableType(new MixedType(true), new UnionType($non_empty_types));
323+
$newType = new IterableType(new MixedType(), new UnionType($non_empty_types));
299324

300325
return $this->typeSpecifier->create(
301326
$node->getArgs()[0]->value,
302327
$newType,
303-
TypeSpecifierContext::createTruthy(),
328+
$context,
304329
$scope,
305330
);
306331
}
@@ -312,8 +337,8 @@ private function specifyAssertAllNumeric(MethodReflection $staticMethodReflectio
312337
{
313338
return $this->typeSpecifier->create(
314339
$node->getArgs()[0]->value,
315-
new IterableType(new MixedType(true), new UnionType([new IntegerType(), new FloatType()])),
316-
TypeSpecifierContext::createTruthy(),
340+
new IterableType(new MixedType(), new UnionType([new IntegerType(), new FloatType()])),
341+
$context,
317342
$scope,
318343
);
319344
}
@@ -325,8 +350,8 @@ private function specifyAssertAllMatch(MethodReflection $staticMethodReflection,
325350
{
326351
return $this->typeSpecifier->create(
327352
$node->getArgs()[1]->value,
328-
new IterableType(new MixedType(true), new StringType()),
329-
TypeSpecifierContext::createTruthy(),
353+
new IterableType(new MixedType(), new StringType()),
354+
$context,
330355
$scope,
331356
);
332357
}
@@ -340,8 +365,8 @@ private function specifyAssertAllRegularExpressionMatch(MethodReflection $static
340365
$node->getArgs()[1]->value,
341366
// Drupal treats any non-string input in traversable as invalid
342367
// value, so it is possible to narrow type here.
343-
new IterableType(new MixedType(true), new StringType()),
344-
TypeSpecifierContext::createTruthy(),
368+
new IterableType(new MixedType(), new StringType()),
369+
$context,
345370
$scope,
346371
);
347372
}
@@ -373,8 +398,8 @@ private function specifyAssertAllObjects(MethodReflection $staticMethodReflectio
373398

374399
return $this->typeSpecifier->create(
375400
$node->getArgs()[0]->value,
376-
new IterableType(new MixedType(true), TypeCombinator::union(...$objectTypes)),
377-
TypeSpecifierContext::createTruthy(),
401+
new IterableType(new MixedType(), TypeCombinator::union(...$objectTypes)),
402+
$context,
378403
$scope,
379404
);
380405
}

‎tests/src/Rules/ImpossibleCheckTypeStaticMethodCallRuleTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace mglaman\PHPStanDrupal\Tests\Rules;
6+
7+
use mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule;
8+
use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
9+
use PHPStan\Rules\Comparison\ImpossibleCheckTypeHelper;
10+
use PHPStan\Rules\Comparison\ImpossibleCheckTypeStaticMethodCallRule;
11+
use PHPStan\Rules\Rule;
12+
13+
class ImpossibleCheckTypeStaticMethodCallRuleTest extends DrupalRuleTestCase
14+
{
15+
16+
private bool $treatPhpDocTypesAsCertain = false;
17+
18+
private bool $reportAlwaysTrueInLastCondition = false;
19+
20+
protected function getRule(): Rule
21+
{
22+
/** @phpstan-ignore phpstanApi.constructor */
23+
return new ImpossibleCheckTypeStaticMethodCallRule(
24+
/** @phpstan-ignore phpstanApi.constructor */
25+
new ImpossibleCheckTypeHelper(
26+
$this->createReflectionProvider(),
27+
$this->getTypeSpecifier(),
28+
[],
29+
$this->treatPhpDocTypesAsCertain,
30+
),
31+
$this->treatPhpDocTypesAsCertain,
32+
$this->reportAlwaysTrueInLastCondition,
33+
true,
34+
);
35+
}
36+
37+
protected function shouldTreatPhpDocTypesAsCertain(): bool
38+
{
39+
return $this->treatPhpDocTypesAsCertain;
40+
}
41+
42+
43+
public function testBug857(): void
44+
{
45+
$this->treatPhpDocTypesAsCertain = true;
46+
// It shouldn't report anything.
47+
$this->analyse([__DIR__ . '/data/bug-857.php'], []);
48+
}
49+
50+
}

‎tests/src/Rules/data/bug-857.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
use Drupal\Component\Assertion\Inspector;
4+
use Drupal\Core\Url;
5+
6+
function foo(array $images) {
7+
// This call is crucial for the bug, because it narrows type to
8+
// 'array<array>'.
9+
Inspector::assertAllArrays($images);
10+
Inspector::assertAll(fn (array $i): bool => $i['file'] instanceof Url, $images);
11+
}

‎tests/src/Type/InspectorTypeExtensionTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ 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');
17+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-857.php');
1618
}
1719

1820
/**
@@ -28,5 +30,5 @@ public function testFileAsserts(
2830
): void {
2931
$this->assertFileAsserts($assertType, $file, ...$args);
3032
}
31-
33+
3234
}

‎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/bug-857.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
use Drupal\Component\Assertion\Inspector;
4+
use function PHPStan\Testing\assertType;
5+
6+
function mixed_function(): mixed {}
7+
8+
// If the type is not narrowed from 'mixed' before call, it is narrowed to
9+
// 'iterable'.
10+
$array = mixed_function();
11+
assertType('mixed', $array);
12+
\assert(Inspector::assertAll(fn (array $i): bool => TRUE, $array));
13+
assertType('iterable', $array);
14+
15+
// If the type is narrowed before '::assertAll()', it shouldn't change it.
16+
$array = mixed_function();
17+
assertType('mixed', $array);
18+
\assert(Inspector::assertAllArrays($array));
19+
assertType('iterable<array>', $array);
20+
\assert(Inspector::assertAll(fn (array $i): bool => TRUE, $array));
21+
assertType('iterable<array>', $array);

0 commit comments

Comments
 (0)