plugin icon indicating copy to clipboard operation
plugin copied to clipboard

[Bug]: Model casts set from traits are not used in generated helper files

Open juse-less opened this issue 2 years ago • 4 comments

Bug description

Apologies in advance. Not sure if this should be more of a feature request than a bug request.

When using trait(s) to populate a Model's cast attribute(s), those data types are not used. Instead, the migration's type is used.


There's a freshly generated Laravel project, with just some small additions to demonstrate, here -> https://github.com/juse-less/laravel-idea-trait-model-casts-issue.

I've added TODO's so the relevant lines should be searchable for // TODO: test trait, with a short description afterwards. I used the default User Model migration and added my trait to the default User Model that sets the attribute in the Model::$casts property. I've tried with and without using Model::$fillable as well, but opted to remove it in the example.

The generated helper file will show the $is_active attribute as bool, and not string, that I used to distinguish the trait from the database in this example. I also added a quick method to quickly inspect the attribute info.


Side note: If the column is commented/removed from the migration, the attribute isn't output at all, in the generated helper file for the Model.

Plugin version

5.7.3.221

Operating system

MacOS

Steps to reproduce

All that should be needed is to check out the provided repo above, composer install and generate the helper files.

Relevant log output

No response

juse-less avatar Aug 09 '22 23:08 juse-less

Thank you for the time spent on this. You wrote on Twitter: "The SoftDeletes/deleted_at being called $6 works in this fresh repo (tested before committing)." What did you mean?

adelf avatar Aug 10 '22 10:08 adelf

No worries. 🙂

What I meant was that, on the project I first ran the generation command on, the $deleted_at attribute ended up being called $6 in the generated class helper file. The reason I'm so sure it's the $deleted_at attribute is because it was the only attribute missing from the generated file, and all others also being present.

Before I committed the code for this example (as it was a freshly generated project), I took the liberty to try adding $table->softDeletes() to the User migration, as well as adding the SoftDeletes trait to the User class. However, in the project I linked to in this issue, $deleted_at correctly showed up in the generated helper class file for User, and nothing called $6. It was also the User class I saw this showing up on in the other project.

So I'm pretty sure it's something I have in the other project that's causing it, and unable to reproduce here.

If it's of interest, I attached the attributes from the generated User helper class from the other project here

They are ordered by occurrence in migrations, from what I can see, and the $deleted_at comes directly after $created_at and $updated_at in the migrations.

/**
 * @property int $id
 * @property bool $is_active
 * @property string $email
 * @property Carbon|null $email_verified_at
 * @property string $first_name
 * @property string $last_name
 * @property string|null $timezone
 * @property \null|\string $password
 * @property string|null $remember_token
 * @property string|null $two_factor_secret
 * @property string|null $two_factor_recovery_codes
 * @property CarbonImmutable|null $two_factor_confirmed_at
 * @property string|null $profile_photo_path
 * @property Carbon $created_at
 * @property Carbon $updated_at
 * @property Carbon|null $6
 * @property int|null $current_team_id
 * @property-read string $profile_photo_url attribute
 * @property-read string $name attribute
 *
 * // Relationship attributes/properties
 */

juse-less avatar Aug 10 '22 11:08 juse-less

That's weird. Sad I can't reproduce it :(

About casts. I don't run PHP code. I only analyze it, so it's hard to check every possible way to change casts, like this "casts +=" call. However, you can just add this to your trait and it will work correctly:

/**
 * @property string $is_active
 */
trait HasActiveState{}

adelf avatar Aug 10 '22 12:08 adelf

Hmm. I actually do that on all my traits, and they are all using the data type from the migrations.

The biggest problem with the project I'm working on is that because of its size, we opted to use custom namespaces. But.. this has caused more bugs than it helped, both within Laravel itself as well as other dependencies. So it's a bit hard to filter out what's our own fault (by doing unsupported/unintended things), and what's an actual bug. This was also the reason I initially asked if there was some documentation or so, so we could adjust accordingly.

Safe to say - it's just better to refactor into the default namespace, which should solve many of our bugs. (Custom namespaces weren't supported in Laravel to begin with.)

Anyway, I did try to add the DocBlock in your snippet to the trait in my linked repo (locally, not committed), but it still adds bool to it in the generated helper class, even if I delete the vendor/_laravel_idea directory. So if you can confirm that it does work for you, then I have a hunch it's something in my PhpStorm, and I can go from there and get back if I can find out what it is. It's not uncommon for plugins disrupt each other, for instance. 🙂

Edit: Actually, I think I misunderstood. It's still bool in the generated helper file, but uncommenting and looking at the property in the autocompletion popup, does indeed show it as string. I've been looking at the generated helper files all the time. 🤦‍♀️

This alternative works for me, to override what I need differently. 🙂 Thanks for the help, and this great plugin. It's very helpful and saves a lot of time. ❤️ It's gonna be interesting how much more help/assistance I'll get (on top of what I already had), now that I know about the helper classes that can be generated, which I had completely missed.

juse-less avatar Aug 10 '22 12:08 juse-less

Closing this as

  1. It's not really a bug
  2. It's not easily solvable
  3. Easy workaround was suggested

juse-less avatar Oct 14 '22 22:10 juse-less