feature #5478 Use PHPStan (javiereguiluz)

This PR was squashed before being merged into the 4.x branch. Discussion ---------- Use PHPStan This PR adds PHPStan and fixes (most) issues up to level 4. I used PHPStan in the past a bit, but this is the first time I'm using it seriously ... so please, tell me anything I did wrong and anything I'm missing. Thanks! Commits ------- 3c0d9cf Use PHPStan
EasyCorp · Nov 25, 2022 · 4d0bbbe · 4d0bbbe
2 parents 2c0921a + 3c0d9cf
commit 4d0bbbe
Show file tree

Hide file tree

Showing 73 changed files with 238 additions and 275 deletions.
diff --git a/.github/workflows/linter-phpstan.yaml b/.github/workflows/linter-phpstan.yaml
@@ -0,0 +1,36 @@
+# It checks the PHPStan rules on the entire codebase
+name: "Linter: PHPStan"
+
+on: [push, pull_request]
+
+env:
+    fail-fast: true
+
+permissions:
+    contents: read
+
+jobs:
+    phpstan:
+        name: PHPStan
+        runs-on: ubuntu-latest
+        steps:
+            - name: 'Checkout code'
+              uses: actions/checkout@v3
+
+            - name: 'Install PHP with extensions'
+              uses: shivammathur/setup-php@v2
+              with:
+                  coverage: none
+                  php-version: 8.2
+                  tools: composer:v2
+                  extensions: mbstring, intl, pdo, pdo_sqlite, sqlite3
+                  ini-values: date.timezone=UTC
+
+            - name: 'Install project dependencies'
+              run: |
+                  composer require --no-progress --no-scripts --no-plugins symfony/flex
+                  composer update --no-interaction --prefer-dist --optimize-autoloader --prefer-stable
+                  vendor/bin/simple-phpunit install
+
+            - name: 'Run PHPStan analysis'
+              run: vendor/bin/phpstan analyse
diff --git a/composer.json b/composer.json
@@ -39,6 +39,11 @@
     },
     "require-dev": {
         "doctrine/doctrine-fixtures-bundle": "^3.4",
+        "phpstan/extension-installer": "^1.2",
+        "phpstan/phpstan": "^1.9",
+        "phpstan/phpstan-phpunit": "^1.2",
+        "phpstan/phpstan-strict-rules": "^1.4",
+        "phpstan/phpstan-symfony": "^1.2",
         "psr/log": "^1.0",
         "symfony/browser-kit": "^5.4|^6.0",
         "symfony/css-selector": "^5.4|^6.0",
@@ -48,7 +53,8 @@
     "config": {
         "sort-packages": true,
         "allow-plugins": {
-            "symfony/flex": true
+            "symfony/flex": true,
+            "phpstan/extension-installer": true
         }
     },
     "autoload": {

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -0,0 +1,16 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Cannot unset offset string on array\\<int, EasyCorp\\\\Bundle\\\\EasyAdminBundle\\\\Dto\\\\ActionDto\\>\\.$#"
+			count: 1
+			path: src/Dto/ActionConfigDto.php
+
+		-
+			message: "#^Offset string on array\\<int, EasyCorp\\\\Bundle\\\\EasyAdminBundle\\\\Dto\\\\ActionDto\\> on left side of \\?\\? does not exist\\.$#"
+			count: 1
+			path: src/Dto/ActionConfigDto.php
+
+		-
+			message: "#^Variable method call on Twig\\\\Extension\\\\RuntimeExtensionInterface\\.$#"
+			count: 1
+			path: src/Twig/EasyAdminTwigExtension.php
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -0,0 +1,11 @@
+includes:
+	- phpstan-baseline.neon
+
+parameters:
+    level: 4
+    paths:
+        - src/
+    bootstrapFiles:
+        - vendor/bin/.phpunit/phpunit/vendor/autoload.php
+    ignoreErrors:
+        - '#Cannot use array destructuring on callable.#'
diff --git a/src/ArgumentResolver/BatchActionDtoResolver.php b/src/ArgumentResolver/BatchActionDtoResolver.php
@@ -42,7 +42,7 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
 
         yield new BatchActionDto(
             $context->getRequest()->request->get(EA::BATCH_ACTION_NAME),
-            $context->getRequest()->request->all()[EA::BATCH_ACTION_ENTITY_IDS] ?: [],
+            $context->getRequest()->request->all()[EA::BATCH_ACTION_ENTITY_IDS] ?? [],
             $context->getRequest()->request->get(EA::ENTITY_FQCN),
             $referrerUrl,
             $context->getRequest()->request->get(EA::BATCH_ACTION_CSRF_TOKEN)

diff --git a/src/Cache/CacheWarmer.php b/src/Cache/CacheWarmer.php
@@ -37,7 +37,7 @@ public function warmUp(string $cacheDir): array
         foreach ($allRoutes as $routeName => $route) {
             $controller = $route->getDefault('_controller') ?? '';
             // controller is defined as $router->add('admin', '/')->controller(DashboardController::class)
-            if (\is_string($controller) && !empty($controller) && class_exists($controller)) {
+            if (\is_string($controller) && '' !== $controller && class_exists($controller)) {
                 $controller .= '::__invoke';
             }
 

diff --git a/src/Collection/ActionCollection.php b/src/Collection/ActionCollection.php
@@ -71,7 +71,7 @@ public function count(): int
     }
 
     /**
-     * @return ActionDto[]
+     * @return \ArrayIterator<ActionDto>
      */
     public function getIterator(): \ArrayIterator
     {

diff --git a/src/Collection/EntityCollection.php b/src/Collection/EntityCollection.php
@@ -61,7 +61,7 @@ public function count(): int
     }
 
     /**
-     * @return EntityDto[]
+     * @return \ArrayIterator<EntityDto>
      */
     public function getIterator(): \ArrayIterator
     {

diff --git a/src/Collection/FieldCollection.php b/src/Collection/FieldCollection.php
@@ -80,7 +80,7 @@ public function prepend(FieldDto $newField): void
 
     public function first(): ?FieldDto
     {
-        if (empty($this->fields)) {
+        if (0 === \count($this->fields)) {
             return null;
         }
 
@@ -118,7 +118,7 @@ public function count(): int
     }
 
     /**
-     * @return FieldDto[]
+     * @return \ArrayIterator<FieldDto>
      */
     public function getIterator(): \ArrayIterator
     {

diff --git a/src/Collection/FilterCollection.php b/src/Collection/FilterCollection.php
@@ -64,7 +64,7 @@ public function count(): int
     }
 
     /**
-     * @return FilterDto[]
+     * @return \ArrayIterator<FilterDto>
      */
     public function getIterator(): \ArrayIterator
     {

diff --git a/src/Command/MakeAdminDashboardCommand.php b/src/Command/MakeAdminDashboardCommand.php
@@ -104,7 +104,7 @@ private function getSiteTitle(string $projectDir): string
             ->trim()
             ->toString();
 
-        return empty($guessedTitle) ? 'EasyAdmin' : $guessedTitle;
+        return '' === $guessedTitle ? 'EasyAdmin' : $guessedTitle;
     }
 
     private function getCommandHelp(): string

diff --git a/src/Config/Action.php b/src/Config/Action.php
@@ -42,10 +42,10 @@ public function __toString()
     }
 
     /**
-     * @param TranslatableInterface|string|false|null $label Use FALSE to hide the label; use NULL to autogenerate it
-     * @param string|null                             $icon  The full CSS classes of the FontAwesome icon to render (see https://fontawesome.com/v6/search?m=free)
+     * @param mixed       $label Use FALSE to hide the label; use NULL to autogenerate it
+     * @param string|null $icon  The full CSS classes of the FontAwesome icon to render (see https://fontawesome.com/v6/search?m=free)
      */
-    public static function new(string $name, /* TranslatableInterface|string|false|null */ $label = null, ?string $icon = null): self
+    public static function new(string $name, /** @var TranslatableInterface|string|false|null */ $label = null, ?string $icon = null): self
     {
         if (!\is_string($label)
             && !$label instanceof TranslatableInterface
@@ -89,9 +89,9 @@ public function createAsBatchAction(): self
     }
 
     /**
-     * @param TranslatableInterface|string|false|null $label Use FALSE to hide the label; use NULL to autogenerate it
+     * @param mixed $label Use FALSE to hide the label; use NULL to autogenerate it
      */
-    public function setLabel(/* TranslatableInterface|string|false|null */ $label): self
+    public function setLabel(/* @var TranslatableInterface|string|false|null */ $label): self
     {
         if (!\is_string($label)
             && !$label instanceof TranslatableInterface
@@ -193,13 +193,9 @@ public function linkToRoute(string $routeName, array|callable $routeParameters =
         return $this;
     }
 
-    /**
-     * @param string|callable $url
-     */
-    public function linkToUrl(/* string|callable */ $url): self
+    public function linkToUrl(/* @var string|callable */ $url): self
     {
-        if (!\is_string($url)
-            && !\is_callable($url)) {
+        if (!\is_string($url) && !\is_callable($url)) {
             trigger_deprecation(
                 'easycorp/easyadmin-bundle',
                 '4.0.5',

diff --git a/src/Config/Actions.php b/src/Config/Actions.php
@@ -122,7 +122,7 @@ public function disable(string ...$disabledActionNames): self
         // if 'delete' is disabled, 'batch delete' is disabled automatically (but the
         // opposite doesn't happen). This is the most common case, but user can re-enable
         // the 'batch delete' action if needed manually
-        if (\in_array(Action::DELETE, $disabledActionNames)) {
+        if (\in_array(Action::DELETE, $disabledActionNames, true)) {
             $disabledActionNames[] = Action::BATCH_DELETE;
         }
 

diff --git a/src/Config/Crud.php b/src/Config/Crud.php
@@ -42,13 +42,11 @@ public static function new(): self
     }
 
     /**
-     * @param TranslatableInterface|string|callable $label The callable signature is: fn ($entityInstance, $pageName): string
+     * @param $label The callable signature is: fn ($entityInstance, $pageName): string
      */
-    public function setEntityLabelInSingular(/* TranslatableInterface|string|callable */ $label): self
+    public function setEntityLabelInSingular(/* @var TranslatableInterface|string|callable */ $label): self
     {
-        if (!\is_string($label)
-            && !$label instanceof TranslatableInterface
-            && !\is_callable($label)) {
+        if (!\is_string($label) && !$label instanceof TranslatableInterface && !\is_callable($label)) {
             trigger_deprecation(
                 'easycorp/easyadmin-bundle',
                 '4.0.5',
@@ -66,13 +64,11 @@ public function setEntityLabelInSingular(/* TranslatableInterface|string|callabl
     }
 
     /**
-     * @param TranslatableInterface|string|callable $label The callable signature is: fn ($entityInstance, $pageName): string
+     * @param $label The callable signature is: fn ($entityInstance, $pageName): string
      */
-    public function setEntityLabelInPlural(/* TranslatableInterface|string|callable */ $label): self
+    public function setEntityLabelInPlural(/* @var TranslatableInterface|string|callable */ $label): self
     {
-        if (!\is_string($label)
-            && !$label instanceof TranslatableInterface
-            && !\is_callable($label)) {
+        if (!\is_string($label) && !$label instanceof TranslatableInterface && !\is_callable($label)) {
             trigger_deprecation(
                 'easycorp/easyadmin-bundle',
                 '4.0.5',
@@ -90,13 +86,11 @@ public function setEntityLabelInPlural(/* TranslatableInterface|string|callable
     }
 
     /**
-     * @param TranslatableInterface|string|callable $title The callable signature is: fn ($entityInstance): string
+     * @param $title The callable signature is: fn ($entityInstance): string
      */
-    public function setPageTitle(string $pageName, /* TranslatableInterface|string|callable */ $title): self
+    public function setPageTitle(string $pageName, /* @var TranslatableInterface|string|callable */ $title): self
     {
-        if (!\is_string($title)
-            && !$title instanceof TranslatableInterface
-            && !\is_callable($title)) {
+        if (!\is_string($title) && !$title instanceof TranslatableInterface && !\is_callable($title)) {
             trigger_deprecation(
                 'easycorp/easyadmin-bundle',
                 '4.0.5',
@@ -178,7 +172,7 @@ public function setDateTimeFormat(string $dateFormatOrPattern, string $timeForma
             throw new \InvalidArgumentException(sprintf('The first argument of the "%s()" method cannot be an empty string. Use either a date format (%s) or a datetime Intl pattern.', __METHOD__, implode(', ', DateTimeField::VALID_DATE_FORMATS)));
         }
 
-        $datePatternIsEmpty = DateTimeField::FORMAT_NONE === $dateFormatOrPattern || '' === trim($dateFormatOrPattern);
+        $datePatternIsEmpty = DateTimeField::FORMAT_NONE === $dateFormatOrPattern;
         $timePatternIsEmpty = DateTimeField::FORMAT_NONE === $timeFormat || '' === trim($timeFormat);
         if ($datePatternIsEmpty && $timePatternIsEmpty) {
             throw new \InvalidArgumentException(sprintf('The values of the arguments of "%s()" cannot be "%s" or an empty string at the same time. Change any of them (or both).', __METHOD__, DateTimeField::FORMAT_NONE));

diff --git a/src/Config/Filters.php b/src/Config/Filters.php
@@ -24,6 +24,9 @@ public static function new(): self
         return new self($dto);
     }
 
+    /**
+     * @param (FilterInterface & \Stringable)|string $propertyNameOrFilter
+     */
     public function add(FilterInterface|string $propertyNameOrFilter): self
     {
         $filterPropertyName = \is_string($propertyNameOrFilter) ? $propertyNameOrFilter : (string) $propertyNameOrFilter;

diff --git a/src/Config/KeyValueStore.php b/src/Config/KeyValueStore.php
@@ -29,7 +29,7 @@ public function isEmpty(): bool
 
     public function has(string $key): bool
     {
-        if (empty($this->map)) {
+        if (0 === \count($this->map)) {
             return false;
         }
 

diff --git a/src/Config/Locale.php b/src/Config/Locale.php
@@ -19,7 +19,7 @@ private function __construct(LocaleDto $localeDto)
 
     public function __toString()
     {
-        return $this->dto->getValue();
+        return $this->dto->getName();
     }
 
     public static function new(string $locale, string|null $label = null, ?string $icon = null): self

diff --git a/src/Controller/AbstractCrudController.php b/src/Controller/AbstractCrudController.php
@@ -482,7 +482,7 @@ public function renderFilters(AdminContext $context): KeyValueStore
         $this->container->get(EntityFactory::class)->processFields($context->getEntity(), $fields);
         $filters = $this->container->get(FilterFactory::class)->create($context->getCrud()->getFiltersConfig(), $context->getEntity()->getFields(), $context->getEntity());
 
-        /** @var FiltersFormType $filtersForm */
+        /** @var FormInterface&FiltersFormType $filtersForm */
         $filtersForm = $this->container->get(FormFactory::class)->createFiltersForm($filters, $context->getRequest());
         $formActionParts = parse_url($filtersForm->getConfig()->getAction());
         $queryString = $formActionParts[EA::QUERY] ?? '';

diff --git a/src/Controller/AbstractDashboardController.php b/src/Controller/AbstractDashboardController.php
@@ -70,14 +70,7 @@ public function configureUserMenu(UserInterface $user): UserMenu
             $userMenuItems[] = MenuItem::linkToExitImpersonation(t('user.exit_impersonation', domain: 'EasyAdminBundle'), 'fa-user-lock');
         }
 
-        $userName = '';
-        if (method_exists($user, '__toString')) {
-            $userName = (string) $user;
-        } elseif (method_exists($user, 'getUserIdentifier')) {
-            $userName = $user->getUserIdentifier();
-        } elseif (method_exists($user, 'getUsername')) {
-            $userName = $user->getUsername();
-        }
+        $userName = method_exists($user, '__toString') ? (string) $user : $user->getUserIdentifier();
 
         return UserMenu::new()
             ->displayUserName()

diff --git a/src/Dto/ActionConfigDto.php b/src/Dto/ActionConfigDto.php
@@ -11,7 +11,7 @@
 final class ActionConfigDto
 {
     private ?string $pageName = null;
-    /** @var ActionDto[] */
+    /** @var array<string, array<int, ActionDto>> */
     private array $actions = [
         Crud::PAGE_DETAIL => [],
         Crud::PAGE_EDIT => [],