From 0386a260d40db46f4577df279fcda33e1ab9aacc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Fri, 30 May 2025 11:23:05 +0200 Subject: [PATCH] Added stricter type hints and update PHPUnit test mocks for improved type safety across various classes and test cases. --- phpstan-baseline.neon | 48 -------------- .../QueryTranslator/Generator/WordVisitor.php | 6 +- ...ntentTypeGroupAggregationKeyMapperTest.php | 18 ++--- .../ObjectStateAggregationKeyMapperTest.php | 24 ++++--- .../SectionAggregationKeyMapperTest.php | 23 ++++--- .../UserMetadataAggregationKeyMapperTest.php | 65 ++++++++++--------- 6 files changed, 75 insertions(+), 109 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6d777b6a..14e91b27 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -756,12 +756,6 @@ parameters: count: 1 path: src/lib/Query/Common/QueryConverter/NativeQueryConverter.php - - - message: '#^Method Ibexa\\Solr\\Query\\Common\\QueryTranslator\\Generator\\WordVisitor\:\:escapeWord\(\) should return string but returns string\|null\.$#' - identifier: return.type - count: 1 - path: src/lib/Query/Common/QueryTranslator/Generator/WordVisitor.php - - message: '#^Method Ibexa\\Contracts\\Solr\\Query\\SortClauseVisitor\:\:visit\(\) invoked with 2 parameters, 1 required\.$#' identifier: arguments.count @@ -1704,48 +1698,6 @@ parameters: count: 1 path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/SubtreeAggregationKeyMapperTest.php - - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapperTest\:\:createExpectedResultForUserGroupKey\(\) has parameter \$userGroupsIds with no value type specified in iterable type iterable\.$#' - identifier: missingType.iterableValue - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapperTest\:\:createExpectedResultForUserGroupKey\(\) return type has no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapperTest\:\:createExpectedResultForUserKey\(\) has parameter \$userIds with no value type specified in iterable type iterable\.$#' - identifier: missingType.iterableValue - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapperTest\:\:createExpectedResultForUserKey\(\) return type has no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapperTest\:\:dataProviderForTestMapUser\(\) return type has no value type specified in iterable type iterable\.$#' - identifier: missingType.iterableValue - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Parameter \#1 \$aggregation of method Ibexa\\Solr\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapper\:\:map\(\) expects Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\Aggregation\\UserMetadataTermAggregation, Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\Aggregation given\.$#' - identifier: argument.type - count: 1 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - - - message: '#^Parameter \#3 \$keys of method Ibexa\\Solr\\ResultExtractor\\AggregationResultExtractor\\TermAggregationKeyMapper\\UserMetadataAggregationKeyMapper\:\:map\(\) expects array\, array\ given\.$#' - identifier: argument.type - count: 2 - path: tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php - - message: '#^Method Ibexa\\Tests\\Solr\\Search\\ResultExtractor\\AggregationResultExtractor\\TermAggregationResultExtractorTest\:\:dataProviderForTestCanVisit\(\) return type has no value type specified in iterable type iterable\.$#' identifier: missingType.iterableValue diff --git a/src/lib/Query/Common/QueryTranslator/Generator/WordVisitor.php b/src/lib/Query/Common/QueryTranslator/Generator/WordVisitor.php index 4a6ec80d..1fd997e8 100644 --- a/src/lib/Query/Common/QueryTranslator/Generator/WordVisitor.php +++ b/src/lib/Query/Common/QueryTranslator/Generator/WordVisitor.php @@ -16,7 +16,7 @@ */ class WordVisitor extends WordBase { - public function visit(Node $node, Visitor $subVisitor = null, $options = null) + public function visit(Node $node, Visitor $subVisitor = null, $options = null): string { $word = parent::visit($node, $subVisitor, $options); @@ -29,14 +29,12 @@ public function visit(Node $node, Visitor $subVisitor = null, $options = null) } /** - * {@inheritdoc} - * * @see http://lucene.apache.org/core/5_0_0/queryparser/org/apache/lucene/queryparser/classic/package-summary.html#Escaping_Special_Characters * * Note: additionally to what is defined above we also escape blank space, * and we don't escape an asterisk. */ - protected function escapeWord($string) + protected function escapeWord($string): ?string { return preg_replace( '/(\\+|-|&&|\\|\\||!|\\(|\\)|\\{|}|\\[|]|\\^|"|~|\\?|:|\\/|\\\\| )/', diff --git a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ContentTypeGroupAggregationKeyMapperTest.php b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ContentTypeGroupAggregationKeyMapperTest.php index 94444ef3..33f70837 100644 --- a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ContentTypeGroupAggregationKeyMapperTest.php +++ b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ContentTypeGroupAggregationKeyMapperTest.php @@ -18,7 +18,7 @@ final class ContentTypeGroupAggregationKeyMapperTest extends TestCase { - private const EXAMPLE_CONTENT_TYPE_GROUPS_IDS = ['1', '2', '3']; + private const array EXAMPLE_CONTENT_TYPE_GROUPS_IDS = ['1', '2', '3']; private ContentTypeService&MockObject $contentTypeService; @@ -55,18 +55,18 @@ private function createExpectedLanguages(): array { $expectedContentTypesGroups = []; - foreach (self::EXAMPLE_CONTENT_TYPE_GROUPS_IDS as $i => $id) { + $map = []; + foreach (self::EXAMPLE_CONTENT_TYPE_GROUPS_IDS as $id) { $contentTypeGroup = $this->createContentTypeGroupWithIds((int)$id); - - $this->contentTypeService - ->expects(self::at($i)) - ->method('loadContentTypeGroup') - ->with((int)$id, []) - ->willReturn($contentTypeGroup); - $expectedContentTypesGroups[$id] = $contentTypeGroup; + + $map[] = [(int)$id, [], $contentTypeGroup]; } + $this->contentTypeService + ->method('loadContentTypeGroup') + ->willReturnMap($map); + return $expectedContentTypesGroups; } } diff --git a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ObjectStateAggregationKeyMapperTest.php b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ObjectStateAggregationKeyMapperTest.php index e437b8c6..50c5ba50 100644 --- a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ObjectStateAggregationKeyMapperTest.php +++ b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/ObjectStateAggregationKeyMapperTest.php @@ -12,6 +12,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Query\Aggregation\ObjectStateTermAggregation; use Ibexa\Contracts\Core\Repository\Values\ObjectState\ObjectState; use Ibexa\Contracts\Core\Repository\Values\ObjectState\ObjectStateGroup; +use Ibexa\Core\Base\Exceptions\InvalidArgumentException; use Ibexa\Solr\ResultExtractor\AggregationResultExtractor\TermAggregationKeyMapper\ObjectStateAggregationKeyMapper; use Ibexa\Tests\Solr\Search\ResultExtractor\AggregationResultExtractor\AggregationResultExtractorTestUtils; use PHPUnit\Framework\MockObject\MockObject; @@ -58,18 +59,25 @@ private function configureObjectStateService( ->willReturn($objectStateGroup); $expectedObjectStates = []; - foreach ($objectStateIdentifiers as $i => $objectStateIdentifier) { + $map = []; + foreach ($objectStateIdentifiers as $objectStateIdentifier) { $objectState = $this->createMock(ObjectState::class); - - $this->objectStateService - ->expects(self::at($i + 1)) - ->method('loadObjectStateByIdentifier') - ->with($objectStateGroup, $objectStateIdentifier, []) - ->willReturn($objectState); - $expectedObjectStates[] = $objectState; + $map[$objectStateIdentifier] = $objectState; } + $this->objectStateService + ->method('loadObjectStateByIdentifier') + ->willReturnCallback( + static function ($group, $identifier, $options) use ($objectStateGroup, $map) { + if ($group === $objectStateGroup && isset($map[$identifier]) && $options === []) { + return $map[$identifier]; + } + + throw new InvalidArgumentException('identifier', "Unexpected arguments: $identifier"); + } + ); + return $expectedObjectStates; } } diff --git a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/SectionAggregationKeyMapperTest.php b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/SectionAggregationKeyMapperTest.php index 80e84c4b..63415977 100644 --- a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/SectionAggregationKeyMapperTest.php +++ b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/SectionAggregationKeyMapperTest.php @@ -11,6 +11,7 @@ use Ibexa\Contracts\Core\Repository\SectionService; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Aggregation; use Ibexa\Contracts\Core\Repository\Values\Content\Section; +use Ibexa\Core\Base\Exceptions\InvalidArgumentException; use Ibexa\Solr\ResultExtractor\AggregationResultExtractor\TermAggregationKeyMapper\SectionAggregationKeyMapper; use Ibexa\Tests\Solr\Search\ResultExtractor\AggregationResultExtractor\AggregationResultExtractorTestUtils; use PHPUnit\Framework\MockObject\MockObject; @@ -18,7 +19,7 @@ final class SectionAggregationKeyMapperTest extends TestCase { - private const EXAMPLE_SECTION_IDS = [1, 2, 3]; + private const array EXAMPLE_SECTION_IDS = [1, 2, 3]; private SectionService&MockObject $sectionService; @@ -47,17 +48,19 @@ public function testMap(): void private function configureSectionServiceMock(iterable $sectionIds): array { $sections = []; - foreach ($sectionIds as $i => $sectionId) { - $section = $this->createMock(Section::class); + foreach ($sectionIds as $sectionId) { + $sections[$sectionId] = $this->createMock(Section::class); + } - $this->sectionService - ->expects(self::at($i)) - ->method('loadSection') - ->with($sectionId) - ->willReturn($section); + $this->sectionService + ->method('loadSection') + ->willReturnCallback(static function ($id) use ($sections) { + if (isset($sections[$id])) { + return $sections[$id]; + } - $sections[$sectionId] = $section; - } + throw new InvalidArgumentException('id', "Unexpected section ID: $id"); + }); return $sections; } diff --git a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php index 77ec0f4e..84da3dc3 100644 --- a/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php +++ b/tests/lib/Search/ResultExtractor/AggregationResultExtractor/TermAggregationKeyMapper/UserMetadataAggregationKeyMapperTest.php @@ -9,10 +9,10 @@ namespace Ibexa\Tests\Solr\Search\ResultExtractor\AggregationResultExtractor\TermAggregationKeyMapper; use Ibexa\Contracts\Core\Repository\UserService; -use Ibexa\Contracts\Core\Repository\Values\Content\Query\Aggregation; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Aggregation\UserMetadataTermAggregation; use Ibexa\Contracts\Core\Repository\Values\User\User; use Ibexa\Contracts\Core\Repository\Values\User\UserGroup; +use Ibexa\Core\Base\Exceptions\InvalidArgumentException; use Ibexa\Solr\ResultExtractor\AggregationResultExtractor\TermAggregationKeyMapper\UserMetadataAggregationKeyMapper; use Ibexa\Tests\Solr\Search\ResultExtractor\AggregationResultExtractor\AggregationResultExtractorTestUtils; use PHPUnit\Framework\MockObject\MockObject; @@ -20,12 +20,11 @@ final class UserMetadataAggregationKeyMapperTest extends TestCase { - private const EXAMPLE_USER_IDS = [1, 2, 3]; - private const EXAMPLE_USER_GROUP_IDS = [1, 2, 3]; + private const array EXAMPLE_USER_IDS = [1, 2, 3]; + private const array EXAMPLE_USER_GROUP_IDS = [1, 2, 3]; private UserService&MockObject $userService; - /** @var \Ibexa\Solr\ResultExtractor\AggregationResultExtractor\TermAggregationKeyMapper\UserMetadataAggregationKeyMapper */ private UserMetadataAggregationKeyMapper $mapper; protected function setUp(): void @@ -37,18 +36,21 @@ protected function setUp(): void /** * @dataProvider dataProviderForTestMapUser */ - public function testMapForUserKey(Aggregation $aggregation): void + public function testMapForUserKey(UserMetadataTermAggregation $aggregation): void { self::assertEquals( $this->createExpectedResultForUserKey(self::EXAMPLE_USER_IDS), $this->mapper->map( $aggregation, AggregationResultExtractorTestUtils::EXAMPLE_LANGUAGE_FILTER, - self::EXAMPLE_USER_IDS, + array_map('strval', self::EXAMPLE_USER_IDS) ) ); } + /** + * @return iterable + */ public function dataProviderForTestMapUser(): iterable { yield UserMetadataTermAggregation::OWNER => [ @@ -69,44 +71,47 @@ public function testMapForUserGroup(): void $this->mapper->map( $aggregation, AggregationResultExtractorTestUtils::EXAMPLE_LANGUAGE_FILTER, - self::EXAMPLE_USER_GROUP_IDS, + array_map('strval', self::EXAMPLE_USER_GROUP_IDS) ) ); } + /** + * @param iterable $userIds + * + * @return array + */ private function createExpectedResultForUserKey(iterable $userIds): array { $users = []; - foreach ($userIds as $i => $userId) { - $user = $this->createMock(User::class); - - $this->userService - ->expects(self::at($i)) - ->method('loadUser') - ->with($userId) - ->willReturn($user); - - $users[$userId] = $user; + foreach ($userIds as $userId) { + $users[$userId] = $this->createMock(User::class); } + $this->userService + ->method('loadUser') + ->willReturnCallback(static fn ($userId) => $users[$userId] ?? throw new InvalidArgumentException('userId', "Unexpected user ID: $userId")); + return $users; } - private function createExpectedResultForUserGroupKey(iterable $userGroupsIds): array + /** + * @param iterable $userGroupIds + * + * @return array + */ + private function createExpectedResultForUserGroupKey(iterable $userGroupIds): array { - $users = []; - foreach ($userGroupsIds as $i => $userGroupId) { - $user = $this->createMock(UserGroup::class); - - $this->userService - ->expects(self::at($i)) - ->method('loadUserGroup') - ->with($userGroupId) - ->willReturn($user); - - $users[$userGroupId] = $user; + $userGroups = []; + foreach ($userGroupIds as $userGroupId) { + $userGroups[$userGroupId] = $this->createMock(UserGroup::class); } - return $users; + $this->userService + ->expects(self::any()) + ->method('loadUserGroup') + ->willReturnCallback(static fn ($userGroupId) => $userGroups[$userGroupId] ?? throw new InvalidArgumentException('userGroupId', "Unexpected user group ID: $userGroupId")); + + return $userGroups; } }