Skip to content

Commit 49e49b0

Browse files
staabmondrejmirtes
staabm
and
ondrejmirtes
authored
Hooked properties cannot be both final and private
Co-authored-by: Ondřej Mirtes <[email protected]>
1 parent d56d084 commit 49e49b0

15 files changed

+403
-6
lines changed

‎Makefile‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ lint:
101101
--exclude tests/PHPStan/Rules/Properties/data/existing-classes-property-hooks.php \
102102
--exclude tests/PHPStan/Rules/Properties/data/set-property-hook-parameter.php \
103103
--exclude tests/PHPStan/Rules/Properties/data/overriding-final-property.php \
104+
--exclude tests/PHPStan/Rules/Properties/data/private-final-property-hooks.php \
105+
--exclude tests/PHPStan/Rules/Properties/data/abstract-final-property-hook.php \
106+
--exclude tests/PHPStan/Rules/Properties/data/final-property-hooks-in-interface.php \
107+
--exclude tests/PHPStan/Rules/Properties/data/final-property-hooks.php \
108+
--exclude tests/PHPStan/Rules/Properties/data/final-properties.php \
109+
--exclude tests/PHPStan/Rules/Properties/data/property-in-interface-explicit-abstract.php \
104110
src tests
105111

106112
cs:

‎src/Node/ClassPropertyNode.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ public function isPrivate(): bool
8686
return (bool) ($this->flags & Modifiers::PRIVATE);
8787
}
8888

89+
public function isFinal(): bool
90+
{
91+
return (bool) ($this->flags & Modifiers::FINAL);
92+
}
93+
8994
public function isStatic(): bool
9095
{
9196
return (bool) ($this->flags & Modifiers::STATIC);

‎src/Php/PhpVersion.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ public function supportsPropertyHooks(): bool
357357
return $this->versionId >= 80400;
358358
}
359359

360+
public function supportsFinalProperties(): bool
361+
{
362+
return $this->versionId >= 80400;
363+
}
364+
360365
public function supportsAsymmetricVisibility(): bool
361366
{
362367
return $this->versionId >= 80400;

‎src/Rules/Properties/PropertiesInInterfaceRule.php‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function processNode(Node $node, Scope $scope): array
3232

3333
if (!$this->phpVersion->supportsPropertyHooks()) {
3434
return [
35-
RuleErrorBuilder::message('Interfaces cannot include properties.')
35+
RuleErrorBuilder::message('Interfaces can include properties only on PHP 8.4 and later.')
3636
->nonIgnorable()
3737
->identifier('property.inInterface')
3838
->build(),
@@ -75,6 +75,37 @@ public function processNode(Node $node, Scope $scope): array
7575
];
7676
}
7777

78+
if ($node->isAbstract()) {
79+
return [
80+
RuleErrorBuilder::message('Property in interface cannot be explicitly abstract.')
81+
->nonIgnorable()
82+
->identifier('property.abstractInInterface')
83+
->build(),
84+
];
85+
}
86+
87+
if ($node->isFinal()) {
88+
return [
89+
RuleErrorBuilder::message('Interfaces cannot include final properties.')
90+
->nonIgnorable()
91+
->identifier('property.finalInInterface')
92+
->build(),
93+
];
94+
}
95+
96+
foreach ($node->getHooks() as $hook) {
97+
if (!$hook->isFinal()) {
98+
continue;
99+
}
100+
101+
return [
102+
RuleErrorBuilder::message('Property hook cannot be both abstract and final.')
103+
->nonIgnorable()
104+
->identifier('property.abstractFinalHook')
105+
->build(),
106+
];
107+
}
108+
78109
if ($this->hasAnyHookBody($node)) {
79110
return [
80111
RuleErrorBuilder::message('Interfaces cannot include property hooks with bodies.')

‎src/Rules/Properties/PropertyInClassRule.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ public function processNode(Node $node, Scope $scope): array
3232
return [];
3333
}
3434

35+
if (
36+
$node->isFinal()
37+
&& !$this->phpVersion->supportsFinalProperties()
38+
) {
39+
return [
40+
RuleErrorBuilder::message('Final properties are supported only on PHP 8.4 and later.')
41+
->nonIgnorable()
42+
->identifier('property.final')
43+
->build(),
44+
];
45+
}
46+
3547
if (!$this->phpVersion->supportsPropertyHooks()) {
3648
if ($node->hasHooks()) {
3749
return [
@@ -81,6 +93,58 @@ public function processNode(Node $node, Scope $scope): array
8193
];
8294
}
8395

96+
if ($node->isPrivate()) {
97+
if ($node->isFinal()) {
98+
return [
99+
RuleErrorBuilder::message('Property cannot be both final and private.')
100+
->nonIgnorable()
101+
->identifier('property.finalPrivate')
102+
->build(),
103+
];
104+
}
105+
106+
foreach ($node->getHooks() as $hook) {
107+
if (!$hook->isFinal()) {
108+
continue;
109+
}
110+
111+
return [
112+
RuleErrorBuilder::message('Private property cannot have a final hook.')
113+
->nonIgnorable()
114+
->identifier('property.finalPrivateHook')
115+
->build(),
116+
];
117+
}
118+
}
119+
120+
if ($node->isAbstract()) {
121+
if ($node->isFinal()) {
122+
return [
123+
RuleErrorBuilder::message('Property cannot be both abstract and final.')
124+
->nonIgnorable()
125+
->identifier('property.abstractFinal')
126+
->build(),
127+
];
128+
}
129+
130+
foreach ($node->getHooks() as $hook) {
131+
if ($hook->body !== null) {
132+
continue;
133+
}
134+
135+
if (!$hook->isFinal()) {
136+
continue;
137+
}
138+
139+
return [
140+
RuleErrorBuilder::message('Property cannot be both abstract and final.')
141+
->nonIgnorable()
142+
->identifier('property.abstractFinal')
143+
->build(),
144+
];
145+
}
146+
}
147+
84148
if ($node->isReadOnly()) {
85149
if ($node->hasHooks()) {
86150
return [

‎tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,21 @@ public function testPhp83AndPropertiesInInterface(): void
2929

3030
$this->analyse([__DIR__ . '/data/properties-in-interface.php'], [
3131
[
32-
'Interfaces cannot include properties.',
32+
'Interfaces can include properties only on PHP 8.4 and later.',
3333
7,
3434
],
3535
[
36-
'Interfaces cannot include properties.',
36+
'Interfaces can include properties only on PHP 8.4 and later.',
3737
9,
3838
],
3939
[
40-
'Interfaces cannot include properties.',
40+
'Interfaces can include properties only on PHP 8.4 and later.',
4141
11,
4242
],
43+
[
44+
'Interfaces can include properties only on PHP 8.4 and later.',
45+
13,
46+
],
4347
]);
4448
}
4549

@@ -54,11 +58,11 @@ public function testPhp83AndPropertyHooksInInterface(): void
5458

5559
$this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [
5660
[
57-
'Interfaces cannot include properties.',
61+
'Interfaces can include properties only on PHP 8.4 and later.',
5862
7,
5963
],
6064
[
61-
'Interfaces cannot include properties.',
65+
'Interfaces can include properties only on PHP 8.4 and later.',
6266
9,
6367
],
6468
]);
@@ -79,6 +83,10 @@ public function testPhp84AndPropertiesInInterface(): void
7983
'Interfaces can only include hooked properties.',
8084
11,
8185
],
86+
[
87+
'Interfaces can only include hooked properties.',
88+
13,
89+
],
8290
]);
8391
}
8492

@@ -140,6 +148,50 @@ public function testPhp84AndReadonlyPropertyHooksInInterface(): void
140148
]);
141149
}
142150

151+
public function testPhp84AndFinalPropertyHooksInInterface(): void
152+
{
153+
if (PHP_VERSION_ID < 80400) {
154+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
155+
}
156+
157+
$this->analyse([__DIR__ . '/data/final-property-hooks-in-interface.php'], [
158+
[
159+
'Interfaces cannot include final properties.',
160+
7,
161+
],
162+
[
163+
'Interfaces cannot include final properties.',
164+
9,
165+
],
166+
[
167+
'Interfaces cannot include final properties.',
168+
11,
169+
],
170+
[
171+
'Property hook cannot be both abstract and final.',
172+
13,
173+
],
174+
[
175+
'Property hook cannot be both abstract and final.',
176+
17,
177+
],
178+
]);
179+
}
180+
181+
public function testPhp84AndExplicitAbstractProperty(): void
182+
{
183+
if (PHP_VERSION_ID < 80400) {
184+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
185+
}
186+
187+
$this->analyse([__DIR__ . '/data/property-in-interface-explicit-abstract.php'], [
188+
[
189+
'Property in interface cannot be explicitly abstract.',
190+
8,
191+
],
192+
]);
193+
}
194+
143195
public function testPhp84AndStaticHookedPropertyInInterface(): void
144196
{
145197
if (PHP_VERSION_ID < 80400) {

‎tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,4 +213,101 @@ public function testPhp84AndStaticHookedProperties(): void
213213
]);
214214
}
215215

216+
public function testPhp84AndPrivateFinalHookedProperties(): void
217+
{
218+
if (PHP_VERSION_ID < 80400) {
219+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
220+
}
221+
222+
$this->analyse([__DIR__ . '/data/private-final-property-hooks.php'], [
223+
[
224+
'Property cannot be both final and private.',
225+
7,
226+
],
227+
[
228+
'Private property cannot have a final hook.',
229+
11,
230+
],
231+
]);
232+
}
233+
234+
public function testPhp84AndAbstractFinalHookedProperties(): void
235+
{
236+
if (PHP_VERSION_ID < 80400) {
237+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
238+
}
239+
240+
$this->analyse([__DIR__ . '/data/abstract-final-property-hook.php'], [
241+
[
242+
'Property cannot be both abstract and final.',
243+
7,
244+
],
245+
]);
246+
}
247+
248+
public function testPhp84AndAbstractFinalHookedPropertiesParseError(): void
249+
{
250+
if (PHP_VERSION_ID < 80400) {
251+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
252+
}
253+
254+
// errors when parsing with php-parser, see https://github.com/nikic/PHP-Parser/issues/1071
255+
$this->analyse([__DIR__ . '/data/abstract-final-property-hook-parse-error.php'], [
256+
[
257+
'Cannot use the final modifier on an abstract class member on line 7',
258+
7,
259+
],
260+
]);
261+
}
262+
263+
public function testPhp84FinalProperties(): void
264+
{
265+
if (PHP_VERSION_ID < 80400) {
266+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
267+
}
268+
269+
$this->analyse([__DIR__ . '/data/final-properties.php'], [
270+
[
271+
'Property cannot be both final and private.',
272+
7,
273+
],
274+
]);
275+
}
276+
277+
public function testBeforePhp84FinalProperties(): void
278+
{
279+
if (PHP_VERSION_ID >= 80400) {
280+
$this->markTestSkipped('Test requires PHP 8.3 or earlier.');
281+
}
282+
283+
$this->analyse([__DIR__ . '/data/final-properties.php'], [
284+
[
285+
'Final properties are supported only on PHP 8.4 and later.',
286+
7,
287+
],
288+
[
289+
'Final properties are supported only on PHP 8.4 and later.',
290+
8,
291+
],
292+
[
293+
'Final properties are supported only on PHP 8.4 and later.',
294+
9,
295+
],
296+
]);
297+
}
298+
299+
public function testPhp84FinalPropertyHooks(): void
300+
{
301+
if (PHP_VERSION_ID < 80400) {
302+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
303+
}
304+
305+
$this->analyse([__DIR__ . '/data/final-property-hooks.php'], [
306+
[
307+
'Cannot use the final modifier on an abstract class member on line 19',
308+
19,
309+
],
310+
]);
311+
}
312+
216313
}

‎tests/PHPStan/Rules/Properties/data/abstract-final-property-hook-parse-error.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php // lint >= 8.4
2+
3+
namespace AbstractFinalHookParseError;
4+
5+
abstract class User
6+
{
7+
final abstract public string $bar {
8+
get;
9+
}
10+
}

‎tests/PHPStan/Rules/Properties/data/abstract-final-property-hook.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php // lint >= 8.4
2+
3+
namespace AbstractFinalHook;
4+
5+
abstract class User
6+
{
7+
abstract public string $foo {
8+
final get;
9+
}
10+
}
11+
12+
abstract class Foo
13+
{
14+
abstract public int $i { final get { return 1;} set; }
15+
}

0 commit comments

Comments
 (0)