Merge pull request #135 from Runroom/hotfix/psalm-2

[All] Increase Psalm to level 2
Runroom · Sep 8, 2022 · bce124d · bce124d
2 parents 78c6efb + 6599343
commit bce124d
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 48 deletions.
diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php
@@ -35,7 +35,7 @@
     'ordered_imports' => true,
     'phpdoc_align' => ['align' => 'left'],
     'phpdoc_order' => true,
-    'phpdoc_to_comment' => ['ignored_tags' => ['psalm-suppress', 'phpstan-ignore-next-line', 'todo']],
+    'phpdoc_to_comment' => ['ignored_tags' => ['psalm-suppress', 'phpstan-ignore-next-line', 'todo', 'return']],
     'compact_nullable_typehint' => true,
     'void_return' => false,
     'strict_comparison' => true,

diff --git a/composer.json b/composer.json
@@ -51,7 +51,7 @@
         "a2lix/auto-form-bundle": "^0.4",
         "a2lix/translation-form-bundle": "^3.0.4",
         "doctrine/data-fixtures": "^1.5.1",
-        "doctrine/dbal": "^2.10 <2.10.3 || ^3.0",
+        "doctrine/dbal": "^2.10 <2.10.3 || ^3.1.4",
         "ergebnis/composer-normalize": "^2.2",
         "fakerphp/faker": "^1.13",
         "friendsofphp/php-cs-fixer": "^3.0",

diff --git a/packages/redirection-bundle/src/EventSubscriber/AutomaticRedirectSubscriber.php b/packages/redirection-bundle/src/EventSubscriber/AutomaticRedirectSubscriber.php
@@ -29,7 +29,6 @@ final class AutomaticRedirectSubscriber implements EventSubscriber
 
     private UrlGeneratorInterface $urlGenerator;
     private PropertyAccessorInterface $propertyAccessor;
-    private EntityManagerInterface $entityManager;
 
     /**
      * @var array<class-string, array{ route: string, routeParameters: array<string, string> }>
@@ -54,27 +53,33 @@ public function getSubscribedEvents(): array
         return [Events::onFlush];
     }
 
+    /**
+     * @todo: Simplify when dropping support for doctrine/orm < 2.13
+     *
+     * @psalm-suppress DeprecatedMethod
+     */
     public function onFlush(OnFlushEventArgs $args): void
     {
-        $this->entityManager = $args->getEntityManager();
-        $unitOfWork = $this->entityManager->getUnitOfWork();
+        // @phpstan-ignore-next-line
+        $entityManager = method_exists($args, 'getObjectManager') ? $args->getObjectManager() : $args->getEntityManager();
+        $unitOfWork = $entityManager->getUnitOfWork();
 
         foreach ($unitOfWork->getScheduledEntityUpdates() as $entity) {
-            if (null !== ($redirect = $this->createRedirectFromEntityChanges($entity))) {
-                $this->entityManager->persist($redirect);
-                $unitOfWork->computeChangeSet($this->entityManager->getClassMetadata(Redirect::class), $redirect);
+            if (null !== ($redirect = $this->createRedirectFromEntityChanges($entityManager, $entity))) {
+                $entityManager->persist($redirect);
+                $unitOfWork->computeChangeSet($entityManager->getClassMetadata(Redirect::class), $redirect);
 
-                $this->modifyRelatedRedirects($redirect);
-                $this->removeLoopRedirects($redirect);
+                $this->modifyRelatedRedirects($entityManager, $redirect);
+                $this->removeLoopRedirects($entityManager, $redirect);
             }
         }
     }
 
-    private function createRedirectFromEntityChanges(object $entity): ?Redirect
+    private function createRedirectFromEntityChanges(EntityManagerInterface $entityManager, object $entity): ?Redirect
     {
         if (isset($this->configuration[\get_class($entity)])) {
-            $source = $this->generateUrl($entity);
-            $destination = $this->generateUrl($entity, self::NEXT_VALUE);
+            $source = $this->generateUrl($entityManager, $entity);
+            $destination = $this->generateUrl($entityManager, $entity, self::NEXT_VALUE);
 
             if ($source !== $destination) {
                 $redirect = new Redirect();
@@ -90,17 +95,20 @@ private function createRedirectFromEntityChanges(object $entity): ?Redirect
         return null;
     }
 
-    private function generateUrl(object $entity, int $state = self::PREVIOUS_VALUE): ?string
+    private function generateUrl(EntityManagerInterface $entityManager, object $entity, int $state = self::PREVIOUS_VALUE): ?string
     {
         $redirectConfiguration = $this->configuration[\get_class($entity)];
-        $uow = $this->entityManager->getUnitOfWork();
+        $uow = $entityManager->getUnitOfWork();
         $changeset = $uow->getEntityChangeSet($entity);
 
         try {
             return $this->urlGenerator->generate(
                 $redirectConfiguration['route'],
                 array_map(
-                    fn ($field) => $changeset[$field][$state] ?? $this->propertyAccessor->getValue($entity, $field),
+                    /**
+                     * @return mixed
+                     */
+                    fn (string $field) => $changeset[$field][$state] ?? $this->propertyAccessor->getValue($entity, $field),
                     $redirectConfiguration['routeParameters']
                 )
             );
@@ -109,11 +117,11 @@ private function generateUrl(object $entity, int $state = self::PREVIOUS_VALUE):
         }
     }
 
-    private function modifyRelatedRedirects(Redirect $redirect): void
+    private function modifyRelatedRedirects(EntityManagerInterface $entityManager, Redirect $redirect): void
     {
-        $repository = $this->entityManager->getRepository(Redirect::class);
-        $metadata = $this->entityManager->getClassMetadata(Redirect::class);
-        $unitOfWork = $this->entityManager->getUnitOfWork();
+        $repository = $entityManager->getRepository(Redirect::class);
+        $metadata = $entityManager->getClassMetadata(Redirect::class);
+        $unitOfWork = $entityManager->getUnitOfWork();
 
         $relatedRedirects = $repository->findBy([
             'destination' => $redirect->getSource(),
@@ -122,24 +130,24 @@ private function modifyRelatedRedirects(Redirect $redirect): void
 
         foreach ($relatedRedirects as $relatedRedirect) {
             $relatedRedirect->setDestination($redirect->getDestination());
-            $this->entityManager->persist($relatedRedirect);
+            $entityManager->persist($relatedRedirect);
             $unitOfWork->computeChangeSet($metadata, $relatedRedirect);
         }
     }
 
-    private function removeLoopRedirects(Redirect $redirect): void
+    private function removeLoopRedirects(EntityManagerInterface $entityManager, Redirect $redirect): void
     {
-        $repository = $this->entityManager->getRepository(Redirect::class);
-        $metadata = $this->entityManager->getClassMetadata(Redirect::class);
-        $unitOfWork = $this->entityManager->getUnitOfWork();
+        $repository = $entityManager->getRepository(Redirect::class);
+        $metadata = $entityManager->getClassMetadata(Redirect::class);
+        $unitOfWork = $entityManager->getUnitOfWork();
 
         $loopRedirects = $repository->findBy([
             'source' => $redirect->getDestination(),
             'automatic' => true,
         ]);
 
         foreach ($loopRedirects as $loopRedirect) {
-            $this->entityManager->remove($loopRedirect);
+            $entityManager->remove($loopRedirect);
             $unitOfWork->computeChangeSet($metadata, $loopRedirect);
         }
     }

diff --git a/packages/redirection-bundle/tests/Unit/RedirectSubscriberTest.php b/packages/redirection-bundle/tests/Unit/RedirectSubscriberTest.php
@@ -98,9 +98,9 @@ public function itDoesARedirectToDestinationUrl(): void
     }
 
     /**
-     * @todo: Change to MAIN_REQUEST when dropping support for Symfony 4
+     * @todo: Change to HttpKernelInterface::MAIN_REQUEST when dropping support for Symfony 4
      */
-    private function getResponseEvent(int $requestType = HttpKernelInterface::MASTER_REQUEST): RequestEvent
+    private function getResponseEvent(int $requestType = 1): RequestEvent
     {
         return new RequestEvent($this->createStub(Kernel::class), new Request(), $requestType);
     }

diff --git a/packages/testing/composer.json b/packages/testing/composer.json
@@ -28,7 +28,7 @@
     },
     "require-dev": {
         "doctrine/data-fixtures": "^1.5.1",
-        "doctrine/dbal": "^2.10 <2.10.3 || ^3.0",
+        "doctrine/dbal": "^2.10 <2.10.3 || ^3.1.4",
         "doctrine/doctrine-bundle": "^2.3",
         "doctrine/orm": "^2.8",
         "knplabs/knp-menu-bundle": "^3.1",

diff --git a/packages/user-bundle/src/Form/RolesMatrixType.php b/packages/user-bundle/src/Form/RolesMatrixType.php
@@ -38,28 +38,34 @@ public function configureOptions(OptionsResolver $resolver): void
 
                 return array_combine($roles, $roles);
             },
-            'choice_translation_domain' => static function (Options $options, $value): ?string {
-                if (true === $value) {
-                    $value = $options['translation_domain'];
-                }
-                if (null === $value) {
-                    $admin = null;
-
-                    if (isset($options['sonata_admin'])) {
-                        $admin = $options['sonata_admin'];
-                    }
-
-                    if (null === $admin && isset($options['sonata_field_description'])) {
-                        $admin = $options['sonata_field_description']->getAdmin();
+            'choice_translation_domain' =>
+                /**
+                 * @param bool|string|null $value
+                 *
+                 * @return bool|string|null
+                 */
+                static function (Options $options, $value) {
+                    // if choice_translation_domain is true, then it's the same as translation_domain
+                    if (true === $value) {
+                        $value = $options['translation_domain'];
                     }
 
-                    if (null !== $admin) {
-                        $value = $admin->getTranslationDomain();
+                    if (null === $value) {
+                        // no translation domain yet, try to ask sonata admin
+                        $admin = null;
+                        if (isset($options['sonata_admin'])) {
+                            $admin = $options['sonata_admin'];
+                        }
+                        if (null === $admin && isset($options['sonata_field_description'])) {
+                            $admin = $options['sonata_field_description']->getAdmin();
+                        }
+                        if (null !== $admin) {
+                            $value = $admin->getTranslationDomain();
+                        }
                     }
-                }
 
-                return $value;
-            },
+                    return $value;
+                },
             'data_class' => null,
         ]);
     }

diff --git a/psalm.xml b/psalm.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <psalm
-    errorLevel="3"
+    errorLevel="2"
     findUnusedPsalmSuppress="true"
     resolveFromConfigFile="true"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"