Add strict rules (#7340)

* Add strict rules

* Fix last issue

* Add message

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",

phpstan-baseline.neon

@@ -1,8 +1,14 @@
 parameters:
     ignoreErrors:
+        - # Disallow VariableMethodCallRule and VariablePropertyFetchRule
+            message: '#^Variable (method call|property access)#'
+            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
@@ -18,3 +24,12 @@ parameters:
         - # PHPStan doesn't solve type of Maker argument/option https://github.com/phpstan/phpstan-symfony/issues/167
             message: '#array<string>\|string\|true(\sgiven)?.$#'
             path: src/Maker/AdminMaker.php
+        - # `treatPhpdocAsCertain: false` should not report this https://github.com/phpstan/phpstan/issues/5333
+            message: '#^Instanceof between Symfony\\Component\\Routing\\Route and Symfony\\Component\\Routing\\Route will always evaluate to true\.$#'
+            path: src/Route/RouteCollection.php
+        - # DataTransformer contravariance is not really mandatory
+            message: '#(reverseTransform|transform)\(\) should be contravariant#'
+            path: src/Form/DataTransformer
+        - # Generic with intersection is buggy https://github.com/phpstan/phpstan/issues/5336
+            message: '#Sonata\\AdminBundle\\Datagrid\\(Datagrid|PagerInterface)<PHPUnit\\Framework\\MockObject\\Stub&Sonata\\AdminBundle\\Datagrid\\ProxyQueryInterface>#'
+            path: tests/Datagrid/DatagridTest.php

src/Admin/AbstractAdmin.php

@@ -2264,15 +2264,15 @@ protected function configureDefaultSortValues(array &$sortValues): void
      */
     final protected function appendParentObject(object $object): void
     {
-        if ($this->isChild() && null !== $this->getParentAssociationMapping()) {
-            $parentAdmin = $this->getParent();
-            $parentObject = $parentAdmin->getObject($this->getRequest()->get($parentAdmin->getIdParameter()));
+        if ($this->isChild()) {
+            $parentAssociationMapping = $this->getParentAssociationMapping();
 
-            if (null !== $parentObject) {
-                $propertyAccessor = PropertyAccess::createPropertyAccessor();
-                $parentAssociationMapping = $this->getParentAssociationMapping();
+            if (null !== $parentAssociationMapping) {
+                $parentAdmin = $this->getParent();
+                $parentObject = $parentAdmin->getObject($this->getRequest()->get($parentAdmin->getIdParameter()));
 
-                if (null !== $parentAssociationMapping) {
+                if (null !== $parentObject) {
+                    $propertyAccessor = PropertyAccess::createPropertyAccessor();
                     $value = $propertyAccessor->getValue($object, $parentAssociationMapping);
 
                     if (\is_array($value) || $value instanceof \ArrayAccess) {
@@ -2282,8 +2282,12 @@ final protected function appendParentObject(object $object): void
                         $propertyAccessor->setValue($object, $parentAssociationMapping, $parentObject);
                     }
                 }
+
+                return;
             }
-        } elseif ($this->hasParentFieldDescription()) {
+        }
+
+        if ($this->hasParentFieldDescription()) {
             $parentAdmin = $this->getParentFieldDescription()->getAdmin();
             $parentObject = $parentAdmin->getObject($this->getRequest()->get($parentAdmin->getIdParameter()));
 

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',

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();
         }

src/Command/GenerateObjectAclCommand.php

@@ -117,7 +117,7 @@ public function execute(InputInterface $input, OutputInterface $output): int
             }
 
             $objectOwner = $input->getOption('object_owner');
-            if (!$input->getOption('step') && $objectOwner) {
+            if (!$input->getOption('step') && null !== $objectOwner) {
                 $securityIdentity = new UserSecurityIdentity($objectOwner, $this->getUserModelClass($input, $output));
             }
 
@@ -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;

src/Controller/CRUDController.php

@@ -431,7 +431,7 @@ public function batchAction(Request $request): Response
 
         $askConfirmation = $batchActions[$action]['ask_confirmation'] ?? true;
 
-        if ($askConfirmation && 'ok' !== $confirmation) {
+        if (true === $askConfirmation && 'ok' !== $confirmation) {
             $actionLabel = $batchActions[$action]['label'];
             $batchTranslationDomain = $batchActions[$action]['translation_domain'] ??
                 $this->admin->getTranslationDomain();
@@ -949,7 +949,9 @@ final protected function renderJson($data, int $status = Response::HTTP_OK, arra
      */
     final protected function isXmlHttpRequest(Request $request): bool
     {
-        return $request->isXmlHttpRequest() || $request->get('_xml_http_request');
+        return $request->isXmlHttpRequest()
+            || $request->request->getBoolean('_xml_http_request')
+            || $request->query->getBoolean('_xml_http_request');
     }
 
     /**
@@ -992,7 +994,9 @@ protected function getBaseTemplate(): string
      */
     protected function handleModelManagerException(\Exception $exception): void
     {
-        if ($this->getParameter('kernel.debug')) {
+        $debug = $this->getParameter('kernel.debug');
+        \assert(\is_bool($debug));
+        if ($debug) {
             throw $exception;
         }
 

src/DependencyInjection/Configuration.php

@@ -13,7 +13,6 @@
 
 namespace Sonata\AdminBundle\DependencyInjection;
 
-use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 
@@ -36,7 +35,6 @@ public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('sonata_admin');
         $rootNode = $treeBuilder->getRootNode();
-        \assert($rootNode instanceof ArrayNodeDefinition);
 
         $rootNode
             ->fixXmlConfig('option')

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();
         }

src/Form/DataTransformer/BooleanToStringTransformer.php

@@ -38,7 +38,7 @@ public function __construct(string $trueValue)
      */
     public function transform($value): ?string
     {
-        return $value ? $this->trueValue : null;
+        return true === $value ? $this->trueValue : null;
     }
 
     /**

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+
         }

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('The option "name" is required.');
+        }
+        $menuItem = $this->menuFactory->createItem($options['name']);
+
+        if (!isset($options['group'])) {
+            throw new \InvalidArgumentException('The option "group" is required.');
+        }
         /** @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;

src/Route/AdminPoolLoader.php

@@ -20,8 +20,6 @@
 
 /**
  * @author Thomas Rabaix <[email protected]>
- *
- * @psalm-suppress PropertyNotSetInConstructor add `parent::__construct()` call when dropping support of symfony/symfony-config < 5.3.
  */
 final class AdminPoolLoader extends Loader
 {
@@ -34,6 +32,12 @@ final class AdminPoolLoader extends Loader
 
     public function __construct(Pool $pool)
     {
+        // Remove this check when dropping support for support of symfony/symfony-config < 5.3.
+        // @phpstan-ignore-next-line
+        if (method_exists(parent::class, '__construct')) {
+            parent::__construct();
+        }
+
         $this->pool = $pool;
     }
 

src/Security/Handler/AclSecurityHandler.php

@@ -168,7 +168,6 @@ public function getObjectAcl(ObjectIdentityInterface $objectIdentity): ?MutableA
     {
         try {
             $acl = $this->aclProvider->findAcl($objectIdentity);
-            \assert($acl instanceof MutableAclInterface);
         } catch (AclNotFoundException $e) {
             return null;
         }
@@ -248,7 +247,8 @@ public function deleteAcl(ObjectIdentityInterface $objectIdentity): void
     public function findClassAceIndexByRole(MutableAclInterface $acl, string $role)
     {
         foreach ($acl->getClassAces() as $index => $entry) {
-            if ($entry->getSecurityIdentity() instanceof RoleSecurityIdentity && $entry->getSecurityIdentity()->getRole() === $role) {
+            $securityIdentity = $entry->getSecurityIdentity();
+            if ($securityIdentity instanceof RoleSecurityIdentity && $securityIdentity->getRole() === $role) {
                 return $index;
             }
         }
@@ -259,7 +259,8 @@ public function findClassAceIndexByRole(MutableAclInterface $acl, string $role)
     public function findClassAceIndexByUsername(MutableAclInterface $acl, string $username)
     {
         foreach ($acl->getClassAces() as $index => $entry) {
-            if ($entry->getSecurityIdentity() instanceof UserSecurityIdentity && $entry->getSecurityIdentity()->getUsername() === $username) {
+            $securityIdentity = $entry->getSecurityIdentity();
+            if ($securityIdentity instanceof UserSecurityIdentity && $securityIdentity->getUsername() === $username) {
                 return $index;
             }
         }

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))

tests/Command/GenerateObjectAclCommandTest.php

@@ -99,35 +99,6 @@ public function testExecuteWithManipulatorNotFound(): void
         self::assertMatchesRegularExpression('/Admin class is using a manager type that has no manipulator implemented : ignoring/', $commandTester->getDisplay());
     }
 
-    /**
-     * @psalm-suppress InvalidArgument
-     */
-    public function testExecuteWithManipulatorNotObjectAclManipulatorInterface(): void
-    {
-        $admin = $this->createStub(AbstractAdmin::class);
-        $container = new Container();
-        $container->set('acme.admin.foo', $admin);
-        $pool = new Pool($container, ['acme.admin.foo']);
-
-        $admin->setManagerType('bar');
-
-        $aclObjectManipulators = [
-            'sonata.admin.manipulator.acl.object.bar' => new \stdClass(),
-        ];
-
-        // @phpstan-ignore-next-line
-        $command = new GenerateObjectAclCommand($pool, $aclObjectManipulators);
-
-        $application = new Application();
-        $application->add($command);
-
-        $command = $application->find('sonata:admin:generate-object-acl');
-        $commandTester = new CommandTester($command);
-        $commandTester->execute(['command' => $command->getName()]);
-
-        self::assertMatchesRegularExpression('/The interface "ObjectAclManipulatorInterface" is not implemented for/', $commandTester->getDisplay());
-    }
-
     public function testExecuteWithManipulator(): void
     {
         $admin = $this->createStub(AbstractAdmin::class);

tests/Controller/CRUDControllerTest.php

@@ -350,7 +350,7 @@ public function testRenderJsonAjax(): void
     {
         $data = ['example' => '123', 'foo' => 'bar'];
 
-        $this->request->attributes->set('_xml_http_request', true);
+        $this->request->query->set('_xml_http_request', true);
         $this->request->headers->set('Content-Type', 'multipart/form-data');
         $response = $this->protectedTestedMethods['renderJson']->invoke($this->controller, $data, 200, [], $this->request);
 
@@ -369,7 +369,7 @@ public function testIsXmlHttpRequest(): void
         $this->request->headers->remove('X-Requested-With');
         self::assertFalse($this->protectedTestedMethods['isXmlHttpRequest']->invoke($this->controller, $this->request));
 
-        $this->request->attributes->set('_xml_http_request', true);
+        $this->request->query->set('_xml_http_request', true);
         self::assertTrue($this->protectedTestedMethods['isXmlHttpRequest']->invoke($this->controller, $this->request));
     }
 
@@ -422,7 +422,7 @@ public function testGetBaseTemplate(): void
             $this->protectedTestedMethods['getBaseTemplate']->invoke($this->controller)
         );
 
-        $this->request->attributes->set('_xml_http_request', true);
+        $this->request->request->set('_xml_http_request', true);
         self::assertSame(
             '@SonataAdmin/ajax_layout.html.twig',
             $this->protectedTestedMethods['getBaseTemplate']->invoke($this->controller)