Add support for generics #108
Conversation
HendrikN
force-pushed
the
feature/generic-types
branch
3 times, most recently
from
299343e to
9999b50
Compare
HendrikN
force-pushed
the
feature/generic-types
branch
from
9999b50 to
d294a72
Compare
| */ | ||
| public function all(); | ||
|
|
||
| /** | ||
| * @return \Swis\JsonApi\Client\Interfaces\ItemDocumentInterface | ||
| * @return \Swis\JsonApi\Client\Interfaces\ItemDocumentInterface<TItem> | ||
| */ | ||
| public function find(string $id); |
There was a problem hiding this comment.
When i check the tests, i think this return type is not correct. It seems this can also return a DocumentInterface. See RepositoryTest::it_can_find_one.
This could also mean the interface on the Document is wrong. seems a little weird, since ItemDocument is pretty much the same class with the extra getData method.
Think there is something wrong here. @JaZo any insights?
There was a problem hiding this comment.
Yeah I had the same questions, but I didnt want to touch actual code in this PR… ItemDocument seems to follow the spec though…
There was a problem hiding this comment.
I would probably make it documentinterface, since itemdocumentinterface does extend documentinterface. But that will mean some errors later with phpstan when we are calling getData.
There was a problem hiding this comment.
It would also make this PR mostly obsolete if find() would not resolve to an Item 😋
There was a problem hiding this comment.
The primary reason to have a CollectionDocumentInterface and an ItemDocumentInterface is to differentiate between the return types of getData. The former returns a collection and the latter an item. This stems from ancient times when generics weren't a thing. I'm okay with changing that to generics, but that would probably be a breaking change, so maybe we should leave that for now and just add a todo.
| /** | ||
| * @param \Swis\JsonApi\Client\Collection<array-key, TItem>|null $data | ||
| * @return static | ||
| */ | ||
| public function setData(?Collection $data); |
There was a problem hiding this comment.
Return static also points at a little weirdness, why isn't that type hinted? And should it be?
| interface OneRelationInterface | ||
| { | ||
| /** | ||
| * @param TItem|null $data | ||
| * @return static |
|
Thank so much for helping out here :) I left some small comments, which might need the help of @JaZo. I think we are going to surface a few incinsistencies when we start running phpstan on max. :x |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
(lets toy with copilot see what happens, boo php) |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces generic type support via PHPDoc annotations to improve static analysis and developer experience when using the package.
- Added generics annotations to repository, document, and relation interfaces.
- Enhanced the HasRelations trait to properly document generic relation methods.
- Updated the Collection class and README documentation to reflect the new generics feature.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Interfaces/RepositoryInterface.php | Added generics to the return types of repository methods. |
| src/Interfaces/OneRelationInterface.php | Annotated relation methods with generics for type consistency. |
| src/Interfaces/ManyRelationInterface.php | Updated phpdoc annotations with generics on collection types. |
| src/Interfaces/ItemDocumentInterface.php | Specified generic return type for document data extraction. |
| src/Interfaces/CollectionDocumentInterface.php | Added generics to collection return types. |
| src/Concerns/HasRelations.php | Enhanced relation factory methods with inline generic templates. |
| src/Collection.php | Added generics annotations to extend the Laravel Collection class. |
| composer.json | Removed an author entry. |
| README.md | Expanded documentation with usage examples on generics. |
Description
Adds support for using generics (using PHPStan types), by adding the correct phpdoc annotations to Interfaces. No actual code has been changed, so this could be released as a minor.
The next step would be completely incorporating PHPStan and use it as a QA-tool in the pipeline.
Motivation and context
Modern IDE's have support for them and this greatly enhances the DX of using this package.
fixes #107
How has this been tested?
N/A
Types of changes
Checklist: