Skip to content

Commit 468fab5

Browse files
committed
MemoizationPropertyRule supports static properties
1 parent 6d8d1e3 commit 468fab5

File tree

4 files changed

+118
-12
lines changed

4 files changed

+118
-12
lines changed

build/PHPStan/Build/MemoizationPropertyRule.php

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,28 @@
33
namespace PHPStan\Build;
44

55
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
67
use PhpParser\Node\Expr\Assign;
78
use PhpParser\Node\Expr\AssignOp\Coalesce;
89
use PhpParser\Node\Expr\BinaryOp\Identical;
910
use PhpParser\Node\Expr\ConstFetch;
1011
use PhpParser\Node\Expr\PropertyFetch;
12+
use PhpParser\Node\Expr\StaticPropertyFetch;
1113
use PhpParser\Node\Expr\Variable;
1214
use PhpParser\Node\Identifier;
15+
use PhpParser\Node\Name;
1316
use PhpParser\Node\Stmt\Expression;
1417
use PhpParser\Node\Stmt\If_;
1518
use PhpParser\Node\Stmt\Return_;
19+
use PhpParser\Node\VarLikeIdentifier;
1620
use PHPStan\Analyser\Scope;
1721
use PHPStan\File\FileHelper;
1822
use PHPStan\Node\InClassMethodNode;
1923
use PHPStan\Rules\Rule;
2024
use PHPStan\Rules\RuleErrorBuilder;
2125
use function count;
2226
use function dirname;
27+
use function is_string;
2328
use function sprintf;
2429
use function str_starts_with;
2530
use function strcasecmp;
@@ -52,11 +57,7 @@ public function processNode(Node $node, Scope $scope): array
5257
}
5358

5459
[$ifNode, $returnNode] = $stmts;
55-
if (!$returnNode instanceof Return_
56-
|| !$returnNode->expr instanceof PropertyFetch
57-
|| !$returnNode->expr->var instanceof Variable
58-
|| !$returnNode->expr->name instanceof Identifier
59-
) {
60+
if (!$returnNode instanceof Return_ || !$this->isSupportedFetchNode($returnNode->expr)) {
6061
return [];
6162
}
6263

@@ -66,23 +67,19 @@ public function processNode(Node $node, Scope $scope): array
6667
|| count($ifNode->elseifs) !== 0
6768
|| $ifNode->else !== null
6869
|| !$ifNode->cond instanceof Identical
69-
|| !$ifNode->cond->left instanceof PropertyFetch
70-
|| !$ifNode->cond->left->var instanceof Variable
71-
|| !$ifNode->cond->left->name instanceof Identifier
70+
|| !$this->isSupportedFetchNode($ifNode->cond->left)
7271
|| !$ifNode->cond->right instanceof ConstFetch
7372
|| strcasecmp($ifNode->cond->right->name->name, 'null') !== 0
7473
) {
7574
return [];
7675
}
7776

7877
$ifThenNode = $ifNode->stmts[0]->expr;
79-
if (!$ifThenNode instanceof Assign || !$ifThenNode->var instanceof PropertyFetch) {
78+
if (!$ifThenNode instanceof Assign || !$this->isSupportedFetchNode($ifThenNode->var)) {
8079
return [];
8180
}
8281

83-
if ($returnNode->expr->var->name !== $ifNode->cond->left->var->name
84-
|| $returnNode->expr->name->name !== $ifNode->cond->left->name->name
85-
) {
82+
if ($this->areNodesNotEqual($returnNode->expr, [$ifNode->cond->left, $ifThenNode->var])) {
8683
return [];
8784
}
8885

@@ -109,4 +106,71 @@ public function processNode(Node $node, Scope $scope): array
109106
];
110107
}
111108

109+
/**
110+
* @phpstan-assert-if-true PropertyFetch|StaticPropertyFetch $node
111+
*/
112+
private function isSupportedFetchNode(?Expr $node): bool
113+
{
114+
return $node instanceof PropertyFetch || $node instanceof StaticPropertyFetch;
115+
}
116+
117+
/**
118+
* @param list<PropertyFetch|StaticPropertyFetch> $otherNodes
119+
*/
120+
private function areNodesNotEqual(PropertyFetch|StaticPropertyFetch $node, array $otherNodes): bool
121+
{
122+
if ($node instanceof PropertyFetch) {
123+
if (!$node->var instanceof Variable
124+
|| !is_string($node->var->name)
125+
|| !$node->name instanceof Identifier
126+
) {
127+
return true;
128+
}
129+
130+
foreach ($otherNodes as $otherNode) {
131+
if (!$otherNode instanceof PropertyFetch) {
132+
return true;
133+
}
134+
if (!$otherNode->var instanceof Variable
135+
|| !is_string($otherNode->var->name)
136+
|| !$otherNode->name instanceof Identifier
137+
) {
138+
return true;
139+
}
140+
141+
if ($node->var->name !== $otherNode->var->name
142+
|| $node->name->name !== $otherNode->name->name
143+
) {
144+
return true;
145+
}
146+
}
147+
148+
return false;
149+
}
150+
151+
if (!$node->class instanceof Name || !$node->name instanceof VarLikeIdentifier) {
152+
return true;
153+
}
154+
155+
foreach ($otherNodes as $otherNode) {
156+
if (!$otherNode instanceof StaticPropertyFetch) {
157+
return true;
158+
}
159+
160+
if (!$otherNode->class instanceof Name
161+
|| !$otherNode->name instanceof VarLikeIdentifier
162+
) {
163+
return true;
164+
}
165+
166+
if ($node->class->toLowerString() !== $otherNode->class->toLowerString()
167+
|| $node->name->toString() !== $otherNode->name->toString()
168+
) {
169+
return true;
170+
}
171+
}
172+
173+
return false;
174+
}
175+
112176
}

tests/PHPStan/Build/MemoizationPropertyRuleTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ public function testRule(): void
2929
'Method MemoizationProperty\A::getBar() for memoization can be simplified.',
3030
20,
3131
],
32+
[
33+
'Method MemoizationProperty\A::singleton() for memoization can be simplified.',
34+
96,
35+
],
3236
]);
3337
}
3438

tests/PHPStan/Build/data/memoization-property.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,25 @@ public function printFoo(): void
9191
echo $this->foo;
9292
}
9393

94+
private static ?self $sigleton = null;
95+
96+
public function singleton(): self
97+
{
98+
if (self::$sigleton === null) {
99+
self::$sigleton = new self();
100+
}
101+
102+
return self::$sigleton;
103+
}
104+
105+
/** Not applicable because property names are not matched. */
106+
public function singletonBadProperty(): self
107+
{
108+
if (self::$sigleton === null) {
109+
self::$sigletom = new self();
110+
}
111+
112+
return self::$sigleton;
113+
}
114+
94115
}

tests/PHPStan/Build/data/memoization-property.php.fixed

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,21 @@ final class A
8383
echo $this->foo;
8484
}
8585

86+
private static ?self $sigleton = null;
87+
88+
public function singleton(): self
89+
{
90+
return self::$sigleton ??= new self();
91+
}
92+
93+
/** Not applicable because property names are not matched. */
94+
public function singletonBadProperty(): self
95+
{
96+
if (self::$sigleton === null) {
97+
self::$sigletom = new self();
98+
}
99+
100+
return self::$sigleton;
101+
}
102+
86103
}

0 commit comments

Comments
 (0)