Skip to content

Commit 36748fc

Browse files
ondrejmirtesondrejmirtes
committed
Extracted the logic from NullCoalesceRule to IssetCheck
1 parent 3608ded commit 36748fc

File tree

4 files changed

+163
-141
lines changed

4 files changed

+163
-141
lines changed

‎conf/config.neon

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,9 @@ services:
547547
-
548548
class: PHPStan\Rules\Generics\VarianceCheck
549549

550+
-
551+
class: PHPStan\Rules\IssetCheck
552+
550553
-
551554
class: PHPStan\Rules\MissingTypehintCheck
552555
arguments:

‎src/Rules/IssetCheck.php

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules;
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\Type\NullType;
11+
use PHPStan\Type\Type;
12+
use PHPStan\Type\VerbosityLevel;
13+
14+
class IssetCheck
15+
{
16+
17+
/** @var \PHPStan\Rules\Properties\PropertyDescriptor */
18+
private $propertyDescriptor;
19+
20+
/** @var \PHPStan\Rules\Properties\PropertyReflectionFinder */
21+
private $propertyReflectionFinder;
22+
23+
public function __construct(
24+
PropertyDescriptor $propertyDescriptor,
25+
PropertyReflectionFinder $propertyReflectionFinder
26+
)
27+
{
28+
$this->propertyDescriptor = $propertyDescriptor;
29+
$this->propertyReflectionFinder = $propertyReflectionFinder;
30+
}
31+
32+
public function check(Expr $expr, Scope $scope, string $operatorDescription, ?RuleError $error = null): ?RuleError
33+
{
34+
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
35+
36+
$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+
46+
if ($hasVariable->maybe()) {
47+
return null;
48+
}
49+
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+
57+
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
58+
59+
$type = $scope->getType($expr->var);
60+
$dimType = $scope->getType($expr->dim);
61+
$hasOffsetValue = $type->hasOffsetValueType($dimType);
62+
if (!$type->isOffsetAccessible()->yes()) {
63+
return $error;
64+
}
65+
66+
if ($hasOffsetValue->no()) {
67+
return $error ?? RuleErrorBuilder::message(
68+
sprintf(
69+
'Offset %s on %s on left side of %s does not exist.',
70+
$dimType->describe(VerbosityLevel::value()),
71+
$type->describe(VerbosityLevel::value()),
72+
$operatorDescription
73+
)
74+
)->build();
75+
}
76+
77+
if ($hasOffsetValue->maybe()) {
78+
return null;
79+
}
80+
81+
// If offset is cannot be null, store this error message and see if one of the earlier offsets is.
82+
// E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null.
83+
if ($hasOffsetValue->yes()) {
84+
85+
$error = $error ?? $this->generateError($type->getOffsetValueType($dimType), sprintf(
86+
'Offset %s on %s on left side of %s always exists and',
87+
$dimType->describe(VerbosityLevel::value()),
88+
$type->describe(VerbosityLevel::value()),
89+
$operatorDescription
90+
));
91+
92+
if ($error !== null) {
93+
return $this->check($expr->var, $scope, $operatorDescription, $error);
94+
}
95+
}
96+
97+
// Has offset, it is nullable
98+
return null;
99+
100+
} elseif ($expr instanceof Node\Expr\PropertyFetch || $expr instanceof Node\Expr\StaticPropertyFetch) {
101+
102+
$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $scope);
103+
104+
if ($propertyReflection === null) {
105+
return null;
106+
}
107+
108+
$propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $expr);
109+
$propertyType = $propertyReflection->getWritableType();
110+
111+
$error = $error ?? $this->generateError(
112+
$propertyReflection->getWritableType(),
113+
sprintf('%s (%s) on left side of %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription)
114+
);
115+
116+
if ($error !== null) {
117+
if ($expr instanceof Node\Expr\PropertyFetch) {
118+
return $this->check($expr->var, $scope, $operatorDescription, $error);
119+
}
120+
121+
if ($expr->class instanceof Expr) {
122+
return $this->check($expr->class, $scope, $operatorDescription, $error);
123+
}
124+
}
125+
126+
return $error;
127+
}
128+
129+
return $error ?? $this->generateError($scope->getType($expr), sprintf('Left side of %s', $operatorDescription));
130+
}
131+
132+
private function generateError(Type $type, string $message): ?RuleError
133+
{
134+
$nullType = new NullType();
135+
136+
if ($type->equals($nullType)) {
137+
return RuleErrorBuilder::message(
138+
sprintf('%s is always null.', $message)
139+
)->build();
140+
}
141+
142+
if ($type->isSuperTypeOf($nullType)->no()) {
143+
return RuleErrorBuilder::message(
144+
sprintf('%s is not nullable.', $message)
145+
)->build();
146+
}
147+
148+
return null;
149+
}
150+
151+
}

‎src/Rules/Variables/NullCoalesceRule.php

Lines changed: 7 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,21 @@
33
namespace PHPStan\Rules\Variables;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Expr;
76
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;
7+
use PHPStan\Rules\IssetCheck;
158

169
/**
1710
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr>
1811
*/
1912
class NullCoalesceRule implements \PHPStan\Rules\Rule
2013
{
2114

22-
/** @var \PHPStan\Rules\Properties\PropertyDescriptor */
23-
private $propertyDescriptor;
15+
/** @var IssetCheck */
16+
private $issetCheck;
2417

25-
/** @var \PHPStan\Rules\Properties\PropertyReflectionFinder */
26-
private $propertyReflectionFinder;
27-
28-
public function __construct(
29-
PropertyDescriptor $propertyDescriptor,
30-
PropertyReflectionFinder $propertyReflectionFinder
31-
)
18+
public function __construct(IssetCheck $issetCheck)
3219
{
33-
$this->propertyDescriptor = $propertyDescriptor;
34-
$this->propertyReflectionFinder = $propertyReflectionFinder;
20+
$this->issetCheck = $issetCheck;
3521
}
3622

3723
public function getNodeType(): string
@@ -42,9 +28,9 @@ public function getNodeType(): string
4228
public function processNode(Node $node, Scope $scope): array
4329
{
4430
if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
45-
$error = $this->canBeCoalesced($node->left, $scope, '??');
31+
$error = $this->issetCheck->check($node->left, $scope, '??');
4632
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
47-
$error = $this->canBeCoalesced($node->var, $scope, '??=');
33+
$error = $this->issetCheck->check($node->var, $scope, '??=');
4834
} else {
4935
return [];
5036
}
@@ -56,123 +42,4 @@ public function processNode(Node $node, Scope $scope): array
5642
return [$error];
5743
}
5844

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-
17845
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Variables;
44

5+
use PHPStan\Rules\IssetCheck;
56
use PHPStan\Rules\Properties\PropertyDescriptor;
67
use PHPStan\Rules\Properties\PropertyReflectionFinder;
78

@@ -13,7 +14,7 @@ class NullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase
1314

1415
protected function getRule(): \PHPStan\Rules\Rule
1516
{
16-
return new NullCoalesceRule(new PropertyDescriptor(), new PropertyReflectionFinder());
17+
return new NullCoalesceRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
1718
}
1819

1920
public function testCoalesceRule(): void

0 commit comments

Comments
 (0)