From 4ac1fc784014a43c99c9687d095e36ee4bb6c03d Mon Sep 17 00:00:00 2001 From: Jacob Tobiasz Date: Mon, 25 Nov 2024 07:55:37 +0100 Subject: [PATCH 01/11] Implement a logic handling Property Hooks on PHP `8.4` or above in interfaces --- Makefile | 3 + build/collision-detector.json | 5 +- src/Node/ClassPropertyNode.php | 13 ++++ src/Php/PhpVersion.php | 5 ++ .../Properties/PropertiesInInterfaceRule.php | 59 +++++++++++++-- .../PropertiesInInterfaceRuleTest.php | 74 ++++++++++++++++++- .../property-hooks-bodies-in-interface.php | 20 +++++ .../data/property-hooks-in-interface.php | 10 +++ ...property-hooks-visibility-in-interface.php | 12 +++ 9 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php create mode 100644 tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php create mode 100644 tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php diff --git a/Makefile b/Makefile index 3282ee2c4e..719f10c92f 100644 --- a/Makefile +++ b/Makefile @@ -76,6 +76,9 @@ lint: --exclude tests/PHPStan/Rules/Classes/data/extends-readonly-class.php \ --exclude tests/PHPStan/Rules/Classes/data/instantiation-promoted-properties.php \ --exclude tests/PHPStan/Rules/Classes/data/bug-11592.php \ + --exclude tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php \ + --exclude tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php \ + --exclude tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php \ src tests cs: diff --git a/build/collision-detector.json b/build/collision-detector.json index 21228704f5..7f6962cc86 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -11,6 +11,9 @@ "../tests/PHPStan/Rules/Names/data/no-namespace.php", "../tests/notAutoloaded", "../tests/PHPStan/Rules/Functions/data/define-bug-3349.php", - "../tests/PHPStan/Levels/data/stubs/function.php" + "../tests/PHPStan/Levels/data/stubs/function.php", + "../tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php", + "../tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php", + "../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php" ] } diff --git a/src/Node/ClassPropertyNode.php b/src/Node/ClassPropertyNode.php index 3f500b62c1..88d4aeba82 100644 --- a/src/Node/ClassPropertyNode.php +++ b/src/Node/ClassPropertyNode.php @@ -142,4 +142,17 @@ public function getSubNodeNames(): array return []; } + public function hasHooks(): bool + { + return $this->getHooks() !== []; + } + + /** + * @return Node\PropertyHook[] + */ + public function getHooks(): array + { + return $this->originalNode->hooks; + } + } diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index 8520f6488d..e636945cc2 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -347,6 +347,11 @@ public function supportsPregCaptureOnlyNamedGroups(): bool return $this->versionId >= 80200; } + public function supportsPropertyHooks(): bool + { + return $this->versionId >= 80400; + } + public function hasDateTimeExceptions(): bool { return $this->versionId >= 80300; diff --git a/src/Rules/Properties/PropertiesInInterfaceRule.php b/src/Rules/Properties/PropertiesInInterfaceRule.php index d9f62fa618..e0605e6841 100644 --- a/src/Rules/Properties/PropertiesInInterfaceRule.php +++ b/src/Rules/Properties/PropertiesInInterfaceRule.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Node\ClassPropertyNode; +use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -14,6 +15,10 @@ final class PropertiesInInterfaceRule implements Rule { + public function __construct(private PhpVersion $phpVersion) + { + } + public function getNodeType(): string { return ClassPropertyNode::class; @@ -25,12 +30,54 @@ public function processNode(Node $node, Scope $scope): array return []; } - return [ - RuleErrorBuilder::message('Interfaces may not include properties.') - ->nonIgnorable() - ->identifier('property.inInterface') - ->build(), - ]; + if (!$this->phpVersion->supportsPropertyHooks()) { + return [ + RuleErrorBuilder::message('Interfaces may not include properties.') + ->nonIgnorable() + ->identifier('property.inInterface') + ->build(), + ]; + } + + if (!$node->hasHooks()) { + return [ + RuleErrorBuilder::message('Interfaces may only include hooked properties.') + ->nonIgnorable() + ->identifier('property.nonHookedInInterface') + ->build(), + ]; + } + + if (!$node->isPublic()) { + return [ + RuleErrorBuilder::message('Interfaces may not include non-public properties.') + ->nonIgnorable() + ->identifier('property.nonPublicInInterface') + ->build(), + ]; + } + + if ($this->hasAnyHookBody($node)) { + return [ + RuleErrorBuilder::message('Interfaces may not include property hooks with bodies.') + ->nonIgnorable() + ->identifier('property.hookBodyInInterface') + ->build(), + ]; + } + + return []; + } + + private function hasAnyHookBody(ClassPropertyNode $node): bool + { + foreach ($node->getHooks() as $hook) { + if ($hook->body !== null) { + return true; + } + } + + return false; } } diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index ecf4597d97..ee0ffe5198 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -2,8 +2,10 @@ namespace PHPStan\Rules\Properties; +use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -11,13 +13,17 @@ class PropertiesInInterfaceRuleTest extends RuleTestCase { + private int $phpVersion = PHP_VERSION_ID; + protected function getRule(): Rule { - return new PropertiesInInterfaceRule(); + return new PropertiesInInterfaceRule(new PhpVersion($this->phpVersion)); } - public function testRule(): void + public function testPhp83AndPropertiesInInterface(): void { + $this->phpVersion = 80300; + $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ 'Interfaces may not include properties.', @@ -30,4 +36,68 @@ public function testRule(): void ]); } + public function testPhp83AndPropertyHooksInInterface(): void + { + $this->phpVersion = 80300; + + $this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [ + [ + 'Interfaces may not include properties.', + 7, + ], + [ + 'Interfaces may not include properties.', + 9, + ], + ]); + } + + public function testPhp84AndPropertiesInInterface(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ + [ + 'Interfaces may only include hooked properties.', + 7, + ], + [ + 'Interfaces may only include hooked properties.', + 9, + ], + ]); + } + + public function testPhp84AndNonPublicPropertyHooksInInterface(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/property-hooks-visibility-in-interface.php'], [ + [ + 'Interfaces may not include non-public properties.', + 7, + ], + [ + 'Interfaces may not include non-public properties.', + 9, + ], + ]); + } + + public function testPhp84AndPropertyHooksWithBodiesInInterface(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/property-hooks-bodies-in-interface.php'], [ + [ + 'Interfaces may not include property hooks with bodies.', + 7, + ], + [ + 'Interfaces may not include property hooks with bodies.', + 13, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php b/tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php new file mode 100644 index 0000000000..58af100248 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php @@ -0,0 +1,20 @@ + Date: Wed, 27 Nov 2024 09:11:20 +0100 Subject: [PATCH 02/11] Implement a logic handling Property Hooks on PHP `8.4` or above in classes --- Makefile | 5 + build/collision-detector.json | 9 +- conf/config.level0.neon | 1 + src/Node/ClassPropertyNode.php | 5 + src/Rules/Properties/PropertyInClassRule.php | 109 ++++++++++++++++++ .../Properties/PropertyInClassRuleTest.php | 103 +++++++++++++++++ .../abstract-hooked-properties-in-class.php | 10 ++ ...abstract-hooked-properties-with-bodies.php | 26 +++++ ...on-hooked-properties-in-abstract-class.php | 10 ++ ...ct-hooked-properties-in-abstract-class.php | 10 ++ ...on-abstract-hooked-properties-in-class.php | 10 ++ 11 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 src/Rules/Properties/PropertyInClassRule.php create mode 100644 tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php create mode 100644 tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php create mode 100644 tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php create mode 100644 tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php create mode 100644 tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php create mode 100644 tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php diff --git a/Makefile b/Makefile index 719f10c92f..bd970c2ae2 100644 --- a/Makefile +++ b/Makefile @@ -79,6 +79,11 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php \ --exclude tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php \ --exclude tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php \ + --exclude tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php \ + --exclude tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php \ src tests cs: diff --git a/build/collision-detector.json b/build/collision-detector.json index 7f6962cc86..30e8a3d31c 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -14,6 +14,11 @@ "../tests/PHPStan/Levels/data/stubs/function.php", "../tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php", "../tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php", - "../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php" - ] + "../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php", + "../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php", + "../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php", + "../tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php", + "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php", + "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php" + ] } diff --git a/conf/config.level0.neon b/conf/config.level0.neon index d4927a56c4..8c2a23a9d4 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -96,6 +96,7 @@ rules: - PHPStan\Rules\Properties\MissingReadOnlyByPhpDocPropertyAssignRule - PHPStan\Rules\Properties\PropertiesInInterfaceRule - PHPStan\Rules\Properties\PropertyAttributesRule + - PHPStan\Rules\Properties\PropertyInClassRule - PHPStan\Rules\Properties\ReadOnlyPropertyRule - PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyRule - PHPStan\Rules\Regexp\RegularExpressionPatternRule diff --git a/src/Node/ClassPropertyNode.php b/src/Node/ClassPropertyNode.php index 88d4aeba82..c8602aef79 100644 --- a/src/Node/ClassPropertyNode.php +++ b/src/Node/ClassPropertyNode.php @@ -111,6 +111,11 @@ public function isAllowedPrivateMutation(): bool return $this->isAllowedPrivateMutation; } + public function isAbstract(): bool + { + return (bool) ($this->flags & Modifiers::ABSTRACT); + } + public function getNativeType(): ?Type { return $this->type; diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php new file mode 100644 index 0000000000..a6bf200945 --- /dev/null +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -0,0 +1,109 @@ + + */ +final class PropertyInClassRule implements Rule +{ + + public function __construct(private PhpVersion $phpVersion) + { + } + + public function getNodeType(): string + { + return ClassPropertyNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $classReflection = $node->getClassReflection(); + + if (!$classReflection->isClass() || !$this->phpVersion->supportsPropertyHooks()) { + return []; + } + + if (!$classReflection->isAbstract() && $node->hasHooks() && $node->isAbstract()) { + return [ + RuleErrorBuilder::message('Classes may not include abstract hooked properties.') + ->nonIgnorable() + ->identifier('property.abstractHookedInClass') + ->build(), + ]; + } + + if (!$classReflection->isAbstract() && $node->hasHooks() && !$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Classes may not include hooked properties without bodies.') + ->nonIgnorable() + ->identifier('property.hookedWithoutBodyInClass') + ->build(), + ]; + } + + if (!$classReflection->isAbstract()) { + return []; + } + + if ($node->hasHooks() && !$node->isAbstract()) { + return [ + RuleErrorBuilder::message('Abstract classes may not include non-abstract hooked properties without bodies.') + ->nonIgnorable() + ->identifier('property.nonAbstractHookedWithoutBodyInAbstractClass') + ->build(), + ]; + } + + if ($node->isAbstract() && !$node->hasHooks()) { + return [ + RuleErrorBuilder::message('Only hooked properties may be declared abstract.') + ->nonIgnorable() + ->identifier('property.nonHookedAbstractInClass') + ->build(), + ]; + } + + if ($node->isAbstract() && !$this->isAtLeastOneHookBodyEmpty($node)) { + return [ + RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') + ->nonIgnorable() + ->identifier('property.hookedAbstractWithBodies') + ->build(), + ]; + } + + return []; + } + + private function doAllHooksHaveBody(ClassPropertyNode $node): bool + { + foreach ($node->getHooks() as $hook) { + if ($hook->body === null) { + return false; + } + } + + return true; + } + + private function isAtLeastOneHookBodyEmpty(ClassPropertyNode $node): bool + { + foreach ($node->getHooks() as $hook) { + if ($hook->body === null) { + return true; + } + } + + return false; + } + +} diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php new file mode 100644 index 0000000000..69b5210423 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -0,0 +1,103 @@ + + */ +class PropertyInClassRuleTest extends RuleTestCase +{ + + private int $phpVersion = PHP_VERSION_ID; + + protected function getRule(): Rule + { + return new PropertyInClassRule(new PhpVersion($this->phpVersion)); + } + + public function testPhp84AndNonAbstractHookedPropertiesInClass(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-class.php'], [ + [ + 'Classes may not include hooked properties without bodies.', + 7, + ], + [ + 'Classes may not include hooked properties without bodies.', + 9, + ], + ]); + } + + public function testPhp84AndAbstractHookedPropertiesInClass(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/abstract-hooked-properties-in-class.php'], [ + [ + 'Classes may not include abstract hooked properties.', + 7, + ], + [ + 'Classes may not include abstract hooked properties.', + 9, + ], + ]); + } + + public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-abstract-class.php'], [ + [ + 'Abstract classes may not include non-abstract hooked properties without bodies.', + 7, + ], + [ + 'Abstract classes may not include non-abstract hooked properties without bodies.', + 9, + ], + ]); + } + + public function testPhp84AndAbstractNonHookedPropertiesInAbstractClass(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/abstract-non-hooked-properties-in-abstract-class.php'], [ + [ + 'Only hooked properties may be declared abstract.', + 7, + ], + [ + 'Only hooked properties may be declared abstract.', + 9, + ], + ]); + } + + public function testPhp84AndAbstractHookedPropertiesWithBodies(): void + { + $this->phpVersion = 80400; + + $this->analyse([__DIR__ . '/data/abstract-hooked-properties-with-bodies.php'], [ + [ + 'Abstract properties must specify at least one abstract hook.', + 7, + ], + [ + 'Abstract properties must specify at least one abstract hook.', + 12, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php b/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php new file mode 100644 index 0000000000..68e6978a36 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php @@ -0,0 +1,10 @@ + $this->name; + set => $this->name = $value; + } + + public abstract string $lastName { + get => $this->lastName; + set => $this->lastName = $value; + } + + public abstract string $middleName { + get => $this->name; + set; + } + + public abstract string $familyName { + get; + set; + } +} diff --git a/tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php b/tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php new file mode 100644 index 0000000000..4822e9ee9c --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php @@ -0,0 +1,10 @@ + Date: Wed, 27 Nov 2024 18:39:30 +0100 Subject: [PATCH 03/11] Improve readability and provide minor fixes --- Makefile | 2 + build/collision-detector.json | 5 +- .../Properties/PropertiesInInterfaceRule.php | 9 ++ src/Rules/Properties/PropertyInClassRule.php | 91 +++++++++++-------- .../PropertiesInInterfaceRuleTest.php | 34 ++++--- .../Properties/PropertyInClassRuleTest.php | 60 ++++++++++-- .../data/hooked-properties-in-class.php | 11 +++ ...ked-properties-without-bodies-in-class.php | 10 ++ .../data/properties-in-interface.php | 2 + 9 files changed, 166 insertions(+), 58 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php create mode 100644 tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php diff --git a/Makefile b/Makefile index bd970c2ae2..be44969c3c 100644 --- a/Makefile +++ b/Makefile @@ -84,6 +84,8 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php \ --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php \ --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \ src tests cs: diff --git a/build/collision-detector.json b/build/collision-detector.json index 30e8a3d31c..9a86709292 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -12,6 +12,7 @@ "../tests/notAutoloaded", "../tests/PHPStan/Rules/Functions/data/define-bug-3349.php", "../tests/PHPStan/Levels/data/stubs/function.php", + "../tests/PHPStan/Rules/Properties/data/properties-in-interface.php", "../tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php", "../tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php", "../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php", @@ -19,6 +20,8 @@ "../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php", "../tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php", "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php", - "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php" + "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php", + "../tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php", + "../tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php" ] } diff --git a/src/Rules/Properties/PropertiesInInterfaceRule.php b/src/Rules/Properties/PropertiesInInterfaceRule.php index e0605e6841..a433193ac3 100644 --- a/src/Rules/Properties/PropertiesInInterfaceRule.php +++ b/src/Rules/Properties/PropertiesInInterfaceRule.php @@ -30,6 +30,15 @@ public function processNode(Node $node, Scope $scope): array return []; } + if (!$this->phpVersion->supportsPropertyHooks() && $node->hasHooks()) { + return [ + RuleErrorBuilder::message('Property hooks in interfaces are supported only on PHP 8.4 and later.') + ->nonIgnorable() + ->identifier('property.unsupportedHooksInInterface') + ->build(), + ]; + } + if (!$this->phpVersion->supportsPropertyHooks()) { return [ RuleErrorBuilder::message('Interfaces may not include properties.') diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index a6bf200945..3b316ed305 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -28,57 +28,76 @@ public function processNode(Node $node, Scope $scope): array { $classReflection = $node->getClassReflection(); - if (!$classReflection->isClass() || !$this->phpVersion->supportsPropertyHooks()) { + if (!$classReflection->isClass()) { return []; } - if (!$classReflection->isAbstract() && $node->hasHooks() && $node->isAbstract()) { + if (!$this->phpVersion->supportsPropertyHooks() && $node->hasHooks()) { return [ - RuleErrorBuilder::message('Classes may not include abstract hooked properties.') + RuleErrorBuilder::message('Property hooks in classes are supported only on PHP 8.4 and later.') ->nonIgnorable() - ->identifier('property.abstractHookedInClass') + ->identifier('property.unsupportedHooksInClass') ->build(), ]; } - if (!$classReflection->isAbstract() && $node->hasHooks() && !$this->doAllHooksHaveBody($node)) { - return [ - RuleErrorBuilder::message('Classes may not include hooked properties without bodies.') - ->nonIgnorable() - ->identifier('property.hookedWithoutBodyInClass') - ->build(), - ]; - } - - if (!$classReflection->isAbstract()) { + if (!$this->phpVersion->supportsPropertyHooks()) { return []; } - if ($node->hasHooks() && !$node->isAbstract()) { - return [ - RuleErrorBuilder::message('Abstract classes may not include non-abstract hooked properties without bodies.') - ->nonIgnorable() - ->identifier('property.nonAbstractHookedWithoutBodyInAbstractClass') - ->build(), - ]; - } + if ($classReflection->isAbstract()) { + if ($node->isAbstract()) { + if (!$node->hasHooks()) { + return [ + RuleErrorBuilder::message('Only hooked properties may be declared abstract.') + ->nonIgnorable() + ->identifier('property.nonHookedAbstractInClass') + ->build(), + ]; + } + + if (!$this->isAtLeastOneHookBodyEmpty($node)) { + return [ + RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') + ->nonIgnorable() + ->identifier('property.hookedAbstractWithBodies') + ->build(), + ]; + } + } - if ($node->isAbstract() && !$node->hasHooks()) { - return [ - RuleErrorBuilder::message('Only hooked properties may be declared abstract.') - ->nonIgnorable() - ->identifier('property.nonHookedAbstractInClass') - ->build(), - ]; + if (!$node->isAbstract()) { + if ($node->hasHooks()) { + return [ + RuleErrorBuilder::message('Abstract classes may not include non-abstract hooked properties without bodies.') + ->nonIgnorable() + ->identifier('property.nonAbstractHookedWithoutBodyInAbstractClass') + ->build(), + ]; + } + } + + return []; } - if ($node->isAbstract() && !$this->isAtLeastOneHookBodyEmpty($node)) { - return [ - RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') - ->nonIgnorable() - ->identifier('property.hookedAbstractWithBodies') - ->build(), - ]; + if ($node->hasHooks()) { + if ($node->isAbstract()) { + return [ + RuleErrorBuilder::message('Classes may not include abstract hooked properties.') + ->nonIgnorable() + ->identifier('property.abstractHookedInClass') + ->build(), + ]; + } + + if (!$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Non-abstract classes may not include hooked properties without bodies.') + ->nonIgnorable() + ->identifier('property.hookedWithoutBodyInClass') + ->build(), + ]; + } } return []; diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index ee0ffe5198..48739a74ba 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -13,32 +13,38 @@ class PropertiesInInterfaceRuleTest extends RuleTestCase { - private int $phpVersion = PHP_VERSION_ID; - protected function getRule(): Rule { - return new PropertiesInInterfaceRule(new PhpVersion($this->phpVersion)); + return new PropertiesInInterfaceRule(new PhpVersion(PHP_VERSION_ID)); } public function testPhp83AndPropertiesInInterface(): void { - $this->phpVersion = 80300; + if (PHP_VERSION_ID >= 80400) { + $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); + } $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ - 'Interfaces may not include properties.', + 'Property hooks in interfaces are supported only on PHP 8.4 and later.', 7, ], [ 'Interfaces may not include properties.', 9, ], + [ + 'Interfaces may not include properties.', + 11, + ], ]); } public function testPhp83AndPropertyHooksInInterface(): void { - $this->phpVersion = 80300; + if (PHP_VERSION_ID >= 80400) { + $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); + } $this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [ [ @@ -54,23 +60,27 @@ public function testPhp83AndPropertyHooksInInterface(): void public function testPhp84AndPropertiesInInterface(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ 'Interfaces may only include hooked properties.', - 7, + 9, ], [ 'Interfaces may only include hooked properties.', - 9, + 11, ], ]); } public function testPhp84AndNonPublicPropertyHooksInInterface(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/property-hooks-visibility-in-interface.php'], [ [ @@ -86,7 +96,9 @@ public function testPhp84AndNonPublicPropertyHooksInInterface(): void public function testPhp84AndPropertyHooksWithBodiesInInterface(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/property-hooks-bodies-in-interface.php'], [ [ diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index 69b5210423..24b8ab68bc 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -13,24 +13,56 @@ class PropertyInClassRuleTest extends RuleTestCase { - private int $phpVersion = PHP_VERSION_ID; - protected function getRule(): Rule { - return new PropertyInClassRule(new PhpVersion($this->phpVersion)); + return new PropertyInClassRule(new PhpVersion(PHP_VERSION_ID)); + } + + public function testPhpLessThan84AndHookedPropertiesInClass(): void + { + if (PHP_VERSION_ID >= 80400) { + $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); + } + + $this->analyse([__DIR__ . '/data/hooked-properties-in-class.php'], [ + [ + 'Property hooks in classes are supported only on PHP 8.4 and later.', + 7, + ], + ]); + } + + public function testPhp84AndHookedPropertiesWithoutBodiesInClass(): void + { + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } + + $this->analyse([__DIR__ . '/data/hooked-properties-without-bodies-in-class.php'], [ + [ + 'Non-abstract classes may not include hooked properties without bodies.', + 7, + ], + [ + 'Non-abstract classes may not include hooked properties without bodies.', + 9, + ], + ]); } public function testPhp84AndNonAbstractHookedPropertiesInClass(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-class.php'], [ [ - 'Classes may not include hooked properties without bodies.', + 'Non-abstract classes may not include hooked properties without bodies.', 7, ], [ - 'Classes may not include hooked properties without bodies.', + 'Non-abstract classes may not include hooked properties without bodies.', 9, ], ]); @@ -38,7 +70,9 @@ public function testPhp84AndNonAbstractHookedPropertiesInClass(): void public function testPhp84AndAbstractHookedPropertiesInClass(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/abstract-hooked-properties-in-class.php'], [ [ @@ -54,7 +88,9 @@ public function testPhp84AndAbstractHookedPropertiesInClass(): void public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-abstract-class.php'], [ [ @@ -70,7 +106,9 @@ public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void public function testPhp84AndAbstractNonHookedPropertiesInAbstractClass(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/abstract-non-hooked-properties-in-abstract-class.php'], [ [ @@ -86,7 +124,9 @@ public function testPhp84AndAbstractNonHookedPropertiesInAbstractClass(): void public function testPhp84AndAbstractHookedPropertiesWithBodies(): void { - $this->phpVersion = 80400; + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } $this->analyse([__DIR__ . '/data/abstract-hooked-properties-with-bodies.php'], [ [ diff --git a/tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php b/tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php new file mode 100644 index 0000000000..e3f45ee60f --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php @@ -0,0 +1,11 @@ + $this->name; + set => $this->name = $value; + } +} diff --git a/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php b/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php new file mode 100644 index 0000000000..dc839f0d2c --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php @@ -0,0 +1,10 @@ + Date: Thu, 28 Nov 2024 22:15:30 +0100 Subject: [PATCH 04/11] Skip tests on right versions --- .../Rules/Properties/PropertiesInInterfaceRuleTest.php | 6 ++++++ tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php | 3 +++ 2 files changed, 9 insertions(+) diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index 48739a74ba..dd2c37ae37 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -23,6 +23,9 @@ public function testPhp83AndPropertiesInInterface(): void if (PHP_VERSION_ID >= 80400) { $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); } + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Property hooks cause syntax error on PHP 7.4'); + } $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ @@ -45,6 +48,9 @@ public function testPhp83AndPropertyHooksInInterface(): void if (PHP_VERSION_ID >= 80400) { $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); } + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Property hooks cause syntax error on PHP 7.4'); + } $this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [ [ diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index 24b8ab68bc..4e9f3ee4f3 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -23,6 +23,9 @@ public function testPhpLessThan84AndHookedPropertiesInClass(): void if (PHP_VERSION_ID >= 80400) { $this->markTestSkipped('Test requires PHP 8.3 or earlier.'); } + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Property hooks cause syntax error on PHP 7.4'); + } $this->analyse([__DIR__ . '/data/hooked-properties-in-class.php'], [ [ From cb53a79a1a1380d4eb282acb22ccad722954cf74 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:17:03 +0100 Subject: [PATCH 05/11] Fix tests --- src/Rules/Properties/PropertiesInInterfaceRule.php | 9 --------- .../Rules/Properties/PropertiesInInterfaceRuleTest.php | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/Rules/Properties/PropertiesInInterfaceRule.php b/src/Rules/Properties/PropertiesInInterfaceRule.php index a433193ac3..e0605e6841 100644 --- a/src/Rules/Properties/PropertiesInInterfaceRule.php +++ b/src/Rules/Properties/PropertiesInInterfaceRule.php @@ -30,15 +30,6 @@ public function processNode(Node $node, Scope $scope): array return []; } - if (!$this->phpVersion->supportsPropertyHooks() && $node->hasHooks()) { - return [ - RuleErrorBuilder::message('Property hooks in interfaces are supported only on PHP 8.4 and later.') - ->nonIgnorable() - ->identifier('property.unsupportedHooksInInterface') - ->build(), - ]; - } - if (!$this->phpVersion->supportsPropertyHooks()) { return [ RuleErrorBuilder::message('Interfaces may not include properties.') diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index dd2c37ae37..d011d3a844 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -29,7 +29,7 @@ public function testPhp83AndPropertiesInInterface(): void $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ - 'Property hooks in interfaces are supported only on PHP 8.4 and later.', + 'Interfaces may not include properties.', 7, ], [ From 21c1d1b1e4edec8d724736faf9d5de0c7852e1c1 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:21:31 +0100 Subject: [PATCH 06/11] Do not modify collision detector --- build/collision-detector.json | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/build/collision-detector.json b/build/collision-detector.json index 9a86709292..12de9af1d3 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -11,17 +11,6 @@ "../tests/PHPStan/Rules/Names/data/no-namespace.php", "../tests/notAutoloaded", "../tests/PHPStan/Rules/Functions/data/define-bug-3349.php", - "../tests/PHPStan/Levels/data/stubs/function.php", - "../tests/PHPStan/Rules/Properties/data/properties-in-interface.php", - "../tests/PHPStan/Rules/Properties/data/property-hooks-bodies-in-interface.php", - "../tests/PHPStan/Rules/Properties/data/property-hooks-in-interface.php", - "../tests/PHPStan/Rules/Properties/data/property-hooks-visibility-in-interface.php", - "../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php", - "../tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-with-bodies.php", - "../tests/PHPStan/Rules/Properties/data/abstract-non-hooked-properties-in-abstract-class.php", - "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php", - "../tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php", - "../tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php", - "../tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php" + "../tests/PHPStan/Levels/data/stubs/function.php" ] } From 07c90abfffae0261eac9fe4ca3cad741f479d326 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:29:19 +0100 Subject: [PATCH 07/11] Run collision-detector on 8.4 --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 4680ddb82d..86ee004f07 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -116,7 +116,7 @@ jobs: uses: "shivammathur/setup-php@v2" with: coverage: "none" - php-version: "8.3" + php-version: "8.4" - name: "Install dependencies" run: "composer install --no-interaction --no-progress" From 0d3edef6b8a6c80de07c3ed9a362e66622fbfd46 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:30:52 +0100 Subject: [PATCH 08/11] Cosmetics according to my taste --- .../Properties/PropertiesInInterfaceRule.php | 8 ++--- src/Rules/Properties/PropertyInClassRule.php | 36 +++++++++---------- .../PropertiesInInterfaceRuleTest.php | 22 ++++++------ .../Properties/PropertyInClassRuleTest.php | 22 ++++++------ 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/Rules/Properties/PropertiesInInterfaceRule.php b/src/Rules/Properties/PropertiesInInterfaceRule.php index e0605e6841..df5354adb3 100644 --- a/src/Rules/Properties/PropertiesInInterfaceRule.php +++ b/src/Rules/Properties/PropertiesInInterfaceRule.php @@ -32,7 +32,7 @@ public function processNode(Node $node, Scope $scope): array if (!$this->phpVersion->supportsPropertyHooks()) { return [ - RuleErrorBuilder::message('Interfaces may not include properties.') + RuleErrorBuilder::message('Interfaces cannot include properties.') ->nonIgnorable() ->identifier('property.inInterface') ->build(), @@ -41,7 +41,7 @@ public function processNode(Node $node, Scope $scope): array if (!$node->hasHooks()) { return [ - RuleErrorBuilder::message('Interfaces may only include hooked properties.') + RuleErrorBuilder::message('Interfaces can only include hooked properties.') ->nonIgnorable() ->identifier('property.nonHookedInInterface') ->build(), @@ -50,7 +50,7 @@ public function processNode(Node $node, Scope $scope): array if (!$node->isPublic()) { return [ - RuleErrorBuilder::message('Interfaces may not include non-public properties.') + RuleErrorBuilder::message('Interfaces cannot include non-public properties.') ->nonIgnorable() ->identifier('property.nonPublicInInterface') ->build(), @@ -59,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array if ($this->hasAnyHookBody($node)) { return [ - RuleErrorBuilder::message('Interfaces may not include property hooks with bodies.') + RuleErrorBuilder::message('Interfaces cannot include property hooks with bodies.') ->nonIgnorable() ->identifier('property.hookBodyInInterface') ->build(), diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index 3b316ed305..c1e62c3c6a 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -32,16 +32,16 @@ public function processNode(Node $node, Scope $scope): array return []; } - if (!$this->phpVersion->supportsPropertyHooks() && $node->hasHooks()) { - return [ - RuleErrorBuilder::message('Property hooks in classes are supported only on PHP 8.4 and later.') - ->nonIgnorable() - ->identifier('property.unsupportedHooksInClass') - ->build(), - ]; - } - if (!$this->phpVersion->supportsPropertyHooks()) { + if ($node->hasHooks()) { + return [ + RuleErrorBuilder::message('Property hooks are supported only on PHP 8.4 and later.') + ->nonIgnorable() + ->identifier('property.hooksNotSupported') + ->build(), + ]; + } + return []; } @@ -49,9 +49,9 @@ public function processNode(Node $node, Scope $scope): array if ($node->isAbstract()) { if (!$node->hasHooks()) { return [ - RuleErrorBuilder::message('Only hooked properties may be declared abstract.') + RuleErrorBuilder::message('Only hooked properties can be declared abstract.') ->nonIgnorable() - ->identifier('property.nonHookedAbstractInClass') + ->identifier('property.abstractNonHooked') ->build(), ]; } @@ -60,7 +60,7 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') ->nonIgnorable() - ->identifier('property.hookedAbstractWithBodies') + ->identifier('property.abstractWithoutAbstractHook') ->build(), ]; } @@ -69,9 +69,9 @@ public function processNode(Node $node, Scope $scope): array if (!$node->isAbstract()) { if ($node->hasHooks()) { return [ - RuleErrorBuilder::message('Abstract classes may not include non-abstract hooked properties without bodies.') + RuleErrorBuilder::message('Abstract classes cannot include non-abstract hooked properties without bodies.') ->nonIgnorable() - ->identifier('property.nonAbstractHookedWithoutBodyInAbstractClass') + ->identifier('property.nonAbstractWithAbstractHook') ->build(), ]; } @@ -83,18 +83,18 @@ public function processNode(Node $node, Scope $scope): array if ($node->hasHooks()) { if ($node->isAbstract()) { return [ - RuleErrorBuilder::message('Classes may not include abstract hooked properties.') + RuleErrorBuilder::message('Non-abstract classes cannot include abstract properties.') ->nonIgnorable() - ->identifier('property.abstractHookedInClass') + ->identifier('property.abstract') ->build(), ]; } if (!$this->doAllHooksHaveBody($node)) { return [ - RuleErrorBuilder::message('Non-abstract classes may not include hooked properties without bodies.') + RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') ->nonIgnorable() - ->identifier('property.hookedWithoutBodyInClass') + ->identifier('property.hookWithoutBody') ->build(), ]; } diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index d011d3a844..de425931da 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -29,15 +29,15 @@ public function testPhp83AndPropertiesInInterface(): void $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ - 'Interfaces may not include properties.', + 'Interfaces cannot include properties.', 7, ], [ - 'Interfaces may not include properties.', + 'Interfaces cannot include properties.', 9, ], [ - 'Interfaces may not include properties.', + 'Interfaces cannot include properties.', 11, ], ]); @@ -54,11 +54,11 @@ public function testPhp83AndPropertyHooksInInterface(): void $this->analyse([__DIR__ . '/data/property-hooks-in-interface.php'], [ [ - 'Interfaces may not include properties.', + 'Interfaces cannot include properties.', 7, ], [ - 'Interfaces may not include properties.', + 'Interfaces cannot include properties.', 9, ], ]); @@ -72,11 +72,11 @@ public function testPhp84AndPropertiesInInterface(): void $this->analyse([__DIR__ . '/data/properties-in-interface.php'], [ [ - 'Interfaces may only include hooked properties.', + 'Interfaces can only include hooked properties.', 9, ], [ - 'Interfaces may only include hooked properties.', + 'Interfaces can only include hooked properties.', 11, ], ]); @@ -90,11 +90,11 @@ public function testPhp84AndNonPublicPropertyHooksInInterface(): void $this->analyse([__DIR__ . '/data/property-hooks-visibility-in-interface.php'], [ [ - 'Interfaces may not include non-public properties.', + 'Interfaces cannot include non-public properties.', 7, ], [ - 'Interfaces may not include non-public properties.', + 'Interfaces cannot include non-public properties.', 9, ], ]); @@ -108,11 +108,11 @@ public function testPhp84AndPropertyHooksWithBodiesInInterface(): void $this->analyse([__DIR__ . '/data/property-hooks-bodies-in-interface.php'], [ [ - 'Interfaces may not include property hooks with bodies.', + 'Interfaces cannot include property hooks with bodies.', 7, ], [ - 'Interfaces may not include property hooks with bodies.', + 'Interfaces cannot include property hooks with bodies.', 13, ], ]); diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index 4e9f3ee4f3..879e43357d 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -29,7 +29,7 @@ public function testPhpLessThan84AndHookedPropertiesInClass(): void $this->analyse([__DIR__ . '/data/hooked-properties-in-class.php'], [ [ - 'Property hooks in classes are supported only on PHP 8.4 and later.', + 'Property hooks are supported only on PHP 8.4 and later.', 7, ], ]); @@ -43,11 +43,11 @@ public function testPhp84AndHookedPropertiesWithoutBodiesInClass(): void $this->analyse([__DIR__ . '/data/hooked-properties-without-bodies-in-class.php'], [ [ - 'Non-abstract classes may not include hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 7, ], [ - 'Non-abstract classes may not include hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 9, ], ]); @@ -61,11 +61,11 @@ public function testPhp84AndNonAbstractHookedPropertiesInClass(): void $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-class.php'], [ [ - 'Non-abstract classes may not include hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 7, ], [ - 'Non-abstract classes may not include hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 9, ], ]); @@ -79,11 +79,11 @@ public function testPhp84AndAbstractHookedPropertiesInClass(): void $this->analyse([__DIR__ . '/data/abstract-hooked-properties-in-class.php'], [ [ - 'Classes may not include abstract hooked properties.', + 'Non-abstract classes cannot include abstract properties.', 7, ], [ - 'Classes may not include abstract hooked properties.', + 'Non-abstract classes cannot include abstract properties.', 9, ], ]); @@ -97,11 +97,11 @@ public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-abstract-class.php'], [ [ - 'Abstract classes may not include non-abstract hooked properties without bodies.', + 'Abstract classes cannot include non-abstract hooked properties without bodies.', 7, ], [ - 'Abstract classes may not include non-abstract hooked properties without bodies.', + 'Abstract classes cannot include non-abstract hooked properties without bodies.', 9, ], ]); @@ -115,11 +115,11 @@ public function testPhp84AndAbstractNonHookedPropertiesInAbstractClass(): void $this->analyse([__DIR__ . '/data/abstract-non-hooked-properties-in-abstract-class.php'], [ [ - 'Only hooked properties may be declared abstract.', + 'Only hooked properties can be declared abstract.', 7, ], [ - 'Only hooked properties may be declared abstract.', + 'Only hooked properties can be declared abstract.', 9, ], ]); From 7eaed3a5799a0dfb268b408cddb179fa70019bca Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:46:08 +0100 Subject: [PATCH 09/11] Fix logic --- src/Rules/Properties/PropertyInClassRule.php | 18 ++++++------- .../Properties/PropertyInClassRuleTest.php | 8 ++++-- ...ct-hooked-properties-in-abstract-class.php | 25 +++++++++++++++++++ 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index c1e62c3c6a..b41c261868 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -64,17 +64,17 @@ public function processNode(Node $node, Scope $scope): array ->build(), ]; } + + return []; } - if (!$node->isAbstract()) { - if ($node->hasHooks()) { - return [ - RuleErrorBuilder::message('Abstract classes cannot include non-abstract hooked properties without bodies.') - ->nonIgnorable() - ->identifier('property.nonAbstractWithAbstractHook') - ->build(), - ]; - } + if (!$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + ->nonIgnorable() + ->identifier('property.hookWithoutBody') + ->build(), + ]; } return []; diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index 879e43357d..0b2ca5ba09 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -97,13 +97,17 @@ public function testPhp84AndNonAbstractHookedPropertiesInAbstractClass(): void $this->analyse([__DIR__ . '/data/non-abstract-hooked-properties-in-abstract-class.php'], [ [ - 'Abstract classes cannot include non-abstract hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 7, ], [ - 'Abstract classes cannot include non-abstract hooked properties without bodies.', + 'Non-abstract properties cannot include hooks without bodies.', 9, ], + [ + 'Non-abstract properties cannot include hooks without bodies.', + 25, + ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php b/tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php index 48aa9dd5f4..a29e8e769d 100644 --- a/tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php +++ b/tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-abstract-class.php @@ -8,3 +8,28 @@ abstract class AbstractPerson public string $lastName { get; set; } } + +abstract class Foo +{ + + public string $name { + get { + + } + + set { + + } + } + + public string $name2 { + get; + + set { + + } + } + + public string $name3; + +} From f8fc25d53a2c18a7e8a04308ee1c99213ffddcc5 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:48:41 +0100 Subject: [PATCH 10/11] Cleanup --- src/Rules/Properties/PropertyInClassRule.php | 57 ++++++++------------ 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index b41c261868..3500959541 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -45,59 +45,44 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($classReflection->isAbstract()) { - if ($node->isAbstract()) { - if (!$node->hasHooks()) { - return [ - RuleErrorBuilder::message('Only hooked properties can be declared abstract.') - ->nonIgnorable() - ->identifier('property.abstractNonHooked') - ->build(), - ]; - } - - if (!$this->isAtLeastOneHookBodyEmpty($node)) { - return [ - RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') - ->nonIgnorable() - ->identifier('property.abstractWithoutAbstractHook') - ->build(), - ]; - } - - return []; - } - - if (!$this->doAllHooksHaveBody($node)) { + if ($node->isAbstract()) { + if (!$node->hasHooks()) { return [ - RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + RuleErrorBuilder::message('Only hooked properties can be declared abstract.') ->nonIgnorable() - ->identifier('property.hookWithoutBody') + ->identifier('property.abstractNonHooked') ->build(), ]; } - return []; - } - - if ($node->hasHooks()) { - if ($node->isAbstract()) { + if (!$this->isAtLeastOneHookBodyEmpty($node)) { return [ - RuleErrorBuilder::message('Non-abstract classes cannot include abstract properties.') + RuleErrorBuilder::message('Abstract properties must specify at least one abstract hook.') ->nonIgnorable() - ->identifier('property.abstract') + ->identifier('property.abstractWithoutAbstractHook') ->build(), ]; } - if (!$this->doAllHooksHaveBody($node)) { + if (!$classReflection->isAbstract()) { return [ - RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + RuleErrorBuilder::message('Non-abstract classes cannot include abstract properties.') ->nonIgnorable() - ->identifier('property.hookWithoutBody') + ->identifier('property.abstract') ->build(), ]; } + + return []; + } + + if (!$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + ->nonIgnorable() + ->identifier('property.hookWithoutBody') + ->build(), + ]; } return []; From ac3dbb36f7532922d8f2ed13ee0d0ec4af854eb3 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Nov 2024 22:55:43 +0100 Subject: [PATCH 11/11] Fix duplicate namespaces --- .../Properties/data/abstract-hooked-properties-in-class.php | 2 +- .../data/abstract-non-hooked-properties-in-abstract-class.php | 2 +- .../Rules/Properties/data/hooked-properties-in-class.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php b/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php index 68e6978a36..d035d36810 100644 --- a/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php +++ b/tests/PHPStan/Rules/Properties/data/abstract-hooked-properties-in-class.php @@ -1,6 +1,6 @@