Skip to content

Add support for generics #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 28, 2025
Merged

Add support for generics #108

merged 11 commits into from
May 28, 2025

Conversation

HendrikN
Copy link
Contributor

@HendrikN HendrikN commented Apr 18, 2025

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the CONTRIBUTING document.
  • My pull request addresses exactly one patch/feature.
  • I have created a branch for this patch/feature.
  • Each individual commit in the pull request is meaningful.
  • I have added tests to cover my changes.
  • If my change requires a change to the documentation, I have updated it accordingly.

@HendrikN HendrikN force-pushed the feature/generic-types branch 3 times, most recently from 299343e to 9999b50 Compare April 22, 2025 06:22
@HendrikN HendrikN force-pushed the feature/generic-types branch from 9999b50 to d294a72 Compare April 22, 2025 06:24
@bbrala
Copy link
Member

bbrala commented Apr 30, 2025

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

@bbrala bbrala requested a review from Copilot May 2, 2025 15:21
Copilot

This comment was marked as outdated.

@bbrala
Copy link
Member

bbrala commented May 2, 2025

(lets toy with copilot see what happens, boo php)

@JaZo JaZo requested a review from Copilot May 8, 2025 18:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@HendrikN
Copy link
Contributor Author

So uhm, how can we move forward with this PR? Do you expect me to follow up on some things or can we merge as-is?

@bbrala
Copy link
Member

bbrala commented May 15, 2025

Think we can just add a todo as @JaZo mentioned for now and move forward further when we have phpstan.

Copy link
Member

@JaZo JaZo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added type tests, inspired by these from Laravel, but they still fail on the repository types. I'd like some help resolving that.

@HendrikN
Copy link
Contributor Author

HendrikN commented May 28, 2025

The type tests are fixed (thx for the initial setup!) by adding some @use annotations for the traits. I also added some type assertions.

Further, upon second reading, I changed the signature of the Collection from array-key, item (array-key being int|string) to int, item. Pretty sure that's the case anyhow since I cant imagine a situation where strings are passed as keys.

@HendrikN HendrikN force-pushed the feature/generic-types branch from c2904d5 to 78c530c Compare May 28, 2025 08:15
@JaZo
Copy link
Member

JaZo commented May 28, 2025

Great! Using int as collection key makes more sense yes.

Copy link
Member

@JaZo JaZo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaZo JaZo merged commit 5157137 into swisnl:master May 28, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PHPStan types
3 participants