Skip to content

Commit e6dc705

Browse files
jakubtobiaszjakubtobiasz
authored
Sanity checks around hooked properties in interfaces and classes
1 parent 86197c9 commit e6dc705

21 files changed

+600
-12
lines changed

‎.github/workflows/lint.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ jobs:
116116
uses: "shivammathur/setup-php@v2"
117117
with:
118118
coverage: "none"
119-
php-version: "8.3"
119+
php-version: "8.4"
120120

121121
- name: "Install dependencies"
122122
run: "composer install --no-interaction --no-progress"

‎Makefile

+10
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ lint:
7676
--exclude tests/PHPStan/Rules/Classes/data/extends-readonly-class.php \
7777
--exclude tests/PHPStan/Rules/Classes/data/instantiation-promoted-properties.php \
7878
--exclude tests/PHPStan/Rules/Classes/data/bug-11592.php \
79+
--exclude tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php \
80+
--exclude tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php \
81+
--exclude tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php \
82+
--exclude tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php \
83+
--exclude tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php \
84+
--exclude tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php \
85+
--exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php \
86+
--exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php \
87+
--exclude tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php \
88+
--exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \
7989
src tests
8090

8191
cs:

‎build/collision-detector.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
"../tests/notAutoloaded",
1313
"../tests/PHPStan/Rules/Functions/data/define-bug-3349.php",
1414
"../tests/PHPStan/Levels/data/stubs/function.php"
15-
]
15+
]
1616
}

‎conf/config.level0.neon

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ rules:
9696
- PHPStan\Rules\Properties\MissingReadOnlyByPhpDocPropertyAssignRule
9797
- PHPStan\Rules\Properties\PropertiesInInterfaceRule
9898
- PHPStan\Rules\Properties\PropertyAttributesRule
99+
- PHPStan\Rules\Properties\PropertyInClassRule
99100
- PHPStan\Rules\Properties\ReadOnlyPropertyRule
100101
- PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyRule
101102
- PHPStan\Rules\Regexp\RegularExpressionPatternRule

‎src/Node/ClassPropertyNode.php

+18
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ public function isAllowedPrivateMutation(): bool
111111
return $this->isAllowedPrivateMutation;
112112
}
113113

114+
public function isAbstract(): bool
115+
{
116+
return (bool) ($this->flags & Modifiers::ABSTRACT);
117+
}
118+
114119
public function getNativeType(): ?Type
115120
{
116121
return $this->type;
@@ -142,4 +147,17 @@ public function getSubNodeNames(): array
142147
return [];
143148
}
144149

150+
public function hasHooks(): bool
151+
{
152+
return $this->getHooks() !== [];
153+
}
154+
155+
/**
156+
* @return Node\PropertyHook[]
157+
*/
158+
public function getHooks(): array
159+
{
160+
return $this->originalNode->hooks;
161+
}
162+
145163
}

‎src/Php/PhpVersion.php

+5
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,11 @@ public function supportsPregCaptureOnlyNamedGroups(): bool
347347
return $this->versionId >= 80200;
348348
}
349349

350+
public function supportsPropertyHooks(): bool
351+
{
352+
return $this->versionId >= 80400;
353+
}
354+
350355
public function hasDateTimeExceptions(): bool
351356
{
352357
return $this->versionId >= 80300;

‎src/Rules/Properties/PropertiesInInterfaceRule.php

+53-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Node\ClassPropertyNode;
8+
use PHPStan\Php\PhpVersion;
89
use PHPStan\Rules\Rule;
910
use PHPStan\Rules\RuleErrorBuilder;
1011

@@ -14,6 +15,10 @@
1415
final class PropertiesInInterfaceRule implements Rule
1516
{
1617

18+
public function __construct(private PhpVersion $phpVersion)
19+
{
20+
}
21+
1722
public function getNodeType(): string
1823
{
1924
return ClassPropertyNode::class;
@@ -25,12 +30,54 @@ public function processNode(Node $node, Scope $scope): array
2530
return [];
2631
}
2732

28-
return [
29-
RuleErrorBuilder::message('Interfaces may not include properties.')
30-
->nonIgnorable()
31-
->identifier('property.inInterface')
32-
->build(),
33-
];
33+
if (!$this->phpVersion->supportsPropertyHooks()) {
34+
return [
35+
RuleErrorBuilder::message('Interfaces cannot include properties.')
36+
->nonIgnorable()
37+
->identifier('property.inInterface')
38+
->build(),
39+
];
40+
}
41+
42+
if (!$node->hasHooks()) {
43+
return [
44+
RuleErrorBuilder::message('Interfaces can only include hooked properties.')
45+
->nonIgnorable()
46+
->identifier('property.nonHookedInInterface')
47+
->build(),
48+
];
49+
}
50+
51+
if (!$node->isPublic()) {
52+
return [
53+
RuleErrorBuilder::message('Interfaces cannot include non-public properties.')
54+
->nonIgnorable()
55+
->identifier('property.nonPublicInInterface')
56+
->build(),
57+
];
58+
}
59+
60+
if ($this->hasAnyHookBody($node)) {
61+
return [
62+
RuleErrorBuilder::message('Interfaces cannot include property hooks with bodies.')
63+
->nonIgnorable()
64+
->identifier('property.hookBodyInInterface')
65+
->build(),
66+
];
67+
}
68+
69+
return [];
70+
}
71+
72+
private function hasAnyHookBody(ClassPropertyNode $node): bool
73+
{
74+
foreach ($node->getHooks() as $hook) {
75+
if ($hook->body !== null) {
76+
return true;
77+
}
78+
}
79+
80+
return false;
3481
}
3582

3683
}

‎src/Rules/Properties/PropertyInClassRule.php

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Node\ClassPropertyNode;
8+
use PHPStan\Php\PhpVersion;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
12+
/**
13+
* @implements Rule<ClassPropertyNode>
14+
*/
15+
final class PropertyInClassRule implements Rule
16+
{
17+
18+
public function __construct(private PhpVersion $phpVersion)
19+
{
20+
}
21+
22+
public function getNodeType(): string
23+
{
24+
return ClassPropertyNode::class;
25+
}
26+
27+
public function processNode(Node $node, Scope $scope): array
28+
{
29+
$classReflection = $node->getClassReflection();
30+
31+
if (!$classReflection->isClass()) {
32+
return [];
33+
}
34+
35+
if (!$this->phpVersion->supportsPropertyHooks()) {
36+
if ($node->hasHooks()) {
37+
return [
38+
RuleErrorBuilder::message('Property hooks are supported only on PHP 8.4 and later.')
39+
->nonIgnorable()
40+
->identifier('property.hooksNotSupported')
41+
->build(),
42+
];
43+
}
44+
45+
return [];
46+
}
47+
48+
if ($node->isAbstract()) {
49+
if (!$node->hasHooks()) {
50+
return [
51+
RuleErrorBuilder::message('Only hooked properties can be declared abstract.')
52+
->nonIgnorable()
53+
->identifier('property.abstractNonHooked')
54+
->build(),
55+
];
56+
}
57+
58+
if (!$this->isAtLeastOneHookBodyEmpty($node)) {
59+
return [
60+
RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.')
61+
->nonIgnorable()
62+
->identifier('property.abstractWithoutAbstractHook')
63+
->build(),
64+
];
65+
}
66+
67+
if (!$classReflection->isAbstract()) {
68+
return [
69+
RuleErrorBuilder::message('Non-abstract classes cannot include abstract properties.')
70+
->nonIgnorable()
71+
->identifier('property.abstract')
72+
->build(),
73+
];
74+
}
75+
76+
return [];
77+
}
78+
79+
if (!$this->doAllHooksHaveBody($node)) {
80+
return [
81+
RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.')
82+
->nonIgnorable()
83+
->identifier('property.hookWithoutBody')
84+
->build(),
85+
];
86+
}
87+
88+
return [];
89+
}
90+
91+
private function doAllHooksHaveBody(ClassPropertyNode $node): bool
92+
{
93+
foreach ($node->getHooks() as $hook) {
94+
if ($hook->body === null) {
95+
return false;
96+
}
97+
}
98+
99+
return true;
100+
}
101+
102+
private function isAtLeastOneHookBodyEmpty(ClassPropertyNode $node): bool
103+
{
104+
foreach ($node->getHooks() as $hook) {
105+
if ($hook->body === null) {
106+
return true;
107+
}
108+
}
109+
110+
return false;
111+
}
112+
113+
}

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

+92-4
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
namespace PHPStan\Rules\Properties;
44

5+
use PHPStan\Php\PhpVersion;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Testing\RuleTestCase;
8+
use const PHP_VERSION_ID;
79

810
/**
911
* @extends RuleTestCase<PropertiesInInterfaceRule>
@@ -13,21 +15,107 @@ class PropertiesInInterfaceRuleTest extends RuleTestCase
1315

1416
protected function getRule(): Rule
1517
{
16-
return new PropertiesInInterfaceRule();
18+
return new PropertiesInInterfaceRule(new PhpVersion(PHP_VERSION_ID));
1719
}
1820

19-
public function testRule(): void
21+
public function testPhp83AndPropertiesInInterface(): void
2022
{
23+
if (PHP_VERSION_ID >= 80400) {
24+
$this->markTestSkipped('Test requires PHP 8.3 or earlier.');
25+
}
26+
if (PHP_VERSION_ID < 80000) {
27+
$this->markTestSkipped('Property hooks cause syntax error on PHP 7.4');
28+
}
29+
30+
$this->analyse([__DIR__ . '/data/properties-in-interface.php'], [
31+
[
32+
'Interfaces cannot include properties.',
33+
7,
34+
],
35+
[
36+
'Interfaces cannot include properties.',
37+
9,
38+
],
39+
[
40+
'Interfaces cannot include properties.',
41+
11,
42+
],
43+
]);
44+
}
45+
46+
public function testPhp83AndPropertyHooksInInterface(): void
47+
{
48+
if (PHP_VERSION_ID >= 80400) {
49+
$this->markTestSkipped('Test requires PHP 8.3 or earlier.');
50+
}
51+
if (PHP_VERSION_ID < 80000) {
52+
$this->markTestSkipped('Property hooks cause syntax error on PHP 7.4');
53+
}
54+
55+
$this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [
56+
[
57+
'Interfaces cannot include properties.',
58+
7,
59+
],
60+
[
61+
'Interfaces cannot include properties.',
62+
9,
63+
],
64+
]);
65+
}
66+
67+
public function testPhp84AndPropertiesInInterface(): void
68+
{
69+
if (PHP_VERSION_ID < 80400) {
70+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
71+
}
72+
2173
$this->analyse([__DIR__ . '/data/properties-in-interface.php'], [
2274
[
23-
'Interfaces may not include properties.',
75+
'Interfaces can only include hooked properties.',
76+
9,
77+
],
78+
[
79+
'Interfaces can only include hooked properties.',
80+
11,
81+
],
82+
]);
83+
}
84+
85+
public function testPhp84AndNonPublicPropertyHooksInInterface(): void
86+
{
87+
if (PHP_VERSION_ID < 80400) {
88+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
89+
}
90+
91+
$this->analyse([__DIR__ . '/data/property-hooks-visibility-in-interface.php'], [
92+
[
93+
'Interfaces cannot include non-public properties.',
2494
7,
2595
],
2696
[
27-
'Interfaces may not include properties.',
97+
'Interfaces cannot include non-public properties.',
2898
9,
2999
],
30100
]);
31101
}
32102

103+
public function testPhp84AndPropertyHooksWithBodiesInInterface(): void
104+
{
105+
if (PHP_VERSION_ID < 80400) {
106+
$this->markTestSkipped('Test requires PHP 8.4 or later.');
107+
}
108+
109+
$this->analyse([__DIR__ . '/data/property-hooks-bodies-in-interface.php'], [
110+
[
111+
'Interfaces cannot include property hooks with bodies.',
112+
7,
113+
],
114+
[
115+
'Interfaces cannot include property hooks with bodies.',
116+
13,
117+
],
118+
]);
119+
}
120+
33121
}

0 commit comments

Comments
 (0)