From 6b89d4dfab0744a42cfeed01638c42fd5c2f6abb Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Mon, 24 Jun 2024 11:28:02 +0200 Subject: [PATCH 1/4] Add static analysis with phpstan --- .github/workflows/ci.yaml | 20 +++++++++++++++++++- composer.json | 4 ++++ phpstan.neon | 15 +++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 phpstan.neon diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2b43d4cb6..92676d21b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,7 +9,7 @@ on: - 3.x jobs: - run: + tests: runs-on: ubuntu-latest strategy: fail-fast: false @@ -52,3 +52,21 @@ jobs: - name: Run PHPUnit tests run: ./vendor/bin/phpunit + + phpstan: + runs-on: ubuntu-latest + name: PHPStan + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.4' + + - name: Install dependencies + run: composer update + + - name: Run analysis + run: ./vendor/bin/phpstan diff --git a/composer.json b/composer.json index 4b603c1e4..74ebffa9e 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,11 @@ }, "require-dev": { "doctrine/doctrine-bundle": "^1.3 || ^2", + "doctrine/orm": "^2.6.3 || ^3", "friendsofphp/php-cs-fixer": "^3.0.2, !=3.5.0", + "phpstan/phpstan": "^2.1", + "phpstan/phpstan-phpunit": "^2.0", + "phpstan/phpstan-symfony": "^2.0", "phpunit/phpunit": "^9.6", "symfony/console": "^6.4 || ^7.0", "symfony/mailer": "^6.4 || ^7.0", diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 000000000..6870ba82e --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,15 @@ +parameters: + level: 8 + inferPrivatePropertyTypeFromConstructor: true + treatPhpDocTypesAsCertain: false + paths: + - src/ + - tests/ + ignoreErrors: + - '~Method FOS\\UserBundle\\Tests\\[\w\\]+Test::test\w+\(\) has no return type specified.~' + +includes: + - vendor/phpstan/phpstan-phpunit/extension.neon + - vendor/phpstan/phpstan-phpunit/rules.neon + - vendor/phpstan/phpstan-symfony/extension.neon + - vendor/phpstan/phpstan-symfony/rules.neon From 8245475419de8a3fa81da78eda6f684ef92b135f Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 10 Feb 2026 15:19:22 +0100 Subject: [PATCH 2/4] Try without the phpstan-symfony extension --- phpstan.neon | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 6870ba82e..af54b616c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -11,5 +11,5 @@ parameters: includes: - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon - - vendor/phpstan/phpstan-symfony/extension.neon - - vendor/phpstan/phpstan-symfony/rules.neon +# - vendor/phpstan/phpstan-symfony/extension.neon +# - vendor/phpstan/phpstan-symfony/rules.neon From be29986b2fe3fd344a673b8a3224fc2466446a7c Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 10 Feb 2026 15:44:12 +0100 Subject: [PATCH 3/4] Install the mongodb-odm for the static analysis The composer config fakes the availability of ext-mongodb during the dependency resolution (affecting only the case where FOSUserBundle is the root package) as we don't need to be able to actually execute the ODM to be able to do static analysis. This allows to avoid requiring ext-mongodb for contributors. --- composer.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/composer.json b/composer.json index 74ebffa9e..a35759d4e 100644 --- a/composer.json +++ b/composer.json @@ -50,6 +50,7 @@ }, "require-dev": { "doctrine/doctrine-bundle": "^1.3 || ^2", + "doctrine/mongodb-odm": "^2.16", "doctrine/orm": "^2.6.3 || ^3", "friendsofphp/php-cs-fixer": "^3.0.2, !=3.5.0", "phpstan/phpstan": "^2.1", @@ -62,6 +63,9 @@ "symfony/phpunit-bridge": "^6.4 || ^7.0" }, "config": { + "platform": { + "ext-mongodb": "2.1.8" + }, "sort-packages": true }, "extra": { From b45559337171bb0aeb532a3270e916fec64ee783 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 13 Feb 2026 18:43:29 +0100 Subject: [PATCH 4/4] Improve types for static analysis --- src/Controller/ResettingController.php | 2 +- src/Doctrine/UserManager.php | 8 +++-- src/Event/FormEvent.php | 2 +- src/Form/Type/UsernameFormType.php | 2 +- src/Util/CanonicalFieldsUpdater.php | 4 +++ .../FOSUserExtensionTest.php | 29 +++++++++---------- tests/Security/UserCheckerTest.php | 2 +- 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/Controller/ResettingController.php b/src/Controller/ResettingController.php index fd36771a8..632a366ef 100644 --- a/src/Controller/ResettingController.php +++ b/src/Controller/ResettingController.php @@ -71,7 +71,7 @@ public function requestAction(): Response */ public function sendEmailAction(Request $request): Response { - $username = $request->request->get('username'); + $username = $request->request->getString('username'); $user = $this->userManager->findUserByUsernameOrEmail($username); diff --git a/src/Doctrine/UserManager.php b/src/Doctrine/UserManager.php index eda9cb958..748ff31a5 100644 --- a/src/Doctrine/UserManager.php +++ b/src/Doctrine/UserManager.php @@ -27,15 +27,17 @@ class UserManager extends BaseUserManager /** * @var string + * + * @phpstan-var class-string */ private $class; /** * Constructor. * - * @param string $class + * @phpstan-param class-string $class */ - public function __construct(PasswordUpdaterInterface $passwordUpdater, CanonicalFieldsUpdater $canonicalFieldsUpdater, ObjectManager $om, $class) + public function __construct(PasswordUpdaterInterface $passwordUpdater, CanonicalFieldsUpdater $canonicalFieldsUpdater, ObjectManager $om, string $class) { parent::__construct($passwordUpdater, $canonicalFieldsUpdater); @@ -80,7 +82,7 @@ public function reloadUser(UserInterface $user): void $this->objectManager->refresh($user); } - public function updateUser(UserInterface $user, $andFlush = true): void + public function updateUser(UserInterface $user, bool $andFlush = true): void { $this->updateCanonicalFields($user); $this->updatePassword($user); diff --git a/src/Event/FormEvent.php b/src/Event/FormEvent.php index fd70535e1..722de3ba9 100644 --- a/src/Event/FormEvent.php +++ b/src/Event/FormEvent.php @@ -29,7 +29,7 @@ final class FormEvent extends Event private $request; /** - * @var Response + * @var Response|null */ private $response; diff --git a/src/Form/Type/UsernameFormType.php b/src/Form/Type/UsernameFormType.php index 22505c6c8..4105b1202 100644 --- a/src/Form/Type/UsernameFormType.php +++ b/src/Form/Type/UsernameFormType.php @@ -41,7 +41,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $builder->addModelTransformer($this->usernameTransformer); } - public function getParent(): ?string + public function getParent(): string { return TextType::class; } diff --git a/src/Util/CanonicalFieldsUpdater.php b/src/Util/CanonicalFieldsUpdater.php index 923f3c7b7..41d800c69 100644 --- a/src/Util/CanonicalFieldsUpdater.php +++ b/src/Util/CanonicalFieldsUpdater.php @@ -41,6 +41,8 @@ public function updateCanonicalFields(UserInterface $user): void * Canonicalizes an email. * * @param string|null $email + * + * @phpstan-return ($email is null ? null : string) */ public function canonicalizeEmail($email): ?string { @@ -51,6 +53,8 @@ public function canonicalizeEmail($email): ?string * Canonicalizes a username. * * @param string|null $username + * + * @phpstan-return ($username is null ? null : string) */ public function canonicalizeUsername($username): ?string { diff --git a/tests/DependencyInjection/FOSUserExtensionTest.php b/tests/DependencyInjection/FOSUserExtensionTest.php index 65cec37c6..4fbae8c24 100644 --- a/tests/DependencyInjection/FOSUserExtensionTest.php +++ b/tests/DependencyInjection/FOSUserExtensionTest.php @@ -20,12 +20,7 @@ class FOSUserExtensionTest extends TestCase { /** @var ContainerBuilder */ - protected $configuration; - - protected function tearDown(): void - { - unset($this->configuration); - } + private $configuration; public function testUserLoadThrowsExceptionUnlessDatabaseDriverSet() { @@ -157,8 +152,12 @@ public function testDisableChangePassword() /** * @dataProvider providerEmailsDisabledFeature + * + * @param array $testConfig + * @param array{address: string, sender_name: string} $registration + * @param array{address: string, sender_name: string} $resetting */ - public function testEmailsDisabledFeature($testConfig, $registration, $resetting) + public function testEmailsDisabledFeature(array $testConfig, array $registration, array $resetting) { $this->configuration = new ContainerBuilder(); $loader = new FOSUserExtension(); @@ -170,7 +169,10 @@ public function testEmailsDisabledFeature($testConfig, $registration, $resetting $this->assertParameter($resetting, 'fos_user.resetting.email.from_address'); } - public function providerEmailsDisabledFeature() + /** + * @return iterable, array{address: string, sender_name: string}, array{address: string, sender_name: string}}> + */ + public function providerEmailsDisabledFeature(): iterable { $configBothFeaturesDisabled = ['registration' => false, 'resetting' => false]; $configResettingDisabled = ['resetting' => false]; @@ -343,7 +345,7 @@ public function testUserLoadFlashesCanBeDisabled() /** * @dataProvider userManagerSetFactoryProvider */ - public function testUserManagerSetFactory($dbDriver, $doctrineService) + public function testUserManagerSetFactory(string $dbDriver, string $doctrineService) { $this->configuration = new ContainerBuilder(); $loader = new FOSUserExtension(); @@ -357,6 +359,7 @@ public function testUserManagerSetFactory($dbDriver, $doctrineService) $factory = $definition->getFactory(); + $this->assertIsArray($factory); $this->assertInstanceOf('Symfony\Component\DependencyInjection\Reference', $factory[0]); $this->assertSame('fos_user.doctrine_registry', (string) $factory[0]); $this->assertSame('getManager', $factory[1]); @@ -373,27 +376,23 @@ public function userManagerSetFactoryProvider(): iterable ]; } - protected function createEmptyConfiguration() + protected function createEmptyConfiguration(): void { $this->configuration = new ContainerBuilder(); $loader = new FOSUserExtension(); $config = $this->getEmptyConfig(); $loader->load([$config], $this->configuration); - $this->assertTrue($this->configuration instanceof ContainerBuilder); } - protected function createFullConfiguration() + protected function createFullConfiguration(): void { $this->configuration = new ContainerBuilder(); $loader = new FOSUserExtension(); $config = $this->getFullConfig(); $loader->load([$config], $this->configuration); - $this->assertTrue($this->configuration instanceof ContainerBuilder); } /** - * getEmptyConfig. - * * @return array */ protected function getEmptyConfig(): array diff --git a/tests/Security/UserCheckerTest.php b/tests/Security/UserCheckerTest.php index 66815d93e..690b4ec41 100644 --- a/tests/Security/UserCheckerTest.php +++ b/tests/Security/UserCheckerTest.php @@ -46,7 +46,7 @@ public function testCheckPostAuthSuccess() $this->expectNotToPerformAssertions(); } - private function getUser($isEnabled): User + private function getUser(bool $isEnabled): User { $userMock = $this->getMockBuilder('FOS\UserBundle\Model\User')->getMock(); $userMock