Skip to content

Commit 9d72e31

Browse files
committed
MemoizationPropertyRule supports static properties
1 parent 6d8d1e3 commit 9d72e31

File tree

4 files changed

+125
-13
lines changed

4 files changed

+125
-13
lines changed

build/PHPStan/Build/MemoizationPropertyRule.php

Lines changed: 83 additions & 13 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

@@ -91,9 +88,15 @@ public function processNode(Node $node, Scope $scope): array
9188
}
9289

9390
$classReflection = $node->getClassReflection();
91+
$methodReflection = $node->getMethodReflection();
9492
$methodName = $methodNode->name->name;
9593
$errorBuilder = RuleErrorBuilder::message(
96-
sprintf('Method %s::%s() for memoization can be simplified.', $classReflection->getDisplayName(), $methodName),
94+
sprintf(
95+
'%s %s::%s() for memoization can be simplified.',
96+
$methodReflection->isStatic() ? 'Static method' : 'Method',
97+
$classReflection->getDisplayName(),
98+
$methodName,
99+
),
97100
)->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) {
98101
$method->stmts = [
99102
new Return_(
@@ -109,4 +112,71 @@ public function processNode(Node $node, Scope $scope): array
109112
];
110113
}
111114

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

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+
'Static 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 $singleton = null;
95+
96+
public static function singleton(): self
97+
{
98+
if (self::$singleton === null) {
99+
self::$singleton = new self();
100+
}
101+
102+
return self::$singleton;
103+
}
104+
105+
/** Not applicable because property names are not matched. */
106+
public static function singletonBadProperty(): self
107+
{
108+
if (self::$singleton === null) {
109+
self::$singletom = new self();
110+
}
111+
112+
return self::$singleton;
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 $singleton = null;
87+
88+
public static function singleton(): self
89+
{
90+
return self::$singleton ??= new self();
91+
}
92+
93+
/** Not applicable because property names are not matched. */
94+
public static function singletonBadProperty(): self
95+
{
96+
if (self::$singleton === null) {
97+
self::$singletom = new self();
98+
}
99+
100+
return self::$singleton;
101+
}
102+
86103
}

0 commit comments

Comments
 (0)