Merge pull request #2258 from acelaya-forks/feature/phpstan-2

Update to PHPStan 2.0

CHANGELOG.md

@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 * Update to Shlink PHP coding standard 2.4
 * Update to `hidehalo/nanoid-php` 2.0
+* Update to PHPStan 2.0
 
 ### Deprecated
 * *Nothing*

composer.json

@@ -64,10 +64,10 @@
     "require-dev": {
         "devizzent/cebe-php-openapi": "^1.0.1",
         "devster/ubench": "^2.1",
-        "phpstan/phpstan": "^1.12",
+        "phpstan/phpstan": "^2.0",
-        "phpstan/phpstan-doctrine": "^1.5",
+        "phpstan/phpstan-doctrine": "^2.0",
-        "phpstan/phpstan-phpunit": "^1.4",
+        "phpstan/phpstan-phpunit": "^2.0",
-        "phpstan/phpstan-symfony": "^1.4",
+        "phpstan/phpstan-symfony": "^2.0",
         "phpunit/php-code-coverage": "^11.0",
         "phpunit/phpcov": "^10.0",
         "phpunit/phpunit": "^11.4",

module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php

@@ -22,7 +22,7 @@ class CreateShortUrlCommand extends Command
 {
     public const NAME = 'short-url:create';
 
-    private SymfonyStyle|null $io;
+    private SymfonyStyle $io;
     private readonly ShortUrlDataInput $shortUrlDataInput;
 
     public function __construct(

module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php

@@ -235,7 +235,7 @@ class ListShortUrlsCommand extends Command
         }
         if ($input->getOption('show-domain')) {
             $columnsMap['Domain'] = static fn (array $_, ShortUrl $shortUrl): string =>
-                $shortUrl->getDomain()?->authority ?? Domain::DEFAULT_AUTHORITY;
+                $shortUrl->getDomain()->authority ?? Domain::DEFAULT_AUTHORITY;
         }
         if ($input->getOption('show-api-key') || $input->getOption('show-api-key-name')) {
             $columnsMap['API Key Name'] = static fn (array $_, ShortUrl $shortUrl): string|null =>

module/CLI/src/Command/Visit/AbstractVisitsListCommand.php

@@ -61,8 +61,8 @@ abstract class AbstractVisitsListCommand extends Command
                 'date' => $visit->date->toAtomString(),
                 'userAgent' => $visit->userAgent,
                 'potentialBot' => $visit->potentialBot,
-                'country' => $visit->getVisitLocation()?->countryName ?? 'Unknown',
+                'country' => $visit->getVisitLocation()->countryName ?? 'Unknown',
-                'city' => $visit->getVisitLocation()?->cityName ?? 'Unknown',
+                'city' => $visit->getVisitLocation()->cityName ?? 'Unknown',
                 ...$extraFields,
             ];
 

module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php

@@ -41,22 +41,24 @@ class GeolocationDbUpdaterTest extends TestCase
     #[Test]
     public function properResultIsReturnedWhenLicenseIsMissing(): void
     {
-        $mustBeUpdated = fn () => self::assertTrue(true);
-
         $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false);
         $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException(
             new MissingLicenseException(''),
         );
         $this->geoLiteDbReader->expects($this->never())->method('metadata');
 
-        $result = $this->geolocationDbUpdater()->checkDbUpdate($mustBeUpdated);
+        $isCalled = false;
+        $result = $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void {
+            $isCalled = true;
+        });
+
+        self::assertTrue($isCalled);
         self::assertEquals(GeolocationResult::LICENSE_MISSING, $result);
     }
 
     #[Test]
     public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void
     {
-        $mustBeUpdated = fn () => self::assertTrue(true);
         $prev = new DbUpdateException('');
 
         $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false);
@@ -65,14 +67,17 @@ class GeolocationDbUpdaterTest extends TestCase
         )->willThrowException($prev);
         $this->geoLiteDbReader->expects($this->never())->method('metadata');
 
+        $isCalled = false;
         try {
-            $this->geolocationDbUpdater()->checkDbUpdate($mustBeUpdated);
+            $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void {
+                $isCalled = true;
+            });
             self::fail();
         } catch (Throwable $e) {
-            /** @var GeolocationDbUpdateFailedException $e */
             self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e);
             self::assertSame($prev, $e->getPrevious());
             self::assertFalse($e->olderDbExists());
+            self::assertTrue($isCalled);
         }
     }
 
@@ -92,7 +97,6 @@ class GeolocationDbUpdaterTest extends TestCase
             $this->geolocationDbUpdater()->checkDbUpdate();
             self::fail();
         } catch (Throwable $e) {
-            /** @var GeolocationDbUpdateFailedException $e */
             self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e);
             self::assertSame($prev, $e->getPrevious());
             self::assertTrue($e->olderDbExists());

module/Core/functions/functions.php

@@ -109,7 +109,7 @@ function normalizeLocale(string $locale): string
  * minimum quality
  *
  * @param non-empty-string $acceptLanguage
- * @return iterable<string>;
+ * @return iterable<string>
  */
 function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): iterable
 {

module/Core/src/Importer/ImportedLinksProcessor.php

@@ -8,11 +8,11 @@ use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface;
-use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
 use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
-use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
+use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository;
 use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkOrphanVisit;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@@ -93,7 +93,7 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface
         bool $importShortCodes,
         callable $skipOnShortCodeConflict,
     ): ShortUrlImporting {
-        /** @var ShortUrlRepositoryInterface $shortUrlRepo */
+        /** @var ShortUrlRepository $shortUrlRepo */
         $shortUrlRepo = $this->em->getRepository(ShortUrl::class);
         $alreadyImportedShortUrl = $shortUrlRepo->findOneByImportedUrl($importedUrl);
         if ($alreadyImportedShortUrl !== null) {
@@ -132,7 +132,7 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface
     {
         $iterable = $this->batchHelper->wrapIterable($orphanVisits, 100);
 
-        /** @var VisitRepositoryInterface $visitRepo */
+        /** @var VisitRepository $visitRepo */
         $visitRepo = $this->em->getRepository(Visit::class);
         $mostRecentOrphanVisit = $visitRepo->findMostRecentOrphanVisit();
 

module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php

@@ -11,8 +11,8 @@ use Doctrine\ORM\Events;
 use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Core\Tag\Entity\Tag;
-use Symfony\Component\Lock\Lock;
 use Symfony\Component\Lock\LockFactory;
+use Symfony\Component\Lock\SharedLockInterface;
 use Symfony\Component\Lock\Store\InMemoryStore;
 
 use function array_map;
@@ -24,9 +24,9 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
     private array $memoizedNewDomains = [];
     /** @var array<string, Tag> */
     private array $memoizedNewTags = [];
-    /** @var array<string, Lock> */
+    /** @var array<string, SharedLockInterface> */
     private array $tagLocks = [];
-    /** @var array<string, Lock> */
+    /** @var array<string, SharedLockInterface> */
     private array $domainLocks = [];
 
     public function __construct(
@@ -100,7 +100,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
     }
 
     /**
-     * @param array<string, Lock> $locks
+     * @param array<string, SharedLockInterface> $locks
      */
     private function lock(array &$locks, string $name): void
     {
@@ -112,7 +112,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
 
     /**
     /**
-     * @param array<string, Lock> $locks
+     * @param array<string, SharedLockInterface> $locks
      */
     private function releaseLock(array &$locks, string $name): void
     {

module/Core/src/Tag/Repository/TagRepository.php

@@ -45,7 +45,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
     public function findTagsWithInfo(TagsListFiltering|null $filtering = null): array
     {
         $orderField = OrderableField::toValidField($filtering?->orderBy?->field);
-        $orderDir = $filtering?->orderBy?->direction ?? 'ASC';
+        $orderDir = $filtering->orderBy->direction ?? 'ASC';
         $apiKey = $filtering?->apiKey;
         $conn = $this->getEntityManager()->getConnection();
 
@@ -113,8 +113,8 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
             ->from('(' . $tagsSubQb->getSQL() . ')', 't')
             ->leftJoin('t', '(' . $allVisitsSubQb->getSQL() . ')', 'v', $mainQb->expr()->eq('t.tag_id', 'v.tag_id'))
             ->leftJoin('t', '(' . $nonBotVisitsSubQb->getSQL() . ')', 'b', $mainQb->expr()->eq('t.tag_id', 'b.tag_id'))
-            ->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
+            ->setMaxResults($filtering->limit ?? PHP_INT_MAX)
-            ->setFirstResult($filtering?->offset ?? 0);
+            ->setFirstResult($filtering->offset ?? 0);
 
         $mainQb->orderBy(camelCaseToSnakeCase($orderField->value), $orderDir);
         if ($orderField !== OrderableField::TAG) {

module/Core/src/Tag/TagService.php

@@ -47,9 +47,11 @@ readonly class TagService implements TagServiceInterface
      */
     private function createPaginator(AdapterInterface $adapter, TagsParams $params): Paginator
     {
-        return (new Paginator($adapter))
+        $paginator = new Paginator($adapter);
-            ->setMaxPerPage($params->itemsPerPage)
+        $paginator->setMaxPerPage($params->itemsPerPage)
                   ->setCurrentPage($params->page);
+
+        return $paginator;
     }
 
     /**

module/Core/src/Visit/Repository/VisitRepository.php

@@ -11,7 +11,7 @@ use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
@@ -48,7 +48,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
         ShortUrlIdentifier $identifier,
         VisitsCountFiltering $filtering,
     ): QueryBuilder {
-        /** @var ShortUrlRepositoryInterface $shortUrlRepo */
+        /** @var ShortUrlRepository $shortUrlRepo */
         $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class);
         $shortUrlId = $shortUrlRepo->findOne($identifier, $filtering->apiKey?->spec())?->getId() ?? '-1';
 

module/Core/src/Visit/VisitsStatsHelper.php

@@ -14,7 +14,7 @@ use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Tag\Entity\Tag;
 use Shlinkio\Shlink\Core\Tag\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Visit\Entity\OrphanVisitsCount;
@@ -32,7 +32,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository;
 use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository;
-use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
+use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
@@ -70,13 +70,13 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
         VisitsParams $params,
         ApiKey|null $apiKey = null,
     ): Paginator {
-        /** @var ShortUrlRepositoryInterface $repo */
+        /** @var ShortUrlRepository $repo */
         $repo = $this->em->getRepository(ShortUrl::class);
         if (! $repo->shortCodeIsInUse($identifier, $apiKey?->spec())) {
             throw ShortUrlNotFoundException::fromNotFound($identifier);
         }
 
-        /** @var VisitRepositoryInterface $repo */
+        /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
 
         return $this->createPaginator(
@@ -96,7 +96,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
             throw TagNotFoundException::fromTag($tag);
         }
 
-        /** @var VisitRepositoryInterface $repo */
+        /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
 
         return $this->createPaginator(new TagVisitsPaginatorAdapter($repo, $tag, $params, $apiKey), $params);
@@ -113,7 +113,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
             throw DomainNotFoundException::fromAuthority($domain);
         }
 
-        /** @var VisitRepositoryInterface $repo */
+        /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
 
         return $this->createPaginator(new DomainVisitsPaginatorAdapter($repo, $domain, $params, $apiKey), $params);
@@ -124,7 +124,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
      */
     public function orphanVisits(OrphanVisitsParams $params, ApiKey|null $apiKey = null): Paginator
     {
-        /** @var VisitRepositoryInterface $repo */
+        /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
 
         return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params);
@@ -132,7 +132,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface
 
     public function nonOrphanVisits(VisitsParams $params, ApiKey|null $apiKey = null): Paginator
     {
-        /** @var VisitRepositoryInterface $repo */
+        /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
 
         return $this->createPaginator(new NonOrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params);

module/Core/test-db/Domain/Repository/DomainRepositoryTest.php

@@ -139,7 +139,7 @@ class DomainRepositoryTest extends DatabaseTestCase
                 {
                 }
 
-                public function resolveDomain(string|null $domain): Domain|null
+                public function resolveDomain(string|null $domain): Domain
                 {
                     return $this->domain;
                 }

module/Core/test/RedirectRule/ShortUrlRedirectRuleServiceTest.php

@@ -99,8 +99,6 @@ class ShortUrlRedirectRuleServiceTest extends TestCase
         $result = $this->ruleService->setRulesForShortUrl($shortUrl, $data);
 
         self::assertCount(2, $result);
-        self::assertInstanceOf(ShortUrlRedirectRule::class, $result[0]);
-        self::assertInstanceOf(ShortUrlRedirectRule::class, $result[1]);
     }
 
     #[Test]

module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php

@@ -90,8 +90,8 @@ class ShortUrlTitleResolutionHelperTest extends TestCase
     }
 
     #[Test]
-    #[TestWith(['TEXT/html; charset=utf-8'], name: 'charset')]
+    #[TestWith(['TEXT/html; charset=utf-8'], 'charset')]
-    #[TestWith(['TEXT/html'], name: 'no charset')]
+    #[TestWith(['TEXT/html'], 'no charset')]
     public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentType): void
     {
         $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]);

module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php

@@ -5,7 +5,6 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Rest\Middleware;
 
 use Laminas\Diactoros\Response\EmptyResponse;
-use Mezzio\Router\Middleware\ImplicitOptionsMiddleware;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ResponseFactoryInterface;
@@ -21,13 +20,6 @@ class EmptyResponseImplicitOptionsMiddlewareFactoryTest extends TestCase
         $this->factory = new EmptyResponseImplicitOptionsMiddlewareFactory();
     }
 
-    #[Test]
-    public function serviceIsCreated(): void
-    {
-        $instance = ($this->factory)();
-        self::assertInstanceOf(ImplicitOptionsMiddleware::class, $instance);
-    }
-
     #[Test]
     public function responsePrototypeIsEmptyResponse(): void
     {

phpstan.neon

@@ -10,7 +10,7 @@ parameters:
         - config
         - docker/config
     symfony:
-        console_application_loader: 'config/cli-app.php'
+        consoleApplicationLoader: 'config/cli-app.php'
     doctrine:
         repositoryClass: Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository
         objectManagerLoader: 'config/entity-manager.php'