From db956a1f40b8ce1d67dd7e12c3f710d8a54753b8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 27 Dec 2017 16:23:54 +0100 Subject: [PATCH] Fixed all possible PHPStan errors --- .../Command/Shortcode/ResolveUrlCommand.php | 8 -------- .../Common/src/Exception/WrongIpException.php | 2 +- .../Common/src/Service/IpLocationResolver.php | 6 ++++-- .../Service/IpLocationResolverInterface.php | 7 +++++-- module/Common/src/Util/DateRange.php | 12 ++++++------ module/Core/src/Action/RedirectAction.php | 6 ------ .../src/Exception/InvalidUrlException.php | 4 ++-- module/Core/src/Service/ShortUrlService.php | 4 ++-- .../src/Service/ShortUrlServiceInterface.php | 2 +- module/Core/src/Service/Tag/TagService.php | 3 ++- module/Core/src/Service/UrlShortener.php | 10 ++++++---- module/Core/src/Service/VisitsTracker.php | 8 ++++---- .../src/Service/VisitsTrackerInterface.php | 4 ++-- .../Rest/src/Action/CreateShortcodeAction.php | 2 +- module/Rest/src/Action/GetVisitsAction.php | 4 ++-- module/Rest/src/Authentication/JWTService.php | 14 +++++++------- .../Authentication/JWTServiceInterface.php | 8 ++++---- .../CheckAuthenticationMiddleware.php | 5 +++-- module/Rest/src/Service/ApiKeyService.php | 19 +++++++++---------- .../src/Service/ApiKeyServiceInterface.php | 4 +++- .../test/Action/AuthenticateActionTest.php | 3 +++ 21 files changed, 67 insertions(+), 68 deletions(-) diff --git a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php index 8c9af2fd..539035b1 100644 --- a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php +++ b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php @@ -71,14 +71,6 @@ class ResolveUrlCommand extends Command try { $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); - if (! isset($longUrl)) { - $output->writeln(sprintf( - '' . $this->translator->translate('No URL found for short code "%s"') . '', - $shortCode - )); - return; - } - $output->writeln(sprintf('%s %s', $this->translator->translate('Long URL:'), $longUrl)); } catch (InvalidShortCodeException $e) { $output->writeln(sprintf('' . $this->translator->translate( diff --git a/module/Common/src/Exception/WrongIpException.php b/module/Common/src/Exception/WrongIpException.php index 0e70d4e2..837f9ec7 100644 --- a/module/Common/src/Exception/WrongIpException.php +++ b/module/Common/src/Exception/WrongIpException.php @@ -5,7 +5,7 @@ namespace Shlinkio\Shlink\Common\Exception; class WrongIpException extends RuntimeException { - public static function fromIpAddress($ipAddress, \Exception $prev = null) + public static function fromIpAddress($ipAddress, \Throwable $prev = null) { return new self(sprintf('Provided IP "%s" is invalid', $ipAddress), 0, $prev); } diff --git a/module/Common/src/Service/IpLocationResolver.php b/module/Common/src/Service/IpLocationResolver.php index 218d2288..93b3ccfe 100644 --- a/module/Common/src/Service/IpLocationResolver.php +++ b/module/Common/src/Service/IpLocationResolver.php @@ -22,15 +22,17 @@ class IpLocationResolver implements IpLocationResolverInterface } /** - * @param $ipAddress + * @param string $ipAddress * @return array + * @throws WrongIpException */ - public function resolveIpLocation($ipAddress) + public function resolveIpLocation(string $ipAddress): array { try { $response = $this->httpClient->get(sprintf(self::SERVICE_PATTERN, $ipAddress)); return json_decode((string) $response->getBody(), true); } catch (GuzzleException $e) { + /** @var \Throwable $e */ throw WrongIpException::fromIpAddress($ipAddress, $e); } } diff --git a/module/Common/src/Service/IpLocationResolverInterface.php b/module/Common/src/Service/IpLocationResolverInterface.php index 4f4279a2..fc1f07a3 100644 --- a/module/Common/src/Service/IpLocationResolverInterface.php +++ b/module/Common/src/Service/IpLocationResolverInterface.php @@ -3,11 +3,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Service; +use Shlinkio\Shlink\Common\Exception\WrongIpException; + interface IpLocationResolverInterface { /** - * @param $ipAddress + * @param string $ipAddress * @return array + * @throws WrongIpException */ - public function resolveIpLocation($ipAddress); + public function resolveIpLocation(string $ipAddress): array; } diff --git a/module/Common/src/Util/DateRange.php b/module/Common/src/Util/DateRange.php index 215de520..f58f2033 100644 --- a/module/Common/src/Util/DateRange.php +++ b/module/Common/src/Util/DateRange.php @@ -6,11 +6,11 @@ namespace Shlinkio\Shlink\Common\Util; class DateRange { /** - * @var \DateTimeInterface + * @var \DateTimeInterface|null */ private $startDate; /** - * @var \DateTimeInterface + * @var \DateTimeInterface|null */ private $endDate; @@ -21,7 +21,7 @@ class DateRange } /** - * @return \DateTimeInterface + * @return \DateTimeInterface|null */ public function getStartDate() { @@ -29,7 +29,7 @@ class DateRange } /** - * @return \DateTimeInterface + * @return \DateTimeInterface|null */ public function getEndDate() { @@ -39,8 +39,8 @@ class DateRange /** * @return bool */ - public function isEmpty() + public function isEmpty(): bool { - return is_null($this->startDate) && is_null($this->endDate); + return $this->startDate === null && $this->endDate === null; } } diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index e22639c7..066e9508 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -49,12 +49,6 @@ class RedirectAction implements MiddlewareInterface try { $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); - // If provided shortCode does not belong to a valid long URL, dispatch next middleware, which will trigger - // a not-found error - if ($longUrl === null) { - return $delegate->process($request); - } - // Track visit to this short code $this->visitTracker->track($shortCode, $request); diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 848cd71b..13e77c5d 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -7,9 +7,9 @@ use Shlinkio\Shlink\Common\Exception\RuntimeException; class InvalidUrlException extends RuntimeException { - public static function fromUrl($url, \Exception $previous = null) + public static function fromUrl($url, \Throwable $previous = null) { $code = isset($previous) ? $previous->getCode() : -1; - return new static(sprintf('Provided URL "%s" is not an exisitng and valid URL', $url), $code, $previous); + return new static(sprintf('Provided URL "%s" is not an existing and valid URL', $url), $code, $previous); } } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 2e22717b..85359060 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -49,9 +49,9 @@ class ShortUrlService implements ShortUrlServiceInterface * @return ShortUrl * @throws InvalidShortCodeException */ - public function setTagsByShortCode($shortCode, array $tags = []) + public function setTagsByShortCode($shortCode, array $tags = []): ShortUrl { - /** @var ShortUrl $shortUrl */ + /** @var ShortUrl|null $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, ]); diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index e299842d..1815c4de 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -24,5 +24,5 @@ interface ShortUrlServiceInterface * @return ShortUrl * @throws InvalidShortCodeException */ - public function setTagsByShortCode($shortCode, array $tags = []); + public function setTagsByShortCode($shortCode, array $tags = []): ShortUrl; } diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Service/Tag/TagService.php index b9427eee..247112c8 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/module/Core/src/Service/Tag/TagService.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service\Tag; use Doctrine\Common\Collections\Collection; +use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; @@ -15,7 +16,7 @@ class TagService implements TagServiceInterface use TagManagerTrait; /** - * @var EntityManagerInterface + * @var EntityManager|EntityManagerInterface */ private $em; diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index cd282035..6515fd0f 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -90,6 +90,7 @@ class UrlShortener implements UrlShortenerInterface int $maxVisits = null ): string { // If the url already exists in the database, just return its short code + /** @var ShortUrl|null $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'originalUrl' => $url, ]); @@ -148,6 +149,7 @@ class UrlShortener implements UrlShortenerInterface 'max' => 15, ]]); } catch (GuzzleException $e) { + /** @var \Throwable $e */ throw InvalidUrlException::fromUrl($url, $e); } } @@ -155,13 +157,13 @@ class UrlShortener implements UrlShortenerInterface /** * Generates the unique shortcode for an autoincrement ID * - * @param int $id + * @param float $id * @return string */ - private function convertAutoincrementIdToShortCode($id): string + private function convertAutoincrementIdToShortCode(float $id): string { - $id = ((int) $id) + 200000; // Increment the Id so that the generated shortcode is not too short - $length = strlen($this->chars); + $id += 200000; // Increment the Id so that the generated shortcode is not too short + $length = \strlen($this->chars); $code = ''; while ($id > 0) { diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 38e44fea..6147046b 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Repository\VisitRepository; class VisitsTracker implements VisitsTrackerInterface { /** - * @var EntityManagerInterface|EntityManager + * @var EntityManager|EntityManagerInterface */ private $em; @@ -66,14 +66,14 @@ class VisitsTracker implements VisitsTrackerInterface /** * Returns the visits on certain short code * - * @param $shortCode + * @param string $shortCode * @param DateRange $dateRange * @return Visit[] * @throws InvalidArgumentException */ - public function info($shortCode, DateRange $dateRange = null): array + public function info(string $shortCode, DateRange $dateRange = null): array { - /** @var ShortUrl $shortUrl */ + /** @var ShortUrl|null $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, ]); diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index d3295f37..a852c030 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -21,10 +21,10 @@ interface VisitsTrackerInterface /** * Returns the visits on certain short code * - * @param $shortCode + * @param string $shortCode * @param DateRange $dateRange * @return Visit[] * @throws InvalidArgumentException */ - public function info($shortCode, DateRange $dateRange = null): array; + public function info(string $shortCode, DateRange $dateRange = null): array; } diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortcodeAction.php index 8edcd499..56169592 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortcodeAction.php @@ -50,7 +50,7 @@ class CreateShortcodeAction extends AbstractRestAction */ public function process(Request $request, DelegateInterface $delegate) { - $postData = $request->getParsedBody(); + $postData = (array) $request->getParsedBody(); if (! isset($postData['longUrl'])) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/GetVisitsAction.php index 0d03acf1..490b0470 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/GetVisitsAction.php @@ -75,10 +75,10 @@ class GetVisitsAction extends AbstractRestAction /** * @param Request $request - * @param $key + * @param string $key * @return \DateTime|null */ - protected function getDateQueryParam(Request $request, $key) + private function getDateQueryParam(Request $request, string $key) { $query = $request->getQueryParams(); if (! isset($query[$key]) || empty($query[$key])) { diff --git a/module/Rest/src/Authentication/JWTService.php b/module/Rest/src/Authentication/JWTService.php index 74130438..6a0d3ac8 100644 --- a/module/Rest/src/Authentication/JWTService.php +++ b/module/Rest/src/Authentication/JWTService.php @@ -27,7 +27,7 @@ class JWTService implements JWTServiceInterface * @param int $lifetime * @return string */ - public function create(ApiKey $apiKey, $lifetime = self::DEFAULT_LIFETIME) + public function create(ApiKey $apiKey, $lifetime = self::DEFAULT_LIFETIME): string { $currentTimestamp = time(); @@ -48,7 +48,7 @@ class JWTService implements JWTServiceInterface * @return string * @throws AuthenticationException If the token has expired */ - public function refresh($jwt, $lifetime = self::DEFAULT_LIFETIME) + public function refresh(string $jwt, $lifetime = self::DEFAULT_LIFETIME): string { $payload = $this->getPayload($jwt); $payload['exp'] = time() + $lifetime; @@ -61,7 +61,7 @@ class JWTService implements JWTServiceInterface * @param string $jwt * @return bool */ - public function verify($jwt) + public function verify(string $jwt): bool { try { // If no exception is thrown while decoding the token, it is considered valid @@ -79,7 +79,7 @@ class JWTService implements JWTServiceInterface * @return array * @throws AuthenticationException If the token has expired */ - public function getPayload($jwt) + public function getPayload(string $jwt): array { try { return $this->decode($jwt); @@ -92,16 +92,16 @@ class JWTService implements JWTServiceInterface * @param array $data * @return string */ - protected function encode(array $data) + protected function encode(array $data): string { return JWT::encode($data, $this->appOptions->getSecretKey(), self::DEFAULT_ENCRYPTION_ALG); } /** - * @param $jwt + * @param string $jwt * @return array */ - protected function decode($jwt) + protected function decode(string $jwt): array { return (array) JWT::decode($jwt, $this->appOptions->getSecretKey(), [self::DEFAULT_ENCRYPTION_ALG]); } diff --git a/module/Rest/src/Authentication/JWTServiceInterface.php b/module/Rest/src/Authentication/JWTServiceInterface.php index 55c8fb36..ba162aa8 100644 --- a/module/Rest/src/Authentication/JWTServiceInterface.php +++ b/module/Rest/src/Authentication/JWTServiceInterface.php @@ -18,7 +18,7 @@ interface JWTServiceInterface * @param int $lifetime * @return string */ - public function create(ApiKey $apiKey, $lifetime = self::DEFAULT_LIFETIME); + public function create(ApiKey $apiKey, $lifetime = self::DEFAULT_LIFETIME): string; /** * Refreshes a token and returns it with the new expiration @@ -28,7 +28,7 @@ interface JWTServiceInterface * @return string * @throws AuthenticationException If the token has expired */ - public function refresh($jwt, $lifetime = self::DEFAULT_LIFETIME); + public function refresh(string $jwt, $lifetime = self::DEFAULT_LIFETIME): string; /** * Verifies that certain JWT is valid @@ -36,7 +36,7 @@ interface JWTServiceInterface * @param string $jwt * @return bool */ - public function verify($jwt); + public function verify(string $jwt): bool; /** * Decodes certain token and returns the payload @@ -45,5 +45,5 @@ interface JWTServiceInterface * @return array * @throws AuthenticationException If the token has expired */ - public function getPayload($jwt); + public function getPayload(string $jwt): array; } diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index b5823f4f..071eadba 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -55,13 +55,14 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeIn * * @return Response * @throws \InvalidArgumentException + * @throws \ErrorException */ public function process(Request $request, DelegateInterface $delegate) { // If current route is the authenticate route or an OPTIONS request, continue to the next middleware - /** @var RouteResult $routeResult */ + /** @var RouteResult|null $routeResult */ $routeResult = $request->getAttribute(RouteResult::class); - if (! isset($routeResult) + if ($routeResult === null || $routeResult->isFailure() || $routeResult->getMatchedRouteName() === AuthenticateAction::class || $request->getMethod() === 'OPTIONS' diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index e2473f1b..6f368e56 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -46,13 +46,9 @@ class ApiKeyService implements ApiKeyServiceInterface */ public function check($key) { - /** @var ApiKey $apiKey */ + /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); - if (! isset($apiKey)) { - return false; - } - - return $apiKey->isValid(); + return $apiKey !== null && $apiKey->isValid(); } /** @@ -60,12 +56,13 @@ class ApiKeyService implements ApiKeyServiceInterface * * @param string $key * @return ApiKey + * @throws InvalidArgumentException */ public function disable($key) { - /** @var ApiKey $apiKey */ + /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); - if (! isset($apiKey)) { + if ($apiKey === null) { throw new InvalidArgumentException(sprintf('API key "%s" does not exist and can\'t be disabled', $key)); } @@ -75,7 +72,7 @@ class ApiKeyService implements ApiKeyServiceInterface } /** - * Lists all existing appi keys + * Lists all existing api keys * * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] @@ -94,8 +91,10 @@ class ApiKeyService implements ApiKeyServiceInterface */ public function getByKey($key) { - return $this->em->getRepository(ApiKey::class)->findOneBy([ + /** @var ApiKey|null $apiKey */ + $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ 'key' => $key, ]); + return $apiKey; } } diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 51dadf7a..ceaa3b96 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Service; +use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ApiKeyServiceInterface @@ -28,11 +29,12 @@ interface ApiKeyServiceInterface * * @param string $key * @return ApiKey + * @throws InvalidArgumentException */ public function disable($key); /** - * Lists all existing appi keys + * Lists all existing api keys * * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 32079e9b..4fb2518f 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\JWTService; @@ -32,6 +33,8 @@ class AuthenticateActionTest extends TestCase { $this->apiKeyService = $this->prophesize(ApiKeyService::class); $this->jwtService = $this->prophesize(JWTService::class); + $this->jwtService->create(Argument::cetera())->willReturn(''); + $this->action = new AuthenticateAction( $this->apiKeyService->reveal(), $this->jwtService->reveal(),