From 7660cd46c4ff2d1aeee30505a2562ec2ddb392ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Wed, 3 Jul 2024 16:25:17 +0200 Subject: [PATCH 01/16] Introduce Well-Typed Collections and Update Documentation for Improved Type Safety --- docs/as-a-data-transfer-object/nesting.md | 23 +++ .../Annotations/CollectionAnnotation.php | 13 ++ .../CollectionAnnotationReader.php | 137 ++++++++++++++++++ src/Support/Factories/DataTypeFactory.php | 13 ++ .../CollectionAnnotationReaderTest.php | 101 +++++++++++++ tests/Support/DataPropertyTypeTest.php | 48 ++++++ 6 files changed, 335 insertions(+) create mode 100644 src/Support/Annotations/CollectionAnnotation.php create mode 100644 src/Support/Annotations/CollectionAnnotationReader.php create mode 100644 tests/Support/Annotations/CollectionAnnotationReaderTest.php diff --git a/docs/as-a-data-transfer-object/nesting.md b/docs/as-a-data-transfer-object/nesting.md index a4bc48f7..67d1ff4f 100644 --- a/docs/as-a-data-transfer-object/nesting.md +++ b/docs/as-a-data-transfer-object/nesting.md @@ -124,6 +124,29 @@ class AlbumData extends Data } ``` +If the collection is well-typed, you don't need to use annotations: + +```php +/** + * @template TKey of array-key + * @template TValue of \App\Data\SongData + * + * @extends \Illuminate\Support\Collection + */ +class SongDataCollection +{ +} + +class AlbumData extends Data +{ + public function __construct( + public string $title, + public SongDataCollection $songs, + ) { + } +} +``` + You can also use an attribute to define the type of data objects that will be stored within a collection: ```php diff --git a/src/Support/Annotations/CollectionAnnotation.php b/src/Support/Annotations/CollectionAnnotation.php new file mode 100644 index 00000000..8be726bb --- /dev/null +++ b/src/Support/Annotations/CollectionAnnotation.php @@ -0,0 +1,13 @@ +isCollection($class)) { + return null; + } + + $type = $this->getCollectionReturnType($class); + + if ($type === null || $type['valueType'] === null) { + return null; + } + + $isData = false; + + if (is_subclass_of($type['valueType'], Data::class)) { + $isData = true; + } + + return new CollectionAnnotation( + type: $type['valueType'], + isData: $isData, + keyType: $type['keyType'] ?? 'array-key', + ); + } + + /** + * @return array{keyType: string|null, valueType: string|null}|null + */ + protected function getCollectionReturnType(ReflectionClass $class): ?array + { + // Initialize TypeResolver and DocBlockFactory + $docBlockFactory = DocBlockFactory::createInstance(); + + // Extract the namespace and uses from the file content + $namespace = $class->getNamespaceName(); + $fileContent = file_get_contents($class->getFileName()); + $this->context = $this->contextFactory->createForNamespace($namespace, $fileContent); + + // Get the PHPDoc comment of the class + $docComment = $class->getDocComment(); + if ($docComment === false) { + return null; + } + + // Create the DocBlock instance + $docBlock = $docBlockFactory->create($docComment, $this->context); + + // Initialize variables + $templateTypes = []; + $keyType = null; + $valueType = null; + + foreach ($docBlock->getTags() as $tag) { + + if (! $tag instanceof Generic) { + continue; + } + + if ($tag->getName() === 'template') { + $description = $tag->getDescription(); + + if (preg_match('/^(\w+)\s+of\s+([^\s]+)/', $description, $matches)) { + $templateTypes[$matches[1]] = $this->resolve($matches[2]); + } + + continue; + } + + if ($tag->getName() === 'extends') { + $description = $tag->getDescription(); + + if (preg_match('/<\s*([^,]+)\s*,\s*([^>]+)\s*>/', $description, $matches)) { + $keyType = $templateTypes[$matches[1]] ?? $this->resolve($matches[1]); + $valueType = $templateTypes[$matches[2]] ?? $this->resolve($matches[2]); + + return [ + 'keyType' => $keyType, + 'valueType' => $valueType + ]; + } + } + } + + return null; + } + + protected function isCollection(ReflectionClass $class): bool + { + // Check if the class implements common collection interfaces + $collectionInterfaces = [ + Traversable::class, + Iterator::class, + IteratorAggregate::class, + Countable::class, + ]; + + foreach ($collectionInterfaces as $interface) { + if ($class->implementsInterface($interface)) { + return true; + } + } + + return false; + } + + protected function resolve(string $type): ?string + { + $type = (string) $this->typeResolver->resolve($type, $this->context); + + return $type ? ltrim($type, '\\') : null; + } +} diff --git a/src/Support/Factories/DataTypeFactory.php b/src/Support/Factories/DataTypeFactory.php index d062f92f..9bbfe73d 100644 --- a/src/Support/Factories/DataTypeFactory.php +++ b/src/Support/Factories/DataTypeFactory.php @@ -17,6 +17,7 @@ use Spatie\LaravelData\Exceptions\CannotFindDataClass; use Spatie\LaravelData\Lazy; use Spatie\LaravelData\Optional; +use Spatie\LaravelData\Support\Annotations\CollectionAnnotationReader; use Spatie\LaravelData\Support\Annotations\DataIterableAnnotation; use Spatie\LaravelData\Support\Annotations\DataIterableAnnotationReader; use Spatie\LaravelData\Support\DataPropertyType; @@ -36,6 +37,7 @@ class DataTypeFactory { public function __construct( protected DataIterableAnnotationReader $iterableAnnotationReader, + protected CollectionAnnotationReader $collectionAnnotationReader, ) { } @@ -354,6 +356,17 @@ protected function inferPropertiesForNamedType( $iterableKeyType = $annotation->keyType; } + if ( + $iterableItemType === null + && $typeable instanceof ReflectionProperty + && class_exists($name) + && $annotation = $this->collectionAnnotationReader->getForClass(new ReflectionClass($name)) + ) { + $isData = $annotation->isData; + $iterableItemType = $annotation->type; + $iterableKeyType = $annotation->keyType; + } + $kind = $isData ? $kind->getDataRelatedEquivalent() : $kind; diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php new file mode 100644 index 00000000..affd2f1d --- /dev/null +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -0,0 +1,101 @@ +getForClass(new ReflectionClass($className)); + + expect($annotations)->toEqual($expected); + } +)->with(function () { + yield DataCollectionWithTemplate::class => [ + 'className' => DataCollectionWithTemplate::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), + ]; + + yield DataCollectionWithoutTemplate::class => [ + 'className' => DataCollectionWithoutTemplate::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), + ]; + + yield DataCollectionWithoutExtends::class => [ + 'className' => DataCollectionWithoutExtends::class, + 'expected' => null, + ]; + + yield NonDataCollectionWithTemplate::class => [ + 'className' => NonDataCollectionWithTemplate::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + + yield NonDataCollectionWithoutTemplate::class => [ + 'className' => NonDataCollectionWithoutTemplate::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + + yield NonDataCollectionWithoutExtends::class => [ + 'className' => NonDataCollectionWithoutExtends::class, + 'expected' => null, + ]; + + yield NonCollectionWithTemplate::class => [ + 'className' => NonCollectionWithTemplate::class, + 'expected' => null, + ]; +}); + + +/** + * @template TKey of array-key + * @template TValue of \Spatie\LaravelData\Tests\Fakes\SimpleData + * + * @extends \Illuminate\Support\Collection + */ +class DataCollectionWithTemplate extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class DataCollectionWithoutTemplate extends Collection +{ +} + +class DataCollectionWithoutExtends extends Collection +{ +} + +/** + * @template TKey of array-key + * @template TValue of \Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum + * + * @extends \Illuminate\Support\Collection + */ +class NonDataCollectionWithTemplate extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class NonDataCollectionWithoutTemplate extends Collection +{ +} + +class NonDataCollectionWithoutExtends extends Collection +{ +} + +/** + * @extends \Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum + */ +class NonCollectionWithTemplate +{ +} diff --git a/tests/Support/DataPropertyTypeTest.php b/tests/Support/DataPropertyTypeTest.php index 3dd121d6..c9476e76 100644 --- a/tests/Support/DataPropertyTypeTest.php +++ b/tests/Support/DataPropertyTypeTest.php @@ -572,6 +572,54 @@ function resolveDataType(object $class, string $property = 'property'): DataProp ->iterableClass->toBe(Collection::class); }); +it('can deduce an enumerable data collection type from collection', function () { + $type = resolveDataType(new class () { + public DataCollectionWithTemplate $property; + }); + + expect($type) + ->isOptional->toBeFalse() + ->isNullable->toBeFalse() + ->isMixed->toBeFalse() + ->lazyType->toBeNull() + ->kind->toBe(DataTypeKind::DataEnumerable) + ->dataClass->toBe(SimpleData::class) + ->iterableClass->toBe(DataCollectionWithTemplate::class) + ->getAcceptedTypes()->toHaveKeys([DataCollectionWithTemplate::class]); + + expect($type->type) + ->toBeInstanceOf(NamedType::class) + ->name->toBe(DataCollectionWithTemplate::class) + ->builtIn->toBeFalse() + ->kind->toBe(DataTypeKind::DataEnumerable) + ->dataClass->toBe(SimpleData::class) + ->iterableClass->toBe(DataCollectionWithTemplate::class); +}); + +it('can deduce an enumerable data collection union type from collection', function () { + $type = resolveDataType(new class () { + public DataCollectionWithTemplate|Lazy $property; + }); + + expect($type) + ->isOptional->toBeFalse() + ->isNullable->toBeFalse() + ->isMixed->toBeFalse() + ->lazyType->toBe(Lazy::class) + ->kind->toBe(DataTypeKind::DataEnumerable) + ->dataClass->toBe(SimpleData::class) + ->iterableClass->toBe(DataCollectionWithTemplate::class) + ->getAcceptedTypes()->toHaveKeys([DataCollectionWithTemplate::class]); + + expect($type->type) + ->toBeInstanceOf(NamedType::class) + ->name->toBe(DataCollectionWithTemplate::class) + ->builtIn->toBeFalse() + ->kind->toBe(DataTypeKind::DataEnumerable) + ->dataClass->toBe(SimpleData::class) + ->iterableClass->toBe(DataCollectionWithTemplate::class); +}); + it('can deduce a paginator data collection type', function () { $type = resolveDataType(new class () { #[DataCollectionOf(SimpleData::class)] From 7b082158ffcfb90ca084e9a122a4d32d1e889e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Wed, 3 Jul 2024 17:17:54 +0200 Subject: [PATCH 02/16] add missing extends in documentation --- docs/as-a-data-transfer-object/nesting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/as-a-data-transfer-object/nesting.md b/docs/as-a-data-transfer-object/nesting.md index 67d1ff4f..21bb66e0 100644 --- a/docs/as-a-data-transfer-object/nesting.md +++ b/docs/as-a-data-transfer-object/nesting.md @@ -133,7 +133,7 @@ If the collection is well-typed, you don't need to use annotations: * * @extends \Illuminate\Support\Collection */ -class SongDataCollection +class SongDataCollection extends Collection { } From cde525560ce5a49b480be84a4078b83d2b2369be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 08:36:26 +0200 Subject: [PATCH 03/16] improve sentance in documentation --- docs/as-a-data-transfer-object/nesting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/as-a-data-transfer-object/nesting.md b/docs/as-a-data-transfer-object/nesting.md index 21bb66e0..5a0e4aaf 100644 --- a/docs/as-a-data-transfer-object/nesting.md +++ b/docs/as-a-data-transfer-object/nesting.md @@ -124,7 +124,7 @@ class AlbumData extends Data } ``` -If the collection is well-typed, you don't need to use annotations: +If the collection is well-annotated, the `Data` class doesn't need to use annotations: ```php /** From dd3a8f2d208e2692ed6ea7fae3009c9f1825be98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 09:11:57 +0200 Subject: [PATCH 04/16] fix combination of type and key + improve test --- .../CollectionAnnotationReader.php | 15 ++- .../CollectionAnnotationReaderTest.php | 98 ++++++++++++++++++- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index 98714a35..10288bed 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -94,9 +94,18 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array if ($tag->getName() === 'extends') { $description = $tag->getDescription(); - if (preg_match('/<\s*([^,]+)\s*,\s*([^>]+)\s*>/', $description, $matches)) { - $keyType = $templateTypes[$matches[1]] ?? $this->resolve($matches[1]); - $valueType = $templateTypes[$matches[2]] ?? $this->resolve($matches[2]); + if (preg_match('/<\s*([^,\s]+)?\s*(?:,\s*([^>\s]+))?\s*>/', $description, $matches)) { + + if (count($matches) === 3) { + $keyType = $templateTypes[$matches[1]] ?? $this->resolve($matches[1]); + $valueType = $templateTypes[$matches[2]] ?? $this->resolve($matches[2]); + } else { + $keyType = null; + $valueType = $templateTypes[$matches[1]] ?? $this->resolve($matches[1]); + } + + $valueType = explode('|', $valueType)[0]; + $keyType = $keyType ? explode('|', $keyType)[0] : null; return [ 'keyType' => $keyType, diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index affd2f1d..98deff84 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -7,7 +7,7 @@ use Spatie\LaravelData\Tests\Fakes\SimpleData; it( - 'can get the data class for a collection by annotation', + 'verifies the correct CollectionAnnotation is returned for a given class', function (string $className, ?CollectionAnnotation $expected) { $annotations = app(CollectionAnnotationReader::class)->getForClass(new ReflectionClass($className)); @@ -24,6 +24,26 @@ function (string $className, ?CollectionAnnotation $expected) { 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), ]; + yield DataCollectionWithCombinationType::class => [ + 'className' => DataCollectionWithCombinationType::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), + ]; + + yield DataCollectionWithIntegerKey::class => [ + 'className' => DataCollectionWithIntegerKey::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true, keyType: 'int'), + ]; + + yield DataCollectionWithCombinationKey::class => [ + 'className' => DataCollectionWithCombinationKey::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true, keyType: 'int'), + ]; + + yield DataCollectionWithoutKey::class => [ + 'className' => DataCollectionWithoutKey::class, + 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), + ]; + yield DataCollectionWithoutExtends::class => [ 'className' => DataCollectionWithoutExtends::class, 'expected' => null, @@ -39,6 +59,26 @@ function (string $className, ?CollectionAnnotation $expected) { 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), ]; + yield NonDataCollectionWithCombinationType::class => [ + 'className' => NonDataCollectionWithCombinationType::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + + yield NonDataCollectionWithIntegerKey::class => [ + 'className' => NonDataCollectionWithIntegerKey::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false, keyType: 'int'), + ]; + + yield NonDataCollectionWithCombinationKey::class => [ + 'className' => NonDataCollectionWithCombinationKey::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false, keyType: 'int'), + ]; + + yield NonDataCollectionWithoutKey::class => [ + 'className' => NonDataCollectionWithoutKey::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + yield NonDataCollectionWithoutExtends::class => [ 'className' => NonDataCollectionWithoutExtends::class, 'expected' => null, @@ -68,6 +108,34 @@ class DataCollectionWithoutTemplate extends Collection { } +/** + * @extends \Illuminate\Support\Collection + */ +class DataCollectionWithCombinationType extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class DataCollectionWithIntegerKey extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class DataCollectionWithCombinationKey extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection<\Spatie\LaravelData\Tests\Fakes\SimpleData> + */ +class DataCollectionWithoutKey extends Collection +{ +} + class DataCollectionWithoutExtends extends Collection { } @@ -89,6 +157,34 @@ class NonDataCollectionWithoutTemplate extends Collection { } +/** + * @extends \Illuminate\Support\Collection + */ +class NonDataCollectionWithCombinationType extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class NonDataCollectionWithIntegerKey extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection + */ +class NonDataCollectionWithCombinationKey extends Collection +{ +} + +/** + * @extends \Illuminate\Support\Collection<\Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum> + */ +class NonDataCollectionWithoutKey extends Collection +{ +} + class NonDataCollectionWithoutExtends extends Collection { } From d336e71ce8d491522e6227c2885d8ef9cd810ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 09:22:55 +0200 Subject: [PATCH 05/16] improve test with specific collection class --- .../SimpleDataCollectionWithAnotations.php | 15 +++++++++++++ tests/Support/DataPropertyTypeTest.php | 21 ++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 tests/Fakes/Collections/SimpleDataCollectionWithAnotations.php diff --git a/tests/Fakes/Collections/SimpleDataCollectionWithAnotations.php b/tests/Fakes/Collections/SimpleDataCollectionWithAnotations.php new file mode 100644 index 00000000..84385a63 --- /dev/null +++ b/tests/Fakes/Collections/SimpleDataCollectionWithAnotations.php @@ -0,0 +1,15 @@ + + */ +class SimpleDataCollectionWithAnotations extends Collection +{ +} diff --git a/tests/Support/DataPropertyTypeTest.php b/tests/Support/DataPropertyTypeTest.php index c9476e76..402c1046 100644 --- a/tests/Support/DataPropertyTypeTest.php +++ b/tests/Support/DataPropertyTypeTest.php @@ -35,6 +35,7 @@ use Spatie\LaravelData\Support\Types\NamedType; use Spatie\LaravelData\Support\Types\UnionType; use Spatie\LaravelData\Tests\Factories\FakeDataStructureFactory; +use Spatie\LaravelData\Tests\Fakes\Collections\SimpleDataCollectionWithAnotations; use Spatie\LaravelData\Tests\Fakes\ComplicatedData; use Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum; use Spatie\LaravelData\Tests\Fakes\SimpleData; @@ -574,7 +575,7 @@ function resolveDataType(object $class, string $property = 'property'): DataProp it('can deduce an enumerable data collection type from collection', function () { $type = resolveDataType(new class () { - public DataCollectionWithTemplate $property; + public SimpleDataCollectionWithAnotations $property; }); expect($type) @@ -584,21 +585,21 @@ function resolveDataType(object $class, string $property = 'property'): DataProp ->lazyType->toBeNull() ->kind->toBe(DataTypeKind::DataEnumerable) ->dataClass->toBe(SimpleData::class) - ->iterableClass->toBe(DataCollectionWithTemplate::class) - ->getAcceptedTypes()->toHaveKeys([DataCollectionWithTemplate::class]); + ->iterableClass->toBe(SimpleDataCollectionWithAnotations::class) + ->getAcceptedTypes()->toHaveKeys([SimpleDataCollectionWithAnotations::class]); expect($type->type) ->toBeInstanceOf(NamedType::class) - ->name->toBe(DataCollectionWithTemplate::class) + ->name->toBe(SimpleDataCollectionWithAnotations::class) ->builtIn->toBeFalse() ->kind->toBe(DataTypeKind::DataEnumerable) ->dataClass->toBe(SimpleData::class) - ->iterableClass->toBe(DataCollectionWithTemplate::class); + ->iterableClass->toBe(SimpleDataCollectionWithAnotations::class); }); it('can deduce an enumerable data collection union type from collection', function () { $type = resolveDataType(new class () { - public DataCollectionWithTemplate|Lazy $property; + public SimpleDataCollectionWithAnotations|Lazy $property; }); expect($type) @@ -608,16 +609,16 @@ function resolveDataType(object $class, string $property = 'property'): DataProp ->lazyType->toBe(Lazy::class) ->kind->toBe(DataTypeKind::DataEnumerable) ->dataClass->toBe(SimpleData::class) - ->iterableClass->toBe(DataCollectionWithTemplate::class) - ->getAcceptedTypes()->toHaveKeys([DataCollectionWithTemplate::class]); + ->iterableClass->toBe(SimpleDataCollectionWithAnotations::class) + ->getAcceptedTypes()->toHaveKeys([SimpleDataCollectionWithAnotations::class]); expect($type->type) ->toBeInstanceOf(NamedType::class) - ->name->toBe(DataCollectionWithTemplate::class) + ->name->toBe(SimpleDataCollectionWithAnotations::class) ->builtIn->toBeFalse() ->kind->toBe(DataTypeKind::DataEnumerable) ->dataClass->toBe(SimpleData::class) - ->iterableClass->toBe(DataCollectionWithTemplate::class); + ->iterableClass->toBe(SimpleDataCollectionWithAnotations::class); }); it('can deduce a paginator data collection type', function () { From e9edc6e731bc24e45dda6071d88970451a862799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 09:24:30 +0200 Subject: [PATCH 06/16] improve documentation example --- docs/as-a-data-transfer-object/nesting.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/as-a-data-transfer-object/nesting.md b/docs/as-a-data-transfer-object/nesting.md index 5a0e4aaf..3b7e98af 100644 --- a/docs/as-a-data-transfer-object/nesting.md +++ b/docs/as-a-data-transfer-object/nesting.md @@ -129,9 +129,9 @@ If the collection is well-annotated, the `Data` class doesn't need to use annota ```php /** * @template TKey of array-key - * @template TValue of \App\Data\SongData + * @template TData of \App\Data\SongData * - * @extends \Illuminate\Support\Collection + * @extends \Illuminate\Support\Collection */ class SongDataCollection extends Collection { From ec222070be53fcc74ef4237925cc933f957ab762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 09:29:34 +0200 Subject: [PATCH 07/16] improve code readability --- src/Support/Annotations/CollectionAnnotationReader.php | 2 +- tests/Support/Annotations/CollectionAnnotationReaderTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index 10288bed..c28f7778 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -104,8 +104,8 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array $valueType = $templateTypes[$matches[1]] ?? $this->resolve($matches[1]); } - $valueType = explode('|', $valueType)[0]; $keyType = $keyType ? explode('|', $keyType)[0] : null; + $valueType = explode('|', $valueType)[0]; return [ 'keyType' => $keyType, diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index 98deff84..4f9eaed2 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -93,9 +93,9 @@ function (string $className, ?CollectionAnnotation $expected) { /** * @template TKey of array-key - * @template TValue of \Spatie\LaravelData\Tests\Fakes\SimpleData + * @template TData of \Spatie\LaravelData\Tests\Fakes\SimpleData * - * @extends \Illuminate\Support\Collection + * @extends \Illuminate\Support\Collection */ class DataCollectionWithTemplate extends Collection { From 437584a3a51c7065226dba130ec84412e2974d74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Thu, 4 Jul 2024 09:31:04 +0200 Subject: [PATCH 08/16] format --- src/Support/Annotations/CollectionAnnotationReader.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index c28f7778..dcc68826 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -19,7 +19,8 @@ class CollectionAnnotationReader public function __construct( protected readonly TypeResolver $typeResolver, protected readonly ContextFactory $contextFactory, - ) {} + ) { + } protected Context $context; @@ -109,7 +110,7 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array return [ 'keyType' => $keyType, - 'valueType' => $valueType + 'valueType' => $valueType, ]; } } From 6c9d573259835396b016214ef0d652ac506d4606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Fri, 5 Jul 2024 15:51:57 +0200 Subject: [PATCH 09/16] improve detection of collections --- .../CollectionAnnotationReader.php | 4 -- .../CollectionAnnotationReaderTest.php | 67 ++++++++++++++----- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index dcc68826..226a0c11 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -2,7 +2,6 @@ namespace Spatie\LaravelData\Support\Annotations; -use Countable; use Iterator; use IteratorAggregate; use phpDocumentor\Reflection\DocBlock\Tags\Generic; @@ -12,7 +11,6 @@ use phpDocumentor\Reflection\Types\ContextFactory; use ReflectionClass; use Spatie\LaravelData\Data; -use Traversable; class CollectionAnnotationReader { @@ -123,10 +121,8 @@ protected function isCollection(ReflectionClass $class): bool { // Check if the class implements common collection interfaces $collectionInterfaces = [ - Traversable::class, Iterator::class, IteratorAggregate::class, - Countable::class, ]; foreach ($collectionInterfaces as $interface) { diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index 4f9eaed2..70b17800 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -44,11 +44,6 @@ function (string $className, ?CollectionAnnotation $expected) { 'expected' => new CollectionAnnotation(type: SimpleData::class, isData: true), ]; - yield DataCollectionWithoutExtends::class => [ - 'className' => DataCollectionWithoutExtends::class, - 'expected' => null, - ]; - yield NonDataCollectionWithTemplate::class => [ 'className' => NonDataCollectionWithTemplate::class, 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), @@ -79,13 +74,23 @@ function (string $className, ?CollectionAnnotation $expected) { 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), ]; - yield NonDataCollectionWithoutExtends::class => [ - 'className' => NonDataCollectionWithoutExtends::class, + yield CollectionWhoImplementsIterator::class => [ + 'className' => CollectionWhoImplementsIterator::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + + yield CollectionWhoImplementsIteratorAggregate::class => [ + 'className' => CollectionWhoImplementsIteratorAggregate::class, + 'expected' => new CollectionAnnotation(type: DummyBackedEnum::class, isData: false), + ]; + + yield CollectionWhoImplementsNothing::class => [ + 'className' => CollectionWhoImplementsNothing::class, 'expected' => null, ]; - yield NonCollectionWithTemplate::class => [ - 'className' => NonCollectionWithTemplate::class, + yield CollectionWithoutDocBlock::class => [ + 'className' => CollectionWithoutDocBlock::class, 'expected' => null, ]; }); @@ -136,10 +141,6 @@ class DataCollectionWithoutKey extends Collection { } -class DataCollectionWithoutExtends extends Collection -{ -} - /** * @template TKey of array-key * @template TValue of \Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum @@ -185,13 +186,47 @@ class NonDataCollectionWithoutKey extends Collection { } -class NonDataCollectionWithoutExtends extends Collection +/** + * @extends \Illuminate\Support\Collection + */ +class CollectionWhoImplementsIterator implements Iterator { + public function current(): mixed + { + } + public function next(): void + { + } + public function key(): mixed + { + } + public function valid(): bool + { + return true; + } + public function rewind(): void + { + } } /** - * @extends \Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum + * @extends \Illuminate\Support\Collection */ -class NonCollectionWithTemplate +class CollectionWhoImplementsIteratorAggregate implements IteratorAggregate +{ + public function getIterator(): Traversable + { + return $this; + } +} + +/** + * @extends \Illuminate\Support\Collection + */ +class CollectionWhoImplementsNothing +{ +} + +class CollectionWithoutDocBlock extends Collection { } From 0c6efb08154bd2b3e4ab023123ddc419d781c3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Fri, 5 Jul 2024 16:11:47 +0200 Subject: [PATCH 10/16] add missing test --- .../Annotations/CollectionAnnotationReaderTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index 70b17800..854c2d75 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -93,6 +93,11 @@ function (string $className, ?CollectionAnnotation $expected) { 'className' => CollectionWithoutDocBlock::class, 'expected' => null, ]; + + yield CollectionWithoutType::class => [ + 'className' => CollectionWithoutType::class, + 'expected' => null, + ]; }); @@ -230,3 +235,10 @@ class CollectionWhoImplementsNothing class CollectionWithoutDocBlock extends Collection { } + +/** + * @extends \Illuminate\Support\Collection + */ +class CollectionWithoutType extends Collection +{ +} From 141a40f8fe2892608b946f3fb961f21492d18456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Fri, 26 Jul 2024 10:10:56 +0200 Subject: [PATCH 11/16] Add ContextResolver class --- src/LaravelDataServiceProvider.php | 3 + src/Resolvers/ContextResolver.php | 24 +++++ .../CollectionAnnotationReader.php | 9 +- .../DataIterableAnnotationReader.php | 20 ++--- tests/Resolvers/ContextResolverTest.php | 87 +++++++++++++++++++ 5 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 src/Resolvers/ContextResolver.php create mode 100644 tests/Resolvers/ContextResolverTest.php diff --git a/src/LaravelDataServiceProvider.php b/src/LaravelDataServiceProvider.php index b5871384..748fe8db 100644 --- a/src/LaravelDataServiceProvider.php +++ b/src/LaravelDataServiceProvider.php @@ -6,6 +6,7 @@ use Spatie\LaravelData\Commands\DataMakeCommand; use Spatie\LaravelData\Commands\DataStructuresCacheCommand; use Spatie\LaravelData\Contracts\BaseData; +use Spatie\LaravelData\Resolvers\ContextResolver; use Spatie\LaravelData\Support\Caching\DataStructureCache; use Spatie\LaravelData\Support\DataConfig; use Spatie\LaravelData\Support\Livewire\LivewireDataCollectionSynth; @@ -43,6 +44,8 @@ function () { } ); + $this->app->singleton(ContextResolver::class); + $this->app->beforeResolving(BaseData::class, function ($class, $parameters, $app) { if ($app->has($class)) { return; diff --git a/src/Resolvers/ContextResolver.php b/src/Resolvers/ContextResolver.php new file mode 100644 index 00000000..4dfa8177 --- /dev/null +++ b/src/Resolvers/ContextResolver.php @@ -0,0 +1,24 @@ + */ + protected array $contexts = []; + + public function get(ReflectionProperty|ReflectionClass|ReflectionMethod $reflection): Context + { + $reflectionClass = $reflection instanceof ReflectionProperty || $reflection instanceof ReflectionMethod + ? $reflection->getDeclaringClass() + : $reflection; + + return $this->contexts[$reflectionClass->getName()] ??= (new ContextFactory())->createFromReflector($reflectionClass); + } +} diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index 226a0c11..9e976585 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -8,15 +8,15 @@ use phpDocumentor\Reflection\DocBlockFactory; use phpDocumentor\Reflection\TypeResolver; use phpDocumentor\Reflection\Types\Context; -use phpDocumentor\Reflection\Types\ContextFactory; use ReflectionClass; use Spatie\LaravelData\Data; +use Spatie\LaravelData\Resolvers\ContextResolver; class CollectionAnnotationReader { public function __construct( + protected readonly ContextResolver $contextResolver, protected readonly TypeResolver $typeResolver, - protected readonly ContextFactory $contextFactory, ) { } @@ -55,10 +55,7 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array // Initialize TypeResolver and DocBlockFactory $docBlockFactory = DocBlockFactory::createInstance(); - // Extract the namespace and uses from the file content - $namespace = $class->getNamespaceName(); - $fileContent = file_get_contents($class->getFileName()); - $this->context = $this->contextFactory->createForNamespace($namespace, $fileContent); + $this->context = $this->contextResolver->get($class); // Get the PHPDoc comment of the class $docComment = $class->getDocComment(); diff --git a/src/Support/Annotations/DataIterableAnnotationReader.php b/src/Support/Annotations/DataIterableAnnotationReader.php index c3a63308..2a09bde8 100644 --- a/src/Support/Annotations/DataIterableAnnotationReader.php +++ b/src/Support/Annotations/DataIterableAnnotationReader.php @@ -4,20 +4,21 @@ use Illuminate\Support\Arr; use phpDocumentor\Reflection\FqsenResolver; -use phpDocumentor\Reflection\Types\Context; -use phpDocumentor\Reflection\Types\ContextFactory; use ReflectionClass; use ReflectionMethod; use ReflectionProperty; use Spatie\LaravelData\Contracts\BaseData; +use Spatie\LaravelData\Resolvers\ContextResolver; /** * @note To myself, always use the fully qualified class names in pest tests when using anonymous classes */ class DataIterableAnnotationReader { - /** @var array */ - protected static array $contexts = []; + public function __construct( + protected readonly ContextResolver $contextResolver, + ) { + } /** @return array */ public function getForClass(ReflectionClass $class): array @@ -196,19 +197,10 @@ protected function resolveFcqn( ReflectionProperty|ReflectionClass|ReflectionMethod $reflection, string $class ): ?string { - $context = $this->getContext($reflection); + $context = $this->contextResolver->get($reflection); $type = (new FqsenResolver())->resolve($class, $context); return ltrim((string) $type, '\\'); } - - protected function getContext(ReflectionProperty|ReflectionClass|ReflectionMethod $reflection): Context - { - $reflectionClass = $reflection instanceof ReflectionProperty || $reflection instanceof ReflectionMethod - ? $reflection->getDeclaringClass() - : $reflection; - - return static::$contexts[$reflectionClass->getName()] ??= (new ContextFactory())->createFromReflector($reflectionClass); - } } diff --git a/tests/Resolvers/ContextResolverTest.php b/tests/Resolvers/ContextResolverTest.php new file mode 100644 index 00000000..202697f0 --- /dev/null +++ b/tests/Resolvers/ContextResolverTest.php @@ -0,0 +1,87 @@ +get($reflectionProperty); + + // Create expected context + $expectedContext = (new ContextFactory())->createFromReflector($reflectionProperty->getDeclaringClass()); + + // Assertions + expect($context)->toBeInstanceOf(Context::class); + expect($context)->toEqual($expectedContext); +}); + +it('can resolve context from class', function () { + $resolver = new ContextResolver(); + + // Create a ReflectionClass for the test class + $reflectionClass = new ReflectionClass(TestContextResolverClass::class); + + // Resolve the context + $context = $resolver->get($reflectionClass); + + // Create expected context + $expectedContext = (new ContextFactory())->createFromReflector($reflectionClass); + + // Assertions + expect($context)->toBeInstanceOf(Context::class); + expect($context)->toEqual($expectedContext); +}); + +it('can resolve context from method', function () { + $resolver = new ContextResolver(); + + // Create a ReflectionMethod for the test class method + $reflectionMethod = new ReflectionMethod(TestContextResolverClass::class, 'testMethod'); + + // Resolve the context + $context = $resolver->get($reflectionMethod); + + // Create expected context + $expectedContext = (new ContextFactory())->createFromReflector($reflectionMethod->getDeclaringClass()); + + // Assertions + expect($context)->toBeInstanceOf(Context::class); + expect($context)->toEqual($expectedContext); +}); + +it('uses cache when resolving the same class multiple times', function () { + $resolver = new ContextResolver(); + + // Create a ReflectionClass for the test class + $reflectionClass = new ReflectionClass(TestContextResolverClass::class); + + // Resolve the context the first time + $context1 = $resolver->get($reflectionClass); + + // Resolve the context the second time + $context2 = $resolver->get($reflectionClass); + + // Assertions + expect($context1)->toBeInstanceOf(Context::class); + expect($context2)->toBeInstanceOf(Context::class); + expect($context1)->toBe($context2); // They should be the same instance, indicating the cache was used +}); + +// Test class +class TestContextResolverClass +{ + public $testProperty; + + public function testMethod() + { + } +} From 6f452f6de991b86e5506caa00e7a56f64dac726f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Fri, 26 Jul 2024 18:13:38 +0200 Subject: [PATCH 12/16] Add a cache for CollectionAnnotationReader --- .../CollectionAnnotationReader.php | 32 +++++++++++++------ src/Support/Factories/DataTypeFactory.php | 2 +- .../CollectionAnnotationReaderTest.php | 2 +- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index 9e976585..2c08e59a 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -20,31 +20,45 @@ public function __construct( ) { } + /** @var array */ + protected static array $cache = []; + protected Context $context; - public function getForClass(ReflectionClass $class): ?CollectionAnnotation + public function getForClass(string $className): ?CollectionAnnotation { + // Check the cache first + if (array_key_exists($className, self::$cache)) { + return self::$cache[$className]; + } + + // Create ReflectionClass from class string + $class = new ReflectionClass($className); + + // Determine if the class is a collection if (! $this->isCollection($class)) { - return null; + return self::$cache[$className] = null; } + // Get the collection return type $type = $this->getCollectionReturnType($class); if ($type === null || $type['valueType'] === null) { - return null; + return self::$cache[$className] = null; } - $isData = false; - - if (is_subclass_of($type['valueType'], Data::class)) { - $isData = true; - } + $isData = is_subclass_of($type['valueType'], Data::class); - return new CollectionAnnotation( + $annotation = new CollectionAnnotation( type: $type['valueType'], isData: $isData, keyType: $type['keyType'] ?? 'array-key', ); + + // Cache the result + self::$cache[$className] = $annotation; + + return $annotation; } /** diff --git a/src/Support/Factories/DataTypeFactory.php b/src/Support/Factories/DataTypeFactory.php index 9bbfe73d..cc9e294a 100644 --- a/src/Support/Factories/DataTypeFactory.php +++ b/src/Support/Factories/DataTypeFactory.php @@ -360,7 +360,7 @@ protected function inferPropertiesForNamedType( $iterableItemType === null && $typeable instanceof ReflectionProperty && class_exists($name) - && $annotation = $this->collectionAnnotationReader->getForClass(new ReflectionClass($name)) + && $annotation = $this->collectionAnnotationReader->getForClass($name) ) { $isData = $annotation->isData; $iterableItemType = $annotation->type; diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index 854c2d75..09033822 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -9,7 +9,7 @@ it( 'verifies the correct CollectionAnnotation is returned for a given class', function (string $className, ?CollectionAnnotation $expected) { - $annotations = app(CollectionAnnotationReader::class)->getForClass(new ReflectionClass($className)); + $annotations = app(CollectionAnnotationReader::class)->getForClass($className); expect($annotations)->toEqual($expected); } From ef9277451bb76b763ec3da1a4a485cb7c11d515a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Fri, 26 Jul 2024 22:02:20 +0200 Subject: [PATCH 13/16] add integration test for collection attribute with anotations --- .../CollectionAttributeWithAnotationsTest.php | 49 +++++++++++++++++++ ...WithSimpleDataCollectionWithAnotations.php | 14 ++++++ 2 files changed, 63 insertions(+) create mode 100644 tests/CollectionAttributeWithAnotationsTest.php create mode 100644 tests/Fakes/DataWithSimpleDataCollectionWithAnotations.php diff --git a/tests/CollectionAttributeWithAnotationsTest.php b/tests/CollectionAttributeWithAnotationsTest.php new file mode 100644 index 00000000..af001b14 --- /dev/null +++ b/tests/CollectionAttributeWithAnotationsTest.php @@ -0,0 +1,49 @@ +payload = [ + 'collection' => [ + ['string' => 'string1'], + ['string' => 'string2'], + ['string' => 'string3'], + ], + ]; +}); + +it('can create a data object with a collection attribute from array and back', function () { + + $data = DataWithSimpleDataCollectionWithAnotations::from($this->payload); + + expect($data)->toEqual(new DataWithSimpleDataCollectionWithAnotations( + collection: new SimpleDataCollectionWithAnotations([ + new SimpleData(string: 'string1'), + new SimpleData(string: 'string2'), + new SimpleData(string: 'string3'), + ]) + )); + + expect($data->toArray())->toBe($this->payload); +}); + +it('can validate a data object with a collection attribute', function () { + + DataValidationAsserter::for(DataWithSimpleDataCollectionWithAnotations::class) + ->assertOk($this->payload) + ->assertErrors(['collection' => [ + ['notExistingAttribute' => 'xxx'], + ]]) + ->assertRules( + rules: [ + 'collection' => ['present', 'array'], + 'collection.0.string' => ['required', 'string'], + 'collection.1.string' => ['required', 'string'], + 'collection.2.string' => ['required', 'string'], + ], + payload: $this->payload + ); +}); diff --git a/tests/Fakes/DataWithSimpleDataCollectionWithAnotations.php b/tests/Fakes/DataWithSimpleDataCollectionWithAnotations.php new file mode 100644 index 00000000..8116ac5e --- /dev/null +++ b/tests/Fakes/DataWithSimpleDataCollectionWithAnotations.php @@ -0,0 +1,14 @@ + Date: Fri, 26 Jul 2024 22:08:40 +0200 Subject: [PATCH 14/16] remove some uses to avoid warning --- tests/Resolvers/ContextResolverTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Resolvers/ContextResolverTest.php b/tests/Resolvers/ContextResolverTest.php index 202697f0..fe026ac2 100644 --- a/tests/Resolvers/ContextResolverTest.php +++ b/tests/Resolvers/ContextResolverTest.php @@ -2,9 +2,6 @@ use phpDocumentor\Reflection\Types\Context; use phpDocumentor\Reflection\Types\ContextFactory; -use ReflectionClass; -use ReflectionMethod; -use ReflectionProperty; use Spatie\LaravelData\Resolvers\ContextResolver; it('can resolve context from property', function () { From 72ac7ef893a27d328af665a9d9547e2591612b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Sat, 27 Jul 2024 22:34:32 +0200 Subject: [PATCH 15/16] Add test for CollectionAnnotationReader cache --- .../CollectionAnnotationReader.php | 52 ++++++++++++------- .../CollectionAnnotationReaderTest.php | 31 +++++++++++ 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index 2c08e59a..b14a903f 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -25,6 +25,9 @@ public function __construct( protected Context $context; + /** + * @param class-string $className + */ public function getForClass(string $className): ?CollectionAnnotation { // Check the cache first @@ -33,7 +36,7 @@ public function getForClass(string $className): ?CollectionAnnotation } // Create ReflectionClass from class string - $class = new ReflectionClass($className); + $class = $this->getReflectionClass($className); // Determine if the class is a collection if (! $this->isCollection($class)) { @@ -61,6 +64,36 @@ public function getForClass(string $className): ?CollectionAnnotation return $annotation; } + public static function clearCache(): void + { + self::$cache = []; + } + + /** + * @param class-string $className + */ + protected function getReflectionClass(string $className): ReflectionClass + { + return new ReflectionClass($className); + } + + protected function isCollection(ReflectionClass $class): bool + { + // Check if the class implements common collection interfaces + $collectionInterfaces = [ + Iterator::class, + IteratorAggregate::class, + ]; + + foreach ($collectionInterfaces as $interface) { + if ($class->implementsInterface($interface)) { + return true; + } + } + + return false; + } + /** * @return array{keyType: string|null, valueType: string|null}|null */ @@ -128,23 +161,6 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array return null; } - protected function isCollection(ReflectionClass $class): bool - { - // Check if the class implements common collection interfaces - $collectionInterfaces = [ - Iterator::class, - IteratorAggregate::class, - ]; - - foreach ($collectionInterfaces as $interface) { - if ($class->implementsInterface($interface)) { - return true; - } - } - - return false; - } - protected function resolve(string $type): ?string { $type = (string) $this->typeResolver->resolve($type, $this->context); diff --git a/tests/Support/Annotations/CollectionAnnotationReaderTest.php b/tests/Support/Annotations/CollectionAnnotationReaderTest.php index 09033822..18be76d9 100644 --- a/tests/Support/Annotations/CollectionAnnotationReaderTest.php +++ b/tests/Support/Annotations/CollectionAnnotationReaderTest.php @@ -1,11 +1,17 @@ makePartial(); + + // Call the getForClass method with a test class + $collectionAnnotation = $collectionAnnotationReader->getForClass($className); + + // Call the getForClass method again to test caching + $cachedCollectionAnnotation = $collectionAnnotationReader->getForClass($className); + + // Assert the cache is used and the same annotation is returned + expect($cachedCollectionAnnotation)->toBe($collectionAnnotation); + + // Check if getReflectionClass was called only once + $collectionAnnotationReader->shouldHaveReceived('getReflectionClass')->once(); + +})->with([ + [CollectionWhoImplementsNothing::class], // first return + [CollectionWithoutDocBlock::class], // second return + [DataCollectionWithTemplate::class], // third return +]); /** * @template TKey of array-key From 9fe2d2254938491dcf3da92e96b9ba5502468501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Birkl=C3=A9?= Date: Sun, 28 Jul 2024 10:35:20 +0200 Subject: [PATCH 16/16] rename method get to execute in ContextResolver for consistency --- src/Resolvers/ContextResolver.php | 2 +- src/Support/Annotations/CollectionAnnotationReader.php | 2 +- .../Annotations/DataIterableAnnotationReader.php | 2 +- tests/Resolvers/ContextResolverTest.php | 10 +++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Resolvers/ContextResolver.php b/src/Resolvers/ContextResolver.php index 4dfa8177..d82c0f6d 100644 --- a/src/Resolvers/ContextResolver.php +++ b/src/Resolvers/ContextResolver.php @@ -13,7 +13,7 @@ class ContextResolver /** @var array */ protected array $contexts = []; - public function get(ReflectionProperty|ReflectionClass|ReflectionMethod $reflection): Context + public function execute(ReflectionProperty|ReflectionClass|ReflectionMethod $reflection): Context { $reflectionClass = $reflection instanceof ReflectionProperty || $reflection instanceof ReflectionMethod ? $reflection->getDeclaringClass() diff --git a/src/Support/Annotations/CollectionAnnotationReader.php b/src/Support/Annotations/CollectionAnnotationReader.php index b14a903f..896940d1 100644 --- a/src/Support/Annotations/CollectionAnnotationReader.php +++ b/src/Support/Annotations/CollectionAnnotationReader.php @@ -102,7 +102,7 @@ protected function getCollectionReturnType(ReflectionClass $class): ?array // Initialize TypeResolver and DocBlockFactory $docBlockFactory = DocBlockFactory::createInstance(); - $this->context = $this->contextResolver->get($class); + $this->context = $this->contextResolver->execute($class); // Get the PHPDoc comment of the class $docComment = $class->getDocComment(); diff --git a/src/Support/Annotations/DataIterableAnnotationReader.php b/src/Support/Annotations/DataIterableAnnotationReader.php index 2a09bde8..61545c6e 100644 --- a/src/Support/Annotations/DataIterableAnnotationReader.php +++ b/src/Support/Annotations/DataIterableAnnotationReader.php @@ -197,7 +197,7 @@ protected function resolveFcqn( ReflectionProperty|ReflectionClass|ReflectionMethod $reflection, string $class ): ?string { - $context = $this->contextResolver->get($reflection); + $context = $this->contextResolver->execute($reflection); $type = (new FqsenResolver())->resolve($class, $context); diff --git a/tests/Resolvers/ContextResolverTest.php b/tests/Resolvers/ContextResolverTest.php index fe026ac2..506bd3e2 100644 --- a/tests/Resolvers/ContextResolverTest.php +++ b/tests/Resolvers/ContextResolverTest.php @@ -11,7 +11,7 @@ $reflectionProperty = new ReflectionProperty(TestContextResolverClass::class, 'testProperty'); // Resolve the context - $context = $resolver->get($reflectionProperty); + $context = $resolver->execute($reflectionProperty); // Create expected context $expectedContext = (new ContextFactory())->createFromReflector($reflectionProperty->getDeclaringClass()); @@ -28,7 +28,7 @@ $reflectionClass = new ReflectionClass(TestContextResolverClass::class); // Resolve the context - $context = $resolver->get($reflectionClass); + $context = $resolver->execute($reflectionClass); // Create expected context $expectedContext = (new ContextFactory())->createFromReflector($reflectionClass); @@ -45,7 +45,7 @@ $reflectionMethod = new ReflectionMethod(TestContextResolverClass::class, 'testMethod'); // Resolve the context - $context = $resolver->get($reflectionMethod); + $context = $resolver->execute($reflectionMethod); // Create expected context $expectedContext = (new ContextFactory())->createFromReflector($reflectionMethod->getDeclaringClass()); @@ -62,10 +62,10 @@ $reflectionClass = new ReflectionClass(TestContextResolverClass::class); // Resolve the context the first time - $context1 = $resolver->get($reflectionClass); + $context1 = $resolver->execute($reflectionClass); // Resolve the context the second time - $context2 = $resolver->get($reflectionClass); + $context2 = $resolver->execute($reflectionClass); // Assertions expect($context1)->toBeInstanceOf(Context::class);