From 4cf9127967e43344a63a729104cdc5fc3663e52c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Attali?= Date: Sat, 4 May 2024 23:02:22 +0200 Subject: [PATCH] Adding PHPStan checks (#76) * Adding PHPStan checks * Add tests * use UnexpectedValueException --- .github/workflows/code_checks.yaml | 11 +++++- composer.json | 3 +- phpunit.xml | 2 - src/Controller/CalendarController.php | 36 ++++++++++++++--- src/Entity/Event.php | 24 +++++++++--- src/Event/SetDataEvent.php | 6 +++ tests/Controller/CalendarControllerTest.php | 39 +++++++++++++++++-- .../CalendarExtensionTest.php | 4 ++ tests/Entity/EventTest.php | 3 +- tests/Event/SetDataEventTest.php | 7 ++-- tests/Serializer/SerializerTest.php | 5 ++- 11 files changed, 114 insertions(+), 26 deletions(-) diff --git a/.github/workflows/code_checks.yaml b/.github/workflows/code_checks.yaml index e765450..4be9f9b 100644 --- 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 index 7406e16..2464eb1 100644 --- 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 index 4fe4af4..910b0ce 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -9,8 +9,6 @@ failOnWarning="true"> - - diff --git a/src/Controller/CalendarController.php b/src/Controller/CalendarController.php index 39c34b0..1225638 100755 --- 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 index bb2b044..c256f16 100755 --- 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 index f772f7a..c60784a 100644 --- 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 index 737353d..ca50aa3 100755 --- 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 index 3b5d8e1..b4c1e57 100644 --- 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> + */ private array $configuration; protected function setUp(): void diff --git a/tests/Entity/EventTest.php b/tests/Entity/EventTest.php index ebd3be4..03a2cba 100755 --- 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 index 6f7ea10..be650ad 100755 --- 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 index 01fc59f..8d2f2e0 100755 --- 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