Skip to content

Commit 039361a

Browse files
committed
fixup! Add MemoizationPropertyRule
1 parent 9d72e31 commit 039361a

File tree

4 files changed

+33
-59
lines changed

4 files changed

+33
-59
lines changed

build/PHPStan/Build/MemoizationPropertyRule.php

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use PhpParser\Node\Name;
1616
use PhpParser\Node\Stmt\Expression;
1717
use PhpParser\Node\Stmt\If_;
18-
use PhpParser\Node\Stmt\Return_;
1918
use PhpParser\Node\VarLikeIdentifier;
2019
use PHPStan\Analyser\Scope;
2120
use PHPStan\File\FileHelper;
@@ -25,12 +24,11 @@
2524
use function count;
2625
use function dirname;
2726
use function is_string;
28-
use function sprintf;
2927
use function str_starts_with;
3028
use function strcasecmp;
3129

3230
/**
33-
* @implements Rule<InClassMethodNode>
31+
* @implements Rule<If_>
3432
*/
3533
final class MemoizationPropertyRule implements Rule
3634
{
@@ -41,28 +39,14 @@ public function __construct(private FileHelper $fileHelper, private bool $skipTe
4139

4240
public function getNodeType(): string
4341
{
44-
return InClassMethodNode::class;
42+
return If_::class;
4543
}
4644

4745
public function processNode(Node $node, Scope $scope): array
4846
{
49-
$methodNode = $node->getOriginalNode();
50-
if (count($methodNode->params) !== 0) {
51-
return [];
52-
}
53-
54-
$stmts = $methodNode->getStmts();
55-
if ($stmts === null || count($stmts) !== 2) {
56-
return [];
57-
}
47+
$ifNode = $node;
5848

59-
[$ifNode, $returnNode] = $stmts;
60-
if (!$returnNode instanceof Return_ || !$this->isSupportedFetchNode($returnNode->expr)) {
61-
return [];
62-
}
63-
64-
if (!$ifNode instanceof If_
65-
|| count($ifNode->stmts) !== 1
49+
if (count($ifNode->stmts) !== 1
6650
|| !$ifNode->stmts[0] instanceof Expression
6751
|| count($ifNode->elseifs) !== 0
6852
|| $ifNode->else !== null
@@ -79,33 +63,17 @@ public function processNode(Node $node, Scope $scope): array
7963
return [];
8064
}
8165

82-
if ($this->areNodesNotEqual($returnNode->expr, [$ifNode->cond->left, $ifThenNode->var])) {
66+
if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) {
8367
return [];
8468
}
8569

86-
if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) {
70+
if ($this->areNodesNotEqual($ifNode->cond->left, [$ifThenNode->var])) {
8771
return [];
8872
}
8973

90-
$classReflection = $node->getClassReflection();
91-
$methodReflection = $node->getMethodReflection();
92-
$methodName = $methodNode->name->name;
93-
$errorBuilder = RuleErrorBuilder::message(
94-
sprintf(
95-
'%s %s::%s() for memoization can be simplified.',
96-
$methodReflection->isStatic() ? 'Static method' : 'Method',
97-
$classReflection->getDisplayName(),
98-
$methodName,
99-
),
100-
)->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) {
101-
$method->stmts = [
102-
new Return_(
103-
new Coalesce($ifThenNode->var, $ifThenNode->expr),
104-
),
105-
];
106-
107-
return $method;
108-
})->identifier('phpstan.memoizationProperty');
74+
$errorBuilder = RuleErrorBuilder::message('Memoization property can be simplified.')
75+
->fixNode($node, static fn (If_ $node) => new Expression(new Coalesce($ifThenNode->var, $ifThenNode->expr)))
76+
->identifier('phpstan.memoizationProperty');
10977

11078
return [
11179
$errorBuilder->build(),

tests/PHPStan/Build/MemoizationPropertyRuleTest.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,23 @@ public function testRule(): void
2222
{
2323
$this->analyse([__DIR__ . '/data/memoization-property.php'], [
2424
[
25-
'Method MemoizationProperty\A::getFoo() for memoization can be simplified.',
26-
11,
25+
'Memoization property can be simplified.',
26+
13,
2727
],
2828
[
29-
'Method MemoizationProperty\A::getBar() for memoization can be simplified.',
30-
20,
29+
'Memoization property can be simplified.',
30+
22,
3131
],
3232
[
33-
'Static method MemoizationProperty\A::singleton() for memoization can be simplified.',
33+
'Memoization property can be simplified.',
34+
55,
35+
],
36+
[
37+
'Memoization property can be simplified.',
38+
85,
39+
],
40+
[
41+
'Memoization property can be simplified.',
3442
96,
3543
],
3644
]);

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public function getBarElseIf()
5050
return $this->bar;
5151
}
5252

53-
/** Not applicable because it receives parameters. */
5453
public function getBarReceiveParam(int $length)
5554
{
5655
if ($this->bar === null) {
@@ -81,7 +80,6 @@ public function getBuz()
8180
return $this->buz;
8281
}
8382

84-
/** Not applicable because it doesn't return a memoized property. */
8583
public function printFoo(): void
8684
{
8785
if ($this->foo === null) {

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ final class A
1010

1111
public function getFoo()
1212
{
13-
return $this->foo ??= random_bytes(1);
13+
$this->foo ??= random_bytes(1);
14+
15+
return $this->foo;
1416
}
1517

1618
public function getBar()
1719
{
18-
return $this->bar ??= random_bytes(1);
20+
$this->bar ??= random_bytes(1);
21+
22+
return $this->bar;
1923
}
2024

2125
/** Not applicable because it has an else clause in the if. */
@@ -42,12 +46,9 @@ final class A
4246
return $this->bar;
4347
}
4448

45-
/** Not applicable because it receives parameters. */
4649
public function getBarReceiveParam(int $length)
4750
{
48-
if ($this->bar === null) {
49-
$this->bar = random_bytes($length);
50-
}
51+
$this->bar ??= random_bytes($length);
5152

5253
return $this->bar;
5354
}
@@ -73,12 +74,9 @@ final class A
7374
return $this->buz;
7475
}
7576

76-
/** Not applicable because it doesn't return a memoized property. */
7777
public function printFoo(): void
7878
{
79-
if ($this->foo === null) {
80-
$this->foo = random_bytes(1);
81-
}
79+
$this->foo ??= random_bytes(1);
8280

8381
echo $this->foo;
8482
}
@@ -87,7 +85,9 @@ final class A
8785

8886
public static function singleton(): self
8987
{
90-
return self::$singleton ??= new self();
88+
self::$singleton ??= new self();
89+
90+
return self::$singleton;
9191
}
9292

9393
/** Not applicable because property names are not matched. */

0 commit comments

Comments
 (0)