From 55ec7695e9a691a3dbbb27685adf1d011e75e7c9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 6 Nov 2024 15:39:54 +0100 Subject: [PATCH 1/4] [command] add downgrade command --- src/Command/DowngradeCommand.php | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/Command/DowngradeCommand.php diff --git a/src/Command/DowngradeCommand.php b/src/Command/DowngradeCommand.php new file mode 100644 index 0000000..4267688 --- /dev/null +++ b/src/Command/DowngradeCommand.php @@ -0,0 +1,38 @@ +setName('downgrade'); + $this->setDescription( + 'Prepare setup for downgrading package with Rector on release - rector.php config, Scoper and Github Workflow' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + // @todo + // @todo copy prepared workflow + // @todo create /build with config + // @todo update composer.json with no PHP version check + + return self::SUCCESS; + } +} From 65dbf5e66a4da17404c03fb4ad266f238a471c57 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 3 Dec 2024 11:03:29 +0100 Subject: [PATCH 2/4] move couple handy rules here --- src/Enum/ClassName.php | 13 +++ .../Rule/NoConstructorOverrideRule.php | 97 +++++++++++++++++++ src/PHPStan/Rule/NoDocumentMockingRule.php | 56 +++++++++++ .../Rule/NoListenerWithoutContractRule.php | 61 ++++++++++++ .../NoStringInGetSubscribedEventsRule.php | 88 +++++++++++++++++ .../Rule/SingleArgEventDispatchRule.php | 60 ++++++++++++ .../EventDispatcherInterface.php | 13 +++ .../Fixture/ReportEventDispatcher.php | 13 +++ .../Fixture/SkipSingleDispatch.php | 13 +++ .../Fixture/SkipUnrelatedDispatch.php | 14 +++ .../SingleArgEventDispatchRuleTest.php | 39 ++++++++ .../Source/NotEventDispatcher.php | 11 +++ 12 files changed, 478 insertions(+) create mode 100644 src/Enum/ClassName.php create mode 100644 src/PHPStan/Rule/NoConstructorOverrideRule.php create mode 100644 src/PHPStan/Rule/NoDocumentMockingRule.php create mode 100644 src/PHPStan/Rule/NoListenerWithoutContractRule.php create mode 100644 src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php create mode 100644 src/PHPStan/Rule/SingleArgEventDispatchRule.php create mode 100644 stubs/Symfony/Component/EventDispatcher/EventDispatcherInterface.php create mode 100644 tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/ReportEventDispatcher.php create mode 100644 tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipSingleDispatch.php create mode 100644 tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php create mode 100644 tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php create mode 100644 tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php diff --git a/src/Enum/ClassName.php b/src/Enum/ClassName.php new file mode 100644 index 0000000..9fcc077 --- /dev/null +++ b/src/Enum/ClassName.php @@ -0,0 +1,13 @@ + + */ +final class NoConstructorOverrideRule implements Rule +{ + /** + * @var string + */ + public const ERROR_MESSAGE = 'Possible __construct() override, this can cause missing dependencies or setup'; + + /** + * @var string + */ + private const CONSTRUCTOR_NAME = '__construct'; + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * @return RuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->name->toLowerString() !== self::CONSTRUCTOR_NAME) { + return []; + } + + // has parent constructor call? + if (! $scope->isInClass()) { + return []; + } + + // parent has no cunstructor, we can skip it + $classReflection = $scope->getClassReflection(); + if ($classReflection->isAnonymous()) { + return []; + } + + $parentClassReflection = $classReflection->getParentClass(); + + // no parent class? let it go + if (! $parentClassReflection instanceof ClassReflection) { + return []; + } + + if (! $parentClassReflection->hasConstructor()) { + return []; + } + + $nodeFinder = new NodeFinder(); + $parentConstructorStaticCall = $nodeFinder->findFirst($node->stmts, function (Node $node): bool { + if (! $node instanceof StaticCall) { + return false; + } + + if (! $node->class instanceof Name) { + return false; + } + + if (! $node->name instanceof Identifier) { + return false; + } + + return $node->name->toString() === '__construct'; + }); + + if ($parentConstructorStaticCall instanceof StaticCall) { + return []; + } + + $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->build(); + + return [$ruleError]; + } +} diff --git a/src/PHPStan/Rule/NoDocumentMockingRule.php b/src/PHPStan/Rule/NoDocumentMockingRule.php new file mode 100644 index 0000000..3a6a276 --- /dev/null +++ b/src/PHPStan/Rule/NoDocumentMockingRule.php @@ -0,0 +1,56 @@ +isFirstClassCallable()) { + return []; + } + + if (! $node->name instanceof Identifier) { + return []; + } + + $methodName = $node->name->toString(); + if ($methodName !== 'createMock') { + return []; + } + + $firstArg = $node->getArgs()[0]; + $mockedClassType = $scope->getType($firstArg->value); + foreach ($mockedClassType->getConstantStrings() as $constantString) { + if (! str_contains($constantString->getValue(), '\\Document\\')) { + continue; + } + + return [self::ERROR_MESSAGE]; + } + + return []; + } +} diff --git a/src/PHPStan/Rule/NoListenerWithoutContractRule.php b/src/PHPStan/Rule/NoListenerWithoutContractRule.php new file mode 100644 index 0000000..1bb2910 --- /dev/null +++ b/src/PHPStan/Rule/NoListenerWithoutContractRule.php @@ -0,0 +1,61 @@ + + */ +final class NoListenerWithoutContractRule implements Rule +{ + /** + * @var string + */ + public const ERROR_MESSAGE = 'There should be no listeners defined in yaml config, use contract + PHP instead'; + + public function getNodeType(): string + { + return InClassNode::class; + } + + /** + * @param InClassNode $node + * @return RuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + if (! $scope->isInClass()) { + return []; + } + + $classReflection = $scope->getClassReflection(); + if (! str_ends_with($classReflection->getName(), 'Listener')) { + return []; + } + + $class = $node->getOriginalNode(); + if (! $class instanceof Class_) { + return []; + } + + if ($class->implements !== []) { + return []; + } + + $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)->build(); + + return [$ruleError]; + } +} diff --git a/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php b/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php new file mode 100644 index 0000000..4099ece --- /dev/null +++ b/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php @@ -0,0 +1,88 @@ + + */ +final class NoStringInGetSubscribedEventsRule implements Rule +{ + /** + * @var string + */ + private const EVENT_SUBSCRIBER_INTERFACE = 'Symfony\Component\EventDispatcher\EventSubscriberInterface'; + + /** + * @var string + */ + private const ERROR_MESSAGE = 'Symfony getSubscribedEvents() method must contain only event class references, no strings'; + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * @return RuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->name->toString() !== 'getSubscribedEvents') { + return []; + } + + $classReflection = $scope->getClassReflection(); + + // only handle symfony one + if (! $classReflection->implementsInterface(self::EVENT_SUBSCRIBER_INTERFACE)) { + return []; + } + + $nodeFinder = new NodeFinder(); + + /** @var ArrayItem[] $arrayItems */ + $arrayItems = $nodeFinder->findInstanceOf($node->stmts, ArrayItem::class); + + foreach ($arrayItems as $arrayItem) { + if (! $arrayItem->key instanceof Expr) { + continue; + } + + // must be class const fetch + if ($arrayItem->key instanceof ClassConstFetch) { + $classConstFetch = $arrayItem->key; + + // skip Symfony FormEvents::class + if ($classConstFetch->class->toString() === FormEvents::class) { + continue; + } + + if ($classConstFetch->name->toString() === 'class') { + continue; + } + + continue; + } + + $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)->build(); + return [$ruleError]; + } + + return []; + } +} diff --git a/src/PHPStan/Rule/SingleArgEventDispatchRule.php b/src/PHPStan/Rule/SingleArgEventDispatchRule.php new file mode 100644 index 0000000..71a46f4 --- /dev/null +++ b/src/PHPStan/Rule/SingleArgEventDispatchRule.php @@ -0,0 +1,60 @@ +name instanceof Identifier) { + return []; + } + + if ($node->name->toString() !== 'dispatch') { + return []; + } + + // all good + if (count($node->getArgs()) === 1) { + return []; + } + + $callerType = $scope->getType($node->var); + if (! $callerType instanceof ObjectType) { + return []; + } + + if (! $callerType->isInstanceOf(ClassName::EVENT_DISPATCHER_INTERFACE)->yes()) { + return []; + } + + $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)->build(); + return [$ruleError]; + } +} diff --git a/stubs/Symfony/Component/EventDispatcher/EventDispatcherInterface.php b/stubs/Symfony/Component/EventDispatcher/EventDispatcherInterface.php new file mode 100644 index 0000000..790a051 --- /dev/null +++ b/stubs/Symfony/Component/EventDispatcher/EventDispatcherInterface.php @@ -0,0 +1,13 @@ +dispatch('one', 'two'); + } +} diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipSingleDispatch.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipSingleDispatch.php new file mode 100644 index 0000000..e32b96f --- /dev/null +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipSingleDispatch.php @@ -0,0 +1,13 @@ +dispatch('one'); + } +} diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php new file mode 100644 index 0000000..8b8fbce --- /dev/null +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php @@ -0,0 +1,14 @@ +dispatch('one', 'two'); + } +} diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php new file mode 100644 index 0000000..0e087e1 --- /dev/null +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php @@ -0,0 +1,39 @@ +analyse([$filePath], $expectedErrorsWithLines); + } + + public static function provideData(): Iterator + { + yield [ + __DIR__ . '/Fixture/ReportEventDispatcher.php', + [[SingleArgEventDispatchRule::ERROR_MESSAGE, 11]], + ]; + + yield [__DIR__ . '/Fixture/SkipSingleDispatch.php', []]; + yield [__DIR__ . '/Fixture/SkipUnrelatedDispatch.php', []]; + } + + protected function getRule(): Rule + { + return new SingleArgEventDispatchRule(); + } +} diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php new file mode 100644 index 0000000..51e953f --- /dev/null +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php @@ -0,0 +1,11 @@ + Date: Tue, 3 Dec 2024 11:06:11 +0100 Subject: [PATCH 3/4] static fixes --- src/Command/DowngradeCommand.php | 2 ++ src/Enum/ClassName.php | 5 +++++ .../Rule/NoConstructorOverrideRule.php | 4 ++++ src/PHPStan/Rule/NoDocumentMockingRule.php | 3 +++ .../NoStringInGetSubscribedEventsRule.php | 20 +++++++++++++++++-- .../Rule/SingleArgEventDispatchRule.php | 3 +++ .../Fixture/SkipUnrelatedDispatch.php | 1 - .../SingleArgEventDispatchRuleTest.php | 5 +---- 8 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/Command/DowngradeCommand.php b/src/Command/DowngradeCommand.php index 4267688..055ff79 100644 --- a/src/Command/DowngradeCommand.php +++ b/src/Command/DowngradeCommand.php @@ -33,6 +33,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int // @todo create /build with config // @todo update composer.json with no PHP version check + $this->symfonyStyle->success('Done'); + return self::SUCCESS; } } diff --git a/src/Enum/ClassName.php b/src/Enum/ClassName.php index 9fcc077..ba3782f 100644 --- a/src/Enum/ClassName.php +++ b/src/Enum/ClassName.php @@ -10,4 +10,9 @@ final class ClassName * @var string */ public const EVENT_DISPATCHER_INTERFACE = 'Symfony\Component\EventDispatcher\EventDispatcherInterface'; + + /** + * @var string + */ + public const FORM_EVENTS = 'Symfony\Component\Form\FormEvents'; } diff --git a/src/PHPStan/Rule/NoConstructorOverrideRule.php b/src/PHPStan/Rule/NoConstructorOverrideRule.php index 57ffbee..9e1691a 100644 --- a/src/PHPStan/Rule/NoConstructorOverrideRule.php +++ b/src/PHPStan/Rule/NoConstructorOverrideRule.php @@ -46,6 +46,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->stmts === null) { + return []; + } + // has parent constructor call? if (! $scope->isInClass()) { return []; diff --git a/src/PHPStan/Rule/NoDocumentMockingRule.php b/src/PHPStan/Rule/NoDocumentMockingRule.php index 3a6a276..65ecfbe 100644 --- a/src/PHPStan/Rule/NoDocumentMockingRule.php +++ b/src/PHPStan/Rule/NoDocumentMockingRule.php @@ -10,6 +10,9 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; +/** + * @implements Rule + */ final class NoDocumentMockingRule implements Rule { /** diff --git a/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php b/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php index 4099ece..05cbfef 100644 --- a/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php +++ b/src/PHPStan/Rule/NoStringInGetSubscribedEventsRule.php @@ -11,10 +11,11 @@ use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeFinder; use PHPStan\Analyser\Scope; +use PHPStan\Reflection\ClassReflection; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; -use Symfony\Component\Form\FormEvents; +use TomasVotruba\Handyman\Enum\ClassName; /** * @implements Rule @@ -42,11 +43,18 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { + if ($node->stmts === null) { + return []; + } + if ($node->name->toString() !== 'getSubscribedEvents') { return []; } $classReflection = $scope->getClassReflection(); + if (! $classReflection instanceof ClassReflection) { + return []; + } // only handle symfony one if (! $classReflection->implementsInterface(self::EVENT_SUBSCRIBER_INTERFACE)) { @@ -67,8 +75,16 @@ public function processNode(Node $node, Scope $scope): array if ($arrayItem->key instanceof ClassConstFetch) { $classConstFetch = $arrayItem->key; + if ($classConstFetch->class instanceof Expr) { + continue; + } + // skip Symfony FormEvents::class - if ($classConstFetch->class->toString() === FormEvents::class) { + if ($classConstFetch->class->toString() === ClassName::FORM_EVENTS) { + continue; + } + + if ($classConstFetch->name instanceof Expr) { continue; } diff --git a/src/PHPStan/Rule/SingleArgEventDispatchRule.php b/src/PHPStan/Rule/SingleArgEventDispatchRule.php index 71a46f4..b1f2868 100644 --- a/src/PHPStan/Rule/SingleArgEventDispatchRule.php +++ b/src/PHPStan/Rule/SingleArgEventDispatchRule.php @@ -14,6 +14,9 @@ use PHPStan\Type\ObjectType; use TomasVotruba\Handyman\Enum\ClassName; +/** + * @implements Rule + */ final class SingleArgEventDispatchRule implements Rule { /** diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php index 8b8fbce..849878d 100644 --- a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Fixture/SkipUnrelatedDispatch.php @@ -2,7 +2,6 @@ namespace TomasVotruba\Handyman\Tests\PHPStan\Rule\SingleArgEventDispatchRule\Fixture; -use Symfony\Component\EventDispatcher\EventDispatcherInterface; use TomasVotruba\Handyman\Tests\PHPStan\Rule\SingleArgEventDispatchRule\Source\NotEventDispatcher; final class SkipUnrelatedDispatch diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php index 0e087e1..62ed6ee 100644 --- a/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/SingleArgEventDispatchRuleTest.php @@ -23,10 +23,7 @@ public function testRule(string $filePath, array $expectedErrorsWithLines): void public static function provideData(): Iterator { - yield [ - __DIR__ . '/Fixture/ReportEventDispatcher.php', - [[SingleArgEventDispatchRule::ERROR_MESSAGE, 11]], - ]; + yield [__DIR__ . '/Fixture/ReportEventDispatcher.php', [[SingleArgEventDispatchRule::ERROR_MESSAGE, 11]]]; yield [__DIR__ . '/Fixture/SkipSingleDispatch.php', []]; yield [__DIR__ . '/Fixture/SkipUnrelatedDispatch.php', []]; From 912fc403d3eb9716e525a4feca10f352e66f1177 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 3 Dec 2024 11:06:46 +0100 Subject: [PATCH 4/4] misc --- .../SingleArgEventDispatchRule/Source/NotEventDispatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php index 51e953f..1cba5c4 100644 --- a/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php +++ b/tests/PHPStan/Rule/SingleArgEventDispatchRule/Source/NotEventDispatcher.php @@ -2,7 +2,7 @@ namespace TomasVotruba\Handyman\Tests\PHPStan\Rule\SingleArgEventDispatchRule\Source; -class NotEventDispatcher +final class NotEventDispatcher { public function dispatch() {