From 1ba473cb8e9e4da15b6253b33c3ed039c9ea4344 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 15:20:33 +0100 Subject: [PATCH 1/7] Require PHPStan and extensions --- composer.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ccb9b5da..0f88442b 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,11 @@ "require-dev": { "symfony/mime": "^5.4 || ^6.0 || ^7.0", "symfony/phpunit-bridge": "^v6.4.1 || ^7.0.1", - "symfony/security-core": "^5.4 || ^6.0 || ^7.0" + "symfony/security-core": "^5.4 || ^6.0 || ^7.0", + "phpstan/phpstan": "^1.10", + "phpstan/phpstan-deprecation-rules": "^1.1", + "phpstan/phpstan-phpunit": "^1.3", + "phpstan/phpstan-symfony": "^1.3" }, "suggest": { "doctrine/doctrine-bundle": "to use the ORM extensions", From d0661a297c5a13d45ed9de4e3818c63bf6fe0c8b Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Mon, 11 Dec 2023 22:49:52 +0100 Subject: [PATCH 2/7] Sort packages in composer.json --- composer.json | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 0f88442b..f2d4a1a4 100644 --- a/composer.json +++ b/composer.json @@ -21,13 +21,13 @@ "gedmo/doctrine-extensions": "^3.5.0" }, "require-dev": { - "symfony/mime": "^5.4 || ^6.0 || ^7.0", - "symfony/phpunit-bridge": "^v6.4.1 || ^7.0.1", - "symfony/security-core": "^5.4 || ^6.0 || ^7.0", "phpstan/phpstan": "^1.10", "phpstan/phpstan-deprecation-rules": "^1.1", "phpstan/phpstan-phpunit": "^1.3", - "phpstan/phpstan-symfony": "^1.3" + "phpstan/phpstan-symfony": "^1.3", + "symfony/mime": "^5.4 || ^6.0 || ^7.0", + "symfony/phpunit-bridge": "^v6.4.1 || ^7.0.1", + "symfony/security-core": "^5.4 || ^6.0 || ^7.0" }, "suggest": { "doctrine/doctrine-bundle": "to use the ORM extensions", @@ -44,5 +44,8 @@ "branch-alias": { "dev-main": "1.x-dev" } + }, + "config": { + "sort-packages": true } } From 5e1d9b0715f35d44074f74632a3c22712542942f Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 17:29:44 +0100 Subject: [PATCH 3/7] Add PHPStan configuration at level 5 --- phpstan.neon | 12 ++++++++++++ .../Compiler/ValidateExtensionConfigurationPass.php | 6 +++++- 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 phpstan.neon diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 00000000..c2da4098 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,12 @@ +parameters: + level: 5 + paths: + - src/ + - tests/ + +includes: + - vendor/phpstan/phpstan-deprecation-rules/rules.neon + - vendor/phpstan/phpstan-phpunit/extension.neon + - vendor/phpstan/phpstan-phpunit/rules.neon + - vendor/phpstan/phpstan-symfony/extension.neon + - vendor/phpstan/phpstan-symfony/rules.neon diff --git a/src/DependencyInjection/Compiler/ValidateExtensionConfigurationPass.php b/src/DependencyInjection/Compiler/ValidateExtensionConfigurationPass.php index eea7bf5a..2e75a18b 100644 --- a/src/DependencyInjection/Compiler/ValidateExtensionConfigurationPass.php +++ b/src/DependencyInjection/Compiler/ValidateExtensionConfigurationPass.php @@ -2,6 +2,7 @@ namespace Stof\DoctrineExtensionsBundle\DependencyInjection\Compiler; +use Stof\DoctrineExtensionsBundle\DependencyInjection\StofDoctrineExtensionsExtension; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -23,6 +24,9 @@ class ValidateExtensionConfigurationPass implements CompilerPassInterface */ public function process(ContainerBuilder $container) { - $container->getExtension('stof_doctrine_extensions')->configValidate($container); + $extension = $container->getExtension('stof_doctrine_extensions'); + \assert($extension instanceof StofDoctrineExtensionsExtension); + + $extension->configValidate($container); } } From 9b44b740bafe54b0ed9b08df32b3155b91d4a34a Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 18:18:37 +0100 Subject: [PATCH 4/7] Move to PHPStan level 6 --- phpstan.neon | 2 +- .../StofDoctrineExtensionsExtension.php | 16 +++++++++------- src/EventListener/BlameListener.php | 5 ++++- src/EventListener/IpTraceListener.php | 3 ++- src/EventListener/LocaleListener.php | 3 ++- src/EventListener/LoggerListener.php | 11 ++++++++++- src/Uploadable/UploadableManager.php | 8 ++++++++ src/Uploadable/UploadedFileInfo.php | 3 ++- src/Uploadable/ValidatorConfigurator.php | 3 ++- .../StofDoctrineExtensionsExtensionTest.php | 11 +++++++---- 10 files changed, 47 insertions(+), 18 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index c2da4098..d58e42b9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 5 + level: 6 paths: - src/ - tests/ diff --git a/src/DependencyInjection/StofDoctrineExtensionsExtension.php b/src/DependencyInjection/StofDoctrineExtensionsExtension.php index fff3f362..278314e5 100644 --- a/src/DependencyInjection/StofDoctrineExtensionsExtension.php +++ b/src/DependencyInjection/StofDoctrineExtensionsExtension.php @@ -81,7 +81,9 @@ class StofDoctrineExtensionsExtension extends Extension ), ); + /** @var list */ private $entityManagers = array(); + /** @var list */ private $documentManagers = array(); /** @@ -150,7 +152,7 @@ public function load(array $configs, ContainerBuilder $container) /** * @internal */ - public function configValidate(ContainerBuilder $container) + public function configValidate(ContainerBuilder $container): void { foreach ($this->entityManagers as $name) { if (!$container->hasDefinition(sprintf('doctrine.dbal.%s_connection', $name))) { @@ -166,13 +168,13 @@ public function configValidate(ContainerBuilder $container) } /** - * @param array $configs - * @param ContainerBuilder $container - * @param LoaderInterface $loader - * @param array $loaded - * @param string $doctrineListenerTag + * @param array> $configs + * @param ContainerBuilder $container + * @param LoaderInterface $loader + * @param array $loaded + * @param string $doctrineListenerTag * - * @return array + * @return list */ private function processObjectManagerConfigurations(array $configs, ContainerBuilder $container, LoaderInterface $loader, array &$loaded, string $doctrineListenerTag) { diff --git a/src/EventListener/BlameListener.php b/src/EventListener/BlameListener.php index 31e1933d..0abd399c 100644 --- a/src/EventListener/BlameListener.php +++ b/src/EventListener/BlameListener.php @@ -18,8 +18,11 @@ */ class BlameListener implements EventSubscriberInterface { + /** @var AuthorizationCheckerInterface|null */ private $authorizationChecker; + /** @var TokenStorageInterface|null */ private $tokenStorage; + /** @var BlameableListener */ private $blameableListener; public function __construct(BlameableListener $blameableListener, TokenStorageInterface $tokenStorage = null, AuthorizationCheckerInterface $authorizationChecker = null) @@ -32,7 +35,7 @@ public function __construct(BlameableListener $blameableListener, TokenStorageIn /** * @internal */ - public function onKernelRequest(RequestEvent $event) + public function onKernelRequest(RequestEvent $event): void { if (!$event->isMainRequest()) { return; diff --git a/src/EventListener/IpTraceListener.php b/src/EventListener/IpTraceListener.php index f3fd21d8..c62b1e37 100644 --- a/src/EventListener/IpTraceListener.php +++ b/src/EventListener/IpTraceListener.php @@ -13,6 +13,7 @@ */ final class IpTraceListener implements EventSubscriberInterface { + /** @var IpTraceableListener */ private $ipTraceableListener; public function __construct(IpTraceableListener $ipTraceableListener) @@ -20,7 +21,7 @@ public function __construct(IpTraceableListener $ipTraceableListener) $this->ipTraceableListener = $ipTraceableListener; } - public function onKernelRequest(RequestEvent $event) + public function onKernelRequest(RequestEvent $event): void { if (!$event->isMainRequest()) { return; diff --git a/src/EventListener/LocaleListener.php b/src/EventListener/LocaleListener.php index 9979c56d..f1fa1163 100644 --- a/src/EventListener/LocaleListener.php +++ b/src/EventListener/LocaleListener.php @@ -14,6 +14,7 @@ */ class LocaleListener implements EventSubscriberInterface { + /** @var TranslatableListener */ private $translatableListener; public function __construct(TranslatableListener $translatableListener) @@ -24,7 +25,7 @@ public function __construct(TranslatableListener $translatableListener) /** * @internal */ - public function onKernelRequest(RequestEvent $event) + public function onKernelRequest(RequestEvent $event): void { $this->translatableListener->setTranslatableLocale($event->getRequest()->getLocale()); } diff --git a/src/EventListener/LoggerListener.php b/src/EventListener/LoggerListener.php index f2acfd7b..ad2c2130 100644 --- a/src/EventListener/LoggerListener.php +++ b/src/EventListener/LoggerListener.php @@ -2,6 +2,7 @@ namespace Stof\DoctrineExtensionsBundle\EventListener; +use Gedmo\Loggable\Loggable; use Gedmo\Loggable\LoggableListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\RequestEvent; @@ -14,13 +15,21 @@ * Sets the username from the security context by listening on kernel.request * * @author Christophe Coevoet + * + * @phpstan-template T of Loggable|object */ class LoggerListener implements EventSubscriberInterface { + /** @var AuthorizationCheckerInterface|null */ private $authorizationChecker; + /** @var TokenStorageInterface|null */ private $tokenStorage; + /** @var LoggableListener */ private $loggableListener; + /** + * @param LoggableListener $loggableListener + */ public function __construct(LoggableListener $loggableListener, TokenStorageInterface $tokenStorage = null, AuthorizationCheckerInterface $authorizationChecker = null) { $this->loggableListener = $loggableListener; @@ -31,7 +40,7 @@ public function __construct(LoggableListener $loggableListener, TokenStorageInte /** * @internal */ - public function onKernelRequest(RequestEvent $event) + public function onKernelRequest(RequestEvent $event): void { if (!$event->isMainRequest()) { return; diff --git a/src/Uploadable/UploadableManager.php b/src/Uploadable/UploadableManager.php index 504e2bda..0d8c2b33 100644 --- a/src/Uploadable/UploadableManager.php +++ b/src/Uploadable/UploadableManager.php @@ -3,14 +3,20 @@ namespace Stof\DoctrineExtensionsBundle\Uploadable; use Symfony\Component\HttpFoundation\File\UploadedFile; +use Gedmo\Uploadable\FileInfo\FileInfoInterface; use Gedmo\Uploadable\UploadableListener; class UploadableManager { /** @var \Gedmo\Uploadable\UploadableListener */ private $listener; + /** @var class-string */ private $fileInfoClass; + /** + * @param UploadableListener $listener + * @param class-string $fileInfoClass + */ public function __construct(UploadableListener $listener, $fileInfoClass) { $this->listener = $listener; @@ -24,6 +30,8 @@ public function __construct(UploadableListener $listener, $fileInfoClass) * * @param object $entity - The entity you are marking to "Upload" as soon as you call "flush". * @param mixed $fileInfo - The file info object or array. In Symfony, this will be typically an UploadedFile instance. + * + * @return void */ public function markEntityToUpload($entity, $fileInfo) { diff --git a/src/Uploadable/UploadedFileInfo.php b/src/Uploadable/UploadedFileInfo.php index 4a7bf9df..22d07915 100644 --- a/src/Uploadable/UploadedFileInfo.php +++ b/src/Uploadable/UploadedFileInfo.php @@ -7,6 +7,7 @@ class UploadedFileInfo implements FileInfoInterface { + /** @var UploadedFile */ private $uploadedFile; public function __construct(UploadedFile $uploadedFile) @@ -31,7 +32,7 @@ public function getName() } /** - * @return ?string + * @return int|false */ public function getSize() { diff --git a/src/Uploadable/ValidatorConfigurator.php b/src/Uploadable/ValidatorConfigurator.php index ee738fc7..84705e3d 100644 --- a/src/Uploadable/ValidatorConfigurator.php +++ b/src/Uploadable/ValidatorConfigurator.php @@ -9,6 +9,7 @@ */ class ValidatorConfigurator { + /** @var bool */ private $validateWritableDirectory; /** @@ -19,7 +20,7 @@ public function __construct($validateWritableDirectory) $this->validateWritableDirectory = $validateWritableDirectory; } - public function configure() + public function configure(): void { Validator::$validateWritableDirectory = $this->validateWritableDirectory; } diff --git a/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php b/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php index 63a380a0..73841bc1 100644 --- a/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php +++ b/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php @@ -10,6 +10,9 @@ class StofDoctrineExtensionsExtensionTest extends TestCase { + /** + * @return iterable + */ public static function provideExtensions() { return array( @@ -30,7 +33,7 @@ public static function provideExtensions() /** * @dataProvider provideExtensions */ - public function testLoadORMConfig($listener) + public function testLoadORMConfig(string $listener): void { $extension = new StofDoctrineExtensionsExtension(); $container = new ContainerBuilder(); @@ -57,7 +60,7 @@ public function testLoadORMConfig($listener) /** * @dataProvider provideExtensions */ - public function testLoadMongodbConfig($listener) + public function testLoadMongodbConfig(string $listener): void { $extension = new StofDoctrineExtensionsExtension(); $container = new ContainerBuilder(); @@ -84,7 +87,7 @@ public function testLoadMongodbConfig($listener) /** * @dataProvider provideExtensions */ - public function testLoadBothConfig($listener) + public function testLoadBothConfig(string $listener): void { $extension = new StofDoctrineExtensionsExtension(); $container = new ContainerBuilder(); @@ -110,7 +113,7 @@ public function testLoadBothConfig($listener) /** * @dataProvider provideExtensions */ - public function testEventConsistency(string $listener) + public function testEventConsistency(string $listener): void { $extension = new StofDoctrineExtensionsExtension(); $container = new ContainerBuilder(); From 1da060e232f93d3ebb00d9ab2b0ed0d114873c57 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 18:49:40 +0100 Subject: [PATCH 5/7] Add phpstan-strict-rules extension --- composer.json | 1 + phpstan.neon | 1 + src/EventListener/BlameListener.php | 3 --- src/EventListener/LocaleListener.php | 3 --- src/EventListener/LoggerListener.php | 3 --- src/Uploadable/UploadedFileInfo.php | 6 +++-- .../StofDoctrineExtensionsExtensionTest.php | 26 +++++++++---------- 7 files changed, 19 insertions(+), 24 deletions(-) diff --git a/composer.json b/composer.json index f2d4a1a4..439f3d65 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ "phpstan/phpstan": "^1.10", "phpstan/phpstan-deprecation-rules": "^1.1", "phpstan/phpstan-phpunit": "^1.3", + "phpstan/phpstan-strict-rules": "^1.5", "phpstan/phpstan-symfony": "^1.3", "symfony/mime": "^5.4 || ^6.0 || ^7.0", "symfony/phpunit-bridge": "^v6.4.1 || ^7.0.1", diff --git a/phpstan.neon b/phpstan.neon index d58e42b9..c1ad7db8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -8,5 +8,6 @@ includes: - vendor/phpstan/phpstan-deprecation-rules/rules.neon - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon + - vendor/phpstan/phpstan-strict-rules/rules.neon - vendor/phpstan/phpstan-symfony/extension.neon - vendor/phpstan/phpstan-symfony/rules.neon diff --git a/src/EventListener/BlameListener.php b/src/EventListener/BlameListener.php index 0abd399c..5ba8214a 100644 --- a/src/EventListener/BlameListener.php +++ b/src/EventListener/BlameListener.php @@ -51,9 +51,6 @@ public function onKernelRequest(RequestEvent $event): void } } - /** - * @return string[] - */ public static function getSubscribedEvents() { return array( diff --git a/src/EventListener/LocaleListener.php b/src/EventListener/LocaleListener.php index f1fa1163..aaf981fa 100644 --- a/src/EventListener/LocaleListener.php +++ b/src/EventListener/LocaleListener.php @@ -30,9 +30,6 @@ public function onKernelRequest(RequestEvent $event): void $this->translatableListener->setTranslatableLocale($event->getRequest()->getLocale()); } - /** - * @return string[] - */ public static function getSubscribedEvents() { return array( diff --git a/src/EventListener/LoggerListener.php b/src/EventListener/LoggerListener.php index ad2c2130..959fc41a 100644 --- a/src/EventListener/LoggerListener.php +++ b/src/EventListener/LoggerListener.php @@ -57,9 +57,6 @@ public function onKernelRequest(RequestEvent $event): void } } - /** - * @return string[] - */ public static function getSubscribedEvents() { return array( diff --git a/src/Uploadable/UploadedFileInfo.php b/src/Uploadable/UploadedFileInfo.php index 22d07915..59aac5fd 100644 --- a/src/Uploadable/UploadedFileInfo.php +++ b/src/Uploadable/UploadedFileInfo.php @@ -32,11 +32,13 @@ public function getName() } /** - * @return int|false + * @return int|null */ public function getSize() { - return $this->uploadedFile->getSize(); + $size = $this->uploadedFile->getSize(); + + return $size !== false ? $size : null; } /** diff --git a/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php b/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php index 73841bc1..56c39c44 100644 --- a/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php +++ b/tests/DependencyInjection/StofDoctrineExtensionsExtensionTest.php @@ -45,16 +45,16 @@ public function testLoadORMConfig(string $listener): void $extension->load(array($config), $container); - $this->assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); + self::assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); $def = $container->getDefinition('stof_doctrine_extensions.listener.'.$listener); - $this->assertTrue($def->hasTag('doctrine.event_listener')); + self::assertTrue($def->hasTag('doctrine.event_listener')); $tags = $def->getTag('doctrine.event_listener'); $configuredManagers = array_unique(array_column($tags, 'connection')); - $this->assertCount(2, $configuredManagers); + self::assertCount(2, $configuredManagers); } /** @@ -72,16 +72,16 @@ public function testLoadMongodbConfig(string $listener): void $extension->load(array($config), $container); - $this->assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); + self::assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); $def = $container->getDefinition('stof_doctrine_extensions.listener.'.$listener); - $this->assertTrue($def->hasTag('doctrine_mongodb.odm.event_listener')); + self::assertTrue($def->hasTag('doctrine_mongodb.odm.event_listener')); $tags = $def->getTag('doctrine_mongodb.odm.event_listener'); $configuredManagers = array_unique(array_column($tags, 'connection')); - $this->assertCount(2, $configuredManagers); + self::assertCount(2, $configuredManagers); } /** @@ -99,15 +99,15 @@ public function testLoadBothConfig(string $listener): void $extension->load(array($config), $container); - $this->assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); + self::assertTrue($container->hasDefinition('stof_doctrine_extensions.listener.'.$listener)); $def = $container->getDefinition('stof_doctrine_extensions.listener.'.$listener); - $this->assertTrue($def->hasTag('doctrine.event_listener')); - $this->assertTrue($def->hasTag('doctrine_mongodb.odm.event_listener')); + self::assertTrue($def->hasTag('doctrine.event_listener')); + self::assertTrue($def->hasTag('doctrine_mongodb.odm.event_listener')); - $this->assertCount(1, array_unique(array_column($def->getTag('doctrine.event_listener'), 'connection'))); - $this->assertCount(1, array_unique(array_column($def->getTag('doctrine_mongodb.odm.event_listener'), 'connection'))); + self::assertCount(1, array_unique(array_column($def->getTag('doctrine.event_listener'), 'connection'))); + self::assertCount(1, array_unique(array_column($def->getTag('doctrine_mongodb.odm.event_listener'), 'connection'))); } /** @@ -131,9 +131,9 @@ public function testEventConsistency(string $listener): void $listenerInstance = $container->get('stof_doctrine_extensions.listener.'.$listener); if (!$listenerInstance instanceof EventSubscriber) { - $this->markTestSkipped(sprintf('The listener for "%s" is not a Doctrine event subscriber.', $listener)); + self::markTestSkipped(sprintf('The listener for "%s" is not a Doctrine event subscriber.', $listener)); } - $this->assertEqualsCanonicalizing($listenerInstance->getSubscribedEvents(), $configuredEvents); + self::assertEqualsCanonicalizing($listenerInstance->getSubscribedEvents(), $configuredEvents); } } From 016d7b8d939af08c9e3368d83d0ad6cdc93dd679 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 18:50:23 +0100 Subject: [PATCH 6/7] Move to PHPStan max level --- phpstan.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index c1ad7db8..fa7c3153 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 6 + level: max paths: - src/ - tests/ From 252bd9b3ad3f0d81d406c7c79b06a03bc2c7e056 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 9 Dec 2023 18:53:34 +0100 Subject: [PATCH 7/7] Add PHPStan to CI --- .github/workflows/ci.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6dc1fdd7..a7ab022d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,25 @@ jobs: - name: Validate composer.json run: composer validate --strict --no-check-lock + static_analysis: + name: Static analysis + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: shivammathur/setup-php@v2 + with: + coverage: none + php-version: '8.2' + + - name: Install dependencies + run: composer update --ansi --no-progress --prefer-dist --no-interaction + + - name: Install PHPUnit + run: vendor/bin/simple-phpunit install + + - run: vendor/bin/phpstan analyze + tests: name: "Tests on PHP ${{ matrix.php }}${{ matrix.name_suffix }}" runs-on: ubuntu-latest