Skip to content

Commit d6214a6

Browse files
committed
Add MemoizationPropertyRule
1 parent 7d6bbec commit d6214a6

File tree

5 files changed

+332
-0
lines changed

5 files changed

+332
-0
lines changed
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 PHPStan\Node\InClassMethodNode;
6+
use PhpParser\Node;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\File\FileHelper;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use PhpParser\Node\Expr\Assign;
12+
use PhpParser\Node\Expr\AssignOp\Coalesce;
13+
use PhpParser\Node\Expr\BinaryOp\Identical;
14+
use PhpParser\Node\Expr\ConstFetch;
15+
use PhpParser\Node\Expr\PropertyFetch;
16+
use PhpParser\Node\Identifier;
17+
use PhpParser\Node\Stmt\Expression;
18+
use PhpParser\Node\Stmt\If_;
19+
use PhpParser\Node\Stmt\Return_;
20+
use function sprintf;
21+
use function str_starts_with;
22+
23+
/**
24+
* @implements Rule<InClassMethodNode>
25+
*/
26+
final class MemoizationPropertyRule implements Rule
27+
{
28+
29+
public function __construct(private FileHelper $fileHelper, private bool $skipTests = true)
30+
{
31+
}
32+
33+
public function getNodeType(): string
34+
{
35+
return InClassMethodNode::class;
36+
}
37+
38+
public function processNode(Node $node, Scope $scope): array
39+
{
40+
$methodNode = $node->getOriginalNode();
41+
if (count($methodNode->params) !== 0) {
42+
return [];
43+
}
44+
45+
$stmts = $methodNode->getStmts();
46+
if ($stmts === null || count($stmts) !== 2) {
47+
return [];
48+
}
49+
50+
[$ifNode, $returnNode] = $stmts;
51+
52+
if (!$returnNode instanceof Return_ ||
53+
!$returnNode->expr instanceof PropertyFetch
54+
) {
55+
return [];
56+
}
57+
58+
if (!$ifNode instanceof If_ ||
59+
count($ifNode->stmts) !== 1 ||
60+
!$ifNode->stmts[0] instanceof Expression ||
61+
count($ifNode->elseifs) !== 0 ||
62+
$ifNode->else !== null ||
63+
!$ifNode->cond instanceof Identical ||
64+
!$ifNode->cond->left instanceof PropertyFetch ||
65+
!$ifNode->cond->right instanceof ConstFetch ||
66+
strcasecmp($ifNode->cond->right->name->name, 'null') !== 0
67+
) {
68+
return [];
69+
}
70+
71+
$ifThenNode = $ifNode->stmts[0]->expr;
72+
73+
if (!$ifThenNode instanceof Assign ||
74+
!$ifThenNode->var instanceof PropertyFetch
75+
) {
76+
return [];
77+
}
78+
79+
if ($returnNode->expr::class !== $ifNode->cond->left::class ||
80+
$returnNode->expr->var::class !== $ifNode->cond->left->var::class ||
81+
!$returnNode->expr->name instanceof Identifier ||
82+
!$ifNode->cond->left->name instanceof Identifier ||
83+
$returnNode->expr->name->name !== $ifNode->cond->left->name->name
84+
) {
85+
return [];
86+
}
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
-
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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+
9+
/**
10+
* @extends RuleTestCase<MemoizationPropertyRule>
11+
*/
12+
final class MemoizationPropertyRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new MemoizationPropertyRule(self::getContainer()->getByType(FileHelper::class), false);
18+
}
19+
20+
public function testRule(): void
21+
{
22+
$this->analyse([__DIR__ . '/data/memoization-property.php'], [
23+
[
24+
'Method MemoizationProperty\A::getFoo() for memoization can be simplified.',
25+
11,
26+
],
27+
[
28+
'Method MemoizationProperty\A::getBar() for memoization can be simplified.',
29+
20,
30+
],
31+
]);
32+
}
33+
34+
public function testFix(): void
35+
{
36+
$this->fix(__DIR__ . '/data/memoization-property.php', __DIR__ . '/data/memoization-property.php.fixed');
37+
}
38+
39+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php
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+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php
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)