Add PHPStan and fix all errors (#61)

Author: yann-eugone

.github/workflows/tests.yml

@@ -80,11 +80,31 @@ jobs:
       - name: "Setup PHP"
         uses: shivammathur/setup-php@v2
         with:
-          coverage: xdebug
+          coverage: none
           php-version: '8.3'
 
       - name: "Install dependencies with composer"
         run: composer update --no-interaction --no-progress
 
       - name: "Run checkstyle with symplify/easy-coding-standard"
         run: vendor/bin/ecs
+
+  phpstan:
+    name: "PHPStan"
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: "Checkout"
+        uses: actions/checkout@v2
+
+      - name: "Setup PHP"
+        uses: shivammathur/setup-php@v2
+        with:
+          coverage: none
+          php-version: '8.3'
+
+      - name: "Install dependencies with composer"
+        run: composer update --no-interaction --no-progress
+
+      - name: "Run static analysis with phpstan/phpstan"
+        run: vendor/bin/phpstan

composer.json

@@ -16,6 +16,7 @@
     "require-dev": {
         "doctrine/annotations": "^1.14",
         "myclabs/php-enum": "^1.8",
+        "phpstan/phpstan": "^1.11",
         "phpunit/phpunit": "^9.6",
         "symfony/form": "^7.0",
         "symfony/http-kernel": "^7.0",

phpstan-baseline.neon

@@ -0,0 +1,6 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Parameter \\#1 \\$objectOrClass of class ReflectionClass constructor expects class\\-string\\<T of object\\>\\|T of object, string given\\.$#"
+			count: 1
+			path: src/ConstantExtractor.php

phpstan.neon

@@ -0,0 +1,7 @@
+includes:
+  - phpstan-baseline.neon
+
+parameters:
+  level: max
+  paths:
+    - src/

src/ConstantExtractor.php

@@ -16,6 +16,7 @@
 final class ConstantExtractor
 {
     /**
+     * @return list<mixed>
      * @throws LogicException
      */
     public static function extract(string $pattern): array
@@ -29,17 +30,25 @@ public static function extract(string $pattern): array
         );
     }
 
+    /**
+     * @param array<string, mixed> $constants
+     *
+     * @return list<mixed>
+     */
     private static function filter(array $constants, string $regexp, string $pattern): array
     {
         $matchingNames = \preg_grep($regexp, \array_keys($constants));
 
-        if (\count($matchingNames) === 0) {
+        if ($matchingNames === false || \count($matchingNames) === 0) {
             throw LogicException::cannotExtractConstants($pattern, 'Pattern matches no constant.');
         }
 
         return \array_values(\array_intersect_key($constants, \array_flip($matchingNames)));
     }
 
+    /**
+     * @return array<string, mixed>
+     */
     private static function publicConstants(string $class, string $pattern): array
     {
         try {
@@ -67,6 +76,9 @@ private static function publicConstants(string $class, string $pattern): array
         return $list;
     }
 
+    /**
+     * @return array{string, string}
+     */
     private static function explode(string $pattern): array
     {
         if (\substr_count($pattern, '::') !== 1) {

src/ConstantListEnum.php

@@ -4,14 +4,24 @@
 
 namespace Yokai\EnumBundle;
 
+use Yokai\EnumBundle\Exception\LogicException;
+
 /**
  * @author Yann Eugoné <[email protected]>
  */
 class ConstantListEnum extends Enum
 {
     public function __construct(string $constantsPattern, ?string $name = null)
     {
-        $values = ConstantExtractor::extract($constantsPattern);
-        parent::__construct(\array_combine($values, $values), $name);
+        $choices = [];
+        foreach (ConstantExtractor::extract($constantsPattern) as $value) {
+            if (!\is_string($value) && !\is_int($value)) {
+                throw new LogicException(
+                    \sprintf('Extracted constant enum value must be string or int, %s given.', \get_debug_type($value)),
+                );
+            }
+            $choices[(string)$value] = $value;
+        }
+        parent::__construct($choices, $name);
     }
 }

src/Enum.php

@@ -92,6 +92,9 @@ protected function build(): array
         throw new LogicException(static::class . '::' . __FUNCTION__ . ' should have been overridden.');
     }
 
+    /**
+     * @phpstan-assert !null $this->choices
+     */
     private function init(): void
     {
         if ($this->choices !== null) {

src/Form/Extension/EnumTypeGuesser.php

@@ -35,17 +35,17 @@ public function guessTypeForConstraint(Constraint $constraint): ?TypeGuess
         );
     }
 
-    public function guessRequired($class, $property): ?ValueGuess
+    public function guessRequired(string $class, string $property): ?ValueGuess
     {
         return null; //override parent : not able to guess
     }
 
-    public function guessMaxLength($class, $property): ?ValueGuess
+    public function guessMaxLength(string $class, string $property): ?ValueGuess
     {
         return null; //override parent : not able to guess
     }
 
-    public function guessPattern($class, $property): ?ValueGuess
+    public function guessPattern(string $class, string $property): ?ValueGuess
     {
         return null; //override parent : not able to guess
     }

src/Form/Type/EnumType.php

@@ -40,7 +40,9 @@ function (string $name): bool {
             ->setDefault(
                 'choices',
                 function (Options $options): array {
-                    $choices = $this->enumRegistry->get($options['enum'])->getChoices();
+                    /** @var string $name */
+                    $name = $options['enum'];
+                    $choices = $this->enumRegistry->get($name)->getChoices();
 
                     if ($options['enum_choice_value'] === null) {
                         foreach ($choices as $value) {

src/TranslatedEnum.php

@@ -13,7 +13,7 @@
 class TranslatedEnum extends Enum
 {
     /**
-     * @var array
+     * @var array<int|string, mixed>
      */
     private $values;
 
@@ -64,6 +64,9 @@ protected function build(): array
             if (\is_string($key)) {
                 $transLabel = $key;
             }
+            if (!\is_scalar($transLabel)) {
+                $transLabel = $key;
+            }
 
             $label = $this->translator->trans(\sprintf($this->transPattern, $transLabel), [], $this->transDomain);
             $choices[$label] = $value;

src/Validator/Constraints/Enum.php

@@ -20,9 +20,13 @@ final class Enum extends Choice
      */
     public $enum;
 
+    /**
+     * @param array<string, mixed> $options
+     */
     public function __construct(
-        $enum = null,
-        $callback = null,
+        array $options = [],
+        string|null $enum = null,
+        callable|null|string $callback = null,
         bool $multiple = null,
         bool $strict = null,
         int $min = null,
@@ -31,57 +35,29 @@ public function __construct(
         string $multipleMessage = null,
         string $minMessage = null,
         string $maxMessage = null,
-        $groups = null,
-        $payload = null,
-        array $options = []
+        array|null $groups = null,
+        mixed $payload = null,
     ) {
-        if (\is_array($enum)) {
-            // Symfony 4.4 Constraints has single constructor argument containing all options
-            parent::__construct($enum);
-        } else {
-            if (\is_string($enum)) {
-                $this->enum = $enum;
-            }
-            // Symfony 5.x Constraints has many constructor arguments for PHP 8.0 Attributes support
-
-            $firstConstructorArg = (new \ReflectionClass(Choice::class))
-                ->getConstructor()->getParameters()[0]->getName();
-            if ($firstConstructorArg === 'choices') {
-                // Prior to Symfony 5.3, first argument of Choice was $choices
-                parent::__construct(
-                    null,
-                    $callback,
-                    $multiple,
-                    $strict,
-                    $min,
-                    $max,
-                    $message,
-                    $multipleMessage,
-                    $minMessage,
-                    $maxMessage,
-                    $groups,
-                    $payload,
-                    $options
-                );
-            } else {
-                // Since Symfony 5.3, first argument of Choice is $options
-                parent::__construct(
-                    $options,
-                    null,
-                    $callback,
-                    $multiple,
-                    $strict,
-                    $min,
-                    $max,
-                    $message,
-                    $multipleMessage,
-                    $minMessage,
-                    $maxMessage,
-                    $groups,
-                    $payload
-                );
-            }
+        if (\is_string($enum)) {
+            $this->enum = $enum;
         }
+
+        // Since Symfony 5.3, first argument of Choice is $options
+        parent::__construct(
+            $options,
+            null,
+            $callback,
+            $multiple,
+            $strict,
+            $min,
+            $max,
+            $message,
+            $multipleMessage,
+            $minMessage,
+            $maxMessage,
+            $groups,
+            $payload
+        );
     }
 
     public function getDefaultOption(): string

src/Validator/Constraints/EnumValidator.php

@@ -25,7 +25,7 @@ public function __construct(EnumRegistry $enumRegistry)
         $this->enumRegistry = $enumRegistry;
     }
 
-    public function validate($value, Constraint $constraint): void
+    public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$constraint instanceof Enum) {
             throw new UnexpectedTypeException($constraint, Enum::class);

tests/Unit/ConstantListEnumTest.php

@@ -67,4 +67,10 @@ public function testLabelNotFound(): void
 
         $enum->getLabel('unknown');
     }
+
+    public function testConstantMustBeStringOrInt(): void
+    {
+        $this->expectException(LogicException::class);
+        new ConstantListEnum(Vehicle::class . '::PERIOD_*', 'vehicle.period');
+    }
 }

tests/Unit/Fixtures/Vehicle.php

@@ -25,4 +25,8 @@ class Vehicle
     public const WHEELS_TWO = 2;
     public const WHEELS_FOUR = 4;
     public const WHEELS_EIGHT = 8;
+
+    public const PERIOD_COLLECTION = [1817, 1980];
+    public const PERIOD_OLD = [1981, 2010];
+    public const PERIOD_RECENT = [2010, 2024];
 }

tests/Unit/Form/Extension/EnumTypeGuesserTest.php

@@ -70,7 +70,7 @@ protected function getConstraints(array $options): array
             ->with(self::TEST_CLASS)
             ->willReturn($metadata);
 
-        $this->guesser = new EnumTypeGuesser($this->metadataFactory, $this->enumRegistry);
+        $this->guesser = new EnumTypeGuesser($this->metadataFactory);
 
         parent::setUp();
     }

tests/Unit/Validator/Constraints/EnumValidatorTest.php

@@ -46,19 +46,19 @@ public function testEnumIsRequired(): void
     public function testValidEnumIsRequired(): void
     {
         $this->expectException(ConstraintDefinitionException::class);
-        $this->validator->validate('foo', new Enum('state'));
+        $this->validator->validate('foo', new Enum(enum: 'state'));
     }
 
     public function testNullIsValid(): void
     {
-        $this->validator->validate(null, new Enum('type'));
+        $this->validator->validate(null, new Enum(enum: 'type'));
 
         $this->assertNoViolation();
     }
 
     public function testValidSingleEnum(): void
     {
-        $this->validator->validate('customer', new Enum('type'));
+        $this->validator->validate('customer', new Enum(enum: 'type'));
 
         $this->assertNoViolation();
     }