Skip to content

Commit 33359b3

Browse files
jakubtobiaszondrejmirtes
jakubtobiasz
authored and
ondrejmirtes
committed
Implement a logic handling Property Hooks on PHP 8.4 or above in classes
1 parent 9faea16 commit 33359b3

11 files changed

+296
-2
lines changed

‎Makefile‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ lint:
7979
--exclude tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php \
8080
--exclude tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php \
8181
--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 \
8287
src tests
8388

8489
cs:

‎build/collision-detector.json‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@
1414
"../tests/PHPStan/Levels/data/stubs/function.php",
1515
"../tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php",
1616
"../tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php",
17-
"../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php"
18-
]
17+
"../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php",
18+
"../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php",
19+
"../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php",
20+
"../tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php",
21+
"../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php",
22+
"../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php"
23+
]
1924
}

‎conf/config.level0.neon‎

Lines changed: 1 addition & 0 deletions
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‎

Lines changed: 5 additions & 0 deletions
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;

‎src/Rules/Properties/PropertyInClassRule.php‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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() || !$this->phpVersion->supportsPropertyHooks()) {
32+
return [];
33+
}
34+
35+
if (!$classReflection->isAbstract() && $node->hasHooks() && $node->isAbstract()) {
36+
return [
37+
RuleErrorBuilder::message('Classes may not include abstract hooked properties.')
38+
->nonIgnorable()
39+
->identifier('property.abstractHookedInClass')
40+
->build(),
41+
];
42+
}
43+
44+
if (!$classReflection->isAbstract() && $node->hasHooks() && !$this->doAllHooksHaveBody($node)) {
45+
return [
46+
RuleErrorBuilder::message('Classes may not include hooked properties without bodies.')
47+
->nonIgnorable()
48+
->identifier('property.hookedWithoutBodyInClass')
49+
->build(),
50+
];
51+
}
52+
53+
if (!$classReflection->isAbstract()) {
54+
return [];
55+
}
56+
57+
if ($node->hasHooks() && !$node->isAbstract()) {
58+
return [
59+
RuleErrorBuilder::message('Abstract classes may not include non-abstract hooked properties without bodies.')
60+
->nonIgnorable()
61+
->identifier('property.nonAbstractHookedWithoutBodyInAbstractClass')
62+
->build(),
63+
];
64+
}
65+
66+
if ($node->isAbstract() && !$node->hasHooks()) {
67+
return [
68+
RuleErrorBuilder::message('Only hooked properties may be declared abstract.')
69+
->nonIgnorable()
70+
->identifier('property.nonHookedAbstractInClass')
71+
->build(),
72+
];
73+
}
74+
75+
if ($node->isAbstract() && !$this->isAtLeastOneHookBodyEmpty($node)) {
76+
return [
77+
RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.')
78+
->nonIgnorable()
79+
->identifier('property.hookedAbstractWithBodies')
80+
->build(),
81+
];
82+
}
83+
84+
return [];
85+
}
86+
87+
private function doAllHooksHaveBody(ClassPropertyNode $node): bool
88+
{
89+
foreach ($node->getHooks() as $hook) {
90+
if ($hook->body === null) {
91+
return false;
92+
}
93+
}
94+
95+
return true;
96+
}
97+
98+
private function isAtLeastOneHookBodyEmpty(ClassPropertyNode $node): bool
99+
{
100+
foreach ($node->getHooks() as $hook) {
101+
if ($hook->body === null) {
102+
return true;
103+
}
104+
}
105+
106+
return false;
107+
}
108+
109+
}

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PHPStan\Php\PhpVersion;
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\Testing\RuleTestCase;
8+
use const PHP_VERSION_ID;
9+
10+
/**
11+
* @extends RuleTestCase<PropertyInClassRule>
12+
*/
13+
class PropertyInClassRuleTest extends RuleTestCase
14+
{
15+
16+
private int $phpVersion = PHP_VERSION_ID;
17+
18+
protected function getRule(): Rule
19+
{
20+
return new PropertyInClassRule(new PhpVersion($this->phpVersion));
21+
}
22+
23+
public function testPhp84AndNonAbstractHookedPropertiesInClass(): void
24+
{
25+
$this->phpVersion = 80400;
26+
27+
$this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-class.php'], [
28+
[
29+
'Classes may not include hooked properties without bodies.',
30+
7,
31+
],
32+
[
33+
'Classes may not include hooked properties without bodies.',
34+
9,
35+
],
36+
]);
37+
}
38+
39+
public function testPhp84AndAbstractHookedPropertiesInClass(): void
40+
{
41+
$this->phpVersion = 80400;
42+
43+
$this->analyse([__DIR__ . '/data/abstract-hooked-properties-in-class.php'], [
44+
[
45+
'Classes may not include abstract hooked properties.',
46+
7,
47+
],
48+
[
49+
'Classes may not include abstract hooked properties.',
50+
9,
51+
],
52+
]);
53+
}
54+
55+
public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void
56+
{
57+
$this->phpVersion = 80400;
58+
59+
$this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-abstract-class.php'], [
60+
[
61+
'Abstract classes may not include non-abstract hooked properties without bodies.',
62+
7,
63+
],
64+
[
65+
'Abstract classes may not include non-abstract hooked properties without bodies.',
66+
9,
67+
],
68+
]);
69+
}
70+
71+
public function testPhp84AndAbstractNonHookedPropertiesInAbstractClass(): void
72+
{
73+
$this->phpVersion = 80400;
74+
75+
$this->analyse([__DIR__ . '/data/abstract-non-hooked-properties-in-abstract-class.php'], [
76+
[
77+
'Only hooked properties may be declared abstract.',
78+
7,
79+
],
80+
[
81+
'Only hooked properties may be declared abstract.',
82+
9,
83+
],
84+
]);
85+
}
86+
87+
public function testPhp84AndAbstractHookedPropertiesWithBodies(): void
88+
{
89+
$this->phpVersion = 80400;
90+
91+
$this->analyse([__DIR__ . '/data/abstract-hooked-properties-with-bodies.php'], [
92+
[
93+
'Abstract properties must specify at least one abstract hook.',
94+
7,
95+
],
96+
[
97+
'Abstract properties must specify at least one abstract hook.',
98+
12,
99+
],
100+
]);
101+
}
102+
103+
}

‎tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace NonAbstractHookedPropertiesInAbstractClass;
4+
5+
class AbstractPerson
6+
{
7+
public abstract string $name { get; set; }
8+
9+
public abstract string $lastName { get; set; }
10+
}

‎tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace AbstractHookedPropertiesWithBodies;
4+
5+
abstract class AbstractPerson
6+
{
7+
public abstract string $name {
8+
get => $this->name;
9+
set => $this->name = $value;
10+
}
11+
12+
public abstract string $lastName {
13+
get => $this->lastName;
14+
set => $this->lastName = $value;
15+
}
16+
17+
public abstract string $middleName {
18+
get => $this->name;
19+
set;
20+
}
21+
22+
public abstract string $familyName {
23+
get;
24+
set;
25+
}
26+
}

‎tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace NonAbstractHookedPropertiesInAbstractClass;
4+
5+
abstract class AbstractPerson
6+
{
7+
public abstract string $name;
8+
9+
public abstract string $lastName;
10+
}

‎tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace NonAbstractHookedPropertiesInAbstractClass;
4+
5+
abstract class AbstractPerson
6+
{
7+
public string $name { get; set; }
8+
9+
public string $lastName { get; set; }
10+
}

0 commit comments

Comments
 (0)