rector-laravel icon indicating copy to clipboard operation
rector-laravel copied to clipboard

ModelCastsPropertyToCastsMethodRector: note that PHP/Larastan will need additional array shape PHPDoc?

Open ghbob opened this issue 8 months ago • 5 comments

https://getrector.com/rule-detail/model-casts-property-to-casts-method-rector

Would it make sense to include a note here that additional steps would be needed to avoid PHP/Larastan errors?

For PHP/Larastan to recognize the new method casts, an array shape is necessary, as per the upgrade doc: https://github.com/larastan/larastan/blob/3.x/UPGRADE.md#upgrading-to-296-from-295 (discussed here: https://github.com/larastan/larastan/issues/1877)

In general I'd love more details in Rector (and Rector Laravel) rule docs, the "why" of why this rule makes sense. In this case:

The default way changed with 11 (but the old one is still supported): https://laravel.com/docs/11.x/releases#model-cast-improvements See also: https://laravel-news.com/model-casts

Would this be okay or are the descriptions meant to be brief?

Thanks.

ghbob avatar Apr 25 '25 07:04 ghbob

@ghbob as far as I know Larastan 3 doesn't need the extra array shape to work. And the rule copies the existing PHPDocs from the old property to the new class method, so no new errors should appear. If there really is a need for array shape, it may be only for the Larastan 2.9.6 version, and I'm not sure it's worth it to indicate extra steps on the rule just for a Larastan version that's meant to help you get to v3.

But in general I agree that we need more details on Rector rules, on the rationale behind them and any gotchas.

GeniJaho avatar Apr 26 '25 10:04 GeniJaho

@GeniJaho hm, I am pretty sure I had everyting up-to-date and it complained about "you can't put this enum on a string" (so, an enum casted value), I'll have another look in a few days, maybe I misconfigured something. Thanks!

ghbob avatar Apr 26 '25 11:04 ghbob

If the array shape makes the error go away, feel free to submit a PR to update the Rule documentation 👍

GeniJaho avatar Apr 26 '25 11:04 GeniJaho

Yup, array shapes help.

And I think this confirms it's still required for Larastan 3 + the new casts() function: https://github.com/larastan/larastan/issues/2250#issuecomment-2777957898

I'll create a PR for this info.

I guess writing that PHPDoc block would be possible with Rector? If I find the time I'll have a look if I could write this rule (by having a look at other rules).

@GeniJaho for ModelCastsPropertyToCastsMethodRector in general, two things I noticed when running it again:

  1. The new functions are added at the bottom of the class, I think it would be better if they were replaced in-place of the old property instead? Probably just a personal preference to still have these at the top of the class but Git would probably be happier as well.
  2. The new function is also added there, at the bottom, without an empty line before it, not sure if that's intended.

Thanks!

ghbob avatar May 09 '25 15:05 ghbob

Just wanted to comment that the array shape will not be needed if https://github.com/larastan/larastan/pull/2197 is merged

calebdw avatar Aug 06 '25 04:08 calebdw