Skip to content

Commit 601460c

Browse files
ondrejmirtesondrejmirtes
committed
Bleeding edge - empty() rule
1 parent 1ef74b7 commit 601460c

File tree

7 files changed

+228
-25
lines changed

7 files changed

+228
-25
lines changed

‎conf/config.level4.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ conditionalTags:
2828
phpstan.rules.rule: %featureToggles.unusedClassElements%
2929
PHPStan\Rules\DeadCode\UnusedPrivatePropertyRule:
3030
phpstan.rules.rule: %featureToggles.unusedClassElements%
31+
PHPStan\Rules\Variables\EmptyRule:
32+
phpstan.rules.rule: %featureToggles.nullCoalesce%
3133
PHPStan\Rules\Variables\IssetRule:
3234
phpstan.rules.rule: %featureToggles.nullCoalesce%
3335
PHPStan\Rules\Variables\NullCoalesceRule:
@@ -164,6 +166,9 @@ services:
164166
tags:
165167
- phpstan.rules.rule
166168

169+
-
170+
class: PHPStan\Rules\Variables\EmptyRule
171+
167172
-
168173
class: PHPStan\Rules\Variables\IssetRule
169174

‎src/Rules/IssetCheck.php

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use PHPStan\Rules\Properties\PropertyDescriptor;
99
use PHPStan\Rules\Properties\PropertyReflectionFinder;
1010
use PHPStan\Type\MixedType;
11-
use PHPStan\Type\NullType;
1211
use PHPStan\Type\Type;
1312
use PHPStan\Type\VerbosityLevel;
1413

@@ -28,7 +27,10 @@ public function __construct(
2827
$this->propertyReflectionFinder = $propertyReflectionFinder;
2928
}
3029

31-
public function check(Expr $expr, Scope $scope, string $operatorDescription, ?RuleError $error = null): ?RuleError
30+
/**
31+
* @param callable(Type): ?string $typeMessageCallback
32+
*/
33+
public function check(Expr $expr, Scope $scope, string $operatorDescription, callable $typeMessageCallback, ?RuleError $error = null): ?RuleError
3234
{
3335
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
3436
$hasVariable = $scope->hasVariableType($expr->name);
@@ -70,10 +72,10 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
7072
$dimType->describe(VerbosityLevel::value()),
7173
$type->describe(VerbosityLevel::value()),
7274
$operatorDescription
73-
));
75+
), $typeMessageCallback);
7476

7577
if ($error !== null) {
76-
return $this->check($expr->var, $scope, $operatorDescription, $error);
78+
return $this->check($expr->var, $scope, $operatorDescription, $typeMessageCallback, $error);
7779
}
7880
}
7981

@@ -104,42 +106,39 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
104106

105107
$error = $error ?? $this->generateError(
106108
$propertyReflection->getWritableType(),
107-
sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription)
109+
sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription),
110+
$typeMessageCallback
108111
);
109112

110113
if ($error !== null) {
111114
if ($expr instanceof Node\Expr\PropertyFetch) {
112-
return $this->check($expr->var, $scope, $operatorDescription, $error);
115+
return $this->check($expr->var, $scope, $operatorDescription, $typeMessageCallback, $error);
113116
}
114117

115118
if ($expr->class instanceof Expr) {
116-
return $this->check($expr->class, $scope, $operatorDescription, $error);
119+
return $this->check($expr->class, $scope, $operatorDescription, $typeMessageCallback, $error);
117120
}
118121
}
119122

120123
return $error;
121124
}
122125

123-
return $error ?? $this->generateError($scope->getType($expr), sprintf('Expression %s', $operatorDescription));
126+
return $error ?? $this->generateError($scope->getType($expr), sprintf('Expression %s', $operatorDescription), $typeMessageCallback);
124127
}
125128

126-
private function generateError(Type $type, string $message): ?RuleError
129+
/**
130+
* @param callable(Type): ?string $typeMessageCallback
131+
*/
132+
private function generateError(Type $type, string $message, callable $typeMessageCallback): ?RuleError
127133
{
128-
$nullType = new NullType();
129-
130-
if ($type->equals($nullType)) {
131-
return RuleErrorBuilder::message(
132-
sprintf('%s is always null.', $message)
133-
)->build();
134-
}
135-
136-
if ($type->isSuperTypeOf($nullType)->no()) {
137-
return RuleErrorBuilder::message(
138-
sprintf('%s is not nullable.', $message)
139-
)->build();
134+
$typeMessage = $typeMessageCallback($type);
135+
if ($typeMessage === null) {
136+
return null;
140137
}
141138

142-
return null;
139+
return RuleErrorBuilder::message(
140+
sprintf('%s %s.', $message, $typeMessage)
141+
)->build();
143142
}
144143

145144
}

‎src/Rules/Variables/EmptyRule.php

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
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\IssetCheck;
8+
use PHPStan\Type\Constant\ConstantBooleanType;
9+
use PHPStan\Type\ErrorType;
10+
use PHPStan\Type\NullType;
11+
use PHPStan\Type\Type;
12+
13+
/**
14+
* @implements \PHPStan\Rules\Rule<Node\Expr\Empty_>
15+
*/
16+
class EmptyRule implements \PHPStan\Rules\Rule
17+
{
18+
19+
private IssetCheck $issetCheck;
20+
21+
public function __construct(IssetCheck $issetCheck)
22+
{
23+
$this->issetCheck = $issetCheck;
24+
}
25+
26+
public function getNodeType(): string
27+
{
28+
return Node\Expr\Empty_::class;
29+
}
30+
31+
public function processNode(Node $node, Scope $scope): array
32+
{
33+
$error = $this->issetCheck->check($node->expr, $scope, 'in empty()', static function (Type $type): ?string {
34+
$isNull = (new NullType())->isSuperTypeOf($type);
35+
$isFalsey = (new ConstantBooleanType(false))->isSuperTypeOf($type->toBoolean());
36+
if ($isNull->maybe()) {
37+
return null;
38+
}
39+
if ($isFalsey->maybe()) {
40+
return null;
41+
}
42+
43+
if ($isNull->yes()) {
44+
if ($isFalsey->yes()) {
45+
return 'is always falsy';
46+
}
47+
if ($isFalsey->no()) {
48+
return 'is not falsy';
49+
}
50+
51+
return 'is always null';
52+
}
53+
54+
if ($isFalsey->yes()) {
55+
return 'is always falsy';
56+
}
57+
58+
if ($isFalsey->no()) {
59+
return 'is not falsy';
60+
}
61+
62+
return 'is not nullable';
63+
});
64+
if ($error === null) {
65+
return [];
66+
}
67+
68+
$exprType = $scope->getType($node->expr);
69+
$exprBooleanType = $exprType->toBoolean();
70+
$isFalse = (new ConstantBooleanType(false))->isSuperTypeOf($exprBooleanType);
71+
if (!$exprType instanceof ErrorType && $isFalse->maybe()) {
72+
return [];
73+
}
74+
75+
return [$error];
76+
}
77+
78+
}

‎src/Rules/Variables/IssetRule.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Rules\IssetCheck;
8+
use PHPStan\Type\NullType;
9+
use PHPStan\Type\Type;
810

911
/**
1012
* @implements \PHPStan\Rules\Rule<Node\Expr\Isset_>
@@ -28,7 +30,18 @@ public function processNode(Node $node, Scope $scope): array
2830
{
2931
$messages = [];
3032
foreach ($node->vars as $var) {
31-
$error = $this->issetCheck->check($var, $scope, 'in isset()');
33+
$error = $this->issetCheck->check($var, $scope, 'in isset()', static function (Type $type): ?string {
34+
$isNull = (new NullType())->isSuperTypeOf($type);
35+
if ($isNull->maybe()) {
36+
return null;
37+
}
38+
39+
if ($isNull->yes()) {
40+
return 'is always null';
41+
}
42+
43+
return 'is not nullable';
44+
});
3245
if ($error === null) {
3346
continue;
3447
}

‎src/Rules/Variables/NullCoalesceRule.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Rules\IssetCheck;
8+
use PHPStan\Type\NullType;
9+
use PHPStan\Type\Type;
810

911
/**
1012
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr>
@@ -26,10 +28,23 @@ public function getNodeType(): string
2628

2729
public function processNode(Node $node, Scope $scope): array
2830
{
31+
$typeMessageCallback = static function (Type $type): ?string {
32+
$isNull = (new NullType())->isSuperTypeOf($type);
33+
if ($isNull->maybe()) {
34+
return null;
35+
}
36+
37+
if ($isNull->yes()) {
38+
return 'is always null';
39+
}
40+
41+
return 'is not nullable';
42+
};
43+
2944
if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
30-
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??');
45+
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??', $typeMessageCallback);
3146
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
32-
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=');
47+
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=', $typeMessageCallback);
3348
} else {
3449
return [];
3550
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PHPStan\Rules\IssetCheck;
6+
use PHPStan\Rules\Properties\PropertyDescriptor;
7+
use PHPStan\Rules\Properties\PropertyReflectionFinder;
8+
use PHPStan\Testing\RuleTestCase;
9+
10+
/**
11+
* @extends RuleTestCase<EmptyRule>
12+
*/
13+
class EmptyRuleTest extends RuleTestCase
14+
{
15+
16+
protected function getRule(): \PHPStan\Rules\Rule
17+
{
18+
return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
19+
}
20+
21+
public function testRule(): void
22+
{
23+
$this->analyse([__DIR__ . '/data/empty-rule.php'], [
24+
[
25+
'Offset \'nonexistent\' on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() does not exist.',
26+
22,
27+
],
28+
[
29+
'Offset 3 on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() always exists and is always falsy.',
30+
24,
31+
],
32+
[
33+
'Offset 4 on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() always exists and is not falsy.',
34+
25,
35+
],
36+
[
37+
'Offset 0 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is always falsy.',
38+
36,
39+
],
40+
[
41+
'Offset 1 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is always falsy.',
42+
37,
43+
],
44+
[
45+
'Offset 2 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is not falsy.',
46+
38,
47+
],
48+
]);
49+
}
50+
51+
}

‎tests/PHPStan/Rules/Variables/data/empty-rule.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace EmptyRule;
4+
5+
class Foo
6+
{
7+
8+
public function doFoo()
9+
{
10+
$a = [];
11+
if (rand(0, 1)) {
12+
$a[0] = rand(0,1) ? true : false;
13+
$a[1] = false;
14+
}
15+
16+
$a[2] = rand(0,1) ? true : false;
17+
$a[3] = false;
18+
$a[4] = true;
19+
20+
empty($a[0]);
21+
empty($a[1]);
22+
empty($a['nonexistent']);
23+
empty($a[2]);
24+
empty($a[3]);
25+
empty($a[4]);
26+
}
27+
28+
public function doBar()
29+
{
30+
$a = [
31+
'',
32+
'0',
33+
'foo',
34+
rand(0, 1) ? '' : 'foo',
35+
];
36+
empty($a[0]);
37+
empty($a[1]);
38+
empty($a[2]);
39+
empty($a[3]);
40+
}
41+
42+
}

0 commit comments

Comments
 (0)