Skip to content

Commit

Permalink
Extract introduced Property Hooks-related functionalities into thei…
Browse files Browse the repository at this point in the history
…r own rules
  • Loading branch information
jakubtobiasz committed Nov 23, 2024
1 parent c263971 commit 09f4b1a
Show file tree
Hide file tree
Showing 14 changed files with 242 additions and 24 deletions.
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ 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/non-public-property-hook.php \
--exclude tests/PHPStan/Rules/Properties/data/non-empty-property-hook.php \
src tests

cs:
Expand Down
2 changes: 2 additions & 0 deletions build/collision-detector.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"../tests/PHPStan/Rules/Names/data/no-namespace.php",
"../tests/notAutoloaded",
"../tests/PHPStan/Rules/Functions/data/define-bug-3349.php",
"../tests/PHPStan/Rules/Properties/data/non-public-property-hook.php",
"../tests/PHPStan/Rules/Properties/data/non-empty-property-hook.php",
"../tests/PHPStan/Levels/data/stubs/function.php"
]
}
2 changes: 2 additions & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ rules:
- PHPStan\Rules\Properties\PropertyAttributesRule
- PHPStan\Rules\Properties\ReadOnlyPropertyRule
- PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyRule
- PHPStan\Rules\PropertyHooks\NonEmptyPropertyHookRule
- PHPStan\Rules\PropertyHooks\NonPublicPropertyHookRule
- PHPStan\Rules\Regexp\RegularExpressionPatternRule
- PHPStan\Rules\Traits\ConflictingTraitConstantsRule
- PHPStan\Rules\Traits\ConstantsInTraitsRule
Expand Down
1 change: 0 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,6 @@ private function processStmtNode(
}
$propStmt = clone $stmt;
$propStmt->setAttributes($prop->getAttributes());
rd($propStmt);
$nodeCallback(
new ClassPropertyNode(
$propertyName,
Expand Down
16 changes: 16 additions & 0 deletions src/Node/ClassPropertyNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,20 @@ public function getSubNodeNames(): array
return [];
}

public function isPropertyHook(): bool
{
return $this->originalNode->hooks !== [];
}

public function areHooksBodiesEmpty(): bool
{
foreach ($this->originalNode->hooks as $hook) {
if ($hook->body !== null) {
return false;
}
}

return true;
}

}
3 changes: 2 additions & 1 deletion src/Php/PhpVersion.php
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public function supportsPregCaptureOnlyNamedGroups(): bool
return $this->versionId >= 80200;
}

public function supportsPublicPropertiesInInterfaces(): bool
public function supportsPropertyHooksInInterfaces(): bool
{
return $this->versionId >= 80400;
}
Expand Down Expand Up @@ -379,4 +379,5 @@ public function substrReturnFalseInsteadOfEmptyString(): bool
{
return $this->versionId < 80000;
}

}
7 changes: 1 addition & 6 deletions src/Rules/Properties/PropertiesInInterfaceRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

Expand All @@ -15,10 +14,6 @@
final class PropertiesInInterfaceRule implements Rule
{

public function __construct (private PhpVersion $phpVersion)
{
}

public function getNodeType(): string
{
return ClassPropertyNode::class;
Expand All @@ -30,7 +25,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($node->isPublic() && $this->phpVersion->supportsPublicPropertiesInInterfaces()) {
if ($node->isPropertyHook()) {
return [];
}

Expand Down
49 changes: 49 additions & 0 deletions src/Rules/PropertyHooks/NonEmptyPropertyHookRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\PropertyHooks;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

/**
* @implements Rule<ClassPropertyNode>
*/
final class NonEmptyPropertyHookRule implements Rule
{

public function __construct(private PhpVersion $phpVersion)
{
}

public function getNodeType(): string
{
return ClassPropertyNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$node->getClassReflection()->isInterface()) {
return [];
}

if (!$this->phpVersion->supportsPropertyHooksInInterfaces() || !$node->isPropertyHook()) {
return [];
}

if ($node->areHooksBodiesEmpty()) {
return [];
}

return [
RuleErrorBuilder::message('Property hook must not be empty.')
->nonIgnorable()
->identifier('propertyHook.nonEmpty')
->build(),
];
}

}
49 changes: 49 additions & 0 deletions src/Rules/PropertyHooks/NonPublicPropertyHookRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\PropertyHooks;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

/**
* @implements Rule<ClassPropertyNode>
*/
final class NonPublicPropertyHookRule implements Rule
{

public function __construct(private PhpVersion $phpVersion)
{
}

public function getNodeType(): string
{
return ClassPropertyNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$node->getClassReflection()->isInterface()) {
return [];
}

if (!$this->phpVersion->supportsPropertyHooksInInterfaces() || !$node->isPropertyHook()) {
return [];
}

if ($node->isPublic()) {
return [];
}

return [
RuleErrorBuilder::message('Property hook must be public.')
->nonIgnorable()
->identifier('propertyHook.nonPublic')
->build(),
];
}

}
40 changes: 40 additions & 0 deletions tests/PHPStan/Rules/Properties/NonEmptyPropertyHookRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\PropertyHooks\NonEmptyPropertyHookRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<NonEmptyPropertyHookRule>
*/
class NonEmptyPropertyHookRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new NonEmptyPropertyHookRule(new PhpVersion(PHP_VERSION_ID));
}

public function testRule(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('This test requires at least PHP 8.4.');
}

$this->analyse([__DIR__ . '/data/non-empty-property-hook.php'], [
[
'Property hook must not be empty.',
7,
],
[
'Property hook must not be empty.',
14,
],
]);
}

}
40 changes: 40 additions & 0 deletions tests/PHPStan/Rules/Properties/NonPublicPropertyHookRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\PropertyHooks\NonPublicPropertyHookRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<NonPublicPropertyHookRule>
*/
class NonPublicPropertyHookRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new NonPublicPropertyHookRule(new PhpVersion(PHP_VERSION_ID));
}

public function testRule(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('This test requires at least PHP 8.4.');
}

$this->analyse([__DIR__ . '/data/non-public-property-hook.php'], [
[
'Property hook must be public.',
7,
],
[
'Property hook must be public.',
9,
],
]);
}

}
17 changes: 1 addition & 16 deletions tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,22 @@

namespace PHPStan\Rules\Properties;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<PropertiesInInterfaceRule>
*/
class PropertiesInInterfaceRuleTest extends RuleTestCase
{

private PhpVersion $phpVersion;

protected function setUp(): void
{
$this->phpVersion = new PhpVersion(PHP_VERSION_ID);
}

protected function getRule(): Rule
{
return new PropertiesInInterfaceRule(new PhpVersion(PHP_VERSION_ID));
return new PropertiesInInterfaceRule();
}

public function testRule(): void
{
if ($this->phpVersion->supportsPublicPropertiesInInterfaces()) {
$this->analyse([__DIR__ . '/data/properties-in-interface.php'], []);

return;
}

$this->analyse([__DIR__ . '/data/properties-in-interface.php'], [
[
'Interfaces may not include properties.',
Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Properties/data/non-empty-property-hook.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php declare(strict_types=1);

namespace NonEmptyPropertyHook;

interface HelloWorld
{
public string $name {
get {
return $this->name;
}
set;
}

public string $surname {
get;
set {
$this->name = $value;
}
}

public string $middleName { get ; }

public string $title { set; }

public string $familyName { get; set ;}
}
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Properties/data/non-public-property-hook.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare(strict_types=1);

namespace NonPublicPropertyHook;

interface HelloWorld
{
private string $firstName { get; }

protected string $lastName { get; }

public string $fullName { get; }
}

0 comments on commit 09f4b1a

Please sign in to comment.