Tighten types that were reported by phpstan. (mautic#13110)

* Fix phpstan reported type errors. * Undo BC breaks. * Review check. * Review check mautic#3. * PHPCS fixer after rebase. * Rector changes after rebase. * Fixed tests. * Fix code review issues. * Fix phpstan. * After rebase.
galvani · Dec 9, 2024 · d5e4c5d · d5e4c5d
1 parent 426e96a
commit d5e4c5d
Show file tree

Hide file tree

Showing 127 changed files with 545 additions and 1,220 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -65,7 +65,7 @@ jobs:
         php-version: ${{ matrix.php-versions }}
         extensions: mbstring, xml, ctype, iconv, intl, pdo_sqlite, mysql, pdo_mysql
         coverage: pcov
-        ini-values: pcov.enabled=0, pcov.directory=., pcov.exclude="~tests|themes|vendor~"
+        ini-values: pcov.enabled=0, pcov.directory=., pcov.exclude="~tests|themes|vendor~", zend.assertions=1
 
     - name: add MySQL config file
       run: |

diff --git a/app/bundles/CacheBundle/Tests/Cache/CacheProviderTest.php b/app/bundles/CacheBundle/Tests/Cache/CacheProviderTest.php
@@ -10,7 +10,6 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Cache\Exception\InvalidArgumentException;
-use Symfony\Component\Cache\Psr16Cache;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 class CacheProviderTest extends TestCase
@@ -78,8 +77,7 @@ public function testSimplePsrCacheIsReturned(): void
             ->with('foo.bar')
             ->willReturn($this->adapter);
 
-        $simpleCache = $this->cacheProvider->getSimpleCache();
-        $this->assertInstanceOf(Psr16Cache::class, $simpleCache);
+        $this->cacheProvider->getSimpleCache();
     }
 
     public function testExceptionThrownIfAdaptorNotFoundInContainer(): void

diff --git a/app/bundles/CampaignBundle/Command/TriggerCampaignCommand.php b/app/bundles/CampaignBundle/Command/TriggerCampaignCommand.php
@@ -165,6 +165,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->scheduleOnly = $input->getOption('scheduled-only');
         $this->inactiveOnly = $input->getOption('inactive-only');
 
+        $id               = $input->getOption('campaign-id');
         $batchLimit       = $input->getOption('batch-limit');
         $campaignLimit    = $input->getOption('campaign-limit');
         $contactMinId     = $input->getOption('min-contact-id');
@@ -175,6 +176,34 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $maxThreads       = $input->getOption('max-threads');
         $excludeCampaigns = $input->getOption('exclude');
 
+        if (is_numeric($id)) {
+            $id = (int) $id;
+        }
+
+        if (is_numeric($maxThreads)) {
+            $maxThreads = (int) $maxThreads;
+        }
+
+        if (is_numeric($threadId)) {
+            $threadId = (int) $threadId;
+        }
+
+        if (is_numeric($contactMaxId)) {
+            $contactMaxId = (int) $contactMaxId;
+        }
+
+        if (is_numeric($contactMinId)) {
+            $contactMinId = (int) $contactMinId;
+        }
+
+        if (is_numeric($contactId)) {
+            $contactId = (int) $contactId;
+        }
+
+        if (is_numeric($campaignLimit)) {
+            $campaignLimit = (int) $campaignLimit;
+        }
+
         if ($threadId && $maxThreads && (int) $threadId > (int) $maxThreads) {
             $this->output->writeln('--thread-id cannot be larger than --max-thread');
 
@@ -184,7 +213,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->limiter = new ContactLimiter($batchLimit, $contactId, $contactMinId, $contactMaxId, $contactIds, $threadId, $maxThreads, $campaignLimit);
 
         defined('MAUTIC_CAMPAIGN_SYSTEM_TRIGGERED') or define('MAUTIC_CAMPAIGN_SYSTEM_TRIGGERED', 1);
-        $id = $input->getOption('campaign-id');
 
         $moderationKey = sprintf('%s-%s', $id, $threadId);
         if (!$this->checkRunStatus($input, $this->output, $moderationKey)) {

diff --git a/app/bundles/CampaignBundle/Command/UpdateLeadCampaignsCommand.php b/app/bundles/CampaignBundle/Command/UpdateLeadCampaignsCommand.php
@@ -122,6 +122,30 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->output     = ($this->quiet) ? new NullOutput() : $output;
         $excludeCampaigns = $input->getOption('exclude');
 
+        if (is_numeric($id)) {
+            $id = (int) $id;
+        }
+
+        if (is_numeric($maxThreads)) {
+            $maxThreads = (int) $maxThreads;
+        }
+
+        if (is_numeric($threadId)) {
+            $threadId = (int) $threadId;
+        }
+
+        if (is_numeric($contactMaxId)) {
+            $contactMaxId = (int) $contactMaxId;
+        }
+
+        if (is_numeric($contactMinId)) {
+            $contactMinId = (int) $contactMinId;
+        }
+
+        if (is_numeric($contactId)) {
+            $contactId = (int) $contactId;
+        }
+
         if ($threadId && $maxThreads && (int) $threadId > (int) $maxThreads) {
             $this->output->writeln('--thread-id cannot be larger than --max-thread');
 

diff --git a/app/bundles/CampaignBundle/Command/ValidateEventCommand.php b/app/bundles/CampaignBundle/Command/ValidateEventCommand.php
@@ -58,6 +58,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         $decisionId = $input->getOption('decision-id');
         $contactId  = $input->getOption('contact-id');
+
+        if (is_numeric($decisionId)) {
+            $decisionId = (int) $decisionId;
+        }
+
+        if (is_numeric($contactId)) {
+            $contactId = (int) $contactId;
+        }
+
         $contactIds = $this->formatterHelper->simpleCsvToArray($input->getOption('contact-ids'), 'int');
 
         if (!$contactIds && !$contactId) {

diff --git a/app/bundles/CampaignBundle/Controller/CampaignController.php b/app/bundles/CampaignBundle/Controller/CampaignController.php
@@ -693,7 +693,7 @@ protected function getCampaignSessionId(Campaign $campaign, $action, $objectId =
         } elseif ('new' === $action && empty($sessionId)) {
             $sessionId = 'mautic_'.sha1(uniqid(mt_rand(), true));
             if ($this->requestStack->getCurrentRequest()->request->has('campaign')) {
-                $campaign  = $this->requestStack->getCurrentRequest()->request->get('campaign') ?? [];
+                $campaign  = $this->requestStack->getCurrentRequest()->request->all()['campaign'] ?? [];
                 $sessionId = $campaign['sessionId'] ?? $sessionId;
             }
         } elseif ('edit' === $action) {

diff --git a/app/bundles/CampaignBundle/Tests/Controller/CampaignControllerFunctionalTest.php b/app/bundles/CampaignBundle/Tests/Controller/CampaignControllerFunctionalTest.php
@@ -46,7 +46,10 @@ protected function setUp(): void
         $this->configParams[self::CAMPAIGN_SUMMARY_PARAM] = in_array($this->getName(), $functionForUseSummary);
         $this->configParams[self::CAMPAIGN_RANGE_PARAM]   = in_array($this->getName(), $functionForUseRange);
         parent::setUp();
-        $this->campaignModel                                           = static::getContainer()->get('mautic.model.factory')->getModel('campaign');
+
+        $model = static::getContainer()->get(CampaignModel::class);
+
+        $this->campaignModel                                           = $model;
         $this->campaignLeadsLabel                                      = static::getContainer()->get('translator')->trans('mautic.campaign.campaign.leads');
         $this->configParams['delete_campaign_event_log_in_background'] = false;
     }

diff --git a/app/bundles/CampaignBundle/Tests/Executioner/Logger/EventLoggerTest.php b/app/bundles/CampaignBundle/Tests/Executioner/Logger/EventLoggerTest.php
@@ -20,7 +20,7 @@
 class EventLoggerTest extends TestCase
 {
     /**
-     * @var LeadRepository|MockObject
+     * @var IpLookupHelper&MockObject
      */
     private MockObject $ipLookupHelper;
 

diff --git a/app/bundles/CampaignBundle/Tests/Membership/MembershipManagerTest.php b/app/bundles/CampaignBundle/Tests/Membership/MembershipManagerTest.php
@@ -35,9 +35,6 @@ class MembershipManagerTest extends \PHPUnit\Framework\TestCase
      */
     private \PHPUnit\Framework\MockObject\MockObject $leadRepository;
 
-    /**
-     * @var NullLogger|\PHPUnit\Framework\MockObject\MockObject
-     */
     private NullLogger $logger;
 
     protected function setUp(): void

diff --git a/app/bundles/ChannelBundle/Command/SendChannelBroadcastCommand.php b/app/bundles/ChannelBundle/Command/SendChannelBroadcastCommand.php
@@ -93,6 +93,30 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $maxThreads    = $input->getOption('max-threads');
         $key           = sprintf('%s-%s-%s-%s', $channel, $channelId, $threadId, $maxThreads);
 
+        if (is_numeric($limit)) {
+            $limit = (int) $limit;
+        }
+
+        if (is_numeric($batch)) {
+            $batch = (int) $batch;
+        }
+
+        if (is_numeric($minContactId)) {
+            $minContactId = (int) $minContactId;
+        }
+
+        if (is_numeric($maxContactId)) {
+            $maxContactId = (int) $maxContactId;
+        }
+
+        if (is_numeric($threadId)) {
+            $threadId = (int) $threadId;
+        }
+
+        if (is_numeric($maxThreads)) {
+            $maxThreads = (int) $maxThreads;
+        }
+
         if ($threadId && $maxThreads) {
             if ((int) $threadId > (int) $maxThreads) {
                 $output->writeln('--thread-id cannot be larger than --max-thread');

diff --git a/app/bundles/ChannelBundle/Tests/EventListener/CampaignSubscriberTest.php b/app/bundles/ChannelBundle/Tests/EventListener/CampaignSubscriberTest.php
@@ -56,9 +56,6 @@ class CampaignSubscriberTest extends \PHPUnit\Framework\TestCase
      */
     private \PHPUnit\Framework\MockObject\MockObject $scheduler;
 
-    /**
-     * @var \PHPUnit\Framework\MockObject\MockObject|LegacyEventDispatcher
-     */
     private LegacyEventDispatcher $legacyDispatcher;
 
     protected function setUp(): void

diff --git a/app/bundles/CoreBundle/Factory/MauticFactory.php b/app/bundles/CoreBundle/Factory/MauticFactory.php
@@ -7,12 +7,18 @@
 use Mautic\CoreBundle\Entity\IpAddress;
 use Mautic\CoreBundle\Exception\FileNotFoundException;
 use Mautic\CoreBundle\Helper\DateTimeHelper;
+use Mautic\CoreBundle\Helper\ThemeHelper;
 use Mautic\CoreBundle\Helper\UserHelper;
 use Mautic\CoreBundle\Model\AbstractCommonModel;
 use Mautic\CoreBundle\Security\Permissions\CorePermissions;
 use Mautic\CoreBundle\Translation\Translator;
+use Mautic\CoreBundle\Twig\Helper\AssetsHelper;
+use Mautic\CoreBundle\Twig\Helper\SlotsHelper;
 use Mautic\EmailBundle\Helper\MailHelper;
+use Mautic\EmailBundle\MonitoredEmail\Mailbox;
+use Mautic\PluginBundle\Helper\IntegrationHelper;
 use Mautic\UserBundle\Entity\User;
+use Psr\Log\LoggerInterface;
 use Symfony\Bundle\FrameworkBundle\Routing\Router;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -36,7 +42,13 @@ public function __construct(
         private UserHelper $userHelper,
         private RequestStack $requestStack,
         private ManagerRegistry $doctrine,
-        private Translator $translator
+        private Translator $translator,
+        private Mailbox $mailbox,
+        private ThemeHelper $themeHelper,
+        private IntegrationHelper $integrationHelper,
+        private SlotsHelper $slotsHelper,
+        private AssetsHelper $assetsHelper,
+        private LoggerInterface $logger
     ) {
     }
 
@@ -324,15 +336,15 @@ public function getVersion()
      *
      * @param bool|false $system
      *
-     * @return \Monolog\Logger
+     * @return LoggerInterface
      */
     public function getLogger($system = false)
     {
         if ($system) {
-            return $this->container->get('logger');
-        } else {
-            return $this->container->get('monolog.logger.mautic');
+            return $this->logger;
         }
+
+        return $this->container->get('monolog.logger.mautic');
     }
 
     /**
@@ -342,14 +354,22 @@ public function getLogger($system = false)
      */
     public function getHelper($helper)
     {
-        return match ($helper) {
-            'template.assets'     => $this->container->get('twig.helper.assets'),
-            'template.slots'      => $this->container->get('twig.helper.slots'),
-            'template.form'       => $this->container->get('twig.helper.form'),
-            'template.translator' => $this->container->get('twig.helper.translator'),
-            'template.router'     => $this->container->get('twig.helper.router'),
-            default               => $this->container->get('mautic.helper.'.$helper),
-        };
+        switch ($helper) {
+            case 'mailbox':
+                return $this->mailbox;
+            case 'theme':
+                return $this->themeHelper;
+            case 'integration':
+                return $this->integrationHelper;
+            case 'template.slots':
+                return $this->slotsHelper;
+            case 'template.assets':
+                return $this->assetsHelper;
+            default:
+                @trigger_error('MauticFactory::getHelper with "'.$helper.'" is deprecated.', E_USER_DEPRECATED);
+
+                return $this->container->get('mautic.helper.'.$helper);
+        }
     }
 
     /**

diff --git a/app/bundles/CoreBundle/Form/ChoiceLoader/EntityLookupChoiceLoader.php b/app/bundles/CoreBundle/Form/ChoiceLoader/EntityLookupChoiceLoader.php
@@ -11,6 +11,7 @@
 use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
 use Symfony\Component\Form\FormEvent;
 use Symfony\Component\OptionsResolver\Options;
+use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 class EntityLookupChoiceLoader implements ChoiceLoaderInterface
@@ -26,19 +27,24 @@ class EntityLookupChoiceLoader implements ChoiceLoaderInterface
     protected $choices = [];
 
     /**
-     * @param ModelFactory<object> $modelFactory
-     * @param array                $options
+     * @param ModelFactory<object>               $modelFactory
+     * @param Options<array<mixed>>|array<mixed> $options
      */
     public function __construct(
         protected ModelFactory $modelFactory,
         protected TranslatorInterface $translator,
         protected Connection $connection,
         protected $options = []
     ) {
+        if (is_array($options)) {
+            $options = (new OptionsResolver())->setDefaults($options);
+        }
+
+        $this->options = $options;
     }
 
     /**
-     * @param Options|array $options
+     * @param Options<array<mixed>>|array<mixed> $options
      */
     public function setOptions($options): void
     {

diff --git a/app/bundles/CoreBundle/Test/AbstractMauticTestCase.php b/app/bundles/CoreBundle/Test/AbstractMauticTestCase.php
@@ -5,6 +5,7 @@
 use Doctrine\Common\DataFixtures\Executor\AbstractExecutor;
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\EntityManagerInterface;
 use Liip\TestFixturesBundle\Services\DatabaseToolCollection;
 use Liip\TestFixturesBundle\Services\DatabaseTools\AbstractDatabaseTool;
 use Mautic\EmailBundle\Mailer\Message\MauticMessage;
@@ -99,7 +100,8 @@ protected function setUpSymfony(array $defaultConfigOptions = []): void
         $this->client->disableReboot();
         $this->client->followRedirects(true);
 
-        $this->em         = static::getContainer()->get('doctrine')->getManager();
+        $this->em = static::getContainer()->get('doctrine')->getManager();
+        \assert($this->em instanceof EntityManagerInterface);
         $this->connection = $this->em->getConnection();
 
         $this->router = static::getContainer()->get('router');

diff --git a/app/bundles/CoreBundle/Tests/Functional/Helper/LanguageHelperTest.php b/app/bundles/CoreBundle/Tests/Functional/Helper/LanguageHelperTest.php
@@ -17,7 +17,7 @@ public function testGettingLanguageFiles(): void
 
         $languageFiles = $languageHelper->getLanguageFiles();
 
-        // As the list depends on linstalled plugins, let's assert only for random files that should exist.
+        // As the list depends on installed plugins, let's assert only for random files that should exist.
         Assert::assertMatchesRegularExpression('/app\/bundles\/EmailBundle\/Translations\/en_US\/(messages|validators|flashes)\.ini/', $languageFiles['EmailBundle'][0]);
         Assert::assertMatchesRegularExpression('/app\/bundles\/LeadBundle\/Translations\/en_US\/(messages|validators|flashes)\.ini/', $languageFiles['LeadBundle'][1]);
     }

diff --git a/app/bundles/CoreBundle/Tests/Traits/ControllerTrait.php b/app/bundles/CoreBundle/Tests/Traits/ControllerTrait.php
@@ -3,6 +3,8 @@
 namespace Mautic\CoreBundle\Tests\Traits;
 
 use Mautic\PageBundle\Tests\Controller\PageControllerTest;
+use PHPUnit\Framework\Assert;
+use Symfony\Component\HttpFoundation\Response;
 
 trait ControllerTrait
 {
@@ -41,6 +43,7 @@ protected function getControllerColumnTests(
             'GET',
             '/s/'.$urlAlias.'?tmpl=list&name='.$routeAlias.'&orderby='.$tableAlias.$column
         );
+        Assert::assertSame(Response::HTTP_OK, $this->client->getResponse()->getStatusCode());
         PageControllerTest::assertEquals(
             1,
             $crawler->filterXPath(

diff --git a/app/bundles/CoreBundle/Tests/Unit/Controller/AbstractFormControllerTest.php b/app/bundles/CoreBundle/Tests/Unit/Controller/AbstractFormControllerTest.php
@@ -19,9 +19,6 @@
 
 class AbstractFormControllerTest extends \PHPUnit\Framework\TestCase
 {
-    /**
-     * @var \PHPUnit\Framework\MockObject\MockObject|AbstractFormController
-     */
     private AbstractFormController $classFromAbstractFormController;
 
     /**
@@ -53,7 +50,7 @@ public function returnIsFormCancelled(Form $form): bool
                 return $this->isFormCancelled($form);
             }
         };
-        $this->formMock             = $this->createMock(Form::class);
+        $this->formMock = $this->createMock(Form::class);
     }
 
     /**