Adding PHPStan checks (#76)

* Adding PHPStan checks * Add tests * use UnexpectedValueException
tattali · May 4, 2024 · 4cf9127 · 4cf9127
1 parent 8f92f8d
commit 4cf9127
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 26 deletions.
diff --git a/.github/workflows/code_checks.yaml b/.github/workflows/code_checks.yaml
@@ -18,6 +18,7 @@ jobs:
         stability: [prefer-stable]
         minimum-stability: [stable]
         symfony-version: [7.0.*]
+        is-current: [true]
         include:
           - php: '8.3'
             symfony-version: 7.0.*
@@ -34,7 +35,7 @@ jobs:
         uses: actions/cache@v4
         with:
           path: ~/.composer/cache/files
-          key: dependencies-php-${{ matrix.php }}-composer-${{ hashFiles('composer.json') }}
+          key: composer-packages-${{ hashFiles('composer.lock') }}
 
       - uses: shivammathur/setup-php@v2
         with:
@@ -50,6 +51,14 @@ jobs:
           composer config minimum-stability ${{ matrix.minimum-stability }}
           composer update --no-interaction --prefer-dist
 
+      - name: PHP-CS-Fixer
+        continue-on-error: ${{ !matrix.is-current }}
+        run: vendor/bin/php-cs-fixer fix --ansi -v --dry-run
+
+      - name: PHPStan
+        continue-on-error: ${{ !matrix.is-current }}
+        run: vendor/bin/phpstan analyse src tests --level 9
+
       - name: Execute tests
         env:
           SYMFONY_DEPRECATIONS_HELPER: weak

diff --git a/composer.json b/composer.json
@@ -29,7 +29,8 @@
         "symfony/dotenv": "^7.0",
         "symfony/phpunit-bridge": "^7.0",
         "symfony/yaml": "^7.0",
-        "friendsofphp/php-cs-fixer": "^3.54"
+        "friendsofphp/php-cs-fixer": "^3.54",
+        "phpstan/phpstan": "^1.10"
     },
     "suggest": {
         "symfony/orm-pack": "To support Doctrine ORM and Migration.",

diff --git a/phpunit.xml b/phpunit.xml
@@ -9,8 +9,6 @@
          failOnWarning="true">
   <php>
     <ini name="error_reporting" value="-1"/>
-    <ini name="intl.default_locale" value="en"/>
-    <ini name="intl.error_level" value="0"/>
     <ini name="memory_limit" value="-1"/>
   </php>
 

diff --git a/src/Controller/CalendarController.php b/src/Controller/CalendarController.php
@@ -6,12 +6,14 @@
 
 use CalendarBundle\Event\SetDataEvent;
 use CalendarBundle\Serializer\SerializerInterface;
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 
-class CalendarController
+class CalendarController extends AbstractController
 {
     public function __construct(
         private readonly EventDispatcherInterface $eventDispatcher,
@@ -20,10 +22,34 @@ public function __construct(
 
     public function load(Request $request): JsonResponse
     {
-        $start = new \DateTime($request->get('start'));
-        $end = new \DateTime($request->get('end'));
-        $filters = $request->get('filters', '{}');
-        $filters = \is_array($filters) ? $filters : json_decode($filters, true);
+        try {
+            $start = $request->get('start');
+            if ($start && \is_string($start)) {
+                $start = new \DateTime($start);
+            } else {
+                throw new \UnexpectedValueException('Query parameter "start" should be a string');
+            }
+
+            $end = $request->get('end');
+            if ($end && \is_string($end)) {
+                $end = new \DateTime($end);
+            } else {
+                throw new \UnexpectedValueException('Query parameter "end" should be a string');
+            }
+
+            $filters = $request->get('filters', '{}');
+            $filters = match (true) {
+                \is_array($filters) => $filters,
+                \is_string($filters) => json_decode($filters, true),
+                default => false,
+            };
+
+            if (!\is_array($filters)) {
+                throw new \UnexpectedValueException('Query parameter "filters" is not valid');
+            }
+        } catch (\UnexpectedValueException $e) {
+            throw new BadRequestHttpException($e->getMessage(), $e);
+        }
 
         $setDataEvent = $this->eventDispatcher->dispatch(new SetDataEvent($start, $end, $filters));
 

diff --git a/src/Entity/Event.php b/src/Entity/Event.php
@@ -8,6 +8,9 @@ class Event
 {
     protected bool $allDay = true;
 
+    /**
+     * @param mixed[] $options
+     */
     public function __construct(
         protected string $title,
         protected \DateTime $start,
@@ -75,27 +78,33 @@ public function setResourceId(?string $resourceId): void
         $this->resourceId = $resourceId;
     }
 
+    /**
+     * @return mixed[]
+     */
     public function getOptions(): array
     {
         return $this->options;
     }
 
+    /**
+     * @param mixed[] $options
+     */
     public function setOptions(array $options): void
     {
         $this->options = $options;
     }
 
-    public function getOption(int|string $name)
+    public function getOption(string $name): mixed
     {
         return $this->options[$name];
     }
 
-    public function addOption(int|string $name, $value): void
+    public function addOption(string $name, mixed $value): void
     {
         $this->options[$name] = $value;
     }
 
-    public function removeOption(int|string $name): mixed
+    public function removeOption(string $name): mixed
     {
         if (!isset($this->options[$name])) {
             return null;
@@ -107,19 +116,22 @@ public function removeOption(int|string $name): mixed
         return $removed;
     }
 
+    /**
+     * @return mixed[]
+     */
     public function toArray(): array
     {
         $event = [
             'title' => $this->getTitle(),
-            'start' => $this->getStart()->format(\DateTime::ATOM),
+            'start' => $this->getStart()?->format(\DateTime::ATOM),
             'allDay' => $this->isAllDay(),
         ];
 
-        if (null !== $this->getEnd()) {
+        if ($this->getEnd()) {
             $event['end'] = $this->getEnd()->format(\DateTime::ATOM);
         }
 
-        if (null !== $this->getResourceId()) {
+        if ($this->getResourceId()) {
             $event['resourceId'] = $this->getResourceId();
         }
 

diff --git a/src/Event/SetDataEvent.php b/src/Event/SetDataEvent.php
@@ -18,6 +18,9 @@ class SetDataEvent
      */
     private array $events;
 
+    /**
+     * @param mixed[] $filters
+     */
     public function __construct(
         private readonly \DateTime $start,
         private readonly \DateTime $end,
@@ -36,6 +39,9 @@ public function getEnd(): \DateTime
         return $this->end;
     }
 
+    /**
+     * @return mixed[]
+     */
     public function getFilters(): array
     {
         return $this->filters;

diff --git a/tests/Controller/CalendarControllerTest.php b/tests/Controller/CalendarControllerTest.php
@@ -13,6 +13,7 @@
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 
 final class CalendarControllerTest extends TestCase
 {
@@ -41,7 +42,7 @@ protected function setUp(): void
     public function testItProvidesAnEventsFeedForACalendar(): void
     {
         $this->request->method('get')
-            ->willReturnCallback(static fn (string $key) => match ($key) {
+            ->willReturnCallback(static fn(string $key) => match ($key) {
                 'start' => '2016-03-01',
                 'end' => '2016-03-19',
                 'filters' => '{}',
@@ -81,14 +82,14 @@ public function testItProvidesAnEventsFeedForACalendar(): void
 
         self::assertInstanceOf(JsonResponse::class, $response);
 
-        self::assertJson($response->getContent());
+        self::assertJson((string) $response->getContent());
         self::assertSame(JsonResponse::HTTP_OK, $response->getStatusCode());
     }
 
     public function testItNotFindAnyEvents(): void
     {
         $this->request->method('get')
-            ->willReturnCallback(static fn (string $key) => match ($key) {
+            ->willReturnCallback(static fn(string $key) => match ($key) {
                 'start' => '2016-03-01',
                 'end' => '2016-03-19',
                 'filters' => '{}',
@@ -116,7 +117,37 @@ public function testItNotFindAnyEvents(): void
 
         self::assertInstanceOf(JsonResponse::class, $response);
 
-        self::assertJson($response->getContent());
+        self::assertJson((string) $response->getContent());
         self::assertSame(JsonResponse::HTTP_NO_CONTENT, $response->getStatusCode());
     }
+
+    public function testItShouldThrowErrorOnStartParam(): void
+    {
+        $this->expectException(BadRequestHttpException::class);
+
+        $this->request->method('get')
+            ->willReturnCallback(static fn(string $key) => match ($key) {
+                'start' => '',
+                'end' => '',
+                default => throw new \LogicException(),
+            })
+        ;
+
+        $this->controller->load($this->request);
+    }
+
+    public function testItShouldThrowErrorOnEndParam(): void
+    {
+        $this->expectException(BadRequestHttpException::class);
+
+        $this->request->method('get')
+            ->willReturnCallback(static fn(string $key) => match ($key) {
+                'start' => '2016-03-01',
+                'end' => '',
+                default => throw new \LogicException(),
+            })
+        ;
+
+        $this->controller->load($this->request);
+    }
 }
diff --git a/tests/DependencyInjection/CalendarExtensionTest.php b/tests/DependencyInjection/CalendarExtensionTest.php
@@ -12,6 +12,10 @@ final class CalendarExtensionTest extends TestCase
 {
     private ContainerBuilder $builder;
     private CalendarExtension $loader;
+
+    /**
+     * @var array<array<mixed>>
+     */
     private array $configuration;
 
     protected function setUp(): void

diff --git a/tests/Entity/EventTest.php b/tests/Entity/EventTest.php
@@ -13,6 +13,7 @@ final class EventTest extends TestCase
     private \DateTime $start;
     private \DateTime $end;
     private string $resourceId;
+    /** @var mixed[] */
     private array $options;
     private Event $entity;
 
@@ -63,7 +64,7 @@ public function testItShouldConvertItsValuesInToArray(): void
         $this->entity->setOptions($options);
         self::assertSame($options, $this->entity->getOptions());
 
-        self::assertSame($optionValue, $this->entity->getOption($optionName, $optionValue));
+        self::assertSame($optionValue, $this->entity->getOption($optionName));
 
         self::assertSame(
             [

diff --git a/tests/Event/SetDataEventTest.php b/tests/Event/SetDataEventTest.php
@@ -13,11 +13,10 @@ final class SetDataEventTest extends TestCase
 {
     private \DateTime $start;
     private \DateTime $end;
+    /** @var mixed[] */
     private array $filters;
-    /** @var Event&MockObject */
-    private $eventEntity;
-    /** @var Event&MockObject */
-    private $eventEntity2;
+    private Event&MockObject $eventEntity;
+    private Event&MockObject $eventEntity2;
     private SetDataEvent $event;
 
     protected function setUp(): void

diff --git a/tests/Serializer/SerializerTest.php b/tests/Serializer/SerializerTest.php
@@ -6,12 +6,13 @@
 
 use CalendarBundle\Entity\Event;
 use CalendarBundle\Serializer\Serializer;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 final class SerializerTest extends TestCase
 {
-    private $eventEntity1;
-    private $eventEntity2;
+    private Event&MockObject $eventEntity1;
+    private Event&MockObject $eventEntity2;
     private Serializer $serializer;
 
     protected function setUp(): void