Add strict rules

sonata-project · Jul 19, 2021 · d060638 · d060638
1 parent 7cf01f9
commit d060638
Show file tree

Hide file tree

Showing 19 changed files with 60 additions and 47 deletions.
diff --git a/composer.json b/composer.json
@@ -79,6 +79,7 @@
         "phpstan/extension-installer": "^1.0",
         "phpstan/phpstan": "^0.12.84",
         "phpstan/phpstan-phpunit": "^0.12.18",
+        "phpstan/phpstan-strict-rules": "^0.12.10",
         "phpstan/phpstan-symfony": "^0.12.21",
         "psalm/plugin-phpunit": "^0.15.1",
         "psalm/plugin-symfony": "^2.0",

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1,8 +1,17 @@
 parameters:
     ignoreErrors:
+        - # Disallow PHPStan\Rules\VariableVariables\VariableMethodCallRule
+            message: '#^Variable method call#'
+            path: .
+        - # Disallow PHPStan\Rules\DisallowedConstructs\DisallowedShortTernaryRule
+            message: '#^Short ternary operator is not allowed\. Use null coalesce operator if applicable or consider using long ternary\.$#'
+            path: .
         - # https://github.com/phpstan/phpstan-phpunit/issues/87
             message: '#^Trying to mock an undefined method [a-zA-Z]*\(\) on class stdClass\.$#'
             path: tests/
+        - # https://github.com/phpstan/phpstan-strict-rules/issues/130
+            message: '#^Call to static method PHPUnit\\Framework\\Assert::.* will always evaluate to true\.$#'
+            path: tests/
         - # BC for Symfony 4 since InputBag was introduce in Symfony 5
             message: '#^Property Symfony\\Component\\HttpFoundation\\Request\:\:\$query \(Symfony\\Component\\HttpFoundation\\InputBag\) does not accept Symfony\\Component\\HttpFoundation\\ParameterBag\.$#'
             path: tests/Admin/AdminTest.php

diff --git a/src/Admin/AbstractAdmin.php b/src/Admin/AbstractAdmin.php
@@ -1798,7 +1798,7 @@ final public function getActionButtons(string $action, ?object $object = null):
             && null !== $this->id($object);
 
         if ($canAccessObject
-            && self::MASK_OF_ACTION_EDIT & $actionBit
+            && 0 !== (self::MASK_OF_ACTION_EDIT & $actionBit)
             && $this->hasRoute('edit')
             && $this->hasAccess('edit', $object)
         ) {
@@ -1808,7 +1808,7 @@ final public function getActionButtons(string $action, ?object $object = null):
         }
 
         if ($canAccessObject
-            && self::MASK_OF_ACTION_HISTORY & $actionBit
+            && 0 !== (self::MASK_OF_ACTION_HISTORY & $actionBit)
             && $this->hasRoute('history')
             && $this->hasAccess('history', $object)
         ) {
@@ -1818,7 +1818,7 @@ final public function getActionButtons(string $action, ?object $object = null):
         }
 
         if ($canAccessObject
-            && self::MASK_OF_ACTION_ACL & $actionBit
+            && 0 !== (self::MASK_OF_ACTION_ACL & $actionBit)
             && $this->isAclEnabled()
             && $this->hasRoute('acl')
             && $this->hasAccess('acl', $object)
@@ -1829,7 +1829,7 @@ final public function getActionButtons(string $action, ?object $object = null):
         }
 
         if ($canAccessObject
-            && self::MASK_OF_ACTION_SHOW & $actionBit
+            && 0 !== (self::MASK_OF_ACTION_SHOW & $actionBit)
             && $this->hasRoute('show')
             && $this->hasAccess('show', $object)
             && \count($this->getShow()) > 0
@@ -2271,16 +2271,15 @@ final protected function appendParentObject(object $object): void
             if (null !== $parentObject) {
                 $propertyAccessor = PropertyAccess::createPropertyAccessor();
                 $parentAssociationMapping = $this->getParentAssociationMapping();
+                \assert(null !== $parentAssociationMapping);
 
-                if (null !== $parentAssociationMapping) {
-                    $value = $propertyAccessor->getValue($object, $parentAssociationMapping);
-
-                    if (\is_array($value) || $value instanceof \ArrayAccess) {
-                        $value[] = $parentObject;
-                        $propertyAccessor->setValue($object, $parentAssociationMapping, $value);
-                    } else {
-                        $propertyAccessor->setValue($object, $parentAssociationMapping, $parentObject);
-                    }
+                $value = $propertyAccessor->getValue($object, $parentAssociationMapping);
+
+                if (\is_array($value) || $value instanceof \ArrayAccess) {
+                    $value[] = $parentObject;
+                    $propertyAccessor->setValue($object, $parentAssociationMapping, $value);
+                } else {
+                    $propertyAccessor->setValue($object, $parentAssociationMapping, $parentObject);
                 }
             }
         } elseif ($this->hasParentFieldDescription()) {

diff --git a/src/BCLayer/BCDeprecationParameters.php b/src/BCLayer/BCDeprecationParameters.php
@@ -28,6 +28,7 @@ final class BCDeprecationParameters
      */
     public static function forConfig(string $message, string $version): array
     {
+        // @phpstan-ignore-next-line
         if (method_exists(BaseNode::class, 'getDeprecation')) {
             return [
                 'sonata-project/admin-bundle',

diff --git a/src/BCLayer/BCUserInterface.php b/src/BCLayer/BCUserInterface.php
@@ -28,6 +28,7 @@ final class BCUserInterface
      */
     public static function getUsername(UserInterface $user): string
     {
+        // @phpstan-ignore-next-line
         if (method_exists($user, 'getUserIdentifier')) {
             return $user->getUserIdentifier();
         }

diff --git a/src/Command/GenerateObjectAclCommand.php b/src/Command/GenerateObjectAclCommand.php
@@ -128,14 +128,7 @@ public function execute(InputInterface $input, OutputInterface $output): int
                 continue;
             }
 
-            $manipulator = $this->aclObjectManipulators[$manipulatorId];
-            if (!$manipulator instanceof ObjectAclManipulatorInterface) {
-                $output->writeln(sprintf('The interface "ObjectAclManipulatorInterface" is not implemented for %s: <info>ignoring</info>', \get_class($manipulator)));
-
-                continue;
-            }
-
-            $manipulator->batchConfigureAcls($output, $admin, $securityIdentity);
+            $this->aclObjectManipulators[$manipulatorId]->batchConfigureAcls($output, $admin, $securityIdentity);
         }
 
         return 0;

diff --git a/src/Command/QuestionableCommand.php b/src/Command/QuestionableCommand.php
@@ -50,7 +50,7 @@ final protected function askConfirmation(
         OutputInterface $output,
         string $questionText,
         string $default
-    ): string {
+    ): bool {
         $questionHelper = $this->getQuestionHelper();
         $question = new ConfirmationQuestion(
             (new Question($questionText, $default))->getQuestion(),

diff --git a/src/Datagrid/SimplePager.php b/src/Datagrid/SimplePager.php
@@ -61,7 +61,7 @@ public function __construct(int $maxPerPage = 10, int $threshold = 1)
 
     public function countResults(): int
     {
-        return ($this->getPage() - 1) * $this->getMaxPerPage() + $this->thresholdCount ?? 0;
+        return ($this->getPage() - 1) * $this->getMaxPerPage() + ($this->thresholdCount ?? 0);
     }
 
     public function getCurrentPageResults(): iterable

diff --git a/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php b/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php
@@ -161,6 +161,8 @@ public function process(ContainerBuilder $container): void
 
         $dashboardGroupsSettings = $container->getParameter('sonata.admin.configuration.dashboard_groups');
         \assert(\is_array($dashboardGroupsSettings));
+        $sortAdmins = $container->getParameter('sonata.admin.configuration.sort_admins');
+        \assert(\is_bool($sortAdmins));
 
         if ([] !== $dashboardGroupsSettings) {
             $groups = $dashboardGroupsSettings;
@@ -220,7 +222,7 @@ public function process(ContainerBuilder $container): void
                     $groups[$resolvedGroupName]['keep_open'] = $groupDefaults[$resolvedGroupName]['keep_open'];
                 }
             }
-        } elseif ($container->getParameter('sonata.admin.configuration.sort_admins')) {
+        } elseif ($sortAdmins) {
             $groups = $groupDefaults;
 
             $elementSort = static function (array &$element): void {

diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -36,7 +36,6 @@ public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('sonata_admin');
         $rootNode = $treeBuilder->getRootNode();
-        \assert($rootNode instanceof ArrayNodeDefinition);
 
         $rootNode
             ->fixXmlConfig('option')

diff --git a/src/Filter/Persister/SessionFilterPersister.php b/src/Filter/Persister/SessionFilterPersister.php
@@ -65,6 +65,7 @@ private function buildStorageKey(string $adminCode): string
      */
     private function getSession(): SessionInterface
     {
+        // @phpstan-ignore-next-line
         if (method_exists($this->requestStack, 'getSession')) {
             return $this->requestStack->getSession();
         }

diff --git a/src/Mapper/BaseGroupedMapper.php b/src/Mapper/BaseGroupedMapper.php
@@ -88,7 +88,7 @@ final public function with(string $name, array $options = []): self
         $code = $name;
 
         // Open
-        if (\array_key_exists('tab', $options) && $options['tab']) {
+        if (\array_key_exists('tab', $options) && true === $options['tab']) {
             $tabs = $this->getTabs();
 
             if (null !== $this->currentTab) {

diff --git a/src/Menu/Matcher/Voter/AdminVoter.php b/src/Menu/Matcher/Voter/AdminVoter.php
@@ -75,6 +75,7 @@ public function matchItem(ItemInterface $item): ?bool
      */
     private function getMainRequest(): ?Request
     {
+        // @phpstan-ignore-next-line
         if (method_exists($this->requestStack, 'getMainRequest')) {
             return $this->requestStack->getMainRequest();   // symfony 5.3+
         }

diff --git a/src/Menu/Provider/GroupMenuProvider.php b/src/Menu/Provider/GroupMenuProvider.php
@@ -55,18 +55,24 @@ public function __construct(FactoryInterface $menuFactory, Pool $pool, Authoriza
     /**
      * Retrieves the menu based on the group options.
      *
-     * @param mixed[] $options
+     * @param array<string, mixed> $options
      *
      * @throws \InvalidArgumentException if the menu does not exists
      */
     public function get(string $name, array $options = []): ItemInterface
     {
+        if (!isset($options['name'])) {
+            throw new \InvalidArgumentException('TODO');
+        }
+        $menuItem = $this->menuFactory->createItem($options['name']);
+
+        if (!isset($options['group'])) {
+            throw new \InvalidArgumentException('TODO');
+        }
         /** @phpstan-var Group $group */
         $group = $options['group'];
 
-        $menuItem = $this->menuFactory->createItem($options['name']);
-
-        if (!\array_key_exists('on_top', $group) || false === $group['on_top']) {
+        if (false === $group['on_top']) {
             foreach ($group['items'] as $item) {
                 if ($this->canGenerateMenuItem($item, $group)) {
                     $menuItem->addChild($this->generateMenuItem($item, $group));
@@ -75,18 +81,20 @@ public function get(string $name, array $options = []): ItemInterface
 
             if (false === $menuItem->hasChildren()) {
                 $menuItem->setDisplay(false);
-            } elseif ($group['keep_open'] ?? false) {
+            } elseif ($group['keep_open']) {
                 $menuItem->setAttribute('class', 'keep-open');
                 $menuItem->setExtra('keep_open', $group['keep_open']);
             }
-        } elseif (1 === \count($group['items'])) {
-            if ($this->canGenerateMenuItem($group['items'][0], $group)) {
-                $menuItem = $this->generateMenuItem($group['items'][0], $group);
-                $menuItem->setExtra('on_top', $group['on_top']);
-            } else {
-                $menuItem->setDisplay(false);
-            }
+        } elseif (
+            1 === \count($group['items'])
+            && $this->canGenerateMenuItem($group['items'][0], $group)
+        ) {
+            $menuItem = $this->generateMenuItem($group['items'][0], $group);
+            $menuItem->setExtra('on_top', $group['on_top']);
+        } else {
+            $menuItem->setDisplay(false);
         }
+
         $menuItem->setLabel($group['label']);
 
         return $menuItem;

diff --git a/tests/Admin/AdminTest.php b/tests/Admin/AdminTest.php
@@ -1611,7 +1611,7 @@ public function testGetFilterFieldDescription(): void
         $datagridBuilder = $this->createMock(DatagridBuilderInterface::class);
         $datagridBuilder->expects(self::once())
             ->method('getBaseDatagrid')
-            ->with($this->identicalTo($modelAdmin))
+            ->with(self::identicalTo($modelAdmin))
             ->willReturn($datagrid);
 
         $datagridBuilder->expects(self::exactly(3))

diff --git a/tests/Form/DataTransformer/ModelToIdPropertyTransformerTest.php b/tests/Form/DataTransformer/ModelToIdPropertyTransformerTest.php
@@ -71,12 +71,12 @@ public function testReverseTransformMultiple(array $expected, $params, Foo $enti
         $transformer = new ModelToIdPropertyTransformer($modelManager, Foo::class, 'bar', true);
         $proxyQuery = $this->createMock(ProxyQueryInterface::class);
         $modelManager
-            ->expects(self::exactly($params ? 1 : 0))
+            ->expects(self::exactly(null !== $params ? 1 : 0))
             ->method('createQuery')
             ->with(self::equalTo(Foo::class))
             ->willReturn($proxyQuery);
         $modelManager
-            ->expects(self::exactly($params ? 1 : 0))
+            ->expects(self::exactly(null !== $params ? 1 : 0))
             ->method('executeQuery')
             ->with(self::equalTo($proxyQuery))
             ->willReturnCallback(static function () use ($params, $entity1, $entity2, $entity3): array {

diff --git a/tests/Mapper/BaseGroupedMapperTest.php b/tests/Mapper/BaseGroupedMapperTest.php
@@ -73,7 +73,7 @@ protected function setUp(): void
 
         $this->baseGroupedMapper
             ->method('getTabs')
-            ->willReturnCallback(function () {
+            ->willReturnCallback(function (): array {
                 return $this->getTabs();
             });
 
@@ -85,7 +85,7 @@ protected function setUp(): void
 
         $this->baseGroupedMapper
             ->method('getGroups')
-            ->willReturnCallback(function () {
+            ->willReturnCallback(function (): array {
                 return $this->getTestGroups();
             });
 

diff --git a/tests/Route/RouteCollectionTest.php b/tests/Route/RouteCollectionTest.php
@@ -179,9 +179,7 @@ public function testRouteWithAllConstructorParameters(): void
         self::assertArrayHasKey('debug', $route->getOptions());
         self::assertSame($host, $route->getHost());
         self::assertSame($methods, $route->getMethods());
-        if (method_exists($route, 'getCondition')) {
-            self::assertSame($condition, $route->getCondition());
-        }
+        self::assertSame($condition, $route->getCondition());
     }
 
     public function testRouteControllerService(): void

diff --git a/tests/Search/SearchHandlerTest.php b/tests/Search/SearchHandlerTest.php
@@ -90,7 +90,7 @@ public function testBuildPagerWithMultipleSearchableFilter(): void
         $admin->method('getCode')->willReturn($adminCode);
 
         $handler = new SearchHandler();
-        $this->assertInstanceOf(PagerInterface::class, $handler->search($admin, 'myservice'));
+        self::assertInstanceOf(PagerInterface::class, $handler->search($admin, 'myservice'));
     }
 
     /**