Skip to content

Hooked properties cannot be both final and private #3830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ lint:
--exclude tests/PHPStan/Rules/Properties/data/existing-classes-property-hooks.php \
--exclude tests/PHPStan/Rules/Properties/data/set-property-hook-parameter.php \
--exclude tests/PHPStan/Rules/Properties/data/overriding-final-property.php \
--exclude tests/PHPStan/Rules/Properties/data/private-final-property-hooks.php \
--exclude tests/PHPStan/Rules/Properties/data/abstract-final-property-hook.php \
--exclude tests/PHPStan/Rules/Properties/data/final-property-hooks-in-interface.php \
--exclude tests/PHPStan/Rules/Properties/data/final-property-hooks.php \
--exclude tests/PHPStan/Rules/Properties/data/final-properties.php \
--exclude tests/PHPStan/Rules/Properties/data/property-in-interface-explicit-abstract.php \
src tests

cs:
Expand Down
5 changes: 5 additions & 0 deletions src/Node/ClassPropertyNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public function isPrivate(): bool
return (bool) ($this->flags & Modifiers::PRIVATE);
}

public function isFinal(): bool
{
return (bool) ($this->flags & Modifiers::FINAL);
}

public function isStatic(): bool
{
return (bool) ($this->flags & Modifiers::STATIC);
Expand Down
5 changes: 5 additions & 0 deletions src/Php/PhpVersion.php
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,11 @@ public function supportsPropertyHooks(): bool
return $this->versionId >= 80400;
}

public function supportsFinalProperties(): bool
{
return $this->versionId >= 80400;
}

public function supportsAsymmetricVisibility(): bool
{
return $this->versionId >= 80400;
Expand Down
33 changes: 32 additions & 1 deletion src/Rules/Properties/PropertiesInInterfaceRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function processNode(Node $node, Scope $scope): array

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

if ($node->isAbstract()) {
return [
RuleErrorBuilder::message('Property in interface cannot be explicitly abstract.')
->nonIgnorable()
->identifier('property.abstractInInterface')
->build(),
];
}

if ($node->isFinal()) {
return [
RuleErrorBuilder::message('Interfaces cannot include final properties.')
->nonIgnorable()
->identifier('property.finalInInterface')
->build(),
];
}

foreach ($node->getHooks() as $hook) {
if (!$hook->isFinal()) {
continue;
}

return [
RuleErrorBuilder::message('Property hook cannot be both abstract and final.')
->nonIgnorable()
->identifier('property.abstractFinalHook')
->build(),
];
}

if ($this->hasAnyHookBody($node)) {
return [
RuleErrorBuilder::message('Interfaces cannot include property hooks with bodies.')
Expand Down
64 changes: 64 additions & 0 deletions src/Rules/Properties/PropertyInClassRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if (
$node->isFinal()
&& !$this->phpVersion->supportsFinalProperties()
) {
return [
RuleErrorBuilder::message('Final properties are supported only on PHP 8.4 and later.')
->nonIgnorable()
->identifier('property.final')
->build(),
];
}

if (!$this->phpVersion->supportsPropertyHooks()) {
if ($node->hasHooks()) {
return [
Expand Down Expand Up @@ -81,6 +93,58 @@ public function processNode(Node $node, Scope $scope): array
];
}

if ($node->isPrivate()) {
if ($node->isFinal()) {
return [
RuleErrorBuilder::message('Property cannot be both final and private.')
->nonIgnorable()
->identifier('property.finalPrivate')
->build(),
];
}

foreach ($node->getHooks() as $hook) {
if (!$hook->isFinal()) {
continue;
}

return [
RuleErrorBuilder::message('Private property cannot have a final hook.')
->nonIgnorable()
->identifier('property.finalPrivateHook')
->build(),
];
}
}

if ($node->isAbstract()) {
if ($node->isFinal()) {
return [
RuleErrorBuilder::message('Property cannot be both abstract and final.')
->nonIgnorable()
->identifier('property.abstractFinal')
->build(),
];
}

foreach ($node->getHooks() as $hook) {
if ($hook->body !== null) {
continue;
}

if (!$hook->isFinal()) {
continue;
}

return [
RuleErrorBuilder::message('Property cannot be both abstract and final.')
->nonIgnorable()
->identifier('property.abstractFinal')
->build(),
];
}
}

if ($node->isReadOnly()) {
if ($node->hasHooks()) {
return [
Expand Down
62 changes: 57 additions & 5 deletions tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,21 @@ public function testPhp83AndPropertiesInInterface(): void

$this->analyse([__DIR__ . '/data/properties-in-interface.php'], [
[
'Interfaces cannot include properties.',
'Interfaces can include properties only on PHP 8.4 and later.',
7,
],
[
'Interfaces cannot include properties.',
'Interfaces can include properties only on PHP 8.4 and later.',
9,
],
[
'Interfaces cannot include properties.',
'Interfaces can include properties only on PHP 8.4 and later.',
11,
],
[
'Interfaces can include properties only on PHP 8.4 and later.',
13,
],
]);
}

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

$this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [
[
'Interfaces cannot include properties.',
'Interfaces can include properties only on PHP 8.4 and later.',
7,
],
[
'Interfaces cannot include properties.',
'Interfaces can include properties only on PHP 8.4 and later.',
9,
],
]);
Expand All @@ -79,6 +83,10 @@ public function testPhp84AndPropertiesInInterface(): void
'Interfaces can only include hooked properties.',
11,
],
[
'Interfaces can only include hooked properties.',
13,
],
]);
}

Expand Down Expand Up @@ -140,6 +148,50 @@ public function testPhp84AndReadonlyPropertyHooksInInterface(): void
]);
}

public function testPhp84AndFinalPropertyHooksInInterface(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/final-property-hooks-in-interface.php'], [
[
'Interfaces cannot include final properties.',
7,
],
[
'Interfaces cannot include final properties.',
9,
],
[
'Interfaces cannot include final properties.',
11,
],
[
'Property hook cannot be both abstract and final.',
13,
],
[
'Property hook cannot be both abstract and final.',
17,
],
]);
}

public function testPhp84AndExplicitAbstractProperty(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/property-in-interface-explicit-abstract.php'], [
[
'Property in interface cannot be explicitly abstract.',
8,
],
]);
}

public function testPhp84AndStaticHookedPropertyInInterface(): void
{
if (PHP_VERSION_ID < 80400) {
Expand Down
97 changes: 97 additions & 0 deletions tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,101 @@ public function testPhp84AndStaticHookedProperties(): void
]);
}

public function testPhp84AndPrivateFinalHookedProperties(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/private-final-property-hooks.php'], [
[
'Property cannot be both final and private.',
7,
],
[
'Private property cannot have a final hook.',
11,
],
]);
}

public function testPhp84AndAbstractFinalHookedProperties(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/abstract-final-property-hook.php'], [
[
'Property cannot be both abstract and final.',
7,
],
]);
}

public function testPhp84AndAbstractFinalHookedPropertiesParseError(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

// errors when parsing with php-parser, see https://github.com/nikic/PHP-Parser/issues/1071
$this->analyse([__DIR__ . '/data/abstract-final-property-hook-parse-error.php'], [
[
'Cannot use the final modifier on an abstract class member on line 7',
7,
],
]);
}

public function testPhp84FinalProperties(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/final-properties.php'], [
[
'Property cannot be both final and private.',
7,
],
]);
}

public function testBeforePhp84FinalProperties(): void
{
if (PHP_VERSION_ID >= 80400) {
$this->markTestSkipped('Test requires PHP 8.3 or earlier.');
}

$this->analyse([__DIR__ . '/data/final-properties.php'], [
[
'Final properties are supported only on PHP 8.4 and later.',
7,
],
[
'Final properties are supported only on PHP 8.4 and later.',
8,
],
[
'Final properties are supported only on PHP 8.4 and later.',
9,
],
]);
}

public function testPhp84FinalPropertyHooks(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4 or later.');
}

$this->analyse([__DIR__ . '/data/final-property-hooks.php'], [
[
'Cannot use the final modifier on an abstract class member on line 19',
19,
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php // lint >= 8.4

namespace AbstractFinalHookParseError;

abstract class User
{
final abstract public string $bar {
get;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php // lint >= 8.4

namespace AbstractFinalHook;

abstract class User
{
abstract public string $foo {
final get;
}
}

abstract class Foo
{
abstract public int $i { final get { return 1;} set; }
}
Loading
Loading