Skip to content

Commit d58874e

Browse files
VincentLangletVincentLanglet
authored
Avoid false inference with instanceof
1 parent ff1feee commit d58874e

File tree

6 files changed

+57
-31
lines changed

6 files changed

+57
-31
lines changed

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,11 +1040,6 @@ parameters:
10401040
count: 3
10411041
path: src/Type/Generic/TemplateIntersectionType.php
10421042

1043-
-
1044-
message: "#^Instanceof between PHPStan\\\\Type\\\\Type and PHPStan\\\\Type\\\\IntersectionType will always evaluate to false\\.$#"
1045-
count: 2
1046-
path: src/Type/Generic/TemplateIntersectionType.php
1047-
10481043
-
10491044
message: "#^Doing instanceof PHPStan\\\\Type\\\\IntersectionType is error\\-prone and deprecated\\.$#"
10501045
count: 3
@@ -1145,11 +1140,6 @@ parameters:
11451140
count: 3
11461141
path: src/Type/Generic/TemplateUnionType.php
11471142

1148-
-
1149-
message: "#^Instanceof between PHPStan\\\\Type\\\\Type and PHPStan\\\\Type\\\\UnionType will always evaluate to false\\.$#"
1150-
count: 2
1151-
path: src/Type/Generic/TemplateUnionType.php
1152-
11531143
-
11541144
message: "#^Doing instanceof PHPStan\\\\Type\\\\ConstantScalarType is error\\-prone and deprecated\\. Use Type\\:\\:isConstantScalarValue\\(\\) or Type\\:\\:getConstantScalarTypes\\(\\) or Type\\:\\:getConstantScalarValues\\(\\) instead\\.$#"
11551145
count: 1

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,17 @@ public function specifyTypesInCondition(
156156
}
157157

158158
$classType = $scope->getType($expr->class);
159-
$type = TypeTraverser::map($classType, static function (Type $type, callable $traverse): Type {
159+
$uncertainty = false;
160+
$type = TypeTraverser::map($classType, static function (Type $type, callable $traverse) use (&$uncertainty): Type {
160161
if ($type instanceof UnionType || $type instanceof IntersectionType) {
161162
return $traverse($type);
162163
}
163164
if ($type->getObjectClassNames() !== []) {
165+
$uncertainty = true;
164166
return $type;
165167
}
166168
if ($type instanceof GenericClassStringType) {
169+
$uncertainty = true;
167170
return $type->getGenericType();
168171
}
169172
if ($type instanceof ConstantStringType) {
@@ -179,7 +182,7 @@ public function specifyTypesInCondition(
179182
new ObjectWithoutClassType(),
180183
);
181184
return $this->create($exprNode, $type, $context, false, $scope, $rootExpr);
182-
} elseif ($context->false()) {
185+
} elseif ($context->false() && !$uncertainty) {
183186
$exprType = $scope->getType($expr->expr);
184187
if (!$type->isSuperTypeOf($exprType)->yes()) {
185188
return $this->create($exprNode, $type, $context, false, $scope, $rootExpr);

‎tests/PHPStan/Analyser/nsrt/bug-12107.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12107;
4+
5+
use LogicException;
6+
use Throwable;
7+
8+
use function PHPStan\Testing\assertType;
9+
10+
class HelloWorld
11+
{
12+
public function sayHello(Throwable $e1, LogicException $e2): void
13+
{
14+
if ($e1 instanceof $e2) {
15+
return;
16+
}
17+
18+
assertType('Throwable', $e1);
19+
assertType('bool', $e1 instanceof $e2); // could be false
20+
}
21+
22+
/** @param class-string<LogicException> $e2 */
23+
public function sayHello2(Throwable $e1, string $e2): void
24+
{
25+
if ($e1 instanceof $e2) {
26+
return;
27+
}
28+
29+
30+
assertType('Throwable', $e1);
31+
assertType('bool', $e1 instanceof $e2); // could be false
32+
}
33+
34+
public function sayHello3(Throwable $e1): void
35+
{
36+
if ($e1 instanceof LogicException) {
37+
return;
38+
}
39+
40+
assertType('Throwable~LogicException', $e1);
41+
assertType('false', $e1 instanceof LogicException);
42+
}
43+
}

‎tests/PHPStan/Analyser/nsrt/instanceof-class-string.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function doBar(Foo $foo, Bar $bar): void
3232
if ($foo instanceof $class) {
3333
assertType(self::class, $foo);
3434
} else {
35-
assertType('InstanceOfClassString\Foo~InstanceOfClassString\Bar', $foo);
35+
assertType('InstanceOfClassString\Foo', $foo);
3636
}
3737
}
3838

‎tests/PHPStan/Analyser/nsrt/instanceof.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ public function testExprInstanceof($subject, string $classString, $union, $inter
8080
assertType('true', $subject instanceof Foo);
8181
assertType('bool', $subject instanceof $classString);
8282
} else {
83-
assertType('mixed~InstanceOfNamespace\Foo', $subject);
84-
assertType('false', $subject instanceof Foo);
85-
assertType('false', $subject instanceof $classString);
83+
assertType('mixed', $subject);
84+
assertType('bool', $subject instanceof Foo);
85+
assertType('bool', $subject instanceof $classString); // could be false
8686
}
8787

8888
$constantString = 'InstanceOfNamespace\BarParent';
@@ -132,23 +132,23 @@ public function testExprInstanceof($subject, string $classString, $union, $inter
132132
assertType('ObjectT of InstanceOfNamespace\BarInterface (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)', $subject);
133133
assertType('bool', $subject instanceof $objectT);
134134
} else {
135-
assertType('mixed~ObjectT of InstanceOfNamespace\BarInterface (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)', $subject);
135+
assertType('mixed', $subject);
136136
assertType('bool', $subject instanceof $objectT); // can be false
137137
}
138138

139139
if ($subject instanceof $objectTString) {
140140
assertType('ObjectT of InstanceOfNamespace\BarInterface (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)', $subject);
141141
assertType('bool', $subject instanceof $objectTString);
142142
} else {
143-
assertType('mixed~ObjectT of InstanceOfNamespace\BarInterface (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)', $subject);
143+
assertType('mixed', $subject);
144144
assertType('bool', $subject instanceof $objectTString); // can be false
145145
}
146146

147147
if ($subject instanceof $mixedTString) {
148148
assertType('MixedT (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)&object', $subject);
149149
assertType('bool', $subject instanceof $mixedTString);
150150
} else {
151-
assertType('mixed~MixedT (method InstanceOfNamespace\Foo::testExprInstanceof(), argument)', $subject);
151+
assertType('mixed', $subject);
152152
assertType('bool', $subject instanceof $mixedTString); // can be false
153153
}
154154

@@ -180,8 +180,8 @@ public function testExprInstanceof($subject, string $classString, $union, $inter
180180
assertType('InstanceOfNamespace\Foo', $object);
181181
assertType('bool', $object instanceof $classString);
182182
} else {
183-
assertType('object~InstanceOfNamespace\Foo', $object);
184-
assertType('false', $object instanceof $classString);
183+
assertType('object', $object);
184+
assertType('bool', $object instanceof $classString); // could be false
185185
}
186186

187187
if ($instance instanceof $string) {

‎tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,6 @@ public function testInstanceof(): void
167167
388,
168168
$tipText,
169169
],
170-
[
171-
'Instanceof between T of Exception and Error will always evaluate to false.',
172-
404,
173-
$tipText,
174-
],
175170
[
176171
'Instanceof between class-string<DateTimeInterface> and DateTimeInterface will always evaluate to false.',
177172
418,
@@ -270,11 +265,6 @@ public function testInstanceofWithoutAlwaysTrue(): void
270265
'Instanceof between mixed and ImpossibleInstanceOf\InvalidTypeTest|int results in an error.',
271266
362,
272267
],*/
273-
[
274-
'Instanceof between T of Exception and Error will always evaluate to false.',
275-
404,
276-
$tipText,
277-
],
278268
[
279269
'Instanceof between class-string<DateTimeInterface> and DateTimeInterface will always evaluate to false.',
280270
418,

0 commit comments

Comments
 (0)