Skip to content

Commit c67cc1f

Browse files
zonuexezonuexe
committed
Add MemoizationPropertyRule
1 parent 7d6bbec commit c67cc1f

File tree

5 files changed

+334
-0
lines changed

5 files changed

+334
-0
lines changed

‎build/PHPStan/Build/MemoizationPropertyRule.php

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\Assign;
7+
use PhpParser\Node\Expr\AssignOp\Coalesce;
8+
use PhpParser\Node\Expr\BinaryOp\Identical;
9+
use PhpParser\Node\Expr\ConstFetch;
10+
use PhpParser\Node\Expr\PropertyFetch;
11+
use PhpParser\Node\Identifier;
12+
use PhpParser\Node\Stmt\Expression;
13+
use PhpParser\Node\Stmt\If_;
14+
use PhpParser\Node\Stmt\Return_;
15+
use PHPStan\Analyser\Scope;
16+
use PHPStan\File\FileHelper;
17+
use PHPStan\Node\InClassMethodNode;
18+
use PHPStan\Rules\Rule;
19+
use PHPStan\Rules\RuleErrorBuilder;
20+
use function count;
21+
use function dirname;
22+
use function sprintf;
23+
use function str_starts_with;
24+
use function strcasecmp;
25+
26+
/**
27+
* @implements Rule<InClassMethodNode>
28+
*/
29+
final class MemoizationPropertyRule implements Rule
30+
{
31+
32+
public function __construct(private FileHelper $fileHelper, private bool $skipTests = true)
33+
{
34+
}
35+
36+
public function getNodeType(): string
37+
{
38+
return InClassMethodNode::class;
39+
}
40+
41+
public function processNode(Node $node, Scope $scope): array
42+
{
43+
$methodNode = $node->getOriginalNode();
44+
if (count($methodNode->params) !== 0) {
45+
return [];
46+
}
47+
48+
$stmts = $methodNode->getStmts();
49+
if ($stmts === null || count($stmts) !== 2) {
50+
return [];
51+
}
52+
53+
[$ifNode, $returnNode] = $stmts;
54+
55+
if (!$returnNode instanceof Return_ ||
56+
!$returnNode->expr instanceof PropertyFetch
57+
) {
58+
return [];
59+
}
60+
61+
if (!$ifNode instanceof If_
62+
|| count($ifNode->stmts) !== 1
63+
|| !$ifNode->stmts[0] instanceof Expression
64+
|| count($ifNode->elseifs) !== 0
65+
|| $ifNode->else !== null
66+
|| !$ifNode->cond instanceof Identical
67+
|| !$ifNode->cond->left instanceof PropertyFetch
68+
|| !$ifNode->cond->right instanceof ConstFetch
69+
|| strcasecmp($ifNode->cond->right->name->name, 'null') !== 0
70+
) {
71+
return [];
72+
}
73+
74+
$ifThenNode = $ifNode->stmts[0]->expr;
75+
76+
if (!$ifThenNode instanceof Assign || !$ifThenNode->var instanceof PropertyFetch) {
77+
return [];
78+
}
79+
80+
if ($returnNode->expr::class !== $ifNode->cond->left::class
81+
|| $returnNode->expr->var::class !== $ifNode->cond->left->var::class
82+
|| !$returnNode->expr->name instanceof Identifier
83+
|| !$ifNode->cond->left->name instanceof Identifier
84+
|| $returnNode->expr->name->name !== $ifNode->cond->left->name->name
85+
) {
86+
return [];
87+
}
88+
89+
if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) {
90+
return [];
91+
}
92+
93+
$classReflection = $node->getClassReflection();
94+
$methodName = $methodNode->name->name;
95+
$errorBuilder = RuleErrorBuilder::message(
96+
sprintf('Method %s::%s() for memoization can be simplified.', $classReflection->getDisplayName(), $methodName),
97+
)->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) {
98+
$method->stmts = [
99+
new Return_(
100+
new Coalesce($ifThenNode->var, $ifThenNode->expr),
101+
),
102+
];
103+
104+
return $method;
105+
})->identifier('phpstan.memoizationProperty');
106+
107+
return [
108+
$errorBuilder->build(),
109+
];
110+
}
111+
112+
}

‎build/phpstan.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ rules:
123123
- PHPStan\Build\NamedArgumentsRule
124124
- PHPStan\Build\OverrideAttributeThirdPartyMethodRule
125125
- PHPStan\Build\SkipTestsWithRequiresPhpAttributeRule
126+
- PHPStan\Build\MemoizationPropertyRule
126127

127128
services:
128129
-

‎tests/PHPStan/Build/MemoizationPropertyRuleTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PHPStan\File\FileHelper;
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\Testing\RuleTestCase;
8+
use PHPUnit\Framework\Attributes\RequiresPhp;
9+
10+
/**
11+
* @extends RuleTestCase<MemoizationPropertyRule>
12+
*/
13+
final class MemoizationPropertyRuleTest extends RuleTestCase
14+
{
15+
16+
protected function getRule(): Rule
17+
{
18+
return new MemoizationPropertyRule(self::getContainer()->getByType(FileHelper::class), false);
19+
}
20+
21+
public function testRule(): void
22+
{
23+
$this->analyse([__DIR__ . '/data/memoization-property.php'], [
24+
[
25+
'Method MemoizationProperty\A::getFoo() for memoization can be simplified.',
26+
11,
27+
],
28+
[
29+
'Method MemoizationProperty\A::getBar() for memoization can be simplified.',
30+
20,
31+
],
32+
]);
33+
}
34+
35+
#[RequiresPhp('>= 8.0')]
36+
public function testFix(): void
37+
{
38+
$this->fix(__DIR__ . '/data/memoization-property.php', __DIR__ . '/data/memoization-property.php.fixed');
39+
}
40+
41+
}

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php // lint >= 8.0
2+
3+
namespace MemoizationProperty;
4+
5+
final class A
6+
{
7+
private ?string $foo = null;
8+
private ?string $bar = null;
9+
private string|false $buz = false;
10+
11+
public function getFoo()
12+
{
13+
if ($this->foo === null) {
14+
$this->foo = random_bytes(1);
15+
}
16+
17+
return $this->foo;
18+
}
19+
20+
public function getBar()
21+
{
22+
if ($this->bar === null) {
23+
$this->bar = random_bytes(1);
24+
}
25+
26+
return $this->bar;
27+
}
28+
29+
/** Not applicable because it has an else clause in the if. */
30+
public function getBarElse()
31+
{
32+
if ($this->bar === null) {
33+
$this->bar = random_bytes(1);
34+
} else {
35+
// no-op
36+
}
37+
38+
return $this->bar;
39+
}
40+
41+
/** Not applicable because it has an elseif clause in the if. */
42+
public function getBarElseIf()
43+
{
44+
if ($this->bar === null) {
45+
$this->bar = random_bytes(1);
46+
} elseif (false) {
47+
// no-op
48+
}
49+
50+
return $this->bar;
51+
}
52+
53+
/** Not applicable because it receives parameters. */
54+
public function getBarReceiveParam(int $length)
55+
{
56+
if ($this->bar === null) {
57+
$this->bar = random_bytes($length);
58+
}
59+
60+
return $this->bar;
61+
}
62+
63+
/** Not applicable because the body of if is not just an assignment. */
64+
public function getBarComplex()
65+
{
66+
if ($this->bar === null) {
67+
$rand = random_bytes(1);
68+
$this->bar = $rand;
69+
}
70+
71+
return $this->bar;
72+
}
73+
74+
/** Not applicable because it is comparing a property with a non-null value. */
75+
public function getBuz()
76+
{
77+
if ($this->buz === false) {
78+
$this->buz = random_bytes(1);
79+
}
80+
81+
return $this->buz;
82+
}
83+
84+
/** Not applicable because it doesn't return a memoized property. */
85+
public function printFoo(): void
86+
{
87+
if ($this->foo === null) {
88+
$this->foo = random_bytes(1);
89+
}
90+
91+
echo $this->foo;
92+
}
93+
94+
}

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php // lint >= 8.0
2+
3+
namespace MemoizationProperty;
4+
5+
final class A
6+
{
7+
private ?string $foo = null;
8+
private ?string $bar = null;
9+
private string|false $buz = false;
10+
11+
public function getFoo()
12+
{
13+
return $this->foo ??= random_bytes(1);
14+
}
15+
16+
public function getBar()
17+
{
18+
return $this->bar ??= random_bytes(1);
19+
}
20+
21+
/** Not applicable because it has an else clause in the if. */
22+
public function getBarElse()
23+
{
24+
if ($this->bar === null) {
25+
$this->bar = random_bytes(1);
26+
} else {
27+
// no-op
28+
}
29+
30+
return $this->bar;
31+
}
32+
33+
/** Not applicable because it has an elseif clause in the if. */
34+
public function getBarElseIf()
35+
{
36+
if ($this->bar === null) {
37+
$this->bar = random_bytes(1);
38+
} elseif (false) {
39+
// no-op
40+
}
41+
42+
return $this->bar;
43+
}
44+
45+
/** Not applicable because it receives parameters. */
46+
public function getBarReceiveParam(int $length)
47+
{
48+
if ($this->bar === null) {
49+
$this->bar = random_bytes($length);
50+
}
51+
52+
return $this->bar;
53+
}
54+
55+
/** Not applicable because the body of if is not just an assignment. */
56+
public function getBarComplex()
57+
{
58+
if ($this->bar === null) {
59+
$rand = random_bytes(1);
60+
$this->bar = $rand;
61+
}
62+
63+
return $this->bar;
64+
}
65+
66+
/** Not applicable because it is comparing a property with a non-null value. */
67+
public function getBuz()
68+
{
69+
if ($this->buz === false) {
70+
$this->buz = random_bytes(1);
71+
}
72+
73+
return $this->buz;
74+
}
75+
76+
/** Not applicable because it doesn't return a memoized property. */
77+
public function printFoo(): void
78+
{
79+
if ($this->foo === null) {
80+
$this->foo = random_bytes(1);
81+
}
82+
83+
echo $this->foo;
84+
}
85+
86+
}

0 commit comments

Comments
 (0)