Skip to content
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

Improve TIntermediateModels generics and add inheritDoc #102

Merged
merged 16 commits into from
Sep 9, 2024

Conversation

SanderMuller
Copy link
Contributor

@calebdw thanks for your feedback on #101 can you help resolve the last two phpstan errors?

composer.json Outdated Show resolved Hide resolved
tests/Models/Comment.php Outdated Show resolved Hide resolved
@staudenmeir
Copy link
Owner

staudenmeir commented Sep 5, 2024

Thanks @SanderMuller and @calebdw.

Regarding @inheritDoc: I tested this last week and also had the issue that it didn't work in every case. When I use it for BelongsToThrough::newRelatedInstanceFor(), PHPStan reports an error:

Method Znck\Eloquent\Relations\BelongsToThrough::newRelatedInstanceFor() has no return type specified.

The method's "parent" is an abstract method in the SupportsDefaultModels trait. Maybe this case isn't supported?

@calebdw
Copy link
Contributor

calebdw commented Sep 5, 2024

Hmm, yeah having an abstract method on a trait that's included on the same class is a little weird---might be a use case to open a ticket at PHPStan for

@staudenmeir
Copy link
Owner

Hmm, yeah having an abstract method on a trait that's included on the same class is a little weird

Yeah, definitely a special case.

@staudenmeir
Copy link
Owner

Thanks @SanderMuller!

@staudenmeir staudenmeir marked this pull request as ready for review September 9, 2024 12:01
@staudenmeir staudenmeir merged commit 28cbfca into staudenmeir:main Sep 9, 2024
7 of 8 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.

3 participants