diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 042a4b3c6..aab499115 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,241 +1,2 @@ parameters: - ignoreErrors: - - - rawMessage: 'Parameter #2 $array of function implode expects array, list given.' - identifier: argument.type - count: 1 - path: src/Command/TtSyncSubticketsCommand.php - - - - rawMessage: 'Parameter #2 $array of function implode expects array, array given.' - identifier: argument.type - count: 2 - path: src/Service/Integration/Jira/JiraAuthenticationService.php - - - - rawMessage: 'Parameter #2 $array of function implode expects array, array given.' - identifier: argument.type - count: 1 - path: src/Service/Integration/Jira/JiraHttpClientService.php - - - - rawMessage: 'Parameter #2 $array of function implode expects array, list given.' - identifier: argument.type - count: 1 - path: src/Service/Integration/Jira/JiraHttpClientService.php - - - - rawMessage: 'Parameter #1 $array of function natcasesort expects an array of values castable to string, list given.' - identifier: argument.type - count: 1 - path: src/Service/SubticketSyncService.php - - - - rawMessage: 'Parameter #2 $array of function implode expects array, array given.' - identifier: argument.type - count: 1 - path: src/Service/SubticketSyncService.php - - - - rawMessage: 'Property Tests\AbstractWebTestCase::$filepath (string) on left side of ?? is not nullable.' - identifier: nullCoalesce.property - count: 1 - path: tests/AbstractWebTestCase.php - - - - rawMessage: Cannot access offset 'error' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 5 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: Cannot access offset 'exception' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 1 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: Cannot access offset 'message' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 5 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: Cannot access offset 'redirect_url' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 2 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Cannot call method getContent() on Symfony\Component\HttpFoundation\Response|null.' - identifier: method.nonObject - count: 3 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Dynamic call to static method PHPUnit\Framework\Assert::callback().' - identifier: staticMethod.dynamicCall - count: 3 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Method App\Tests\EventSubscriber\ExceptionSubscriberTest::provideDefaultMessages() should return array but returns array{400: array{400, ''The request was…''}, 401: array{401, ''Authentication is…''}, 403: array{403, ''You do not have…''}, 404: array{404, ''The requested…''}, 405: array{405, ''The request method…''}, 406: array{406, ''The requested…''}, 409: array{409, ''The request…''}, 422: array{422, ''The request was…''}, ...}.' - identifier: return.type - count: 1 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Parameter #2 $array of method PHPUnit\Framework\Assert::assertArrayHasKey() expects array|ArrayAccess<(int|string), mixed>, mixed given.' - identifier: argument.type - count: 1 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Parameter #2 $array of method PHPUnit\Framework\Assert::assertArrayNotHasKey() expects array|ArrayAccess<(int|string), mixed>, mixed given.' - identifier: argument.type - count: 4 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Parameter #2 $haystack of method PHPUnit\Framework\Assert::assertStringContainsString() expects string, mixed given.' - identifier: argument.type - count: 1 - path: tests/EventSubscriber/ExceptionSubscriberTest.php - - - - rawMessage: 'Parameter #1 $array of function array_sum expects an array of values castable to number, list given.' - identifier: argument.type - count: 2 - path: tests/Performance/PerformanceBenchmarkRunner.php - - - - rawMessage: 'Parameter #1 $callbacks of method App\Service\Cache\QueryCacheService::warmUp() expects array, array{key1: Closure(): ''value1'', key2: ''not_a_callable''} given.' - identifier: argument.type - count: 1 - path: tests/Service/Cache/QueryCacheServiceTest.php - - - - rawMessage: 'Parameter #2 $haystack of method PHPUnit\Framework\Assert::assertContains() expects iterable, mixed given.' - identifier: argument.type - count: 4 - path: tests/Service/Cache/QueryCacheServiceTest.php - - - - rawMessage: 'Parameter #2 $haystack of method PHPUnit\Framework\Assert::assertNotContains() expects iterable, mixed given.' - identifier: argument.type - count: 1 - path: tests/Service/Cache/QueryCacheServiceTest.php - - - - rawMessage: Binary operation "." between 'encrypted_' and mixed results in an error. - identifier: binaryOp.invalid - count: 3 - path: tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php - - - - rawMessage: 'Call to method PHPUnit\Framework\Assert::assertTrue() with true will always evaluate to true.' - identifier: method.alreadyNarrowedType - count: 2 - path: tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php - - - - rawMessage: Unreachable statement - code above always terminates. - identifier: deadCode.unreachable - count: 1 - path: tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php - - - - rawMessage: Access to an undefined property object::$id. - identifier: property.notFound - count: 1 - path: tests/Service/Integration/Jira/JiraHttpClientServiceTest.php - - - - rawMessage: Access to an undefined property object::$key. - identifier: property.notFound - count: 1 - path: tests/Service/Integration/Jira/JiraHttpClientServiceTest.php - - - - rawMessage: 'Call to method PHPUnit\Framework\Assert::assertInstanceOf() with ''GuzzleHttp\\Client'' and GuzzleHttp\Client will always evaluate to true.' - identifier: method.alreadyNarrowedType - count: 3 - path: tests/Service/Integration/Jira/JiraHttpClientServiceTest.php - - - - rawMessage: Cannot access property $id on mixed. - identifier: property.nonObject - count: 2 - path: tests/Service/Integration/Jira/JiraHttpClientServiceTest.php - - - - rawMessage: 'Parameter #1 $exception of method PHPUnit\Framework\TestCase::expectException() expects class-string, string given.' - identifier: argument.type - count: 1 - path: tests/Service/Integration/Jira/JiraHttpClientServiceTest.php - - - - rawMessage: Cannot access offset 'description' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 2 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access offset 'issuetype' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 2 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access offset 'key' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 1 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access offset 'name' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 2 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access offset 'project' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 1 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access offset 'summary' on mixed. - identifier: offsetAccess.nonOffsetAccessible - count: 3 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access property $id on mixed. - identifier: property.nonObject - count: 1 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access property $key on mixed. - identifier: property.nonObject - count: 2 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: Cannot access property $total on mixed. - identifier: property.nonObject - count: 1 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: 'Dynamic call to static method PHPUnit\Framework\Assert::callback().' - identifier: staticMethod.dynamicCall - count: 5 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php - - - - rawMessage: 'Parameter #1 $haystack of function str_contains expects string, mixed given.' - identifier: argument.type - count: 4 - path: tests/Service/Integration/Jira/JiraTicketServiceTest.php + ignoreErrors: [] diff --git a/src/Service/Cache/QueryCacheService.php b/src/Service/Cache/QueryCacheService.php index fff86650d..c34a82beb 100644 --- a/src/Service/Cache/QueryCacheService.php +++ b/src/Service/Cache/QueryCacheService.php @@ -216,7 +216,7 @@ public function warmUp(array $callbacks): void /** * Gets cache statistics. * - * @return array + * @return array{adapter: class-string, tags: list, tag_count: int<0, max>} */ public function getStats(): array { diff --git a/src/Service/Integration/Jira/JiraAuthenticationService.php b/src/Service/Integration/Jira/JiraAuthenticationService.php index 003a4568a..fbff967b1 100644 --- a/src/Service/Integration/Jira/JiraAuthenticationService.php +++ b/src/Service/Integration/Jira/JiraAuthenticationService.php @@ -24,6 +24,7 @@ use Throwable; use function is_array; +use function is_scalar; use function is_string; use function sprintf; @@ -136,9 +137,7 @@ private function extractTokens(ResponseInterface $response): array parse_str($responseBody, $tokens); if (isset($tokens['oauth_problem'])) { - $problem = is_array($tokens['oauth_problem']) - ? implode(', ', $tokens['oauth_problem']) - : $tokens['oauth_problem']; + $problem = $this->stringifyParsedValue($tokens['oauth_problem'], ', '); throw new JiraApiException(sprintf('OAuth problem: %s', $problem), 401); } @@ -146,13 +145,38 @@ private function extractTokens(ResponseInterface $response): array $result = []; foreach ($tokens as $key => $value) { if (is_string($key)) { - $result[$key] = is_array($value) ? implode(',', $value) : $value; + $result[$key] = $this->stringifyParsedValue($value, ','); } } return $result; } + /** + * Recursively coerces a value parsed from {@see parse_str()} into a string. + */ + private function stringifyParsedValue(mixed $value, string $separator): string + { + if (is_array($value)) { + $parts = []; + foreach ($value as $item) { + $parts[] = $this->stringifyParsedValue($item, $separator); + } + + return implode($separator, $parts); + } + + if (is_string($value)) { + return $value; + } + + if (is_scalar($value)) { + return (string) $value; + } + + return ''; + } + /** * Stores OAuth tokens for user and ticket system. * diff --git a/src/Service/Integration/Jira/JiraHttpClientService.php b/src/Service/Integration/Jira/JiraHttpClientService.php index f37bb5a2f..54255486a 100644 --- a/src/Service/Integration/Jira/JiraHttpClientService.php +++ b/src/Service/Integration/Jira/JiraHttpClientService.php @@ -25,6 +25,7 @@ use UnexpectedValueException; use function is_array; +use function is_scalar; use function sprintf; use const JSON_THROW_ON_ERROR; @@ -271,11 +272,11 @@ private function extractErrorMessage(string $body): string } if (isset($data['errorMessages']) && is_array($data['errorMessages'])) { - return implode(', ', $data['errorMessages']); + return implode(', ', array_map(static fn (mixed $v): string => (string) (is_scalar($v) ? $v : ''), $data['errorMessages'])); } if (isset($data['errors']) && is_array($data['errors'])) { - return implode(', ', array_values($data['errors'])); + return implode(', ', array_map(static fn (mixed $v): string => (string) (is_scalar($v) ? $v : ''), array_values($data['errors']))); } return $body; diff --git a/src/Service/Integration/Jira/JiraOAuthApiService.php b/src/Service/Integration/Jira/JiraOAuthApiService.php index ac5550d3f..b99ef53b3 100644 --- a/src/Service/Integration/Jira/JiraOAuthApiService.php +++ b/src/Service/Integration/Jira/JiraOAuthApiService.php @@ -459,7 +459,7 @@ public function doesTicketExist(string $sTicket): bool /** * Get an array of ticket numbers that are subtickets of the given issue. * - * @psalm-return list + * @return list */ public function getSubtickets(string $sTicket): array { diff --git a/src/Service/Integration/Jira/JiraTicketService.php b/src/Service/Integration/Jira/JiraTicketService.php index 533cca441..46a5057d5 100644 --- a/src/Service/Integration/Jira/JiraTicketService.php +++ b/src/Service/Integration/Jira/JiraTicketService.php @@ -37,7 +37,7 @@ public function __construct( * * @throws JiraApiException */ - public function createTicket(Entry $entry): mixed + public function createTicket(Entry $entry): object { $project = $entry->getProject(); diff --git a/src/Service/SubticketSyncService.php b/src/Service/SubticketSyncService.php index a14f029b3..0151b5d11 100644 --- a/src/Service/SubticketSyncService.php +++ b/src/Service/SubticketSyncService.php @@ -30,9 +30,7 @@ public function __construct(private readonly ManagerRegistry $managerRegistry, p * @throws Exception When something goes wrong. * Exception codes are sensible HTTP status codes * - * @return (mixed|string)[] Array of subticket keys - * - * @psalm-return list + * @return list Array of subticket keys */ public function syncProjectSubtickets(int|Project $projectOrProjectId): array { diff --git a/tests/EventSubscriber/ExceptionSubscriberTest.php b/tests/EventSubscriber/ExceptionSubscriberTest.php index 5e40942f1..1ec4dcc3c 100755 --- a/tests/EventSubscriber/ExceptionSubscriberTest.php +++ b/tests/EventSubscriber/ExceptionSubscriberTest.php @@ -59,6 +59,26 @@ private function createExceptionEvent(Request $request, Throwable $exception): E ); } + /** + * Decodes a JsonResponse body to an associative array, asserting structure. + * + * @return array + */ + private function decodeJsonResponse(ExceptionEvent $event): array + { + $response = $event->getResponse(); + self::assertInstanceOf(JsonResponse::class, $response); + + $content = $response->getContent(); + self::assertIsString($content); + + $decoded = json_decode($content, true); + self::assertIsArray($decoded); + + /** @var array $decoded */ + return $decoded; + } + #[Test] public function getSubscribedEventsReturnsCorrectEvents(): void { @@ -231,7 +251,7 @@ public function handlesJiraApiUnauthorizedException(): void $this->assertInstanceOf(JsonResponse::class, $response); $this->assertSame(401, $response->getStatusCode()); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame('Jira authentication required', $content['error']); $this->assertSame('/oauth/redirect', $content['redirect_url']); } @@ -252,9 +272,11 @@ public function handlesJiraApiException(): void $this->assertInstanceOf(JsonResponse::class, $response); $this->assertSame(502, $response->getStatusCode()); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame('Jira API error', $content['error']); - $this->assertStringContainsString('Connection failed', $content['message']); + $message = $content['message']; + self::assertIsString($message); + $this->assertStringContainsString('Connection failed', $message); } #[Test] @@ -275,7 +297,7 @@ public function handlesHttpExceptions( $this->assertInstanceOf(JsonResponse::class, $response); $this->assertSame($expectedStatusCode, $response->getStatusCode()); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame($expectedErrorType, $content['error']); } @@ -313,8 +335,7 @@ public function handlesHttpExceptionWithEmptyMessage(): void $subscriber->onKernelException($event); - $response = $event->getResponse(); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); // Should use default message $this->assertSame('The requested resource was not found.', $content['message']); @@ -336,7 +357,7 @@ public function handlesGenericExceptionInDevMode(): void $this->assertInstanceOf(JsonResponse::class, $response); $this->assertSame(500, $response->getStatusCode()); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame('Internal server error', $content['error']); $this->assertSame('Something went wrong', $content['message']); $this->assertSame(Exception::class, $content['exception']); @@ -361,7 +382,7 @@ public function handlesGenericExceptionInProdMode(): void $this->assertInstanceOf(JsonResponse::class, $response); $this->assertSame(500, $response->getStatusCode()); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame('Internal server error', $content['error']); // Should hide sensitive details in production $this->assertSame('An unexpected error occurred. Please try again later.', $content['message']); @@ -376,8 +397,8 @@ public function logsServerErrors(): void { $this->logger->expects($this->once()) ->method('error') - ->with('Server error occurred', $this->callback( - static fn (array $context) => isset($context['exception']) && $context['exception'] instanceof HttpException, + ->with('Server error occurred', self::callback( + static fn (array $context): bool => isset($context['exception']) && $context['exception'] instanceof HttpException, )); $subscriber = $this->createSubscriber(); @@ -394,8 +415,8 @@ public function logsClientErrors(): void { $this->logger->expects($this->once()) ->method('warning') - ->with('Client error occurred', $this->callback( - static fn (array $context) => isset($context['exception']) && $context['exception'] instanceof NotFoundHttpException, + ->with('Client error occurred', self::callback( + static fn (array $context): bool => isset($context['exception']) && $context['exception'] instanceof NotFoundHttpException, )); $subscriber = $this->createSubscriber(); @@ -412,8 +433,8 @@ public function logsUnexpectedExceptions(): void { $this->logger->expects($this->once()) ->method('error') - ->with('Unexpected exception occurred', $this->callback( - static fn (array $context) => isset($context['exception']) && $context['exception'] instanceof Exception, + ->with('Unexpected exception occurred', self::callback( + static fn (array $context): bool => isset($context['exception']) && $context['exception'] instanceof Exception, )); $subscriber = $this->createSubscriber(); @@ -468,8 +489,7 @@ public function providesDefaultMessagesForStatusCodes(int $statusCode, string $e $subscriber->onKernelException($event); - $response = $event->getResponse(); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertSame($expectedMessage, $content['message']); } @@ -480,19 +500,19 @@ public function providesDefaultMessagesForStatusCodes(int $statusCode, string $e public static function provideDefaultMessages(): array { return [ - '400' => [400, 'The request was invalid or cannot be processed.'], - '401' => [401, 'Authentication is required to access this resource.'], - '403' => [403, 'You do not have permission to access this resource.'], - '404' => [404, 'The requested resource was not found.'], - '405' => [405, 'The request method is not allowed for this resource.'], - '406' => [406, 'The requested format is not acceptable.'], - '409' => [409, 'The request conflicts with the current state of the resource.'], - '422' => [422, 'The request was well-formed but contains semantic errors.'], - '429' => [429, 'Too many requests have been sent in a given amount of time.'], - '500' => [500, 'An internal server error occurred.'], - '502' => [502, 'The server received an invalid response from an upstream server.'], - '503' => [503, 'The service is temporarily unavailable.'], - '418' => [418, 'An error occurred while processing your request.'], // Unknown code + 'status 400' => [400, 'The request was invalid or cannot be processed.'], + 'status 401' => [401, 'Authentication is required to access this resource.'], + 'status 403' => [403, 'You do not have permission to access this resource.'], + 'status 404' => [404, 'The requested resource was not found.'], + 'status 405' => [405, 'The request method is not allowed for this resource.'], + 'status 406' => [406, 'The requested format is not acceptable.'], + 'status 409' => [409, 'The request conflicts with the current state of the resource.'], + 'status 422' => [422, 'The request was well-formed but contains semantic errors.'], + 'status 429' => [429, 'Too many requests have been sent in a given amount of time.'], + 'status 500' => [500, 'An internal server error occurred.'], + 'status 502' => [502, 'The server received an invalid response from an upstream server.'], + 'status 503' => [503, 'The service is temporarily unavailable.'], + 'status 418' => [418, 'An error occurred while processing your request.'], // Unknown code ]; } @@ -508,8 +528,7 @@ public function jiraUnauthorizedExceptionWithNullRedirectUrl(): void $subscriber->onKernelException($event); - $response = $event->getResponse(); - $content = json_decode((string) $response->getContent(), true); + $content = $this->decodeJsonResponse($event); $this->assertNull($content['redirect_url']); } diff --git a/tests/Performance/PerformanceBenchmarkRunner.php b/tests/Performance/PerformanceBenchmarkRunner.php index c06819e2a..62e381b26 100644 --- a/tests/Performance/PerformanceBenchmarkRunner.php +++ b/tests/Performance/PerformanceBenchmarkRunner.php @@ -395,8 +395,14 @@ private function addSummaryStatistics(array &$report): void $successfulTests = count($successfulResults); $failedTests = $totalTests - $successfulTests; - $executionTimes = array_column($successfulResults, 'execution_time_ms'); - $memoryUsages = array_column($successfulResults, 'memory_usage_bytes'); + $executionTimes = array_map( + static fn (mixed $value): float => is_numeric($value) ? (float) $value : 0.0, + array_column($successfulResults, 'execution_time_ms'), + ); + $memoryUsages = array_map( + static fn (mixed $value): float => is_numeric($value) ? (float) $value : 0.0, + array_column($successfulResults, 'memory_usage_bytes'), + ); if ([] === $executionTimes || [] === $memoryUsages) { $report[] = 'No valid performance data to analyze.'; @@ -406,11 +412,9 @@ private function addSummaryStatistics(array &$report): void $report[] = sprintf('Total Tests: %d (✅ %d, ❌ %d)', $totalTests, $successfulTests, $failedTests); $report[] = sprintf('Average Execution Time: %.1fms', array_sum($executionTimes) / count($executionTimes)); - /** @var int|float $maxExecutionTime */ $maxExecutionTime = max($executionTimes); $report[] = sprintf('Max Execution Time: %.1fms', $maxExecutionTime); $report[] = sprintf('Average Memory Usage: %.2fMB', array_sum($memoryUsages) / count($memoryUsages) / 1024 / 1024); - /** @var int|float $maxMemoryUsage */ $maxMemoryUsage = max($memoryUsages); $report[] = sprintf('Max Memory Usage: %.2fMB', $maxMemoryUsage / 1024 / 1024); } diff --git a/tests/Service/Cache/QueryCacheServiceTest.php b/tests/Service/Cache/QueryCacheServiceTest.php index c2219e078..6b7fa980f 100755 --- a/tests/Service/Cache/QueryCacheServiceTest.php +++ b/tests/Service/Cache/QueryCacheServiceTest.php @@ -313,14 +313,18 @@ public function warmUpSkipsNonCallableEntries(): void $this->cachePool->method('save'); $executed = []; - $callbacks = [ - 'key1' => static function () use (&$executed) { - $executed[] = 'key1'; - - return 'value1'; - }, - 'key2' => 'not_a_callable', - ]; + $validCallback = static function () use (&$executed) { + $executed[] = 'key1'; + + return 'value1'; + }; + + // Build the array as mixed so the second entry can be a non-callable + // value, exercising the runtime guard inside warmUp(). + /** @var array $rawCallbacks */ + $rawCallbacks = ['key1' => $validCallback, 'key2' => 'not_a_callable']; + /** @var array $callbacks */ + $callbacks = $rawCallbacks; $this->service->warmUp($callbacks); diff --git a/tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php b/tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php index 699287786..f6945246e 100755 --- a/tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php +++ b/tests/Service/Integration/Jira/JiraAuthenticationServiceTest.php @@ -124,10 +124,14 @@ public function throwUnauthorizedRedirectIncludesOriginalException(): void try { $this->service->throwUnauthorizedRedirect($ticketSystem, $originalException); - $this->fail('Expected JiraApiUnauthorizedException'); } catch (JiraApiUnauthorizedException $e) { $this->assertSame($originalException, $e->getPrevious()); + + return; } + + // @phpstan-ignore-next-line deadCode.unreachable — method is declared `never` so PHPStan considers this unreachable; we still want to fail loudly if the runtime contract regresses. + $this->fail('Expected JiraApiUnauthorizedException to be thrown'); } #[Test] @@ -336,9 +340,10 @@ public function authenticateSucceedsWithValidTokens(): void ['encrypted_secret', 'valid_secret'], ]); - // Should not throw + // Should not throw - assert side effects (decrypted tokens were read) $this->service->authenticate($user, $ticketSystem); - $this->assertTrue(true); // Reached without exception + self::assertSame('encrypted_token', $userTicketSystem->getAccessToken()); + self::assertSame('encrypted_secret', $userTicketSystem->getTokenSecret()); } #[Test] @@ -416,7 +421,7 @@ public function fetchOAuthRequestTokenSucceeds(): void $this->repository->method('findOneBy')->willReturn(null); $this->tokenEncryptionService->method('encryptToken') - ->willReturnCallback(static fn ($token) => 'encrypted_' . $token); + ->willReturnCallback(static fn (string $token): string => 'encrypted_' . $token); $this->entityManager->expects($this->once())->method('persist'); $this->entityManager->expects($this->once())->method('flush'); @@ -519,14 +524,13 @@ public function fetchOAuthAccessTokenSucceeds(): void $this->repository->method('findOneBy')->willReturn(null); $this->tokenEncryptionService->method('encryptToken') - ->willReturnCallback(static fn ($token) => 'encrypted_' . $token); + ->willReturnCallback(static fn (string $token): string => 'encrypted_' . $token); $this->entityManager->expects($this->once())->method('persist'); $this->entityManager->expects($this->once())->method('flush'); - // Should not throw + // Should not throw - persist/flush expectations above act as the assertions $this->service->fetchOAuthAccessToken($httpClientService, 'request_token', 'verifier_code'); - $this->assertTrue(true); } #[Test] @@ -577,7 +581,7 @@ public function storeTokenUpdatesExistingUserTicketSystem(): void $this->repository->method('findOneBy')->willReturn($existingUserTicketSystem); $this->tokenEncryptionService->method('encryptToken') - ->willReturnCallback(static fn ($token) => 'encrypted_' . $token); + ->willReturnCallback(static fn (string $token): string => 'encrypted_' . $token); // Should persist the same object, not create a new one $this->entityManager->expects($this->once()) diff --git a/tests/Service/Integration/Jira/JiraHttpClientServiceTest.php b/tests/Service/Integration/Jira/JiraHttpClientServiceTest.php index 2e9fcceb4..73ca9ab70 100755 --- a/tests/Service/Integration/Jira/JiraHttpClientServiceTest.php +++ b/tests/Service/Integration/Jira/JiraHttpClientServiceTest.php @@ -23,6 +23,7 @@ use PHPUnit\Framework\TestCase; use ReflectionClass; use stdClass; +use Throwable; use UnexpectedValueException; #[CoversClass(JiraHttpClientService::class)] @@ -91,9 +92,9 @@ public function getClientWithUserModeGetsTokensFromAuthService(): void ->willReturn(['token' => 'user_token', 'secret' => 'user_secret']); $service = $this->createService(); - $client = $service->getClient('user'); - - $this->assertInstanceOf(Client::class, $client); + // The expects(once()) on getTokens above is the actual assertion; + // building the client here verifies it doesn't throw with valid tokens. + $service->getClient('user'); } #[Test] @@ -121,9 +122,8 @@ public function getClientWithNewModeReturnsEmptyTokens(): void $this->authService->expects($this->never())->method('getTokens'); $service = $this->createService(); - $client = $service->getClient('new'); - - $this->assertInstanceOf(Client::class, $client); + // expects(never()) on getTokens above is the actual assertion. + $service->getClient('new'); } #[Test] @@ -133,9 +133,8 @@ public function getClientWithRequestModeUsesProvidedToken(): void $this->authService->expects($this->never())->method('getTokens'); $service = $this->createService(); - $client = $service->getClient('request', 'provided_request_token'); - - $this->assertInstanceOf(Client::class, $client); + // expects(never()) on getTokens above is the actual assertion. + $service->getClient('request', 'provided_request_token'); } #[Test] @@ -249,7 +248,7 @@ public function getReturnsDecodedJsonResponse(): void $result = $service->get('issue/TEST-123'); - $this->assertIsObject($result); + self::assertInstanceOf(stdClass::class, $result); $this->assertSame(123, $result->id); $this->assertSame('TEST-123', $result->key); } @@ -302,6 +301,7 @@ public function postSendsJsonData(): void $result = $service->post('issue', ['fields' => ['summary' => 'Test issue']]); + self::assertInstanceOf(stdClass::class, $result); $this->assertSame(456, $result->id); } @@ -331,6 +331,7 @@ public function putSendsJsonData(): void $result = $service->put('issue/TEST-123', ['fields' => ['summary' => 'Updated']]); + self::assertInstanceOf(stdClass::class, $result); $this->assertSame(789, $result->id); } @@ -584,6 +585,9 @@ public function urlWithLeadingSlashIsNormalized(): void $service->get('/issue/TEST-123'); } + /** + * @param class-string $expectedExceptionClass + */ #[Test] #[DataProvider('provideStatusCodes')] public function handlesDifferentStatusCodes(int $statusCode, string $expectedExceptionClass): void diff --git a/tests/Service/Integration/Jira/JiraTicketServiceTest.php b/tests/Service/Integration/Jira/JiraTicketServiceTest.php index 41039b7bb..759efd984 100755 --- a/tests/Service/Integration/Jira/JiraTicketServiceTest.php +++ b/tests/Service/Integration/Jira/JiraTicketServiceTest.php @@ -93,15 +93,19 @@ public function createTicketSucceeds(): void $this->httpClient->expects($this->once()) ->method('post') - ->with('issue', $this->callback(static fn (array $data) => 'PROJ' === $data['fields']['project']['key'] + ->with('issue', self::callback(static function (array $data): bool { + /** @var array{fields: array{project: array{key: string}, summary: string, description: string, issuetype: array{name: string}}} $data */ + return 'PROJ' === $data['fields']['project']['key'] && str_contains($data['fields']['summary'], 'ACME Corp') && 'Working on feature X' === $data['fields']['description'] - && 'Story' === $data['fields']['issuetype']['name'])) + && 'Story' === $data['fields']['issuetype']['name']; + })) ->willReturn($response); $result = $this->service->createTicket($entry); - $this->assertSame('PROJ-123', $result->key); + self::assertInstanceOf(stdClass::class, $result); + self::assertSame('PROJ-123', $result->key); } #[Test] @@ -150,8 +154,11 @@ public function createTicketUsesDefaultDescriptionWhenEmpty(): void $this->httpClient->expects($this->once()) ->method('post') - ->with('issue', $this->callback( - static fn (array $data) => 'No description provided' === $data['fields']['description'], + ->with('issue', self::callback( + static function (array $data): bool { + /** @var array{fields: array{description: string}} $data */ + return 'No description provided' === $data['fields']['description']; + }, )) ->willReturn($response); @@ -199,8 +206,11 @@ public function createTicketDeterminesIssueTypeFromActivity(string $activityName $this->httpClient->expects($this->once()) ->method('post') - ->with('issue', $this->callback( - static fn (array $data) => $expectedIssueType === $data['fields']['issuetype']['name'], + ->with('issue', self::callback( + static function (array $data) use ($expectedIssueType): bool { + /** @var array{fields: array{issuetype: array{name: string}}} $data */ + return $expectedIssueType === $data['fields']['issuetype']['name']; + }, )) ->willReturn($response); @@ -240,7 +250,8 @@ public function searchTicketsPostsJqlQuery(): void $result = $this->service->searchTickets('project = TEST AND status = Open', [], 10); - $this->assertSame(0, $result->total); + self::assertInstanceOf(stdClass::class, $result); + self::assertSame(0, $result->total); } #[Test] @@ -313,7 +324,8 @@ public function getTicketReturnsTicketDetails(): void $result = $this->service->getTicket('TEST-123'); - $this->assertSame('TEST-123', $result->key); + self::assertInstanceOf(stdClass::class, $result); + self::assertSame('TEST-123', $result->key); } #[Test] @@ -373,7 +385,8 @@ public function addCommentPostsComment(): void $result = $this->service->addComment('TEST-123', 'This is a comment'); - $this->assertSame('10001', $result->id); + self::assertInstanceOf(stdClass::class, $result); + self::assertSame('10001', $result->id); } #[Test] @@ -590,7 +603,8 @@ public function createTicketGeneratesSummaryFromParts(): void $this->httpClient->expects($this->once()) ->method('post') - ->with('issue', $this->callback(static function (array $data) { + ->with('issue', self::callback(static function (array $data): bool { + /** @var array{fields: array{summary: string}} $data */ $summary = $data['fields']['summary']; return str_contains($summary, 'CustomerY') @@ -614,8 +628,11 @@ public function createTicketBuildsPartialSummaryWhenSomePartsEmpty(): void $this->httpClient->expects($this->once()) ->method('post') - ->with('issue', $this->callback( - static fn (array $data) => 'MyProject' === $data['fields']['summary'], + ->with('issue', self::callback( + static function (array $data): bool { + /** @var array{fields: array{summary: string}} $data */ + return 'MyProject' === $data['fields']['summary']; + }, )) ->willReturn($response); diff --git a/tests/Traits/TestDataTrait.php b/tests/Traits/TestDataTrait.php index 037778a4b..149a0aef8 100644 --- a/tests/Traits/TestDataTrait.php +++ b/tests/Traits/TestDataTrait.php @@ -109,9 +109,9 @@ private function resolveTestDataPath(?string $filepath = null): ?string $baseDir = __DIR__; // Use provided filepath or default from instance - $targetPath = $filepath ?? ($this->filepath ?? null); + $targetPath = $filepath ?? $this->filepath; - if (null === $targetPath || '' === $targetPath) { + if ('' === $targetPath) { return null; }