Skip to content

Commit aa14780

Browse files
ondrejmirtesondrejmirtes
committed
Null coalesce checks - variables on level 1, rest on level 4
1 parent 2003cda commit aa14780

10 files changed

+197
-45
lines changed

‎conf/config.level1.neon

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ parameters:
77
reportMagicMethods: true
88
reportMagicProperties: true
99

10+
1011
conditionalTags:
11-
PHPStan\Rules\Variables\NullCoalesceRule:
12+
PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule:
1213
phpstan.rules.rule: %featureToggles.nullCoalesce%
1314

1415
rules:
@@ -19,4 +20,4 @@ rules:
1920

2021
services:
2122
-
22-
class: PHPStan\Rules\Variables\NullCoalesceRule
23+
class: PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule

‎conf/config.level4.neon

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ rules:
1414
- PHPStan\Rules\TooWideTypehints\TooWideClosureReturnTypehintRule
1515
- PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule
1616

17+
conditionalTags:
18+
PHPStan\Rules\Variables\NullCoalesceRule:
19+
phpstan.rules.rule: %featureToggles.nullCoalesce%
20+
1721
services:
1822
-
1923
class: PHPStan\Rules\Classes\ImpossibleInstanceOfRule
@@ -116,3 +120,6 @@ services:
116120
checkProtectedAndPublicMethods: %checkTooWideReturnTypesInProtectedAndPublicMethods%
117121
tags:
118122
- phpstan.rules.rule
123+
124+
-
125+
class: PHPStan\Rules\Variables\NullCoalesceRule

‎src/Rules/IssetCheck.php

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,12 @@ public function __construct(
3232
public function check(Expr $expr, Scope $scope, string $operatorDescription, ?RuleError $error = null): ?RuleError
3333
{
3434
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
35-
3635
$hasVariable = $scope->hasVariableType($expr->name);
37-
38-
if ($hasVariable->no()) {
39-
return $error ?? RuleErrorBuilder::message(
40-
sprintf('Variable $%s on left side of %s is never defined.', $expr->name, $operatorDescription)
41-
)->build();
42-
}
43-
44-
$variableType = $scope->getType($expr);
45-
4636
if ($hasVariable->maybe()) {
4737
return null;
4838
}
4939

50-
if ($hasVariable->yes()) {
51-
return $error ?? $this->generateError(
52-
$variableType,
53-
sprintf('Variable $%s on left side of %s always exists and', $expr->name, $operatorDescription)
54-
);
55-
}
56-
40+
return $error;
5741
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
5842

5943
$type = $scope->getType($expr->var);

‎src/Rules/Variables/VariableCertaintyNullCoalesceRule.php

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Rules\RuleErrorBuilder;
8+
use PHPStan\Type\NullType;
9+
10+
/**
11+
* @implements \PHPStan\Rules\Rule<Node\Expr>
12+
*/
13+
class VariableCertaintyNullCoalesceRule implements \PHPStan\Rules\Rule
14+
{
15+
16+
public function getNodeType(): string
17+
{
18+
return Node\Expr::class;
19+
}
20+
21+
public function processNode(Node $node, Scope $scope): array
22+
{
23+
if ($node instanceof Node\Expr\AssignOp\Coalesce) {
24+
$var = $node->var;
25+
$description = '??=';
26+
} elseif ($node instanceof Node\Expr\BinaryOp\Coalesce) {
27+
$var = $node->left;
28+
$description = '??';
29+
} else {
30+
return [];
31+
}
32+
33+
$isSubNode = false;
34+
while (
35+
$var instanceof Node\Expr\ArrayDimFetch
36+
|| $var instanceof Node\Expr\PropertyFetch
37+
|| (
38+
$var instanceof Node\Expr\StaticPropertyFetch
39+
&& $var->class instanceof Node\Expr
40+
)
41+
) {
42+
if ($var instanceof Node\Expr\StaticPropertyFetch) {
43+
$var = $var->class;
44+
} else {
45+
$var = $var->var;
46+
}
47+
$isSubNode = true;
48+
}
49+
50+
if (!$var instanceof Node\Expr\Variable || !is_string($var->name)) {
51+
return [];
52+
}
53+
54+
$certainty = $scope->hasVariableType($var->name);
55+
if ($certainty->no()) {
56+
if (
57+
$scope->getFunction() !== null
58+
|| $scope->isInAnonymousFunction()
59+
) {
60+
return [RuleErrorBuilder::message(sprintf(
61+
'Variable $%s on left side of %s is never defined.',
62+
$var->name,
63+
$description
64+
))->build()];
65+
}
66+
} elseif ($certainty->yes() && !$isSubNode) {
67+
$variableType = $scope->getVariableType($var->name);
68+
if ($variableType->isSuperTypeOf(new NullType())->no()) {
69+
return [RuleErrorBuilder::message(sprintf(
70+
'Variable $%s on left side of %s always exists and is not nullable.',
71+
$var->name,
72+
$description
73+
))->build()];
74+
} elseif ((new NullType())->isSuperTypeOf($variableType)->yes()) {
75+
return [RuleErrorBuilder::message(sprintf(
76+
'Variable $%s on left side of %s is always null.',
77+
$var->name,
78+
$description
79+
))->build()];
80+
}
81+
}
82+
83+
return [];
84+
}
85+
86+
}

‎tests/PHPStan/Levels/data/coalesce-1.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
"ignorable": true
66
},
77
{
8-
"message": "Property ReflectionClass<object>::$name (class-string<object>) on left side of ?? is not nullable.",
9-
"line": 10,
8+
"message": "Variable $bar on left side of ?? is never defined.",
9+
"line": 6,
1010
"ignorable": true
1111
}
1212
]

‎tests/PHPStan/Levels/data/coalesce-4.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[
2+
{
3+
"message": "Property ReflectionClass<object>::$name (class-string<object>) on left side of ?? is not nullable.",
4+
"line": 10,
5+
"ignorable": true
6+
}
7+
]

‎tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ public function testCoalesceRule(): void
2525
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
2626
32,
2727
],
28-
[
29-
'Variable $scalar on left side of ?? always exists and is not nullable.',
30-
41,
31-
],
3228
[
3329
'Offset \'string\' on array(1, 2, 3) on left side of ?? does not exist.',
3430
45,
@@ -37,10 +33,6 @@ public function testCoalesceRule(): void
3733
'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ?? does not exist.',
3834
49,
3935
],
40-
[
41-
'Variable $doesNotExist on left side of ?? is never defined.',
42-
51,
43-
],
4436
[
4537
'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ?? always exists and is not nullable.',
4638
67,
@@ -73,10 +65,6 @@ public function testCoalesceRule(): void
7365
'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.',
7466
101,
7567
],
76-
[
77-
'Variable $a on left side of ?? always exists and is always null.',
78-
115,
79-
],
8068
[
8169
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
8270
120,
@@ -116,10 +104,6 @@ public function testCoalesceAssignRule(): void
116104
'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
117105
32,
118106
],
119-
[
120-
'Variable $scalar on left side of ??= always exists and is not nullable.',
121-
41,
122-
],
123107
[
124108
'Offset \'string\' on array(1, 2, 3) on left side of ??= does not exist.',
125109
45,
@@ -128,10 +112,6 @@ public function testCoalesceAssignRule(): void
128112
'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ??= does not exist.',
129113
49,
130114
],
131-
[
132-
'Variable $doesNotExist on left side of ??= is never defined.',
133-
51,
134-
],
135115
[
136116
'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ??= always exists and is not nullable.',
137117
67,
@@ -160,10 +140,6 @@ public function testCoalesceAssignRule(): void
160140
'Static property CoalesceAssignRule\FooCoalesce::$staticAlwaysNull (null) on left side of ??= is always null.',
161141
101,
162142
],
163-
[
164-
'Variable $a on left side of ??= always exists and is always null.',
165-
115,
166-
],
167143
]);
168144
}
169145

‎tests/PHPStan/Rules/Variables/VariableCertaintyNullCoalesceRuleTest.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
/**
6+
* @extends \PHPStan\Testing\RuleTestCase<VariableCertaintyNullCoalesceRule>
7+
*/
8+
class VariableCertaintyNullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase
9+
{
10+
11+
protected function getRule(): \PHPStan\Rules\Rule
12+
{
13+
return new VariableCertaintyNullCoalesceRule();
14+
}
15+
16+
public function testVariableCertaintyInNullCoalesce(): void
17+
{
18+
$this->analyse([__DIR__ . '/data/variable-certainty-null.php'], [
19+
[
20+
'Variable $scalar on left side of ?? always exists and is not nullable.',
21+
6,
22+
],
23+
[
24+
'Variable $doesNotExist on left side of ?? is never defined.',
25+
8,
26+
],
27+
[
28+
'Variable $a on left side of ?? is always null.',
29+
13,
30+
],
31+
[
32+
'Variable $scalar on left side of ??= always exists and is not nullable.',
33+
20,
34+
],
35+
[
36+
'Variable $doesNotExist on left side of ??= is never defined.',
37+
22,
38+
],
39+
[
40+
'Variable $a on left side of ??= is always null.',
41+
27,
42+
],
43+
]);
44+
}
45+
46+
public function testNullCoalesceInGlobalScope(): void
47+
{
48+
$this->analyse([__DIR__ . '/data/null-coalesce-global-scope.php'], [
49+
[
50+
'Variable $bar on left side of ?? always exists and is not nullable.',
51+
6,
52+
],
53+
]);
54+
}
55+
56+
}

‎tests/PHPStan/Rules/Variables/data/null-coalesce-global-scope.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
3+
echo $foo ?? 'foo';
4+
5+
$bar = 'abc';
6+
echo $bar ?? 'bar';

‎tests/PHPStan/Rules/Variables/data/variable-certainty-null.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
function (): void {
4+
$scalar = 3;
5+
6+
echo $scalar ?? 4;
7+
8+
echo $doesNotExist ?? 0;
9+
};
10+
11+
function (?string $a): void {
12+
if (!is_string($a)) {
13+
echo $a ?? 'foo';
14+
}
15+
};
16+
17+
function (): void {
18+
$scalar = 3;
19+
20+
echo $scalar ??= 4;
21+
22+
echo $doesNotExist ??= 0;
23+
};
24+
25+
function (?string $a): void {
26+
if (!is_string($a)) {
27+
echo $a ??= 'foo';
28+
}
29+
};

0 commit comments

Comments
 (0)