Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit b143449

Browse files
authored
Hooked properties cannot be unset() 
1 parent f87b890 commit b143449

File tree

4 files changed

+163
-3
lines changed

4 files changed

+163
-3
lines changed

‎src/Reflection/Php/PhpPropertyReflection.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ public function hasHook(string $hookType): bool
256256
return $this->setHook !== null;
257257
}
258258

259+
public function isHooked(): bool
260+
{
261+
return $this->getHook !== null || $this->setHook !== null;
262+
}
263+
259264
public function getHook(string $hookType): ExtendedMethodReflection
260265
{
261266
if ($hookType === 'get') {

‎src/Rules/Variables/UnsetRule.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
7+
use PHPStan\Php\PhpVersion;
78
use PHPStan\Rules\IdentifierRuleError;
89
use PHPStan\Rules\Properties\PropertyReflectionFinder;
910
use PHPStan\Rules\Rule;
@@ -20,6 +21,7 @@ final class UnsetRule implements Rule
2021

2122
public function __construct(
2223
private PropertyReflectionFinder $propertyReflectionFinder,
24+
private PhpVersion $phpVersion,
2325
)
2426
{
2527
}
@@ -103,6 +105,36 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError
103105
->identifier($propertyReflection->isReadOnly() ? 'unset.readOnlyProperty' : 'unset.readOnlyPropertyByPhpDoc')
104106
->build();
105107
}
108+
109+
if ($propertyReflection->isHooked()) {
110+
return RuleErrorBuilder::message(
111+
sprintf(
112+
'Cannot unset hooked %s::$%s property.',
113+
$propertyReflection->getDeclaringClass()->getDisplayName(),
114+
$foundPropertyReflection->getName(),
115+
),
116+
)
117+
->line($node->getStartLine())
118+
->identifier('unset.hookedProperty')
119+
->build();
120+
} elseif ($this->phpVersion->supportsPropertyHooks()) {
121+
if (
122+
!$propertyReflection->isPrivate()
123+
&& !$propertyReflection->isFinal()->yes()
124+
&& !$propertyReflection->getDeclaringClass()->isFinal()
125+
) {
126+
return RuleErrorBuilder::message(
127+
sprintf(
128+
'Cannot unset property %s::$%s because it might have hooks in a subclass.',
129+
$propertyReflection->getDeclaringClass()->getDisplayName(),
130+
$foundPropertyReflection->getName(),
131+
),
132+
)
133+
->line($node->getStartLine())
134+
->identifier('unset.possiblyHookedProperty')
135+
->build();
136+
}
137+
}
106138
}
107139

108140
return null;

‎tests/PHPStan/Rules/Variables/UnsetRuleTest.php‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
namespace PHPStan\Rules\Variables;
44

5+
use PHPStan\Php\PhpVersion;
56
use PHPStan\Rules\Properties\PropertyReflectionFinder;
67
use PHPStan\Rules\Rule;
78
use PHPStan\Testing\RuleTestCase;
9+
use function array_merge;
10+
use const PHP_VERSION_ID;
811

912
/**
1013
* @extends RuleTestCase<UnsetRule>
@@ -14,7 +17,10 @@ class UnsetRuleTest extends RuleTestCase
1417

1518
protected function getRule(): Rule
1619
{
17-
return new UnsetRule(self::getContainer()->getByType(PropertyReflectionFinder::class));
20+
return new UnsetRule(
21+
self::getContainer()->getByType(PropertyReflectionFinder::class),
22+
self::getContainer()->getByType(PhpVersion::class),
23+
);
1824
}
1925

2026
public function testUnsetRule(): void
@@ -55,7 +61,18 @@ public function testBug2752(): void
5561

5662
public function testBug4289(): void
5763
{
58-
$this->analyse([__DIR__ . '/data/bug-4289.php'], []);
64+
$errors = [];
65+
66+
if (PHP_VERSION_ID >= 80400) {
67+
$errors = [
68+
[
69+
'Cannot unset property Bug4289\BaseClass::$fields because it might have hooks in a subclass.',
70+
25,
71+
],
72+
];
73+
}
74+
75+
$this->analyse([__DIR__ . '/data/bug-4289.php'], $errors);
5976
}
6077

6178
public function testBug5223(): void
@@ -94,7 +111,15 @@ public function testBug4565(): void
94111

95112
public function testBug12421(): void
96113
{
97-
$this->analyse([__DIR__ . '/data/bug-12421.php'], [
114+
$errors = [];
115+
if (PHP_VERSION_ID >= 80400) {
116+
$errors[] = [
117+
'Cannot unset property Bug12421\RegularProperty::$y because it might have hooks in a subclass.',
118+
7,
119+
];
120+
}
121+
122+
$errors = array_merge($errors, [
98123
[
99124
'Cannot unset readonly Bug12421\NativeReadonlyClass::$y property.',
100125
11,
@@ -120,6 +145,38 @@ public function testBug12421(): void
120145
34,
121146
],
122147
]);
148+
149+
$this->analyse([__DIR__ . '/data/bug-12421.php'], $errors);
150+
}
151+
152+
public function testUnsetHookedProperty(): void
153+
{
154+
if (PHP_VERSION_ID < 80400) {
155+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
156+
}
157+
158+
$this->analyse([__DIR__ . '/data/unset-hooked-property.php'], [
159+
[
160+
'Cannot unset hooked UnsetHookedProperty\User::$name property.',
161+
6,
162+
],
163+
[
164+
'Cannot unset hooked UnsetHookedProperty\User::$fullName property.',
165+
7,
166+
],
167+
[
168+
'Cannot unset hooked UnsetHookedProperty\Foo::$ii property.',
169+
9,
170+
],
171+
[
172+
'Cannot unset hooked UnsetHookedProperty\Foo::$iii property.',
173+
10,
174+
],
175+
[
176+
'Cannot unset property UnsetHookedProperty\NonFinalClass::$publicProperty because it might have hooks in a subclass.',
177+
13,
178+
],
179+
]);
123180
}
124181

125182
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php // lint >= 8.4
2+
3+
namespace UnsetHookedProperty;
4+
5+
function doUnset(Foo $foo, User $user, NonFinalClass $nonFinalClass, FinalClass $finalClass): void {
6+
unset($user->name);
7+
unset($user->fullName);
8+
9+
unset($foo->ii);
10+
unset($foo->iii);
11+
12+
unset($nonFinalClass->publicFinalProperty);
13+
unset($nonFinalClass->publicProperty);
14+
15+
unset($finalClass->publicFinalProperty);
16+
unset($finalClass->publicProperty);
17+
}
18+
19+
class User
20+
{
21+
public string $name {
22+
set {
23+
if (strlen($value) === 0) {
24+
throw new \ValueError("Name must be non-empty");
25+
}
26+
$this->name = $value;
27+
}
28+
}
29+
30+
public string $fullName {
31+
get {
32+
return "Yennefer of Vengerberg";
33+
}
34+
}
35+
36+
public function __construct(string $name) {
37+
$this->name = $name;
38+
}
39+
}
40+
41+
abstract class Foo
42+
{
43+
abstract protected int $ii { get; }
44+
45+
abstract public int $iii { get; }
46+
}
47+
48+
class NonFinalClass {
49+
private string $privateProperty;
50+
public string $publicProperty;
51+
final public string $publicFinalProperty;
52+
53+
function doFoo() {
54+
unset($this->privateProperty);
55+
}
56+
}
57+
58+
final class FinalClass {
59+
private string $privateProperty;
60+
public string $publicProperty;
61+
final public string $publicFinalProperty;
62+
63+
function doFoo() {
64+
unset($this->privateProperty);
65+
}
66+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /