Skip to content

Commit 551aab5

Browse files
ondrejmirtesondrejmirtes
committed
Fix UnusedPrivatePropertyRule in regard to property hooks
1 parent 53025c4 commit 551aab5

File tree

3 files changed

+179
-6
lines changed

3 files changed

+179
-6
lines changed

‎src/Rules/DeadCode/UnusedPrivatePropertyRule.php

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Node\ClassPropertiesNode;
88
use PHPStan\Node\Property\PropertyRead;
9+
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
910
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
1011
use PHPStan\Rules\Rule;
1112
use PHPStan\Rules\RuleErrorBuilder;
@@ -14,6 +15,7 @@
1415
use function array_key_exists;
1516
use function array_map;
1617
use function count;
18+
use function is_string;
1719
use function sprintf;
1820
use function str_contains;
1921

@@ -113,11 +115,29 @@ public function processNode(Node $node, Scope $scope): array
113115
}
114116

115117
foreach ($node->getPropertyUsages() as $usage) {
118+
$usageScope = $usage->getScope();
116119
$fetch = $usage->getFetch();
117120
if ($fetch->name instanceof Node\Identifier) {
118-
$propertyNames = [$fetch->name->toString()];
121+
$propertyName = $fetch->name->toString();
122+
$propertyNames = [$propertyName];
123+
if (
124+
$usageScope->getFunction() !== null
125+
&& $fetch instanceof Node\Expr\PropertyFetch
126+
&& $fetch->var instanceof Node\Expr\Variable
127+
&& is_string($fetch->var->name)
128+
&& $fetch->var->name === 'this'
129+
) {
130+
$methodReflection = $usageScope->getFunction();
131+
if (
132+
$methodReflection instanceof PhpMethodFromParserNodeReflection
133+
&& $methodReflection->isPropertyHook()
134+
&& $methodReflection->getHookedPropertyName() === $propertyName
135+
) {
136+
continue;
137+
}
138+
}
119139
} else {
120-
$propertyNameType = $usage->getScope()->getType($fetch->name);
140+
$propertyNameType = $usageScope->getType($fetch->name);
121141
$strings = $propertyNameType->getConstantStrings();
122142
if (count($strings) === 0) {
123143
// handle subtractions of a dynamic property fetch
@@ -134,21 +154,22 @@ public function processNode(Node $node, Scope $scope): array
134154

135155
$propertyNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $strings);
136156
}
157+
137158
if ($fetch instanceof Node\Expr\PropertyFetch) {
138-
$fetchedOnType = $usage->getScope()->getType($fetch->var);
159+
$fetchedOnType = $usageScope->getType($fetch->var);
139160
} else {
140161
if ($fetch->class instanceof Node\Name) {
141-
$fetchedOnType = $usage->getScope()->resolveTypeByName($fetch->class);
162+
$fetchedOnType = $usageScope->resolveTypeByName($fetch->class);
142163
} else {
143-
$fetchedOnType = $usage->getScope()->getType($fetch->class);
164+
$fetchedOnType = $usageScope->getType($fetch->class);
144165
}
145166
}
146167

147168
foreach ($propertyNames as $propertyName) {
148169
if (!array_key_exists($propertyName, $properties)) {
149170
continue;
150171
}
151-
$propertyReflection = $usage->getScope()->getPropertyReflection($fetchedOnType, $propertyName);
172+
$propertyReflection = $usageScope->getPropertyReflection($fetchedOnType, $propertyName);
152173
if ($propertyReflection === null) {
153174
if (!$classType->isSuperTypeOf($fetchedOnType)->no()) {
154175
if ($usage instanceof PropertyRead) {

‎tests/PHPStan/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,4 +347,44 @@ public function testBug11802(): void
347347
]);
348348
}
349349

350+
public function testPropertyHooks(): void
351+
{
352+
if (PHP_VERSION_ID < 80400) {
353+
$this->markTestSkipped('Test requires PHP 8.4.');
354+
}
355+
356+
$tip = 'See: https://phpstan.org/developing-extensions/always-read-written-properties';
357+
358+
$this->alwaysWrittenTags = [];
359+
$this->alwaysReadTags = [];
360+
361+
$this->analyse([__DIR__ . '/data/property-hooks-unused-property.php'], [
362+
[
363+
'Property PropertyHooksUnusedProperty\FooUnused::$a is unused.',
364+
32,
365+
$tip,
366+
],
367+
[
368+
'Property PropertyHooksUnusedProperty\FooOnlyRead::$a is never written, only read.',
369+
46,
370+
$tip,
371+
],
372+
[
373+
'Property PropertyHooksUnusedProperty\FooOnlyWritten::$a is never read, only written.',
374+
65,
375+
$tip,
376+
],
377+
[
378+
'Property PropertyHooksUnusedProperty\ReadInAnotherPropertyHook2::$bar is never written, only read.',
379+
95,
380+
$tip,
381+
],
382+
[
383+
'Property PropertyHooksUnusedProperty\WrittenInAnotherPropertyHook::$bar is never read, only written.',
384+
105,
385+
$tip,
386+
],
387+
]);
388+
}
389+
350390
}

‎tests/PHPStan/Rules/DeadCode/data/property-hooks-unused-property.php

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php // lint >= 8.4
2+
3+
namespace PropertyHooksUnusedProperty;
4+
5+
class FooUsed
6+
{
7+
8+
private int $a {
9+
get {
10+
return $this->a + 100;
11+
}
12+
set {
13+
$this->a = $value - 100;
14+
}
15+
}
16+
17+
public function setA(int $a): void
18+
{
19+
$this->a = $a;
20+
}
21+
22+
public function getA(): int
23+
{
24+
return $this->a;
25+
}
26+
27+
}
28+
29+
class FooUnused
30+
{
31+
32+
private int $a {
33+
get {
34+
return $this->a + 100;
35+
}
36+
set {
37+
$this->a = $value - 100;
38+
}
39+
}
40+
41+
}
42+
43+
class FooOnlyRead
44+
{
45+
46+
private int $a {
47+
get {
48+
return $this->a + 100;
49+
}
50+
set {
51+
$this->a = $value - 100;
52+
}
53+
}
54+
55+
public function getA(): int
56+
{
57+
return $this->a;
58+
}
59+
60+
}
61+
62+
class FooOnlyWritten
63+
{
64+
65+
private int $a {
66+
get {
67+
return $this->a + 100;
68+
}
69+
set {
70+
$this->a = $value - 100;
71+
}
72+
}
73+
74+
public function setA(int $a): void
75+
{
76+
$this->a = $a;
77+
}
78+
79+
}
80+
81+
class ReadInAnotherPropertyHook
82+
{
83+
public function __construct(
84+
private readonly string $bar,
85+
) {}
86+
87+
public string $virtualProperty {
88+
get => $this->bar;
89+
}
90+
}
91+
92+
class ReadInAnotherPropertyHook2
93+
{
94+
95+
private string $bar;
96+
97+
public string $virtualProperty {
98+
get => $this->bar;
99+
}
100+
}
101+
102+
class WrittenInAnotherPropertyHook
103+
{
104+
105+
private string $bar;
106+
107+
public string $virtualProperty {
108+
set {
109+
$this->bar = 'test';
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)