Skip to content

Commit d141309

Browse files
ondrejmirtesondrejmirtes
committed
More nullsafe fixes
1 parent 93b025b commit d141309

File tree

5 files changed

+149
-83
lines changed

5 files changed

+149
-83
lines changed

‎src/Analyser/MutatingScope.php‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,6 +1971,11 @@ private function resolveType(Expr $node): Type
19711971
}
19721972

19731973
if ($node instanceof Expr\NullsafeMethodCall) {
1974+
$varType = $this->getType($node->var);
1975+
if (!TypeCombinator::containsNull($varType)) {
1976+
return $this->getType(new MethodCall($node->var, $node->name, $node->args));
1977+
}
1978+
19741979
return TypeCombinator::union(
19751980
$this->filterByTruthyValue(new BinaryOp\NotIdentical($node->var, new ConstFetch(new Name('null'))))
19761981
->getType(new MethodCall($node->var, $node->name, $node->args)),
@@ -2025,6 +2030,11 @@ private function resolveType(Expr $node): Type
20252030
}
20262031

20272032
if ($node instanceof Expr\NullsafePropertyFetch) {
2033+
$varType = $this->getType($node->var);
2034+
if (!TypeCombinator::containsNull($varType)) {
2035+
return $this->getType(new PropertyFetch($node->var, $node->name));
2036+
}
2037+
20282038
return TypeCombinator::union(
20292039
$this->filterByTruthyValue(new BinaryOp\NotIdentical($node->var, new ConstFetch(new Name('null'))))
20302040
->getType(new PropertyFetch($node->var, $node->name)),
@@ -2102,30 +2112,35 @@ private function resolveType(Expr $node): Type
21022112
return new MixedType();
21032113
}
21042114

2105-
private function getNullsafeShortCircuitingType(Expr $var, Type $type): Type
2115+
private function getNullsafeShortCircuitingType(Expr $expr, Type $type): Type
21062116
{
2107-
if ($var instanceof Expr\NullsafePropertyFetch || $var instanceof Expr\NullsafeMethodCall) {
2108-
return TypeCombinator::addNull($type);
2117+
if ($expr instanceof Expr\NullsafePropertyFetch || $expr instanceof Expr\NullsafeMethodCall) {
2118+
$varType = $this->getType($expr->var);
2119+
if (TypeCombinator::containsNull($varType)) {
2120+
return TypeCombinator::addNull($type);
2121+
}
2122+
2123+
return $type;
21092124
}
21102125

2111-
if ($var instanceof Expr\ArrayDimFetch) {
2112-
return $this->getNullsafeShortCircuitingType($var->var, $type);
2126+
if ($expr instanceof Expr\ArrayDimFetch) {
2127+
return $this->getNullsafeShortCircuitingType($expr->var, $type);
21132128
}
21142129

2115-
if ($var instanceof PropertyFetch) {
2116-
return $this->getNullsafeShortCircuitingType($var->var, $type);
2130+
if ($expr instanceof PropertyFetch) {
2131+
return $this->getNullsafeShortCircuitingType($expr->var, $type);
21172132
}
21182133

2119-
if ($var instanceof Expr\StaticPropertyFetch && $var->class instanceof Expr) {
2120-
return $this->getNullsafeShortCircuitingType($var->class, $type);
2134+
if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) {
2135+
return $this->getNullsafeShortCircuitingType($expr->class, $type);
21212136
}
21222137

2123-
if ($var instanceof MethodCall) {
2124-
return $this->getNullsafeShortCircuitingType($var->var, $type);
2138+
if ($expr instanceof MethodCall) {
2139+
return $this->getNullsafeShortCircuitingType($expr->var, $type);
21252140
}
21262141

2127-
if ($var instanceof Expr\StaticCall && $var->class instanceof Expr) {
2128-
return $this->getNullsafeShortCircuitingType($var->class, $type);
2142+
if ($expr instanceof Expr\StaticCall && $expr->class instanceof Expr) {
2143+
return $this->getNullsafeShortCircuitingType($expr->class, $type);
21292144
}
21302145

21312146
return $type;

‎src/Analyser/NullsafeOperatorHelper.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PhpParser\Node\Expr;
6+
7+
class NullsafeOperatorHelper
8+
{
9+
10+
public static function getNullsafeShortcircuitedExpr(Expr $expr): Expr
11+
{
12+
if ($expr instanceof Expr\NullsafeMethodCall) {
13+
return new Expr\MethodCall(self::getNullsafeShortcircuitedExpr($expr->var), $expr->name, $expr->args);
14+
}
15+
16+
if ($expr instanceof Expr\MethodCall) {
17+
$var = self::getNullsafeShortcircuitedExpr($expr->var);
18+
if ($expr->var === $var) {
19+
return $expr;
20+
}
21+
22+
return new Expr\MethodCall($var, $expr->name, $expr->args);
23+
}
24+
25+
if ($expr instanceof Expr\StaticCall && $expr->class instanceof Expr) {
26+
$class = self::getNullsafeShortcircuitedExpr($expr->class);
27+
if ($expr->class === $class) {
28+
return $expr;
29+
}
30+
31+
return new Expr\StaticCall($class, $expr->name, $expr->args);
32+
}
33+
34+
if ($expr instanceof Expr\ArrayDimFetch) {
35+
$var = self::getNullsafeShortcircuitedExpr($expr->var);
36+
if ($expr->var === $var) {
37+
return $expr;
38+
}
39+
40+
return new Expr\ArrayDimFetch($var, $expr->dim);
41+
}
42+
43+
if ($expr instanceof Expr\NullsafePropertyFetch) {
44+
return new Expr\PropertyFetch(self::getNullsafeShortcircuitedExpr($expr->var), $expr->name);
45+
}
46+
47+
if ($expr instanceof Expr\PropertyFetch) {
48+
$var = self::getNullsafeShortcircuitedExpr($expr->var);
49+
if ($expr->var === $var) {
50+
return $expr;
51+
}
52+
53+
return new Expr\PropertyFetch($var, $expr->name);
54+
}
55+
56+
if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) {
57+
$class = self::getNullsafeShortcircuitedExpr($expr->class);
58+
if ($expr->class === $class) {
59+
return $expr;
60+
}
61+
62+
return new Expr\StaticPropertyFetch($class, $expr->name);
63+
}
64+
65+
return $expr;
66+
}
67+
68+
}

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,19 @@ public function create(
914914
return new SpecifiedTypes();
915915
}
916916

917+
if ($scope !== null) {
918+
if ($context->true()) {
919+
$resultType = TypeCombinator::intersect($scope->getType($expr), $type);
920+
} elseif ($context->false()) {
921+
$resultType = TypeCombinator::remove($scope->getType($expr), $type);
922+
}
923+
}
924+
925+
$originalExpr = $expr;
926+
if (isset($resultType) && !TypeCombinator::containsNull($resultType)) {
927+
$expr = NullsafeOperatorHelper::getNullsafeShortcircuitedExpr($expr);
928+
}
929+
917930
if (
918931
$expr instanceof FuncCall
919932
&& $expr->name instanceof Name
@@ -939,16 +952,8 @@ public function create(
939952
$calledOnType = $scope->getType($expr->var);
940953
$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
941954
if ($methodReflection === null || $methodReflection->hasSideEffects()->yes()) {
942-
if ($context->true()) {
943-
$resultType = TypeCombinator::intersect($scope->getType($expr), $type);
944-
if (!TypeCombinator::containsNull($resultType)) {
945-
return $this->createNullsafeTypes($expr, $scope, $context, $type);
946-
}
947-
} elseif ($context->false()) {
948-
$resultType = TypeCombinator::remove($scope->getType($expr), $type);
949-
if (!TypeCombinator::containsNull($resultType)) {
950-
return $this->createNullsafeTypes($expr, $scope, $context, $type);
951-
}
955+
if (isset($resultType) && !TypeCombinator::containsNull($resultType)) {
956+
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
952957
}
953958

954959
return new SpecifiedTypes();
@@ -957,7 +962,6 @@ public function create(
957962

958963
$sureTypes = [];
959964
$sureNotTypes = [];
960-
961965
$exprString = $this->printer->prettyPrintExpr($expr);
962966
if ($context->false()) {
963967
$sureNotTypes[$exprString] = [$expr, $type];
@@ -966,18 +970,8 @@ public function create(
966970
}
967971

968972
$types = new SpecifiedTypes($sureTypes, $sureNotTypes, $overwrite);
969-
if ($scope !== null) {
970-
if ($context->true()) {
971-
$resultType = TypeCombinator::intersect($scope->getType($expr), $type);
972-
if (!TypeCombinator::containsNull($resultType)) {
973-
return $this->createNullsafeTypes($expr, $scope, $context, $type)->unionWith($types);
974-
}
975-
} elseif ($context->false()) {
976-
$resultType = TypeCombinator::remove($scope->getType($expr), $type);
977-
if (!TypeCombinator::containsNull($resultType)) {
978-
return $this->createNullsafeTypes($expr, $scope, $context, $type)->unionWith($types);
979-
}
980-
}
973+
if ($scope !== null && isset($resultType) && !TypeCombinator::containsNull($resultType)) {
974+
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type)->unionWith($types);
981975
}
982976

983977
return $types;

‎src/Rules/RuleLevelHelper.php‎

Lines changed: 2 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PHPStan\Rules;
44

55
use PhpParser\Node\Expr;
6+
use PHPStan\Analyser\NullsafeOperatorHelper;
67
use PHPStan\Analyser\Scope;
78
use PHPStan\Reflection\ReflectionProvider;
89
use PHPStan\Type\BenevolentUnionType;
@@ -128,7 +129,7 @@ public function findTypeToCheck(
128129
}
129130

130131
if (TypeCombinator::containsNull($type)) {
131-
$type = $scope->getType($this->getNullsafeShortcircuitedExpr($var));
132+
$type = $scope->getType(NullsafeOperatorHelper::getNullsafeShortcircuitedExpr($var));
132133
}
133134

134135
if (
@@ -193,50 +194,4 @@ public function findTypeToCheck(
193194
return new FoundTypeResult($type, $directClassNames, []);
194195
}
195196

196-
private function getNullsafeShortcircuitedExpr(Expr $expr): Expr
197-
{
198-
if ($expr instanceof Expr\NullsafeMethodCall) {
199-
return new Expr\MethodCall($this->getNullsafeShortcircuitedExpr($expr->var), $expr->name, $expr->args);
200-
}
201-
202-
if ($expr instanceof Expr\MethodCall) {
203-
return new Expr\MethodCall($this->getNullsafeShortcircuitedExpr($expr->var), $expr->name, $expr->args);
204-
}
205-
206-
if ($expr instanceof Expr\StaticCall && $expr->class instanceof Expr) {
207-
return new Expr\StaticCall(
208-
$this->getNullsafeShortcircuitedExpr($expr->class),
209-
$expr->name,
210-
$expr->args
211-
);
212-
}
213-
214-
if ($expr instanceof Expr\ArrayDimFetch) {
215-
return new Expr\ArrayDimFetch($this->getNullsafeShortcircuitedExpr($expr->var), $expr->dim);
216-
}
217-
218-
if ($expr instanceof Expr\NullsafePropertyFetch) {
219-
return new Expr\PropertyFetch(
220-
$this->getNullsafeShortcircuitedExpr($expr->var),
221-
$expr->name
222-
);
223-
}
224-
225-
if ($expr instanceof Expr\PropertyFetch) {
226-
return new Expr\PropertyFetch(
227-
$this->getNullsafeShortcircuitedExpr($expr->var),
228-
$expr->name
229-
);
230-
}
231-
232-
if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) {
233-
return new Expr\StaticPropertyFetch(
234-
$this->getNullsafeShortcircuitedExpr($expr->class),
235-
$expr->name
236-
);
237-
}
238-
239-
return $expr;
240-
}
241-
242197
}

‎tests/PHPStan/Analyser/data/bug-4757.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,17 @@ public function sayHello6(?Reservation $oldReservation): void
7575
assertType('bool', $oldReservation->isFoo());
7676
}
7777

78+
public function sayHelloPure(?Reservation $oldReservation): void
79+
{
80+
if ($oldReservation?->isFoo()) {
81+
assertType(Reservation::class, $oldReservation);
82+
assertType('true', $oldReservation->isFoo());
83+
return;
84+
}
85+
86+
assertType(Reservation::class . '|null', $oldReservation);
87+
}
88+
7889
public function sayHelloImpure(?Reservation $oldReservation): void
7990
{
8091
if ($oldReservation?->isFooImpure()) {
@@ -172,6 +183,7 @@ public function doFoo(Bar $b): void
172183

173184
assertType(Bar::class, $barOrNull);
174185
assertType('int', $barOrNull->get());
186+
assertType('int', $barOrNull?->get());
175187
}
176188

177189
public function doFooImpire(Bar $b): void
@@ -186,6 +198,7 @@ public function doFooImpire(Bar $b): void
186198

187199
assertType(Bar::class, $barOrNull);
188200
assertType('int|null', $barOrNull->getImpure());
201+
assertType('int|null', $barOrNull?->getImpure());
189202
}
190203

191204
public function doFoo2(Bar $b): void
@@ -194,6 +207,7 @@ public function doFoo2(Bar $b): void
194207
if ($barOrNull?->get() !== null) {
195208
assertType(Bar::class, $barOrNull);
196209
assertType('int', $barOrNull->get());
210+
assertType('int', $barOrNull?->get());
197211
return;
198212
}
199213

@@ -207,11 +221,13 @@ public function doFoo2Impure(Bar $b): void
207221
if ($barOrNull?->getImpure() !== null) {
208222
assertType(Bar::class, $barOrNull);
209223
assertType('int|null', $barOrNull->getImpure());
224+
assertType('int|null', $barOrNull?->getImpure());
210225
return;
211226
}
212227

213228
assertType(Bar::class . '|null', $barOrNull);
214229
assertType('int|null', $barOrNull->getImpure());
230+
assertType('int|null', $barOrNull?->getImpure());
215231
}
216232

217233
public function doFoo3(Bar $b): void
@@ -332,4 +348,22 @@ public function doBaz4(): void
332348
}
333349
}
334350

351+
public function doVariable(): void
352+
{
353+
$foo = $this->selfOrNull;
354+
if ($foo?->get()->selfOrNull !== null) {
355+
assertType(self::class, $foo);
356+
assertType(self::class, $foo->get()->selfOrNull);
357+
}
358+
}
359+
360+
public function doLorem(): void
361+
{
362+
if ($this->find()?->find()?->find() !== null) {
363+
assertType(self::class, $this->find());
364+
assertType(self::class, $this->find()->find());
365+
assertType(self::class, $this->find()->find()->find());
366+
}
367+
}
368+
335369
}

0 commit comments

Comments
 (0)