From d11416ce96ca1a58c603a66a6a4b3c6b2aa304e9 Mon Sep 17 00:00:00 2001 From: Simtel Date: Wed, 26 Aug 2026 11:06:50 +0400 Subject: [PATCH] Refactor rules: configurable suffixes, message registry, InClassNode - Make class name suffixes configurable via constructor args (Command, CommandHandler, EventListener, AsEventListener) with defaults - Add RuleMessages registry for error templates and identifiers - Switch class-level rules to InClassNode and use ClassReflection (getDisplayName, hasNativeMethod, getAttributes) instead of raw AST names - Convert command rule test to a #[DataProvider] dataset - Document suffix customization in README --- AGENTS.md | 7 ++- README.md | 32 +++++++++++-- ...andClassShouldHaveCommandHandlerSeeTag.php | 48 +++++++++++-------- ...enerShouldHaveAsEventListenerAttribute.php | 31 +++++++----- src/Rule/RuleMessages.php | 28 +++++++++++ ...houldNotPhpDocReturnWhenTypeHintExists.php | 6 +-- ...lassShouldHaveCommandHandlerSeeTagTest.php | 48 ++++++++++--------- ...ShouldHaveAsEventListenerAttributeTest.php | 3 +- ...dNotPhpDocReturnWhenTypeHintExistsTest.php | 5 +- 9 files changed, 140 insertions(+), 68 deletions(-) create mode 100644 src/Rule/RuleMessages.php diff --git a/AGENTS.md b/AGENTS.md index 246132d..5a22a30 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,9 +12,12 @@ Custom PHPStan rules package (`simtel/phpstan-rules`). Adds static-analysis rule ## Architecture - `AbstractPhpDocRule` (in `src/Rule/`) is the shared base for rules that parse PHPDoc: it injects `PhpDocParser` + `Lexer` and exposes `parsePhpDoc(string $doc): PhpDocNode`. Subclasses implement `Rule` themselves. -- `EventListenerShouldHaveAsEventListenerAttribute` inspects `$node->attrGroups` directly (no reflection, no base class). +- The two class-level rules target PHPStan's `InClassNode` (not `Class_`): at a raw `Class_` node the scope is the *outer* scope, so `$scope->getClassReflection()` would return the wrong class. Use `$node->getClassReflection()` (and `$node->getOriginalNode()` for the AST node). +- `EventListenerShouldHaveAsEventListenerAttribute` reads attributes via `$classReflection->getAttributes()` (no base class, no manual `$node->attrGroups`). - `ShouldNotPhpDocReturnWhenTypeHintExists` works on `ClassMethod` nodes and reads the native type from `$node->returnType` (no reflection needed). Only `Identifier`/`Name` native types and `IdentifierTypeNode` PHPDoc types are compared — union/nullable/generic types are skipped, not errors. -- Errors are reported via `RuleErrorBuilder::message(...)->identifier('rule.group')` (never throw). PHPStan 2.x requires identifiers. +- `RuleMessages` holds all error message templates (with `%s` placeholders) and identifiers — rules and tests both reference these constants. +- Class name suffixes (`Command`, `CommandHandler`, `EventListener`, `AsEventListener`) are constructor args with defaults; override them via `services:` with the `phpstan.rules.rule` tag. +- Errors are reported via `RuleErrorBuilder::message(...)->identifier(RuleMessages::..._ID)` (never throw). PHPStan 2.x requires identifiers. ## Adding a rule diff --git a/README.md b/README.md index d5bb974..259eef5 100644 --- a/README.md +++ b/README.md @@ -100,11 +100,33 @@ includes: For granular control, register specific rules: ```neon -parameters: - rules: - - Simtel\PHPStanRules\Rule\CommandClassShouldHaveCommandHandlerSeeTag - - Simtel\PHPStanRules\Rule\EventListenerShouldHaveAsEventListenerAttribute - - Simtel\PHPStanRules\Rule\ShouldNotPhpDocReturnWhenTypeHintExists +rules: + - Simtel\PHPStanRules\Rule\CommandClassShouldHaveCommandHandlerSeeTag + - Simtel\PHPStanRules\Rule\EventListenerShouldHaveAsEventListenerAttribute + - Simtel\PHPStanRules\Rule\ShouldNotPhpDocReturnWhenTypeHintExists +``` + +### Customizing class name suffixes + +The name suffixes the rules match on are configurable. Register a rule as a service +and pass the suffix arguments (the PHPDoc parser/lexer are autowired): + +```neon +services: + - + class: Simtel\PHPStanRules\Rule\CommandClassShouldHaveCommandHandlerSeeTag + arguments: + commandSuffix: 'Command' + commandHandlerSuffix: 'CommandHandler' + tags: + - phpstan.rules.rule + - + class: Simtel\PHPStanRules\Rule\EventListenerShouldHaveAsEventListenerAttribute + arguments: + eventListenerSuffix: 'EventListener' + asEventListenerSuffix: 'AsEventListener' + tags: + - phpstan.rules.rule ``` ### Complete Configuration Example diff --git a/src/Rule/CommandClassShouldHaveCommandHandlerSeeTag.php b/src/Rule/CommandClassShouldHaveCommandHandlerSeeTag.php index 0aee6f6..f5490f5 100644 --- a/src/Rule/CommandClassShouldHaveCommandHandlerSeeTag.php +++ b/src/Rule/CommandClassShouldHaveCommandHandlerSeeTag.php @@ -5,43 +5,54 @@ namespace Simtel\PHPStanRules\Rule; use PhpParser\Node; -use PhpParser\Node\Stmt\Class_; use PHPStan\Analyser\Scope; +use PHPStan\Node\InClassNode; use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; +use PHPStan\PhpDocParser\Lexer\Lexer; +use PHPStan\PhpDocParser\Parser\PhpDocParser; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; /** - * @implements Rule + * @implements Rule */ final class CommandClassShouldHaveCommandHandlerSeeTag extends AbstractPhpDocRule implements Rule { + public function __construct( + PhpDocParser $phpDocParser, + Lexer $phpDocLexer, + private readonly string $commandSuffix = 'Command', + private readonly string $commandHandlerSuffix = 'CommandHandler', + ) { + parent::__construct($phpDocParser, $phpDocLexer); + } + public function getNodeType(): string { - return Class_::class; + return InClassNode::class; } public function processNode(Node $node, Scope $scope): array { - if ($node->name === null) { + $classReflection = $node->getClassReflection(); + if (! $classReflection->isClass()) { return []; } - if (! str_ends_with($node->name->name, 'Command')) { + if (! str_ends_with($classReflection->getDisplayName(), $this->commandSuffix)) { return []; } - foreach ($node->getMethods() as $method) { - if ($method->name->name === '__invoke') { - return []; - } + if ($classReflection->hasNativeMethod('__invoke')) { + return []; } - $doc = $node->getDocComment()?->getText() ?? ''; + $doc = $node->getOriginalNode() + ->getDocComment()?->getText() ?? ''; if ($doc === '') { return [ - RuleErrorBuilder::message('Command class should be include phpDoc with @see attribute') - ->identifier('commandClass.missingPhpDoc') + RuleErrorBuilder::message(RuleMessages::COMMAND_MISSING_PHP_DOC) + ->identifier(RuleMessages::IDENTIFIER_COMMAND_MISSING_PHP_DOC) ->build(), ]; } @@ -55,15 +66,16 @@ public function processNode(Node $node, Scope $scope): array continue; } $hasSeeTag = true; - if (! str_ends_with($tag->value->value, 'CommandHandler')) { + if (! str_ends_with($tag->value->value, $this->commandHandlerSuffix)) { return [ RuleErrorBuilder::message( sprintf( - 'PhpDoc command class should be include @see attribute with CommandHandler class name, but include %s', + RuleMessages::COMMAND_INVALID_SEE_VALUE, + $this->commandHandlerSuffix, $tag->value->value ) ) - ->identifier('commandClass.invalidSeeValue') + ->identifier(RuleMessages::IDENTIFIER_COMMAND_INVALID_SEE_VALUE) ->build(), ]; } @@ -71,10 +83,8 @@ public function processNode(Node $node, Scope $scope): array if (! $hasSeeTag) { return [ - RuleErrorBuilder::message( - 'PhpDoc command class should be include @see attribute with CommandHandler class name' - ) - ->identifier('commandClass.missingSee') + RuleErrorBuilder::message(sprintf(RuleMessages::COMMAND_MISSING_SEE, $this->commandHandlerSuffix)) + ->identifier(RuleMessages::IDENTIFIER_COMMAND_MISSING_SEE) ->build(), ]; } diff --git a/src/Rule/EventListenerShouldHaveAsEventListenerAttribute.php b/src/Rule/EventListenerShouldHaveAsEventListenerAttribute.php index 22b9552..38bc5ab 100644 --- a/src/Rule/EventListenerShouldHaveAsEventListenerAttribute.php +++ b/src/Rule/EventListenerShouldHaveAsEventListenerAttribute.php @@ -5,42 +5,49 @@ namespace Simtel\PHPStanRules\Rule; use PhpParser\Node; -use PhpParser\Node\Stmt\Class_; use PHPStan\Analyser\Scope; +use PHPStan\Node\InClassNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; /** - * @implements Rule + * @implements Rule */ final class EventListenerShouldHaveAsEventListenerAttribute implements Rule { + public function __construct( + private readonly string $eventListenerSuffix = 'EventListener', + private readonly string $asEventListenerSuffix = 'AsEventListener', + ) { + } + public function getNodeType(): string { - return Class_::class; + return InClassNode::class; } public function processNode(Node $node, Scope $scope): array { - if ($node->name === null) { + $classReflection = $node->getClassReflection(); + if (! $classReflection->isClass()) { return []; } - if (! str_ends_with($node->name->name, 'EventListener')) { + if (! str_ends_with($classReflection->getDisplayName(), $this->eventListenerSuffix)) { return []; } - foreach ($node->attrGroups as $attrGroup) { - foreach ($attrGroup->attrs as $attribute) { - if (str_ends_with($attribute->name->toString(), 'AsEventListener')) { - return []; - } + foreach ($classReflection->getAttributes() as $attribute) { + if (str_ends_with($attribute->getName(), $this->asEventListenerSuffix)) { + return []; } } return [ - RuleErrorBuilder::message('Event listener class should be include attribute #[AsEventListener]') - ->identifier('eventListener.missingAttribute') + RuleErrorBuilder::message( + sprintf(RuleMessages::EVENT_LISTENER_MISSING_ATTRIBUTE, $this->asEventListenerSuffix) + ) + ->identifier(RuleMessages::IDENTIFIER_EVENT_LISTENER_MISSING_ATTRIBUTE) ->build(), ]; } diff --git a/src/Rule/RuleMessages.php b/src/Rule/RuleMessages.php new file mode 100644 index 0000000..654b537 --- /dev/null +++ b/src/Rule/RuleMessages.php @@ -0,0 +1,28 @@ +parsePhpDoc($doc)->getReturnTagValues() as $returnTag) { if ($returnTag->type instanceof IdentifierTypeNode && $returnTag->type->name === $nativeTypeName) { return [ - RuleErrorBuilder::message( - 'PhpDoc attribute @return for method ' . $node->name->name . ' can be remove' - ) + RuleErrorBuilder::message(sprintf(RuleMessages::RETURN_REDUNDANT_PHP_DOC, $node->name->name)) ->line($node->getStartLine()) - ->identifier('returnType.redundantPhpDoc') + ->identifier(RuleMessages::IDENTIFIER_RETURN_REDUNDANT_PHP_DOC) ->build(), ]; } diff --git a/tests/Rules/CommandClassShouldHaveCommandHandlerSeeTagTest.php b/tests/Rules/CommandClassShouldHaveCommandHandlerSeeTagTest.php index bc4fdc5..7847b71 100644 --- a/tests/Rules/CommandClassShouldHaveCommandHandlerSeeTagTest.php +++ b/tests/Rules/CommandClassShouldHaveCommandHandlerSeeTagTest.php @@ -8,7 +8,9 @@ use PHPStan\PhpDocParser\Parser\PhpDocParser; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\DataProvider; use Simtel\PHPStanRules\Rule\CommandClassShouldHaveCommandHandlerSeeTag; +use Simtel\PHPStanRules\Rule\RuleMessages; class CommandClassShouldHaveCommandHandlerSeeTagTest extends RuleTestCase { @@ -22,32 +24,32 @@ protected function getRule(): Rule ); } - public function testCorrectSeeAttribute(): void + #[DataProvider('provideCommandCases')] + public function testRule(string $file, array $expectedErrors): void { - $this->analyse([__DIR__ . '/../data/command_handler_data1.php'], [ - [ - 'PhpDoc command class should be include @see attribute with CommandHandler class name, but include TestClassCommand', - 10, - ], - ]); + $this->analyse([$file], $expectedErrors); } - public function testExistsSeeAttribute(): void + /** + * @return iterable}> + */ + public static function provideCommandCases(): iterable { - $this->analyse([__DIR__ . '/../data/command_handler_data2.php'], [ - ['PhpDoc command class should be include @see attribute with CommandHandler class name', 10], - ]); - } - - public function testExistsPhpDoc(): void - { - $this->analyse([__DIR__ . '/../data/command_handler_data3.php'], [ - ['Command class should be include phpDoc with @see attribute', 7], - ]); - } - - public function testIfExistInvokeMethod(): void - { - $this->analyse([__DIR__ . '/../data/command_handler_data4.php'], []); + yield 'invalid see value' => [ + __DIR__ . '/../data/command_handler_data1.php', + [[sprintf(RuleMessages::COMMAND_INVALID_SEE_VALUE, 'CommandHandler', 'TestClassCommand'), 10, ], ], + ]; + + yield 'missing see tag' => [ + __DIR__ . '/../data/command_handler_data2.php', + [[sprintf(RuleMessages::COMMAND_MISSING_SEE, 'CommandHandler'), 10], ], + ]; + + yield 'missing phpdoc' => [ + __DIR__ . '/../data/command_handler_data3.php', + [[RuleMessages::COMMAND_MISSING_PHP_DOC, 7], ], + ]; + + yield 'has invoke method' => [__DIR__ . '/../data/command_handler_data4.php', [], ]; } } diff --git a/tests/Rules/EventListenerShouldHaveAsEventListenerAttributeTest.php b/tests/Rules/EventListenerShouldHaveAsEventListenerAttributeTest.php index 97af05c..c4c7e4a 100644 --- a/tests/Rules/EventListenerShouldHaveAsEventListenerAttributeTest.php +++ b/tests/Rules/EventListenerShouldHaveAsEventListenerAttributeTest.php @@ -7,6 +7,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use Simtel\PHPStanRules\Rule\EventListenerShouldHaveAsEventListenerAttribute; +use Simtel\PHPStanRules\Rule\RuleMessages; class EventListenerShouldHaveAsEventListenerAttributeTest extends RuleTestCase { @@ -23,7 +24,7 @@ public function testExistsNeedAttribute(): void public function testExistsAttribute(): void { $this->analyse([__DIR__ . '/../Fixture/EventListener/TestNotCorrectClassEventListener.php'], [ - ['Event listener class should be include attribute #[AsEventListener]', 7], + [sprintf(RuleMessages::EVENT_LISTENER_MISSING_ATTRIBUTE, 'AsEventListener'), 7], ]); } } diff --git a/tests/Rules/ShouldNotPhpDocReturnWhenTypeHintExistsTest.php b/tests/Rules/ShouldNotPhpDocReturnWhenTypeHintExistsTest.php index a98cd15..e782b60 100644 --- a/tests/Rules/ShouldNotPhpDocReturnWhenTypeHintExistsTest.php +++ b/tests/Rules/ShouldNotPhpDocReturnWhenTypeHintExistsTest.php @@ -8,6 +8,7 @@ use PHPStan\PhpDocParser\Parser\PhpDocParser; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use Simtel\PHPStanRules\Rule\RuleMessages; use Simtel\PHPStanRules\Rule\ShouldNotPhpDocReturnWhenTypeHintExists; class ShouldNotPhpDocReturnWhenTypeHintExistsTest extends RuleTestCase @@ -25,8 +26,8 @@ protected function getRule(): Rule public function testWithError(): void { $this->analyse([__DIR__ . '/../Fixture/Return/MethodsWithTypeHintAndReturn.php'], [ - ['PhpDoc attribute @return for method someMethod can be remove', 12], - ['PhpDoc attribute @return for method getInt can be remove', 20], + [sprintf(RuleMessages::RETURN_REDUNDANT_PHP_DOC, 'someMethod'), 12], + [sprintf(RuleMessages::RETURN_REDUNDANT_PHP_DOC, 'getInt'), 20], ]); }