Skip to content

Commit 1dc9205

Browse files
committed
feat: add optional rule to report properties that should be promoted
1 parent 5878035 commit 1dc9205

File tree

5 files changed

+175
-0
lines changed

5 files changed

+175
-0
lines changed

conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ parameters:
6969
reportPossiblyNonexistentGeneralArrayOffset: false
7070
reportPossiblyNonexistentConstantArrayOffset: false
7171
checkMissingOverrideMethodAttribute: false
72+
reportPropertiesThatShouldBePromoted: false
7273
mixinExcludeClasses: []
7374
scanFiles: []
7475
scanDirectories: []

conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ parametersSchema:
7979
reportPossiblyNonexistentGeneralArrayOffset: bool()
8080
reportPossiblyNonexistentConstantArrayOffset: bool()
8181
checkMissingOverrideMethodAttribute: bool()
82+
reportPropertiesThatShouldBePromoted: bool()
8283
parallel: structure([
8384
jobSize: int(),
8485
processTimeout: float(),
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use Override;
6+
use PhpParser\Node;
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\Assign;
9+
use PhpParser\Node\Expr\Error;
10+
use PhpParser\Node\Expr\PropertyFetch;
11+
use PhpParser\Node\Expr\Variable;
12+
use PhpParser\Node\Identifier;
13+
use PhpParser\Node\Stmt\ClassMethod;
14+
use PhpParser\Node\Stmt\Expression;
15+
use PHPStan\Analyser\Scope;
16+
use PHPStan\DependencyInjection\AutowiredParameter;
17+
use PHPStan\DependencyInjection\RegisteredRule;
18+
use PHPStan\Rules\Rule;
19+
use PHPStan\Rules\RuleErrorBuilder;
20+
use function sprintf;
21+
22+
/**
23+
* @implements Rule<ClassMethod>
24+
*/
25+
#[RegisteredRule(level: 0)]
26+
final class ReportPropertiesThatShouldBePromotedRule implements Rule
27+
{
28+
29+
public function __construct(
30+
#[AutowiredParameter]
31+
private bool $reportPropertiesThatShouldBePromoted,
32+
)
33+
{
34+
}
35+
36+
public function getNodeType(): string
37+
{
38+
return ClassMethod::class;
39+
}
40+
41+
#[Override]
42+
public function processNode(Node $node, Scope $scope): array
43+
{
44+
if (! $this->reportPropertiesThatShouldBePromoted) {
45+
return [];
46+
}
47+
if ($node->name->toLowerString() !== '__construct') {
48+
return [];
49+
}
50+
$errors = [];
51+
foreach ($node->params as $param) {
52+
if (
53+
$param->isPromoted()
54+
|| $param->var instanceof Error
55+
|| $param->var->name instanceof Expr
56+
|| ! $this->assignsUnmodifiedVariableToProperty($param->var->name, $node)
57+
) {
58+
continue;
59+
}
60+
$errors[] = RuleErrorBuilder::message(sprintf('Property [%s] should be promoted.', $param->var->name))
61+
->identifier('property.shouldBePromoted')
62+
->line($param->var->getStartLine())
63+
->build();
64+
}
65+
return $errors;
66+
}
67+
68+
private function assignsUnmodifiedVariableToProperty(string $variable, ClassMethod $node): bool
69+
{
70+
foreach ($node->getStmts() as $stmt) {
71+
if (! $stmt instanceof Expression || ! $stmt->expr instanceof Assign) {
72+
continue;
73+
}
74+
$var = $stmt->expr->var;
75+
$expr = $stmt->expr->expr;
76+
if (! $var instanceof Variable && ! $var instanceof PropertyFetch) {
77+
continue;
78+
}
79+
if ($var instanceof Variable) {
80+
// The variable has been modified, so can't promote it.
81+
if ($var->name === $variable) {
82+
return false;
83+
}
84+
continue;
85+
}
86+
if (
87+
! $var->var instanceof Variable
88+
|| $var->var->name !== 'this'
89+
|| ! $var->name instanceof Identifier
90+
|| $var->name->toString() !== $variable
91+
) {
92+
continue;
93+
}
94+
if (! $expr instanceof Variable) {
95+
continue;
96+
}
97+
// The variable is being assigned to a property
98+
// of the same name, safe to promote it.
99+
if ($expr->name === $variable) {
100+
return true;
101+
}
102+
}
103+
return false;
104+
}
105+
106+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use function sprintf;
8+
9+
/**
10+
* @extends RuleTestCase<ReportPropertiesThatShouldBePromoted>
11+
*/
12+
class ReportPropertiesThatShouldBePromotedRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new ReportPropertiesThatShouldBePromoted(true);
18+
}
19+
20+
public function testOverridingFinalMethod(): void
21+
{
22+
$error = static fn (string $property) => sprintf('Property [%s] should be promoted.', $property);
23+
24+
$this->analyse([__DIR__ . '/data/properties-that-should-be-promoted.php'], [
25+
[$error('name'), 20],
26+
]);
27+
}
28+
29+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
namespace PropertiesThatShouldBePromoted;
4+
5+
class PromotedProperties
6+
{
7+
public string $name;
8+
9+
/** @var list<string> */
10+
public array $tags;
11+
12+
/** @var array<string, mixed> */
13+
public array $options;
14+
15+
public int $count;
16+
17+
public int $foo;
18+
19+
public function __construct(
20+
int $count,
21+
?string $name,
22+
public ?string $email,
23+
/** @var array<int, string> */
24+
array $tags,
25+
/** @var array<string, mixed> */
26+
array $options,
27+
int $bar,
28+
) {
29+
$this->count = $count;
30+
31+
$tags = array_values($tags);
32+
$this->tags = $tags;
33+
$this->options = array_filter($options);
34+
$this->email ??= '[email protected]';
35+
$this->name = $name ?? 'Default Name';
36+
$this->foo = $bar;
37+
}
38+
}

0 commit comments

Comments
 (0)