Skip to content

Commit 9689fbd

Browse files
committed
Bleeding edge - teach IssetRule everything what VariableCertaintyInIssetRule does
1 parent 601460c commit 9689fbd

13 files changed

+286
-166
lines changed

conf/config.level1.neon

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,15 @@ parameters:
77
reportMagicMethods: true
88
reportMagicProperties: true
99

10-
11-
conditionalTags:
12-
PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule:
13-
phpstan.rules.rule: %featureToggles.nullCoalesce%
14-
1510
rules:
1611
- PHPStan\Rules\Classes\UnusedConstructorParametersRule
1712
- PHPStan\Rules\Constants\ConstantRule
1813
- PHPStan\Rules\Functions\UnusedClosureUsesRule
19-
- PHPStan\Rules\Variables\VariableCertaintyInIssetRule
2014

2115
services:
2216
-
23-
class: PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule
17+
class: PHPStan\Rules\Variables\VariableCertaintyInIssetRule
18+
arguments:
19+
bleedingEdge: %featureToggles.bleedingEdge%
20+
tags:
21+
- phpstan.rules.rule

conf/config.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,8 @@ services:
859859

860860
-
861861
class: PHPStan\Rules\IssetCheck
862+
arguments:
863+
bleedingEdge: %featureToggles.bleedingEdge%
862864

863865
-
864866
# checked as part of OverridingMethodRule

src/Rules/IssetCheck.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,17 @@ class IssetCheck
1818

1919
private \PHPStan\Rules\Properties\PropertyReflectionFinder $propertyReflectionFinder;
2020

21+
private bool $bleedingEdge;
22+
2123
public function __construct(
2224
PropertyDescriptor $propertyDescriptor,
23-
PropertyReflectionFinder $propertyReflectionFinder
25+
PropertyReflectionFinder $propertyReflectionFinder,
26+
bool $bleedingEdge = false
2427
)
2528
{
2629
$this->propertyDescriptor = $propertyDescriptor;
2730
$this->propertyReflectionFinder = $propertyReflectionFinder;
31+
$this->bleedingEdge = $bleedingEdge;
2832
}
2933

3034
/**
@@ -38,6 +42,25 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, cal
3842
return null;
3943
}
4044

45+
if (
46+
$error === null
47+
&& $this->bleedingEdge
48+
) {
49+
if ($hasVariable->yes()) {
50+
if ($expr->name === '_SESSION') {
51+
return null;
52+
}
53+
54+
return $this->generateError(
55+
$scope->getVariableType($expr->name),
56+
sprintf('Variable $%s %s always exists and', $expr->name, $operatorDescription),
57+
$typeMessageCallback
58+
);
59+
}
60+
61+
return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription))->build();
62+
}
63+
4164
return $error;
4265
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
4366

src/Rules/Variables/EmptyRule.php

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Rules\IssetCheck;
88
use PHPStan\Type\Constant\ConstantBooleanType;
9-
use PHPStan\Type\ErrorType;
109
use PHPStan\Type\NullType;
1110
use PHPStan\Type\Type;
1211

@@ -61,14 +60,8 @@ public function processNode(Node $node, Scope $scope): array
6160

6261
return 'is not nullable';
6362
});
64-
if ($error === null) {
65-
return [];
66-
}
6763

68-
$exprType = $scope->getType($node->expr);
69-
$exprBooleanType = $exprType->toBoolean();
70-
$isFalse = (new ConstantBooleanType(false))->isSuperTypeOf($exprBooleanType);
71-
if (!$exprType instanceof ErrorType && $isFalse->maybe()) {
64+
if ($error === null) {
7265
return [];
7366
}
7467

src/Rules/Variables/VariableCertaintyInIssetRule.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,24 @@
1313
class VariableCertaintyInIssetRule implements \PHPStan\Rules\Rule
1414
{
1515

16+
private bool $bleedingEdge;
17+
18+
public function __construct(bool $bleedingEdge = false)
19+
{
20+
$this->bleedingEdge = $bleedingEdge;
21+
}
22+
1623
public function getNodeType(): string
1724
{
1825
return Node\Expr\Isset_::class;
1926
}
2027

2128
public function processNode(Node $node, Scope $scope): array
2229
{
30+
if ($this->bleedingEdge) {
31+
return [];
32+
}
33+
2334
$messages = [];
2435
foreach ($node->vars as $var) {
2536
$isSubNode = false;

src/Rules/Variables/VariableCertaintyNullCoalesceRule.php

Lines changed: 0 additions & 81 deletions
This file was deleted.

tests/PHPStan/Levels/data/coalesce.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,15 @@ function (\ReflectionClass $ref): void {
1010
echo $ref->name ?? 'foo';
1111
echo $ref->nonexistent ?? 'bar';
1212
};
13+
14+
function (?string $s): void {
15+
echo $a ?? 'foo';
16+
17+
echo $s ?? 'bar';
18+
19+
if ($s !== null) {
20+
return;
21+
}
22+
23+
echo $s ?? 'bar';
24+
};

tests/PHPStan/Rules/Variables/EmptyRuleTest.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class EmptyRuleTest extends RuleTestCase
1515

1616
protected function getRule(): \PHPStan\Rules\Rule
1717
{
18-
return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
18+
return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder(), true));
1919
}
2020

2121
public function testRule(): void
@@ -45,6 +45,24 @@ public function testRule(): void
4545
'Offset 2 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is not falsy.',
4646
38,
4747
],
48+
[
49+
'Variable $a in empty() is never defined.',
50+
44,
51+
],
52+
[
53+
'Variable $b in empty() always exists and is not falsy.',
54+
47,
55+
],
56+
]);
57+
}
58+
59+
public function testBug970(): void
60+
{
61+
$this->analyse([__DIR__ . '/data/bug-970.php'], [
62+
[
63+
'aaa',
64+
10,
65+
],
4866
]);
4967
}
5068

tests/PHPStan/Rules/Variables/IssetRuleTest.php

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class IssetRuleTest extends RuleTestCase
1616

1717
protected function getRule(): Rule
1818
{
19-
return new IssetRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
19+
return new IssetRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder(), true));
2020
}
2121

2222
public function testRule(): void
@@ -26,6 +26,10 @@ public function testRule(): void
2626
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
2727
32,
2828
],
29+
[
30+
'Variable $scalar in isset() always exists and is not nullable.',
31+
41,
32+
],
2933
[
3034
'Offset \'string\' on array(1, 2, 3) in isset() does not exist.',
3135
45,
@@ -34,6 +38,10 @@ public function testRule(): void
3438
'Offset \'string\' on array(array(1), array(2), array(3)) in isset() does not exist.',
3539
49,
3640
],
41+
[
42+
'Variable $doesNotExist in isset() is never defined.',
43+
51,
44+
],
3745
[
3846
'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.',
3947
67,
@@ -62,6 +70,10 @@ public function testRule(): void
6270
'Static property IssetRule\FooCoalesce::$staticAlwaysNull (null) in isset() is always null.',
6371
97,
6472
],
73+
[
74+
'Variable $a in isset() always exists and is always null.',
75+
111,
76+
],
6577
[
6678
'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.',
6779
116,
@@ -114,4 +126,97 @@ public function testBug4671(): void
114126
]]);
115127
}
116128

129+
public function testVariableCertaintyInIsset(): void
130+
{
131+
$this->analyse([__DIR__ . '/data/variable-certainty-isset.php'], [
132+
[
133+
'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.',
134+
14,
135+
],
136+
[
137+
'Variable $neverDefinedVariable in isset() is never defined.',
138+
22,
139+
],
140+
[
141+
'Variable $anotherNeverDefinedVariable in isset() is never defined.',
142+
42,
143+
],
144+
[
145+
'Variable $yetAnotherNeverDefinedVariable in isset() is never defined.',
146+
46,
147+
],
148+
[
149+
'Variable $yetYetAnotherNeverDefinedVariableInIsset in isset() is never defined.',
150+
56,
151+
],
152+
[
153+
'Variable $anotherVariableInDoWhile in isset() always exists and is not nullable.',
154+
104,
155+
],
156+
[
157+
'Variable $variableInSecondCase in isset() is never defined.',
158+
110,
159+
],
160+
[
161+
'Variable $variableInFirstCase in isset() always exists and is not nullable.',
162+
112,
163+
],
164+
[
165+
'Variable $variableInFirstCase in isset() always exists and is not nullable.',
166+
116,
167+
],
168+
[
169+
'Variable $variableInSecondCase in isset() always exists and is not nullable.',
170+
117,
171+
],
172+
[
173+
'Variable $variableAssignedInSecondCase in isset() is never defined.',
174+
119,
175+
],
176+
[
177+
'Variable $alwaysDefinedForSwitchCondition in isset() always exists and is not nullable.',
178+
139,
179+
],
180+
[
181+
'Variable $alwaysDefinedForCaseNodeCondition in isset() always exists and is not nullable.',
182+
140,
183+
],
184+
[
185+
'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.',
186+
152,
187+
],
188+
[
189+
'Variable $neverDefinedVariable in isset() is never defined.',
190+
152,
191+
],
192+
[
193+
'Variable $a in isset() always exists and is not nullable.',
194+
214,
195+
],
196+
[
197+
'Variable $null in isset() always exists and is always null.',
198+
225,
199+
],
200+
]);
201+
}
202+
203+
public function testIssetInGlobalScope(): void
204+
{
205+
$this->analyse([__DIR__ . '/data/isset-global-scope.php'], [
206+
[
207+
'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.',
208+
8,
209+
],
210+
]);
211+
}
212+
213+
public function testNullsafe(): void
214+
{
215+
if (PHP_VERSION_ID < 80000 && !self::$useStaticReflectionProvider) {
216+
$this->markTestSkipped('Test requires PHP 8.0.');
217+
}
218+
219+
$this->analyse([__DIR__ . '/data/isset-nullsafe.php'], []);
220+
}
221+
117222
}

0 commit comments

Comments
 (0)