Skip to content

Commit 3608ded

Browse files
leongersenondrejmirtes
leongersen
authored and
ondrejmirtes
committed
Rule for checking null-coalescing operator (level 1) - bleeding edge
1 parent a68512e commit 3608ded

File tree

7 files changed

+609
-1
lines changed

7 files changed

+609
-1
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@ parameters:
33
closureUsesThis: true
44
randomIntParameters: true
55
resultCache: true
6+
nullCoalesce: true

‎conf/config.level1.neon

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

10+
conditionalTags:
11+
PHPStan\Rules\Variables\NullCoalesceRule:
12+
phpstan.rules.rule: %featureToggles.nullCoalesce%
13+
1014
rules:
1115
- PHPStan\Rules\Classes\UnusedConstructorParametersRule
1216
- PHPStan\Rules\Constants\ConstantRule
1317
- PHPStan\Rules\Functions\UnusedClosureUsesRule
1418
- PHPStan\Rules\Variables\VariableCertaintyInIssetRule
19+
20+
services:
21+
-
22+
class: PHPStan\Rules\Variables\NullCoalesceRule

‎conf/config.neon

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ parameters:
1313
resultCache: false
1414
closureUsesThis: false
1515
randomIntParameters: false
16+
nullCoalesce: false
1617
fileExtensions:
1718
- php
1819
checkAlwaysTrueCheckTypeFunctionCall: false
@@ -134,7 +135,8 @@ parametersSchema:
134135
enableScanningPaths: bool(),
135136
resultCache: bool(),
136137
closureUsesThis: bool(),
137-
randomIntParameters: bool()
138+
randomIntParameters: bool(),
139+
nullCoalesce: bool()
138140
])
139141
fileExtensions: listOf(string())
140142
checkAlwaysTrueCheckTypeFunctionCall: bool()

‎src/Rules/Variables/NullCoalesceRule.php

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Rules\Properties\PropertyDescriptor;
9+
use PHPStan\Rules\Properties\PropertyReflectionFinder;
10+
use PHPStan\Rules\RuleError;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
use PHPStan\Type\NullType;
13+
use PHPStan\Type\Type;
14+
use PHPStan\Type\VerbosityLevel;
15+
16+
/**
17+
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr>
18+
*/
19+
class NullCoalesceRule implements \PHPStan\Rules\Rule
20+
{
21+
22+
/** @var \PHPStan\Rules\Properties\PropertyDescriptor */
23+
private $propertyDescriptor;
24+
25+
/** @var \PHPStan\Rules\Properties\PropertyReflectionFinder */
26+
private $propertyReflectionFinder;
27+
28+
public function __construct(
29+
PropertyDescriptor $propertyDescriptor,
30+
PropertyReflectionFinder $propertyReflectionFinder
31+
)
32+
{
33+
$this->propertyDescriptor = $propertyDescriptor;
34+
$this->propertyReflectionFinder = $propertyReflectionFinder;
35+
}
36+
37+
public function getNodeType(): string
38+
{
39+
return \PhpParser\Node\Expr::class;
40+
}
41+
42+
public function processNode(Node $node, Scope $scope): array
43+
{
44+
if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
45+
$error = $this->canBeCoalesced($node->left, $scope, '??');
46+
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
47+
$error = $this->canBeCoalesced($node->var, $scope, '??=');
48+
} else {
49+
return [];
50+
}
51+
52+
if ($error === null) {
53+
return [];
54+
}
55+
56+
return [$error];
57+
}
58+
59+
private function canBeCoalesced(Expr $expr, Scope $scope, string $action, ?RuleError $error = null): ?RuleError
60+
{
61+
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
62+
63+
$hasVariable = $scope->hasVariableType($expr->name);
64+
65+
if ($hasVariable->no()) {
66+
return $error ?? RuleErrorBuilder::message(
67+
sprintf('Variable $%s on left side of %s is never defined.', $expr->name, $action)
68+
)->build();
69+
}
70+
71+
$variableType = $scope->getType($expr);
72+
73+
if ($hasVariable->maybe()) {
74+
return null;
75+
}
76+
77+
if ($hasVariable->yes()) {
78+
return $error ?? $this->generateError(
79+
$variableType,
80+
sprintf('Variable $%s on left side of %s always exists and', $expr->name, $action)
81+
);
82+
}
83+
84+
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
85+
86+
$type = $scope->getType($expr->var);
87+
$dimType = $scope->getType($expr->dim);
88+
$hasOffsetValue = $type->hasOffsetValueType($dimType);
89+
if (!$type->isOffsetAccessible()->yes()) {
90+
return $error;
91+
}
92+
93+
if ($hasOffsetValue->no()) {
94+
return $error ?? RuleErrorBuilder::message(
95+
sprintf(
96+
'Offset %s on %s on left side of %s does not exist.',
97+
$dimType->describe(VerbosityLevel::value()),
98+
$type->describe(VerbosityLevel::value()),
99+
$action
100+
)
101+
)->build();
102+
}
103+
104+
if ($hasOffsetValue->maybe()) {
105+
return null;
106+
}
107+
108+
// If offset is cannot be null, store this error message and see if one of the earlier offsets is.
109+
// E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null.
110+
if ($hasOffsetValue->yes()) {
111+
112+
$error = $error ?? $this->generateError($type->getOffsetValueType($dimType), sprintf(
113+
'Offset %s on %s on left side of %s always exists and',
114+
$dimType->describe(VerbosityLevel::value()),
115+
$type->describe(VerbosityLevel::value()),
116+
$action
117+
));
118+
119+
if ($error !== null) {
120+
return $this->canBeCoalesced($expr->var, $scope, $action, $error);
121+
}
122+
}
123+
124+
// Has offset, it is nullable
125+
return null;
126+
127+
} elseif ($expr instanceof Node\Expr\PropertyFetch || $expr instanceof Node\Expr\StaticPropertyFetch) {
128+
129+
$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $scope);
130+
131+
if ($propertyReflection === null) {
132+
return null;
133+
}
134+
135+
$propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $expr);
136+
$propertyType = $propertyReflection->getWritableType();
137+
138+
$error = $error ?? $this->generateError(
139+
$propertyReflection->getWritableType(),
140+
sprintf('%s (%s) on left side of %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $action)
141+
);
142+
143+
if ($error !== null) {
144+
if ($expr instanceof Node\Expr\PropertyFetch) {
145+
return $this->canBeCoalesced($expr->var, $scope, $action, $error);
146+
}
147+
148+
if ($expr->class instanceof Expr) {
149+
return $this->canBeCoalesced($expr->class, $scope, $action, $error);
150+
}
151+
}
152+
153+
return $error;
154+
}
155+
156+
return $error ?? $this->generateError($scope->getType($expr), sprintf('Left side of %s', $action));
157+
}
158+
159+
private function generateError(Type $type, string $message): ?RuleError
160+
{
161+
$nullType = new NullType();
162+
163+
if ($type->equals($nullType)) {
164+
return RuleErrorBuilder::message(
165+
sprintf('%s is always null.', $message)
166+
)->build();
167+
}
168+
169+
if ($type->isSuperTypeOf($nullType)->no()) {
170+
return RuleErrorBuilder::message(
171+
sprintf('%s is not nullable.', $message)
172+
)->build();
173+
}
174+
175+
return null;
176+
}
177+
178+
}

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

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PHPStan\Rules\Properties\PropertyDescriptor;
6+
use PHPStan\Rules\Properties\PropertyReflectionFinder;
7+
8+
/**
9+
* @extends \PHPStan\Testing\RuleTestCase<NullCoalesceRule>
10+
*/
11+
class NullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase
12+
{
13+
14+
protected function getRule(): \PHPStan\Rules\Rule
15+
{
16+
return new NullCoalesceRule(new PropertyDescriptor(), new PropertyReflectionFinder());
17+
}
18+
19+
public function testCoalesceRule(): void
20+
{
21+
require_once __DIR__ . '/data/null-coalesce.php';
22+
$this->analyse([__DIR__ . '/data/null-coalesce.php'], [
23+
[
24+
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
25+
32,
26+
],
27+
[
28+
'Variable $scalar on left side of ?? always exists and is not nullable.',
29+
41,
30+
],
31+
[
32+
'Offset \'string\' on array(1, 2, 3) on left side of ?? does not exist.',
33+
45,
34+
],
35+
[
36+
'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ?? does not exist.',
37+
49,
38+
],
39+
[
40+
'Variable $doesNotExist on left side of ?? is never defined.',
41+
51,
42+
],
43+
[
44+
'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.',
45+
67,
46+
],
47+
[
48+
'Offset \'b\' on array() on left side of ?? does not exist.',
49+
79,
50+
],
51+
[
52+
'Left side of ?? is not nullable.',
53+
81,
54+
],
55+
[
56+
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
57+
89,
58+
],
59+
[
60+
'Property CoalesceRule\FooCoalesce::$alwaysNull (null) on left side of ?? is always null.',
61+
91,
62+
],
63+
[
64+
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
65+
93,
66+
],
67+
[
68+
'Static property CoalesceRule\FooCoalesce::$staticString (string) on left side of ?? is not nullable.',
69+
99,
70+
],
71+
[
72+
'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.',
73+
101,
74+
],
75+
[
76+
'Variable $a on left side of ?? always exists and is always null.',
77+
115,
78+
],
79+
[
80+
'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
81+
120,
82+
],
83+
[
84+
'Property CoalesceRule\FooCoalesce::$alwaysNull (null) on left side of ?? is always null.',
85+
122,
86+
],
87+
[
88+
'Left side of ?? is not nullable.',
89+
124,
90+
],
91+
[
92+
'Left side of ?? is always null.',
93+
125,
94+
],
95+
[
96+
'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.',
97+
130,
98+
],
99+
[
100+
'Static property CoalesceRule\FooCoalesce::$staticString (string) on left side of ?? is not nullable.',
101+
131,
102+
],
103+
]);
104+
}
105+
106+
public function testCoalesceAssignRule(): void
107+
{
108+
if (PHP_VERSION_ID < 70400) {
109+
$this->markTestSkipped('Test requires PHP 7.4.');
110+
}
111+
112+
require_once __DIR__ . '/data/null-coalesce-assign.php';
113+
$this->analyse([__DIR__ . '/data/null-coalesce-assign.php'], [
114+
[
115+
'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
116+
32,
117+
],
118+
[
119+
'Variable $scalar on left side of ??= always exists and is not nullable.',
120+
41,
121+
],
122+
[
123+
'Offset \'string\' on array(1, 2, 3) on left side of ??= does not exist.',
124+
45,
125+
],
126+
[
127+
'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ??= does not exist.',
128+
49,
129+
],
130+
[
131+
'Variable $doesNotExist on left side of ??= is never defined.',
132+
51,
133+
],
134+
[
135+
'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.',
136+
67,
137+
],
138+
[
139+
'Offset \'b\' on array() on left side of ??= does not exist.',
140+
79,
141+
],
142+
[
143+
'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
144+
89,
145+
],
146+
[
147+
'Property CoalesceAssignRule\FooCoalesce::$alwaysNull (null) on left side of ??= is always null.',
148+
91,
149+
],
150+
[
151+
'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
152+
93,
153+
],
154+
[
155+
'Static property CoalesceAssignRule\FooCoalesce::$staticString (string) on left side of ??= is not nullable.',
156+
99,
157+
],
158+
[
159+
'Static property CoalesceAssignRule\FooCoalesce::$staticAlwaysNull (null) on left side of ??= is always null.',
160+
101,
161+
],
162+
[
163+
'Variable $a on left side of ??= always exists and is always null.',
164+
115,
165+
],
166+
]);
167+
}
168+
169+
}

0 commit comments

Comments
 (0)