Raise phpstan to max level (#1386)

composer.json

@@ -26,7 +26,7 @@
     ],
     "require": {
         "php": "^7.3 || ^8.0",
-        "doctrine/doctrine-bundle": "^1.8 || ^2.0",
+        "doctrine/doctrine-bundle": "^1.8 || ^2.3",
         "doctrine/orm": "^2.5",
         "doctrine/persistence": "^1.3.4 || ^2.0",
         "sonata-project/admin-bundle": "4.x-dev",

phpstan-baseline.neon

@@ -6,3 +6,6 @@ parameters:
         - # https://github.com/phpstan/phpstan/issues/4650 & https://github.com/phpstan/phpstan-src/pull/476
             message: '#^Strict comparison using === between array\(\) and array<int\|string>\&nonEmpty will always evaluate to false.$#'
             path: src/Model/ModelManager.php
+        - # https://github.com/doctrine/persistence/pull/163
+            message: '#Parameter \#1 \$class of method Sonata\\DoctrineORMAdminBundle\\FieldDescription\\FieldDescriptionFactory\:\:getMetadata\(\) expects class-string, string given.$#'
+            path: src/FieldDescription/FieldDescriptionFactory.php

phpstan.neon.dist

@@ -2,7 +2,7 @@ includes:
     - phpstan-baseline.neon
 
 parameters:
-    level: 6
+    level: max
     bootstrapFiles:
         - vendor/bin/.phpunit/phpunit/vendor/autoload.php
     paths:

src/Block/AuditBlockService.php

@@ -39,16 +39,21 @@ public function __construct(Environment $twig, AuditReader $auditReader)
 
     public function execute(BlockContextInterface $blockContext, ?Response $response = null): Response
     {
+        $template = $blockContext->getTemplate();
+        \assert(null !== $template);
+        $limit = $blockContext->getSetting('limit');
+        \assert(\is_int($limit));
+
         $revisions = [];
 
-        foreach ($this->auditReader->findRevisionHistory($blockContext->getSetting('limit'), 0) as $revision) {
+        foreach ($this->auditReader->findRevisionHistory($limit, 0) as $revision) {
             $revisions[] = [
                 'revision' => $revision,
                 'entities' => $this->auditReader->findEntitiesChangedAtRevision($revision->getRev()),
             ];
         }
 
-        return $this->renderResponse($blockContext->getTemplate(), [
+        return $this->renderResponse($template, [
             'block' => $blockContext->getBlock(),
             'settings' => $blockContext->getSettings(),
             'revisions' => $revisions,

src/Builder/DatagridBuilder.php

@@ -108,6 +108,12 @@ public function addFilter(DatagridInterface $datagrid, ?string $type, FieldDescr
     {
         if (null === $type) {
             $guessType = $this->guesser->guess($fieldDescription);
+            if (null === $guessType) {
+                throw new \InvalidArgumentException(sprintf(
+                    'Cannot guess a type for the field description "%s", You MUST provide a type.',
+                    $fieldDescription->getName()
+                ));
+            }
 
             $type = $guessType->getType();
             $fieldDescription->setType($type);

src/Builder/ListBuilder.php

@@ -50,6 +50,12 @@ public function buildField(?string $type, FieldDescriptionInterface $fieldDescri
     {
         if (null === $type) {
             $guessType = $this->guesser->guess($fieldDescription);
+            if (null === $guessType) {
+                throw new \InvalidArgumentException(sprintf(
+                    'Cannot guess a type for the field description "%s", You MUST provide a type.',
+                    $fieldDescription->getName()
+                ));
+            }
 
             $fieldDescription->setType($guessType->getType());
         } else {
@@ -69,7 +75,16 @@ public function addField(FieldDescriptionCollection $list, ?string $type, FieldD
 
     public function fixFieldDescription(FieldDescriptionInterface $fieldDescription): void
     {
-        if (ListMapper::TYPE_ACTIONS === $fieldDescription->getType()) {
+        $type = $fieldDescription->getType();
+        if (!$type) {
+            throw new \RuntimeException(sprintf(
+                'Please define a type for field `%s` in `%s`',
+                $fieldDescription->getName(),
+                \get_class($fieldDescription->getAdmin())
+            ));
+        }
+
+        if (ListMapper::TYPE_ACTIONS === $type) {
             $this->buildActionFieldDescription($fieldDescription);
         }
 
@@ -83,16 +98,8 @@ public function fixFieldDescription(FieldDescriptionInterface $fieldDescription)
             $fieldDescription->setOption('_sort_order', $fieldDescription->getOption('_sort_order', 'ASC'));
         }
 
-        if (!$fieldDescription->getType()) {
-            throw new \RuntimeException(sprintf(
-                'Please define a type for field `%s` in `%s`',
-                $fieldDescription->getName(),
-                \get_class($fieldDescription->getAdmin())
-            ));
-        }
-
         if (!$fieldDescription->getTemplate()) {
-            $fieldDescription->setTemplate($this->getTemplate($fieldDescription->getType()));
+            $fieldDescription->setTemplate($this->getTemplate($type));
 
             if (!$fieldDescription->getTemplate()) {
                 switch ($fieldDescription->getMappingType()) {

src/Builder/ShowBuilder.php

@@ -49,6 +49,13 @@ public function addField(FieldDescriptionCollection $list, ?string $type, FieldD
     {
         if (null === $type) {
             $guessType = $this->guesser->guess($fieldDescription);
+            if (null === $guessType) {
+                throw new \InvalidArgumentException(sprintf(
+                    'Cannot guess a type for the field description "%s", You MUST provide a type.',
+                    $fieldDescription->getName()
+                ));
+            }
+
             $fieldDescription->setType($guessType->getType());
         } else {
             $fieldDescription->setType($type);
@@ -62,7 +69,8 @@ public function addField(FieldDescriptionCollection $list, ?string $type, FieldD
 
     public function fixFieldDescription(FieldDescriptionInterface $fieldDescription): void
     {
-        if (!$fieldDescription->getType()) {
+        $type = $fieldDescription->getType();
+        if (!$type) {
             throw new \RuntimeException(sprintf(
                 'Please define a type for field `%s` in `%s`',
                 $fieldDescription->getName(),
@@ -71,7 +79,7 @@ public function fixFieldDescription(FieldDescriptionInterface $fieldDescription)
         }
 
         if (!$fieldDescription->getTemplate()) {
-            $fieldDescription->setTemplate($this->getTemplate($fieldDescription->getType()));
+            $fieldDescription->setTemplate($this->getTemplate($type));
         }
 
         switch ($fieldDescription->getMappingType()) {

src/Datagrid/Pager.php

@@ -47,14 +47,9 @@ public function getCurrentPageResults(): iterable
             ->getMetadataFor(current($query->getQueryBuilder()->getRootEntities()))
             ->getIdentifierFieldNames();
 
-        // NEXT_MAJOR: Remove the check and the else part.
-        if (method_exists($query, 'getDoctrineQuery')) {
-            // Paginator with fetchJoinCollection doesn't work with composite primary keys
-            // https://github.com/doctrine/orm/issues/2910
-            $paginator = new Paginator($query->getDoctrineQuery(), 1 === \count($identifierFieldNames));
-        } else {
-            $paginator = new Paginator($query->getQueryBuilder(), 1 === \count($identifierFieldNames));
-        }
+        // Paginator with fetchJoinCollection doesn't work with composite primary keys
+        // https://github.com/doctrine/orm/issues/2910
+        $paginator = new Paginator($query->getDoctrineQuery(), 1 === \count($identifierFieldNames));
 
         return $paginator->getIterator();
     }
@@ -68,8 +63,13 @@ public function init(): void
     {
         $this->resultsCount = $this->computeResultsCount();
 
-        $this->getQuery()->setFirstResult(null);
-        $this->getQuery()->setMaxResults(null);
+        $query = $this->getQuery();
+        if (null === $query) {
+            throw new \LogicException('The pager need a query to be initialised');
+        }
+
+        $query->setFirstResult(null);
+        $query->setMaxResults(null);
 
         if (0 === $this->getPage() || 0 === $this->getMaxPerPage() || 0 === $this->countResults()) {
             $this->setLastPage(0);
@@ -78,8 +78,8 @@ public function init(): void
 
             $this->setLastPage((int) ceil($this->countResults() / $this->getMaxPerPage()));
 
-            $this->getQuery()->setFirstResult($offset);
-            $this->getQuery()->setMaxResults($this->getMaxPerPage());
+            $query->setFirstResult($offset);
+            $query->setMaxResults($this->getMaxPerPage());
         }
     }
 
@@ -91,12 +91,7 @@ private function computeResultsCount(): int
             throw new \TypeError(sprintf('The pager query MUST implement %s.', ProxyQueryInterface::class));
         }
 
-        // NEXT_MAJOR: Remove the check and the else part.
-        if (method_exists($query, 'getDoctrineQuery')) {
-            $paginator = new Paginator($query->getDoctrineQuery());
-        } else {
-            $paginator = new Paginator($query->getQueryBuilder());
-        }
+        $paginator = new Paginator($query->getDoctrineQuery());
 
         return \count($paginator);
     }

src/Datagrid/ProxyQuery.php

@@ -163,11 +163,11 @@ public function getDoctrineQuery(): Query
 
         $rootAlias = current($queryBuilder->getRootAliases());
 
-        if ($this->getSortBy()) {
+        $sortBy = $this->getSortBy();
+        if (null !== $sortBy) {
             $orderByDQLPart = $queryBuilder->getDQLPart('orderBy');
             $queryBuilder->resetDQLPart('orderBy');
 
-            $sortBy = $this->getSortBy();
             if (false === strpos($sortBy, '.')) {
                 $sortBy = $rootAlias.'.'.$sortBy;
             }

src/DependencyInjection/Compiler/AddAuditEntityCompilerPass.php

@@ -58,7 +58,12 @@ public function process(ContainerBuilder $container): void
     private function getModelName(ContainerBuilder $container, string $name): string
     {
         if ('%' === $name[0]) {
-            return $container->getParameter(substr($name, 1, -1));
+            $parameter = $container->getParameter(substr($name, 1, -1));
+            if (!\is_string($parameter)) {
+                throw new \InvalidArgumentException(sprintf('Cannot find the model name "%s"', $name));
+            }
+
+            return $parameter;
         }
 
         return $name;

src/DependencyInjection/Compiler/AddTemplatesCompilerPass.php

@@ -25,7 +25,9 @@ final class AddTemplatesCompilerPass implements CompilerPassInterface
     public function process(ContainerBuilder $container): void
     {
         $overwrite = $container->getParameter('sonata.admin.configuration.admin_services');
+        \assert(\is_array($overwrite));
         $templates = $container->getParameter('sonata_doctrine_orm_admin.templates');
+        \assert(\is_array($templates));
 
         foreach ($container->findTaggedServiceIds('sonata.admin') as $id => $attributes) {
             if (!isset($attributes[0]['manager_type']) || 'orm' !== $attributes[0]['manager_type']) {

src/DependencyInjection/SonataDoctrineORMAdminExtension.php

@@ -38,6 +38,7 @@ public function load(array $configs, ContainerBuilder $container): void
         $loader->load('doctrine_orm_filter_types.xml');
 
         $bundles = $container->getParameter('kernel.bundles');
+        \assert(\is_array($bundles));
 
         if (isset($bundles['SimpleThingsEntityAuditBundle'])) {
             $loader->load('audit.xml');

src/FieldDescription/FieldDescriptionFactory.php

@@ -54,6 +54,8 @@ public function create(string $class, string $name, array $options = []): FieldD
     }
 
     /**
+     * @phpstan-param class-string $baseClass
+     *
      * @phpstan-return array{ClassMetadata, string, mixed[]}
      */
     private function getParentMetadataForProperty(string $baseClass, string $propertyFullName): array

src/Filter/ChoiceFilter.php

@@ -54,9 +54,9 @@ public function getRenderSettings(): array
     /**
      * @param mixed[] $data
      *
-     * @phpstan-param array{type?: string|int|null, value?: mixed} $data
+     * @phpstan-param array{type: int|null, value: mixed} $data
      */
-    private function filterWithMultipleValues(ProxyQueryInterface $query, string $alias, string $field, array $data = []): void
+    private function filterWithMultipleValues(ProxyQueryInterface $query, string $alias, string $field, array $data): void
     {
         if (0 === \count($data['value'])) {
             return;
@@ -88,9 +88,9 @@ private function filterWithMultipleValues(ProxyQueryInterface $query, string $al
     /**
      * @param mixed[] $data
      *
-     * @phpstan-param array{type?: string|int|null, value?: mixed} $data
+     * @phpstan-param array{type: int|null, value: mixed} $data
      */
-    private function filterWithSingleValue(ProxyQueryInterface $query, string $alias, string $field, array $data = []): void
+    private function filterWithSingleValue(ProxyQueryInterface $query, string $alias, string $field, array $data): void
     {
         if ('' === $data['value'] || false === $data['value']) {
             return;

src/Filter/Filter.php

@@ -38,7 +38,7 @@ abstract class Filter extends BaseFilter
      *
      * @param mixed[] $data
      *
-     * @phpstan-param array{type?: string|int|null, value?: mixed} $data
+     * @phpstan-param array{type?: int|null, value?: mixed} $data
      */
     abstract public function filter(ProxyQueryInterface $query, string $alias, string $field, array $data): void;
 
@@ -65,7 +65,7 @@ public function isActive(): bool
      *
      * @return string[]
      *
-     * @phpstan-param array{type?: string|int|null, value?: mixed} $data
+     * @phpstan-param array{type?: int|null, value?: mixed} $data
      */
     protected function association(ProxyQueryInterface $query, array $data): array
     {

src/Model/ModelManager.php

@@ -126,7 +126,7 @@ public function getLockVersion(object $object)
     {
         $metadata = $this->getMetadata(ClassUtils::getClass($object));
 
-        if (!$metadata->isVersioned) {
+        if (!$metadata->isVersioned || !isset($metadata->reflFields[$metadata->versionField])) {
             return null;
         }
 
@@ -243,17 +243,16 @@ public function getIdentifierValues(object $entity): array
             }
 
             $fieldType = $metadata->getTypeOfField($name);
-            $type = $fieldType && Type::hasType($fieldType) ? Type::getType($fieldType) : null;
-            if ($type) {
-                $identifiers[] = $this->getValueFromType($value, $type, $fieldType, $platform);
+            if (null !== $fieldType && Type::hasType($fieldType)) {
+                $identifiers[] = $this->getValueFromType($value, Type::getType($fieldType), $fieldType, $platform);
 
                 continue;
             }
 
             $identifierMetadata = $this->getMetadata(ClassUtils::getClass($value));
 
-            foreach ($identifierMetadata->getIdentifierValues($value) as $value) {
-                $identifiers[] = $value;
+            foreach ($identifierMetadata->getIdentifierValues($value) as $identifierValue) {
+                $identifiers[] = $identifierValue;
             }
         }
 
@@ -396,10 +395,7 @@ private function getFieldName(ClassMetadata $metadata, string $name): string
         return $name;
     }
 
-    /**
-     * @param mixed $value
-     */
-    private function getValueFromType($value, Type $type, string $fieldType, AbstractPlatform $platform): string
+    private function getValueFromType(object $value, Type $type, string $fieldType, AbstractPlatform $platform): string
     {
         if ($platform->hasDoctrineTypeMappingFor($fieldType) &&
             'binary' === $platform->getDoctrineTypeMapping($fieldType)

tests/App/DataFixtures/BookFixtures.php

@@ -16,7 +16,9 @@
 use Doctrine\Bundle\FixturesBundle\Fixture;
 use Doctrine\Common\DataFixtures\DependentFixtureInterface;
 use Doctrine\Persistence\ObjectManager;
+use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Author;
 use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Book;
+use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Category;
 use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Reader;
 
 final class BookFixtures extends Fixture implements DependentFixtureInterface
@@ -25,18 +27,26 @@ final class BookFixtures extends Fixture implements DependentFixtureInterface
 
     public function load(ObjectManager $manager): void
     {
-        $book = new Book('book_id', 'Don Quixote', $this->getReference(AuthorFixtures::AUTHOR));
-        $book->addCategory($this->getReference(CategoryFixtures::CATEGORY));
+        $author = $this->getReference(AuthorFixtures::AUTHOR);
+        \assert($author instanceof Author);
+        $category = $this->getReference(CategoryFixtures::CATEGORY);
+        \assert($category instanceof Category);
+
+        $book = new Book('book_id', 'Don Quixote', $author);
+        $book->addCategory($category);
 
         $manager->persist($book);
 
-        $book1 = new Book('book_1', 'Book 1', $this->getReference(AuthorFixtures::AUTHOR_WITH_TWO_BOOKS));
-        $book1->addCategory($this->getReference(CategoryFixtures::CATEGORY));
+        $authorWithTwoBooks = $this->getReference(AuthorFixtures::AUTHOR_WITH_TWO_BOOKS);
+        \assert($authorWithTwoBooks instanceof Author);
+
+        $book1 = new Book('book_1', 'Book 1', $authorWithTwoBooks);
+        $book1->addCategory($category);
 
         $this->addReaders($book1, 100);
 
-        $book2 = new Book('book_2', 'Book 2', $this->getReference(AuthorFixtures::AUTHOR_WITH_TWO_BOOKS));
-        $book2->addCategory($this->getReference(CategoryFixtures::CATEGORY));
+        $book2 = new Book('book_2', 'Book 2', $authorWithTwoBooks);
+        $book2->addCategory($category);
 
         $this->addReaders($book2, 100);