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 handling of read types on custom model casts #1444

Closed
wants to merge 3 commits into from

Conversation

tadhgboyle
Copy link

@tadhgboyle tadhgboyle commented Nov 11, 2022

  • Added or updated tests
  • Documented user facing changes (I don't think this is needed?)
  • Updated CHANGELOG.md

Changes
This PR fixes a pretty annoying issue while using Larastan and model casts.
For example, when using the Laravel Money package, it has a custom cast class \Cknow\Money\Casts\MoneyIntegerCast.
This casts get method returns a \Cknow\Money\Money object, but Larastan only detected the MoneyIntegerCast as the return type.
This causes dozens of errors similar to:

Parameter #1 $price of static method                              
         `App\Helpers\TaxHelper::calculateFor()` expects `Cknow\Money\Money`
         `Cknow\Money\Casts\MoneyIntegerCast` given.

To fix this issue, I have used reflection to determine the return type of the casts underlying get method, and (if it is defined), use that as the read type for the attribute.

I am aware of this PR #1333, but I thought a smaller PR to fix this problem faster would be nice, until that one gets merged.

@canvural
Copy link
Collaborator

Since #1333 is merged, we no longer need this I guess. Please send a new PR if you see that something can be improved. Thanks.

@canvural canvural closed this Dec 29, 2022
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.

None yet

2 participants