From 2b10e9126b0c5029b274c13496b825d5751eda9d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 Jan 2025 11:01:18 +0100 Subject: [PATCH] Add SingleRequiredMethodRule to spot multiple @required methods in Symfony projects (#163) * split identifiers by group to easily search through them * Add SingleRequiredMethodRule --- README.md | 11 ++++ phpstan.neon | 1 - src/Enum/ClassName.php | 5 ++ src/Enum/DoctrineRuleIdentifier.php | 18 +++++++ src/Enum/PHPUnitRuleIdentifier.php | 14 +++++ src/Enum/RectorRuleIdentifier.php | 16 ++++++ src/Enum/RuleIdentifier.php | 40 -------------- src/Enum/SymfonyRuleIdentifier.php | 26 +++++++++ .../SymfonyRequiredMethodAnalyzer.php | 39 ++++++++++++++ src/Rules/Doctrine/NoDocumentMockingRule.php | 4 +- src/Rules/Doctrine/NoEntityMockingRule.php | 4 +- .../NoGetRepositoryOutsideServiceRule.php | 6 +-- src/Rules/Doctrine/NoParentRepositoryRule.php | 4 +- .../NoRepositoryCallInDataFixtureRule.php | 4 +- .../RequireQueryBuilderOnRepositoryRule.php | 4 +- src/Rules/PHPUnit/NoMockOnlyTestRule.php | 4 +- .../PHPUnit/PublicStaticDataProviderRule.php | 4 +- .../NoClassReflectionStaticReflectionRule.php | 4 +- .../NoInstanceOfStaticReflectionRule.php | 4 +- ...PhpUpgradeDowngradeRegisteredInSetRule.php | 4 +- ...deImplementsMinPhpVersionInterfaceRule.php | 4 +- .../NoAbstractControllerConstructorRule.php | 4 +- .../Symfony/NoGetDoctrineInControllerRule.php | 4 +- src/Rules/Symfony/NoGetInControllerRule.php | 4 +- .../Symfony/NoListenerWithoutContractRule.php | 4 +- .../Symfony/NoRequiredOutsideClassRule.php | 38 ++++--------- .../NoStringInGetSubscribedEventsRule.php | 4 +- .../RequireInvokableControllerRule.php | 4 +- .../Symfony/SingleArgEventDispatchRule.php | 4 +- .../Symfony/SingleRequiredMethodRule.php | 54 +++++++++++++++++++ ...edInterfaceInContractNamespaceRuleTest.php | 3 -- .../ExplicitClassPrefixSuffixRuleTest.php | 3 -- .../NoReturnSetterMethodRuleTest.php | 3 -- ...lassReflectionStaticReflectionRuleTest.php | 3 -- .../NoInstanceOfStaticReflectionRuleTest.php | 3 -- .../NoLeadingBackslashInNameRuleTest.php | 3 -- ...pgradeDowngradeRegisteredInSetRuleTest.php | 3 -- ...plementsMinPhpVersionInterfaceRuleTest.php | 3 -- .../RequireAttributeNameRuleTest.php | 3 -- .../SingleArgEventDispatchRuleTest.php | 3 -- .../MultipleRequiredAttributeMethods.php | 18 +++++++ .../Fixture/MultipleRequiredMethods.php | 20 +++++++ .../Fixture/SkipSingleRequiredMethod.php | 13 +++++ .../SingleRequiredMethodRuleTest.php | 35 ++++++++++++ 44 files changed, 318 insertions(+), 138 deletions(-) create mode 100644 src/Enum/DoctrineRuleIdentifier.php create mode 100644 src/Enum/PHPUnitRuleIdentifier.php create mode 100644 src/Enum/RectorRuleIdentifier.php create mode 100644 src/Enum/SymfonyRuleIdentifier.php create mode 100644 src/NodeAnalyzer/SymfonyRequiredMethodAnalyzer.php create mode 100644 src/Rules/Symfony/SingleRequiredMethodRule.php create mode 100644 tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/MultipleRequiredAttributeMethods.php create mode 100644 tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/MultipleRequiredMethods.php create mode 100644 tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/SkipSingleRequiredMethod.php create mode 100644 tests/Rules/Symfony/SingleRequiredMethodRule/SingleRequiredMethodRuleTest.php diff --git a/README.md b/README.md index 0519d4df..0c7a6b17 100644 --- a/README.md +++ b/README.md @@ -794,6 +794,17 @@ class SomeClass extends SomeParentClass
+### SingleRequiredMethodRule + +There must be maximum 1 @required method in the class. Merge it to one to avoid possible injection collision or duplicated injects. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Symfony\SingleRequiredMethodRule +``` + +
+ ### RequireAttributeNameRule Attribute must have all names explicitly defined diff --git a/phpstan.neon b/phpstan.neon index 857e080a..40b14680 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -48,5 +48,4 @@ parameters: - '#Public constant "Symplify\\PHPStanRules\\(.*?)Rule\:\:ERROR_MESSAGE" is never used#' - '#Although PHPStan\\Node\\InClassNode is covered by backward compatibility promise, this instanceof assumption might break because (.*?) not guaranteed to always stay the same#' - - '#PHPStan\\DependencyInjection\\NeonAdapter#' diff --git a/src/Enum/ClassName.php b/src/Enum/ClassName.php index 5a10f870..50e02d10 100644 --- a/src/Enum/ClassName.php +++ b/src/Enum/ClassName.php @@ -80,4 +80,9 @@ final class ClassName * @var string */ public const MOCK_OBJECT_CLASS = 'PHPUnit\Framework\MockObject\MockObject'; + + /** + * @var string + */ + public const REQUIRED_ATTRIBUTE = 'Symfony\Contracts\Service\Attribute\Required'; } diff --git a/src/Enum/DoctrineRuleIdentifier.php b/src/Enum/DoctrineRuleIdentifier.php new file mode 100644 index 00000000..69727615 --- /dev/null +++ b/src/Enum/DoctrineRuleIdentifier.php @@ -0,0 +1,18 @@ +isPublic()) { + return false; + } + + if ($classMethod->isMagic()) { + return false; + } + + foreach ($classMethod->getAttrGroups() as $attributeGroup) { + foreach ($attributeGroup->attrs as $attr) { + if ($attr->name->toString() === ClassName::REQUIRED_ATTRIBUTE) { + return true; + } + } + } + + $docComment = $classMethod->getDocComment(); + if (! $docComment instanceof Doc) { + return false; + } + + return str_contains($docComment->getText(), '@required'); + } +} diff --git a/src/Rules/Doctrine/NoDocumentMockingRule.php b/src/Rules/Doctrine/NoDocumentMockingRule.php index 7b34f823..15af597d 100644 --- a/src/Rules/Doctrine/NoDocumentMockingRule.php +++ b/src/Rules/Doctrine/NoDocumentMockingRule.php @@ -10,7 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\PHPUnitRuleIdentifier; /** * @implements Rule @@ -53,7 +53,7 @@ public function processNode(Node $node, Scope $scope): array } $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::PHPUNIT_NO_DOCUMENT_MOCKING) + ->identifier(PHPUnitRuleIdentifier::NO_DOCUMENT_MOCKING) ->build(); return [$ruleError]; diff --git a/src/Rules/Doctrine/NoEntityMockingRule.php b/src/Rules/Doctrine/NoEntityMockingRule.php index a3611766..2857ad5e 100644 --- a/src/Rules/Doctrine/NoEntityMockingRule.php +++ b/src/Rules/Doctrine/NoEntityMockingRule.php @@ -11,7 +11,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Doctrine\DoctrineEntityDocumentAnalyser; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\DoctrineRuleIdentifier; use Symplify\PHPStanRules\NodeAnalyzer\MethodCallNameAnalyzer; /** @@ -62,7 +62,7 @@ public function processNode(Node $node, Scope $scope): array } $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::NO_ENTITY_MOCKING) + ->identifier(DoctrineRuleIdentifier::NO_ENTITY_MOCKING) ->build(); return [$ruleError]; diff --git a/src/Rules/Doctrine/NoGetRepositoryOutsideServiceRule.php b/src/Rules/Doctrine/NoGetRepositoryOutsideServiceRule.php index a731e598..0158ba54 100644 --- a/src/Rules/Doctrine/NoGetRepositoryOutsideServiceRule.php +++ b/src/Rules/Doctrine/NoGetRepositoryOutsideServiceRule.php @@ -10,7 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\DoctrineRuleIdentifier; /** * @see \Symplify\PHPStanRules\Tests\Rules\Doctrine\NoGetRepositoryOutsideServiceRule\NoGetRepositoryOutsideServiceRuleTest @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array if (! $scope->isInClass()) { $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE) + ->identifier(DoctrineRuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE) ->build(); return [$ruleError]; @@ -57,7 +57,7 @@ public function processNode(Node $node, Scope $scope): array } $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE) + ->identifier(DoctrineRuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE) ->build(); return [$ruleError]; diff --git a/src/Rules/Doctrine/NoParentRepositoryRule.php b/src/Rules/Doctrine/NoParentRepositoryRule.php index ec34f858..fbd1ec7f 100644 --- a/src/Rules/Doctrine/NoParentRepositoryRule.php +++ b/src/Rules/Doctrine/NoParentRepositoryRule.php @@ -11,7 +11,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\DoctrineRuleIdentifier; /** * Check if class extends repository class, @@ -48,7 +48,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::DOCTRINE_NO_PARENT_REPOSITORY) + ->identifier(DoctrineRuleIdentifier::DOCTRINE_NO_PARENT_REPOSITORY) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Doctrine/NoRepositoryCallInDataFixtureRule.php b/src/Rules/Doctrine/NoRepositoryCallInDataFixtureRule.php index 40f811be..1b7b5c4d 100644 --- a/src/Rules/Doctrine/NoRepositoryCallInDataFixtureRule.php +++ b/src/Rules/Doctrine/NoRepositoryCallInDataFixtureRule.php @@ -12,7 +12,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\DoctrineRuleIdentifier; use Symplify\PHPStanRules\Tests\Rules\Doctrine\NoRepositoryCallInDataFixtureRule\NoRepositoryCallInDataFixtureRuleTest; /** @@ -56,7 +56,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::DOCTRINE_NO_REPOSITORY_CALL_IN_DATA_FIXTURES) + ->identifier(DoctrineRuleIdentifier::DOCTRINE_NO_REPOSITORY_CALL_IN_DATA_FIXTURES) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php b/src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php index f8472c86..3f650165 100644 --- a/src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php +++ b/src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php @@ -12,7 +12,7 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\ObjectType; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\DoctrineRuleIdentifier; /** * @implements Rule @@ -53,7 +53,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::REQUIRE_QUERY_BUILDER_ON_REPOSITORY) + ->identifier(DoctrineRuleIdentifier::REQUIRE_QUERY_BUILDER_ON_REPOSITORY) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/PHPUnit/NoMockOnlyTestRule.php b/src/Rules/PHPUnit/NoMockOnlyTestRule.php index facc7989..cedc05b8 100644 --- a/src/Rules/PHPUnit/NoMockOnlyTestRule.php +++ b/src/Rules/PHPUnit/NoMockOnlyTestRule.php @@ -12,7 +12,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\PHPUnitRuleIdentifier; use Symplify\PHPStanRules\Testing\PHPUnitTestAnalyser; /** @@ -69,7 +69,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::PHPUNIT_NO_MOCK_ONLY) + ->identifier(PHPUnitRuleIdentifier::NO_MOCK_ONLY) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/PHPUnit/PublicStaticDataProviderRule.php b/src/Rules/PHPUnit/PublicStaticDataProviderRule.php index aec9c8ff..89ac64a1 100644 --- a/src/Rules/PHPUnit/PublicStaticDataProviderRule.php +++ b/src/Rules/PHPUnit/PublicStaticDataProviderRule.php @@ -10,7 +10,7 @@ use PHPStan\Node\InClassNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\PHPUnitRuleIdentifier; use Symplify\PHPStanRules\PHPUnit\DataProviderMethodResolver; use Symplify\PHPStanRules\Testing\PHPUnitTestAnalyser; @@ -76,7 +76,7 @@ public function processNode(Node $node, Scope $scope): array if (! $dataProviderClassMethod->isPublic()) { $errorMessage = sprintf(self::PUBLIC_ERROR_MESSAGE, $dataProviderMethodName); $ruleErrors[] = RuleErrorBuilder::message($errorMessage) - ->identifier(RuleIdentifier::PHPUNIT_PUBLIC_STATIC_DATA_PROVIDER) + ->identifier(PHPUnitRuleIdentifier::PUBLIC_STATIC_DATA_PROVIDER) ->line($dataProviderClassMethod->getLine()) ->build(); } diff --git a/src/Rules/Rector/NoClassReflectionStaticReflectionRule.php b/src/Rules/Rector/NoClassReflectionStaticReflectionRule.php index 0ed92c54..f6603f52 100644 --- a/src/Rules/Rector/NoClassReflectionStaticReflectionRule.php +++ b/src/Rules/Rector/NoClassReflectionStaticReflectionRule.php @@ -11,7 +11,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use ReflectionClass; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\RectorRuleIdentifier; use Symplify\PHPStanRules\TypeAnalyzer\RectorAllowedAutoloadedTypeAnalyzer; /** @@ -56,7 +56,7 @@ public function processNode(Node $node, Scope $scope): array } return [RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::RECTOR_NO_CLASS_REFLECTION_STATIC_REFLECTION) + ->identifier(RectorRuleIdentifier::NO_CLASS_REFLECTION_STATIC_REFLECTION) ->build()]; } } diff --git a/src/Rules/Rector/NoInstanceOfStaticReflectionRule.php b/src/Rules/Rector/NoInstanceOfStaticReflectionRule.php index 668370cd..97e069ef 100644 --- a/src/Rules/Rector/NoInstanceOfStaticReflectionRule.php +++ b/src/Rules/Rector/NoInstanceOfStaticReflectionRule.php @@ -14,7 +14,7 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\Type; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\RectorRuleIdentifier; use Symplify\PHPStanRules\TypeAnalyzer\RectorAllowedAutoloadedTypeAnalyzer; /** @@ -55,7 +55,7 @@ public function processNode(Node $node, Scope $scope): array } return [RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::RECTOR_NO_INSTANCE_OF_STATIC_REFLECTION) + ->identifier(RectorRuleIdentifier::NO_INSTANCE_OF_STATIC_REFLECTION) ->build()]; } diff --git a/src/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule.php b/src/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule.php index 0bcc9b61..955ae7bc 100644 --- a/src/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule.php +++ b/src/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule.php @@ -15,7 +15,7 @@ use Rector\Set\ValueObject\SetList; use SplFileInfo; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\RectorRuleIdentifier; use Symplify\PHPStanRules\Exception\ShouldNotHappenException; use Symplify\PHPStanRules\FileSystem\FileSystem; @@ -66,7 +66,7 @@ public function processNode(Node $node, Scope $scope): array $errorMessage = $this->createErrorMessage($configFilePath, $className); return [RuleErrorBuilder::message($errorMessage) - ->identifier(RuleIdentifier::RECTOR_UPGRADE_DOWNGRADE_REGISTERED_IN_SET) + ->identifier(RectorRuleIdentifier::UPGRADE_DOWNGRADE_REGISTERED_IN_SET) ->build()]; } diff --git a/src/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule.php b/src/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule.php index c60de44d..0541a7cc 100644 --- a/src/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule.php +++ b/src/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule.php @@ -12,7 +12,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Rector\VersionBonding\Contract\MinPhpVersionInterface; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\RectorRuleIdentifier; /** * @see \Symplify\PHPStanRules\Tests\Rules\Rector\PhpUpgradeImplementsMinPhpVersionInterfaceRule\PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest @@ -66,7 +66,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $className)) - ->identifier(RuleIdentifier::RECTOR_PHP_RULE_IMPLEMENTS_MIN_VERSION) + ->identifier(RectorRuleIdentifier::PHP_RULE_IMPLEMENTS_MIN_VERSION) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Symfony/NoAbstractControllerConstructorRule.php b/src/Rules/Symfony/NoAbstractControllerConstructorRule.php index cfa82021..b30c1be1 100644 --- a/src/Rules/Symfony/NoAbstractControllerConstructorRule.php +++ b/src/Rules/Symfony/NoAbstractControllerConstructorRule.php @@ -10,7 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; /** * Check if abstract controller has constructor, as it should use @@ -55,7 +55,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::SYMFONY_NO_ABSTRACT_CONTROLLER_CONSTRUCTOR) + ->identifier(SymfonyRuleIdentifier::SYMFONY_NO_ABSTRACT_CONTROLLER_CONSTRUCTOR) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Symfony/NoGetDoctrineInControllerRule.php b/src/Rules/Symfony/NoGetDoctrineInControllerRule.php index 0d3c39a9..db594f6c 100644 --- a/src/Rules/Symfony/NoGetDoctrineInControllerRule.php +++ b/src/Rules/Symfony/NoGetDoctrineInControllerRule.php @@ -9,7 +9,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; use Symplify\PHPStanRules\NodeAnalyzer\MethodCallNameAnalyzer; use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyControllerAnalyzer; @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) ->file($scope->getFile()) ->line($node->getStartLine()) - ->identifier(RuleIdentifier::NO_GET_DOCTRINE_IN_CONTROLLER) + ->identifier(SymfonyRuleIdentifier::NO_GET_DOCTRINE_IN_CONTROLLER) ->build(); return [$ruleError]; diff --git a/src/Rules/Symfony/NoGetInControllerRule.php b/src/Rules/Symfony/NoGetInControllerRule.php index f6bef823..a07375b4 100644 --- a/src/Rules/Symfony/NoGetInControllerRule.php +++ b/src/Rules/Symfony/NoGetInControllerRule.php @@ -9,7 +9,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; use Symplify\PHPStanRules\NodeAnalyzer\MethodCallNameAnalyzer; use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyControllerAnalyzer; @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) ->file($scope->getFile()) ->line($node->getStartLine()) - ->identifier(RuleIdentifier::NO_GET_IN_CONTROLLER) + ->identifier(SymfonyRuleIdentifier::NO_GET_IN_CONTROLLER) ->build(); return [$ruleError]; diff --git a/src/Rules/Symfony/NoListenerWithoutContractRule.php b/src/Rules/Symfony/NoListenerWithoutContractRule.php index c8b7cfa5..a413c9fe 100644 --- a/src/Rules/Symfony/NoListenerWithoutContractRule.php +++ b/src/Rules/Symfony/NoListenerWithoutContractRule.php @@ -10,7 +10,7 @@ use PHPStan\Node\InClassNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; /** * Based on https://tomasvotruba.com/blog/2019/07/22/how-to-convert-listeners-to-subscribers-and-reduce-your-configs @@ -54,7 +54,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::NO_LISTENER_WITHOUT_CONTRACT) + ->identifier(SymfonyRuleIdentifier::NO_LISTENER_WITHOUT_CONTRACT) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Symfony/NoRequiredOutsideClassRule.php b/src/Rules/Symfony/NoRequiredOutsideClassRule.php index f322f0ee..088b22f1 100644 --- a/src/Rules/Symfony/NoRequiredOutsideClassRule.php +++ b/src/Rules/Symfony/NoRequiredOutsideClassRule.php @@ -4,14 +4,13 @@ namespace Symplify\PHPStanRules\Rules\Symfony; -use PhpParser\Comment\Doc; use PhpParser\Node; -use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Trait_; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; +use Symplify\PHPStanRules\NodeAnalyzer\SymfonyRequiredMethodAnalyzer; /** * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\NoRequiredOutsideClassRule\NoRequiredOutsideClassRuleTest @@ -25,11 +24,6 @@ final class NoRequiredOutsideClassRule implements Rule */ public const ERROR_MESSAGE = 'Symfony #[Require]/@required should be used only in classes to avoid misuse'; - /** - * @var string - */ - private const REQUIRED_ATTRIBUTE = 'Symfony\Contracts\Service\Attribute\Required'; - public function getNodeType(): string { return Trait_::class; @@ -43,29 +37,17 @@ public function processNode(Node $node, Scope $scope): array $ruleErrors = []; foreach ($node->getMethods() as $classMethod) { - if ($this->isAutowiredClassMethod($classMethod)) { - $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->file($scope->getFile()) - ->identifier(RuleIdentifier::SYMFONY_NO_REQUIRED_OUTSIDE_CLASS) - ->line($classMethod->getLine()) - ->build(); + if (! SymfonyRequiredMethodAnalyzer::detect($classMethod)) { + continue; } - } - return $ruleErrors; - } - - private function isAutowiredClassMethod(ClassMethod $classMethod): bool - { - foreach ($classMethod->getAttrGroups() as $attributeGroup) { - foreach ($attributeGroup->attrs as $attr) { - if ($attr->name->toString() === self::REQUIRED_ATTRIBUTE) { - return true; - } - } + $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->file($scope->getFile()) + ->identifier(SymfonyRuleIdentifier::SYMFONY_NO_REQUIRED_OUTSIDE_CLASS) + ->line($classMethod->getLine()) + ->build(); } - $docComment = $classMethod->getDocComment(); - return $docComment instanceof Doc && str_contains($docComment->getText(), '@required'); + return $ruleErrors; } } diff --git a/src/Rules/Symfony/NoStringInGetSubscribedEventsRule.php b/src/Rules/Symfony/NoStringInGetSubscribedEventsRule.php index 2ae635fd..d90a93c6 100644 --- a/src/Rules/Symfony/NoStringInGetSubscribedEventsRule.php +++ b/src/Rules/Symfony/NoStringInGetSubscribedEventsRule.php @@ -15,7 +15,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; /** * @implements Rule @@ -90,7 +90,7 @@ public function processNode(Node $node, Scope $scope): array } $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::NO_STRING_IN_GET_SUBSCRIBED_EVENTS) + ->identifier(SymfonyRuleIdentifier::NO_STRING_IN_GET_SUBSCRIBED_EVENTS) ->build(); return [$ruleError]; diff --git a/src/Rules/Symfony/RequireInvokableControllerRule.php b/src/Rules/Symfony/RequireInvokableControllerRule.php index 2e0121c1..79f434c4 100644 --- a/src/Rules/Symfony/RequireInvokableControllerRule.php +++ b/src/Rules/Symfony/RequireInvokableControllerRule.php @@ -11,7 +11,7 @@ use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\ClassName; use Symplify\PHPStanRules\Enum\MethodName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyControllerAnalyzer; /** @@ -63,7 +63,7 @@ public function processNode(Node $node, Scope $scope): array } $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::SYMFONY_REQUIRE_INVOKABLE_CONTROLLER) + ->identifier(SymfonyRuleIdentifier::SYMFONY_REQUIRE_INVOKABLE_CONTROLLER) ->line($classMethod->getLine()) ->build(); } diff --git a/src/Rules/Symfony/SingleArgEventDispatchRule.php b/src/Rules/Symfony/SingleArgEventDispatchRule.php index fdceba3d..b257fd85 100644 --- a/src/Rules/Symfony/SingleArgEventDispatchRule.php +++ b/src/Rules/Symfony/SingleArgEventDispatchRule.php @@ -12,7 +12,7 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\ObjectType; use Symplify\PHPStanRules\Enum\ClassName; -use Symplify\PHPStanRules\Enum\RuleIdentifier; +use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier; /** * @implements Rule @@ -57,7 +57,7 @@ public function processNode(Node $node, Scope $scope): array } $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(RuleIdentifier::SINGLE_ARG_EVENT_DISPATCH) + ->identifier(SymfonyRuleIdentifier::SINGLE_ARG_EVENT_DISPATCH) ->build(); return [$identifierRuleError]; diff --git a/src/Rules/Symfony/SingleRequiredMethodRule.php b/src/Rules/Symfony/SingleRequiredMethodRule.php new file mode 100644 index 00000000..6818cf8e --- /dev/null +++ b/src/Rules/Symfony/SingleRequiredMethodRule.php @@ -0,0 +1,54 @@ + + */ +final class SingleRequiredMethodRule implements Rule +{ + public const ERROR_MESSAGE = 'Found %d @required methods. Use only one method to avoid unexpected behavior.'; + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + * @return RuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + $requiredClassMethodCount = 0; + + foreach ($node->getMethods() as $classMethod) { + if (! SymfonyRequiredMethodAnalyzer::detect($classMethod)) { + continue; + } + + ++$requiredClassMethodCount; + } + + if ($requiredClassMethodCount < 2) { + return []; + } + + $errorMessage = sprintf(self::ERROR_MESSAGE, $requiredClassMethodCount); + + $identifierRuleError = RuleErrorBuilder::message($errorMessage) + ->identifier(SymfonyRuleIdentifier::SINGLE_REQUIRED_METHOD) + ->build(); + + return [$identifierRuleError]; + } +} diff --git a/tests/Rules/CheckRequiredInterfaceInContractNamespaceRule/CheckRequiredInterfaceInContractNamespaceRuleTest.php b/tests/Rules/CheckRequiredInterfaceInContractNamespaceRule/CheckRequiredInterfaceInContractNamespaceRuleTest.php index ed027667..11df6dc7 100644 --- a/tests/Rules/CheckRequiredInterfaceInContractNamespaceRule/CheckRequiredInterfaceInContractNamespaceRuleTest.php +++ b/tests/Rules/CheckRequiredInterfaceInContractNamespaceRule/CheckRequiredInterfaceInContractNamespaceRuleTest.php @@ -12,9 +12,6 @@ final class CheckRequiredInterfaceInContractNamespaceRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Explicit/ExplicitClassPrefixSuffixRule/ExplicitClassPrefixSuffixRuleTest.php b/tests/Rules/Explicit/ExplicitClassPrefixSuffixRule/ExplicitClassPrefixSuffixRuleTest.php index 92f581bc..c4c6752b 100644 --- a/tests/Rules/Explicit/ExplicitClassPrefixSuffixRule/ExplicitClassPrefixSuffixRuleTest.php +++ b/tests/Rules/Explicit/ExplicitClassPrefixSuffixRule/ExplicitClassPrefixSuffixRuleTest.php @@ -12,9 +12,6 @@ final class ExplicitClassPrefixSuffixRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/NoReturnSetterMethodRule/NoReturnSetterMethodRuleTest.php b/tests/Rules/NoReturnSetterMethodRule/NoReturnSetterMethodRuleTest.php index f4e2c187..fa921b86 100644 --- a/tests/Rules/NoReturnSetterMethodRule/NoReturnSetterMethodRuleTest.php +++ b/tests/Rules/NoReturnSetterMethodRule/NoReturnSetterMethodRuleTest.php @@ -12,9 +12,6 @@ final class NoReturnSetterMethodRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Rector/NoClassReflectionStaticReflectionRule/NoClassReflectionStaticReflectionRuleTest.php b/tests/Rules/Rector/NoClassReflectionStaticReflectionRule/NoClassReflectionStaticReflectionRuleTest.php index 51aaa172..5c2a3ea8 100644 --- a/tests/Rules/Rector/NoClassReflectionStaticReflectionRule/NoClassReflectionStaticReflectionRuleTest.php +++ b/tests/Rules/Rector/NoClassReflectionStaticReflectionRule/NoClassReflectionStaticReflectionRuleTest.php @@ -12,9 +12,6 @@ final class NoClassReflectionStaticReflectionRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Rector/NoInstanceOfStaticReflectionRule/NoInstanceOfStaticReflectionRuleTest.php b/tests/Rules/Rector/NoInstanceOfStaticReflectionRule/NoInstanceOfStaticReflectionRuleTest.php index faf88c76..a8ce9813 100644 --- a/tests/Rules/Rector/NoInstanceOfStaticReflectionRule/NoInstanceOfStaticReflectionRuleTest.php +++ b/tests/Rules/Rector/NoInstanceOfStaticReflectionRule/NoInstanceOfStaticReflectionRuleTest.php @@ -12,9 +12,6 @@ final class NoInstanceOfStaticReflectionRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Rector/NoLeadingBackslashInNameRule/NoLeadingBackslashInNameRuleTest.php b/tests/Rules/Rector/NoLeadingBackslashInNameRule/NoLeadingBackslashInNameRuleTest.php index 582c956d..592f0422 100644 --- a/tests/Rules/Rector/NoLeadingBackslashInNameRule/NoLeadingBackslashInNameRuleTest.php +++ b/tests/Rules/Rector/NoLeadingBackslashInNameRule/NoLeadingBackslashInNameRuleTest.php @@ -12,9 +12,6 @@ final class NoLeadingBackslashInNameRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule/PhpUpgradeDowngradeRegisteredInSetRuleTest.php b/tests/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule/PhpUpgradeDowngradeRegisteredInSetRuleTest.php index 1edbb403..fdec8a2d 100644 --- a/tests/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule/PhpUpgradeDowngradeRegisteredInSetRuleTest.php +++ b/tests/Rules/Rector/PhpUpgradeDowngradeRegisteredInSetRule/PhpUpgradeDowngradeRegisteredInSetRuleTest.php @@ -14,9 +14,6 @@ final class PhpUpgradeDowngradeRegisteredInSetRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule/PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest.php b/tests/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule/PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest.php index 0e1c8c38..a9c5aafc 100644 --- a/tests/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule/PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest.php +++ b/tests/Rules/Rector/PhpUpgradeImplementsMinPhpVersionInterfaceRule/PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest.php @@ -12,9 +12,6 @@ final class PhpUpgradeImplementsMinPhpVersionInterfaceRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/RequireAttributeNameRule/RequireAttributeNameRuleTest.php b/tests/Rules/RequireAttributeNameRule/RequireAttributeNameRuleTest.php index e1aac220..27cd36e5 100644 --- a/tests/Rules/RequireAttributeNameRule/RequireAttributeNameRuleTest.php +++ b/tests/Rules/RequireAttributeNameRule/RequireAttributeNameRuleTest.php @@ -12,9 +12,6 @@ final class RequireAttributeNameRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Symfony/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php b/tests/Rules/Symfony/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php index e65c0697..485f7b9c 100644 --- a/tests/Rules/Symfony/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php +++ b/tests/Rules/Symfony/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php @@ -12,9 +12,6 @@ final class SingleArgEventDispatchRuleTest extends RuleTestCase { - /** - * @param mixed[] $expectedErrorsWithLines - */ #[DataProvider('provideData')] public function testRule(string $filePath, array $expectedErrorsWithLines): void { diff --git a/tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/MultipleRequiredAttributeMethods.php b/tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/MultipleRequiredAttributeMethods.php new file mode 100644 index 00000000..e1085896 --- /dev/null +++ b/tests/Rules/Symfony/SingleRequiredMethodRule/Fixture/MultipleRequiredAttributeMethods.php @@ -0,0 +1,18 @@ +analyse([$filePath], $expectedErrorsWithLines); + } + + public static function provideData(): Iterator + { + $expectedErrorMessage = sprintf(SingleRequiredMethodRule::ERROR_MESSAGE, 2); + + yield [__DIR__ . '/Fixture/MultipleRequiredMethods.php', [[$expectedErrorMessage, 5]]]; + yield [__DIR__ . '/Fixture/MultipleRequiredAttributeMethods.php', [[$expectedErrorMessage, 7]]]; + + yield [__DIR__ . '/Fixture/SkipSingleRequiredMethod.php', []]; + } + + protected function getRule(): Rule + { + return new SingleRequiredMethodRule(); + } +}