Skip to content

Commit 8e07b4e

Browse files
committed
Fix false positive comparing identical generic types below level 8
RuleLevelHelper relaxes the nullability of the accepted type but not of the accepting one, which is what lets a nullable value be passed where a non-nullable one is expected below level 8. TypeTraverser applied that relaxation to generic type arguments as well, so the argument of an accepted Collection<string, int|null> became int while the accepting side kept int|null. Compared invariantly, two identical types then stopped matching and the reported message printed the same type on both sides. Map the arguments of a concrete generic type instead of feeding them through the relaxation. A template type keeps going through the regular traversal, as rebuilding it as a plain generic would lose its bound.
1 parent 7a08648 commit 8e07b4e

4 files changed

Lines changed: 137 additions & 2 deletions

File tree

phpstan-baseline.neon

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,12 @@ parameters:
708708
count: 2
709709
path: src/Rules/RuleErrorBuilder.php
710710

711+
-
712+
rawMessage: Doing instanceof PHPStan\Type\Generic\GenericObjectType is error-prone and deprecated.
713+
identifier: phpstanApi.instanceofType
714+
count: 1
715+
path: src/Rules/RuleLevelHelper.php
716+
711717
-
712718
rawMessage: Doing instanceof PHPStan\Type\IntersectionType is error-prone and deprecated.
713719
identifier: phpstanApi.instanceofType

src/Rules/RuleLevelHelper.php

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
use PHPStan\Type\CallableType;
1212
use PHPStan\Type\ClosureType;
1313
use PHPStan\Type\ErrorType;
14+
use PHPStan\Type\Generic\GenericObjectType;
1415
use PHPStan\Type\Generic\TemplateMixedType;
16+
use PHPStan\Type\Generic\TemplateType;
1517
use PHPStan\Type\IntersectionType;
1618
use PHPStan\Type\MixedType;
1719
use PHPStan\Type\NeverType;
@@ -49,6 +51,57 @@ public function __construct(
4951
{
5052
}
5153

54+
/**
55+
* A callable return type is traversed directly instead of going through the
56+
* mapper, so a generic one needs the same handling here.
57+
*
58+
* @param callable(Type): Type $traverse
59+
*/
60+
private function traverseNestedType(Type $type, callable $traverse): Type
61+
{
62+
if ($this->isConcreteGenericObjectType($type)) {
63+
return $this->traverseGenericObjectType($type, $traverse);
64+
}
65+
66+
return $traverse($type);
67+
}
68+
69+
/**
70+
* A template type is compared through its bound, and rebuilding it as a
71+
* plain generic would lose that, so only a concrete generic gets its
72+
* arguments mapped explicitly.
73+
*
74+
* @phpstan-assert-if-true GenericObjectType $type
75+
*/
76+
private function isConcreteGenericObjectType(Type $type): bool
77+
{
78+
return $type instanceof GenericObjectType && !$type instanceof TemplateType;
79+
}
80+
81+
/**
82+
* The accepted type gets its nullability relaxed below level 8, but the
83+
* accepting type does not. Doing that to a generic argument compared
84+
* invariantly turns an otherwise identical pair into a mismatch, so map the
85+
* arguments themselves instead of feeding them through the relaxation.
86+
*
87+
* @param callable(Type): Type $traverse
88+
*/
89+
private function traverseGenericObjectType(GenericObjectType $type, callable $traverse): Type
90+
{
91+
$types = [];
92+
foreach ($type->getTypes() as $innerType) {
93+
$types[] = $traverse($innerType);
94+
}
95+
96+
return new GenericObjectType(
97+
$type->getClassName(),
98+
$types,
99+
$type->getSubtractedType(),
100+
$type->getClassReflection(),
101+
$type->getVariances(),
102+
);
103+
}
104+
52105
/** @api */
53106
public function isThis(Expr $expression): bool
54107
{
@@ -95,7 +148,7 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):
95148

96149
return new CallableType(
97150
$acceptedType->getParameters(),
98-
$traverse($acceptedType->getReturnType()),
151+
$this->traverseNestedType($acceptedType->getReturnType(), $traverse),
99152
$acceptedType->isVariadic(),
100153
$acceptedType->getTemplateTypeMap(),
101154
$acceptedType->getResolvedTemplateTypeMap(),
@@ -111,7 +164,7 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):
111164

112165
return new ClosureType(
113166
$acceptedType->getParameters(),
114-
$traverse($acceptedType->getReturnType()),
167+
$this->traverseNestedType($acceptedType->getReturnType(), $traverse),
115168
$acceptedType->isVariadic(),
116169
$acceptedType->getTemplateTypeMap(),
117170
$acceptedType->getResolvedTemplateTypeMap(),
@@ -127,6 +180,10 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):
127180
);
128181
}
129182

183+
if ($this->isConcreteGenericObjectType($acceptedType)) {
184+
return $this->traverseGenericObjectType($acceptedType, $traverse);
185+
}
186+
130187
if (
131188
!$this->checkNullables
132189
&& !$acceptingType instanceof NullType

tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3014,6 +3014,21 @@ public function testCallablesWithoutCheckNullables(bool $checkNullables, bool $c
30143014
$this->analyse([__DIR__ . '/data/callables-without-check-nullables.php'], $expectedErrors);
30153015
}
30163016

3017+
public static function dataGenericArgumentNullability(): iterable
3018+
{
3019+
yield [false];
3020+
yield [true];
3021+
}
3022+
3023+
#[DataProvider('dataGenericArgumentNullability')]
3024+
public function testGenericArgumentNullability(bool $checkNullables): void
3025+
{
3026+
$this->checkThisOnly = false;
3027+
$this->checkNullables = $checkNullables;
3028+
$this->checkUnionTypes = true;
3029+
$this->analyse([__DIR__ . '/data/generic-argument-nullability.php'], []);
3030+
}
3031+
30173032
#[RequiresPhp('>= 8.0.0')]
30183033
public function testBug8713(): void
30193034
{
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace GenericArgumentNullability;
4+
5+
/**
6+
* @template TKey of array-key
7+
* @template TValue
8+
*/
9+
class Collection
10+
{
11+
12+
}
13+
14+
class Foo
15+
{
16+
17+
/**
18+
* @template T
19+
* @param callable(): T $cb
20+
* @return T
21+
*/
22+
public function grab(callable $cb)
23+
{
24+
return $cb();
25+
}
26+
27+
/**
28+
* @template T
29+
* @param callable(mixed, array<string, mixed>): T $cb
30+
* @return T
31+
*/
32+
public function grabTwoArgs(callable $cb)
33+
{
34+
return $cb(null, []);
35+
}
36+
37+
/** @param Collection<string, int|null> $collection */
38+
public function acceptNullable(Collection $collection): void
39+
{
40+
}
41+
42+
}
43+
44+
/**
45+
* @param Collection<string, int> $plain
46+
* @param Collection<string, int|null> $nullable
47+
* @param array<string, int|null> $array
48+
*/
49+
function test(Foo $foo, Collection $plain, Collection $nullable, array $array, ?int $scalar): void
50+
{
51+
$foo->grab(fn () => $plain);
52+
$foo->grab(fn () => $nullable);
53+
$foo->grab(fn () => $array);
54+
$foo->grab(fn () => $scalar);
55+
$foo->grabTwoArgs(fn () => $nullable);
56+
$foo->acceptNullable($nullable);
57+
}

0 commit comments

Comments
 (0)