Skip to content

Commit 25b61d9

Browse files
ondrejmirtesondrejmirtes
committed
Isset rule for checking always-defined/never-defined properties, array offsets etc. (level 4) - bleeding edge
1 parent 70ee0a4 commit 25b61d9

File tree

10 files changed

+315
-12
lines changed

10 files changed

+315
-12
lines changed

‎conf/config.level4.neon

+5
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ rules:
1515
- PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule
1616

1717
conditionalTags:
18+
PHPStan\Rules\Variables\IssetRule:
19+
phpstan.rules.rule: %featureToggles.nullCoalesce%
1820
PHPStan\Rules\Variables\NullCoalesceRule:
1921
phpstan.rules.rule: %featureToggles.nullCoalesce%
2022

@@ -121,5 +123,8 @@ services:
121123
tags:
122124
- phpstan.rules.rule
123125

126+
-
127+
class: PHPStan\Rules\Variables\IssetRule
128+
124129
-
125130
class: PHPStan\Rules\Variables\NullCoalesceRule

‎src/Reflection/ResolvedPropertyReflection.php

+5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ public function __construct(PropertyReflection $reflection, TemplateTypeMap $tem
2929
$this->templateTypeMap = $templateTypeMap;
3030
}
3131

32+
public function getOriginalReflection(): PropertyReflection
33+
{
34+
return $this->reflection;
35+
}
36+
3237
public function getDeclaringClass(): ClassReflection
3338
{
3439
return $this->reflection->getDeclaringClass();

‎src/Rules/IssetCheck.php

+8-4
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
5050
if ($hasOffsetValue->no()) {
5151
return $error ?? RuleErrorBuilder::message(
5252
sprintf(
53-
'Offset %s on %s on left side of %s does not exist.',
53+
'Offset %s on %s %s does not exist.',
5454
$dimType->describe(VerbosityLevel::value()),
5555
$type->describe(VerbosityLevel::value()),
5656
$operatorDescription
@@ -67,7 +67,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
6767
if ($hasOffsetValue->yes()) {
6868

6969
$error = $error ?? $this->generateError($type->getOffsetValueType($dimType), sprintf(
70-
'Offset %s on %s on left side of %s always exists and',
70+
'Offset %s on %s %s always exists and',
7171
$dimType->describe(VerbosityLevel::value()),
7272
$type->describe(VerbosityLevel::value()),
7373
$operatorDescription
@@ -89,12 +89,16 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
8989
return null;
9090
}
9191

92+
if (!$propertyReflection->isNative()) {
93+
return null;
94+
}
95+
9296
$propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $expr);
9397
$propertyType = $propertyReflection->getWritableType();
9498

9599
$error = $error ?? $this->generateError(
96100
$propertyReflection->getWritableType(),
97-
sprintf('%s (%s) on left side of %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription)
101+
sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription)
98102
);
99103

100104
if ($error !== null) {
@@ -110,7 +114,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
110114
return $error;
111115
}
112116

113-
return $error ?? $this->generateError($scope->getType($expr), sprintf('Left side of %s', $operatorDescription));
117+
return $error ?? $this->generateError($scope->getType($expr), sprintf('Expression %s', $operatorDescription));
114118
}
115119

116120
private function generateError(Type $type, string $message): ?RuleError

‎src/Rules/Properties/FoundPropertyReflection.php

+14-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Reflection\ClassReflection;
66
use PHPStan\Reflection\Php\PhpPropertyReflection;
77
use PHPStan\Reflection\PropertyReflection;
8+
use PHPStan\Reflection\ResolvedPropertyReflection;
89
use PHPStan\TrinaryLogic;
910
use PHPStan\Type\Type;
1011

@@ -98,16 +99,26 @@ public function isInternal(): TrinaryLogic
9899

99100
public function isNative(): bool
100101
{
101-
return $this->originalPropertyReflection instanceof PhpPropertyReflection;
102+
$reflection = $this->originalPropertyReflection;
103+
if ($reflection instanceof ResolvedPropertyReflection) {
104+
$reflection = $reflection->getOriginalReflection();
105+
}
106+
107+
return $reflection instanceof PhpPropertyReflection;
102108
}
103109

104110
public function getNativeType(): ?Type
105111
{
106-
if (!$this->originalPropertyReflection instanceof PhpPropertyReflection) {
112+
$reflection = $this->originalPropertyReflection;
113+
if ($reflection instanceof ResolvedPropertyReflection) {
114+
$reflection = $reflection->getOriginalReflection();
115+
}
116+
117+
if (!$reflection instanceof PhpPropertyReflection) {
107118
return null;
108119
}
109120

110-
return $this->originalPropertyReflection->getNativeType();
121+
return $reflection->getNativeType();
111122
}
112123

113124
}

‎src/Rules/Variables/IssetRule.php

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
9+
/**
10+
* @implements \PHPStan\Rules\Rule<Node\Expr\Isset_>
11+
*/
12+
class IssetRule implements \PHPStan\Rules\Rule
13+
{
14+
15+
/** @var IssetCheck */
16+
private $issetCheck;
17+
18+
public function __construct(IssetCheck $issetCheck)
19+
{
20+
$this->issetCheck = $issetCheck;
21+
}
22+
23+
public function getNodeType(): string
24+
{
25+
return Node\Expr\Isset_::class;
26+
}
27+
28+
public function processNode(Node $node, Scope $scope): array
29+
{
30+
$messages = [];
31+
foreach ($node->vars as $var) {
32+
$error = $this->issetCheck->check($var, $scope, 'in isset()');
33+
if ($error === null) {
34+
continue;
35+
}
36+
$messages[] = $error;
37+
}
38+
39+
return $messages;
40+
}
41+
42+
}

‎src/Rules/Variables/NullCoalesceRule.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public function getNodeType(): string
2828
public function processNode(Node $node, Scope $scope): array
2929
{
3030
if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
31-
$error = $this->issetCheck->check($node->left, $scope, '??');
31+
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??');
3232
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
33-
$error = $this->issetCheck->check($node->var, $scope, '??=');
33+
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=');
3434
} else {
3535
return [];
3636
}

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

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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\Rules\Rule;
9+
use PHPStan\Testing\RuleTestCase;
10+
11+
/**
12+
* @extends RuleTestCase<IssetRule>
13+
*/
14+
class IssetRuleTest extends RuleTestCase
15+
{
16+
17+
protected function getRule(): Rule
18+
{
19+
return new IssetRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
20+
}
21+
22+
public function testRule(): void
23+
{
24+
$this->analyse([__DIR__ . '/data/isset.php'], [
25+
[
26+
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
27+
32,
28+
],
29+
[
30+
'Offset \'string\' on array(1, 2, 3) in isset() does not exist.',
31+
45,
32+
],
33+
[
34+
'Offset \'string\' on array(array(1), array(2), array(3)) in isset() does not exist.',
35+
49,
36+
],
37+
[
38+
'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) in isset() always exists and is not nullable.',
39+
67,
40+
],
41+
[
42+
'Offset \'b\' on array() in isset() does not exist.',
43+
79,
44+
],
45+
[
46+
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
47+
85,
48+
],
49+
[
50+
'Property IssetRule\FooCoalesce::$alwaysNull (null) in isset() is always null.',
51+
87,
52+
],
53+
[
54+
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
55+
89,
56+
],
57+
[
58+
'Static property IssetRule\FooCoalesce::$staticString (string) in isset() is not nullable.',
59+
95,
60+
],
61+
[
62+
'Static property IssetRule\FooCoalesce::$staticAlwaysNull (null) in isset() is always null.',
63+
97,
64+
],
65+
[
66+
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
67+
116,
68+
],
69+
[
70+
'Property IssetRule\FooCoalesce::$alwaysNull (null) in isset() is always null.',
71+
118,
72+
],
73+
[
74+
'Static property IssetRule\FooCoalesce::$staticAlwaysNull (null) in isset() is always null.',
75+
123,
76+
],
77+
[
78+
'Static property IssetRule\FooCoalesce::$staticString (string) in isset() is not nullable.',
79+
124,
80+
],
81+
]);
82+
}
83+
84+
}

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function testCoalesceRule(): void
4242
79,
4343
],
4444
[
45-
'Left side of ?? is not nullable.',
45+
'Expression on left side of ?? is not nullable.',
4646
81,
4747
],
4848
[
@@ -74,11 +74,11 @@ public function testCoalesceRule(): void
7474
122,
7575
],
7676
[
77-
'Left side of ?? is not nullable.',
77+
'Expression on left side of ?? is not nullable.',
7878
124,
7979
],
8080
[
81-
'Left side of ?? is always null.',
81+
'Expression on left side of ?? is always null.',
8282
125,
8383
],
8484
[
@@ -89,6 +89,10 @@ public function testCoalesceRule(): void
8989
'Static property CoalesceRule\FooCoalesce::$staticString (string) on left side of ?? is not nullable.',
9090
131,
9191
],
92+
[
93+
'Property ReflectionClass<object>::$name (class-string<object>) on left side of ?? is not nullable.',
94+
136,
95+
],
9296
]);
9397
}
9498

0 commit comments

Comments
 (0)