barryvdh / laravel-ide-helper

IDE Helper for Laravel
MIT License
14.17k stars 1.16k forks source link

Duplicate property creation every time it runs #1420

Closed faytekin closed 1 year ago

faytekin commented 1 year ago

Versions:

Description:

After updating to the latest version, every time I run the php artisan ide-helper:models --write command, the property-read in the model is added again, causing duplicate values in the model.

/**
 * App\Models\Branch
 *
 * @property int $id
 * @property int $company_id
 * @property string $name
 * @property string|null $phone
 * @property bool $active
 * @property \Illuminate\Support\Carbon|null $created_at
 * @property \Illuminate\Support\Carbon|null $updated_at
 * @property-read \Illuminate\Database\Eloquent\Collection<int, \App\Models\QrMenu> $activeQrMenus
 * @property-read int|null $active_qr_menus_count
 * @property-read \Illuminate\Database\Eloquent\Collection<int, User> $admins
 * @property-read int|null $admins_count
 * @property-read \App\Models\Company $company
 * @method static \Database\Factories\Company\BranchFactory factory($count = null, $state = [])
 * @method static \Illuminate\Database\Eloquent\Builder|Branch newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Branch newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|Branch query()
 * @property-read \Illuminate\Database\Eloquent\Collection<int, \App\Models\QrMenu> $activeQrMenus
 * @property-read \Illuminate\Database\Eloquent\Collection<int, User> $admins
 * @mixin \Eloquent
 */

The activeQrMenus and admins values ​​here are repeating.

There are many models in the project, but this error only occurs in 5 models.

mfn commented 1 year ago

Is it always on Collections?

faytekin commented 1 year ago

yes, it is always on Collection

DarkStoorM commented 1 year ago

Same issue. However, I found out, that use_generics_annotations set to true will produce duplicates like this

I ran it three times with this setting and it always seem to duplicate the @property-read from HasApiTokens and Notifiable in my default User model. Switching the generics annotations off fixes this issue (requires deleting the previous model docBlock)

* @property-read \Illuminate\Notifications\DatabaseNotificationCollection<int, \Illuminate\Notifications\DatabaseNotification> $notifications
* @property-read \Illuminate\Database\Eloquent\Collection<int, \Laravel\Sanctum\PersonalAccessToken> $tokens
+* @property-read \Illuminate\Notifications\DatabaseNotificationCollection<int, \Illuminate\Notifications\DatabaseNotification> $notifications
+* @property-read \Illuminate\Database\Eloquent\Collection<int, \Laravel\Sanctum\PersonalAccessToken> $tokens
+* @property-read \Illuminate\Notifications\DatabaseNotificationCollection<int, \Illuminate\Notifications\DatabaseNotification> $notifications
+* @property-read \Illuminate\Database\Eloquent\Collection<int, \Laravel\Sanctum\PersonalAccessToken> $tokens

Related block:

https://github.com/barryvdh/laravel-ide-helper/blob/4dc20b028adaeadf603860f179f09da5aa21f5b5/src/Console/ModelsCommand.php#L748

Both mentioned traits indeed extend from Relation, so maybe it just can't match the correct type for collections of those relations.

matthiastjong commented 1 year ago

+1 on this issue

matthiastjong commented 1 year ago

FYI I needed to fix this because it caused my CI to fail every time. How I fixed is by changing the command to php artisan ide-helper:models --no-interaction --write --reset. The --reset flag resets the lookup and re-populates the models.

reachuniversal commented 1 year ago

+1, having the same issue

jorycn commented 1 year ago

+1 on this issue

mfn commented 1 year ago

I acknowledge this seems buggy, so we definitely appreciate if someone steps up and looks into fixing this.

Btw, for those affected: can you use --reset?

(I realized I'm not affected by this bug, because my pipeline uses ide-helper:models --write --reset)

faytekin commented 1 year ago

I acknowledge this seems buggy, so we definitely appreciate if someone steps up and looks into fixing this.

Btw, for those affected: can you use --reset?

(I realized I'm not affected by this bug, because my pipeline uses ide-helper:models --write --reset)

I can't use --reset because I've defined some model getters using Laravel's Attribute, and when I add --reset, it deletes the manually added properties.

Mark-van-Seeters-LW commented 1 year ago

+1 on this, using the reset switch helps but it's quite annoying

JeRabix commented 1 year ago

image image

Not work with generic types?

JeRabix commented 1 year ago

In latest version phpDocumentor/ReflectionDocBlock generic type parsed correctly, but this package use fork barryvdh/ReflectionDocBlock and this fork can't parse generic type in DocBlock correcly. So need update fork package, maybe).

https://github.com/barryvdh/laravel-ide-helper/blob/master/src/Console/ModelsCommand.php#L939 This line ($tag->getVariableName()) - get empty string for generic type.

JeRabix commented 1 year ago

I create PR to fork repository

https://github.com/barryvdh/ReflectionDocBlock/pull/14

JeRabix commented 1 year ago

@barryvdh can you help please?)

ghost commented 1 year ago

Running into this issue too:


* @property-read \Illuminate\Database\Eloquent\Collection<int, \Laravel\Sanctum\PersonalAccessToken> $tokens
* @property-read \Illuminate\Database\Eloquent\Collection<int, \Laravel\Sanctum\PersonalAccessToken> $tokens
*/
class User extends Authenticatable {
}
classicalguss commented 1 year ago

Same problem here. Just a bit of knowledge since it's not mentioned in this thread. In my case, it only happens if there was something "irregular" with the naming convention.

For example the model is User but the relation is called admins. Or if it's two words like payment_logs. Or if it's 1 word, but you're not using standard column naming like user_id for a foreign key on users

faytekin commented 1 year ago

Hello @mfn,

The bug located here: https://github.com/barryvdh/ReflectionDocBlock/pull/14, has been successfully corrected with this pull request. If possible, I kindly request you to release a new version incorporating these fixes.

raymadrona commented 1 year ago

Still encountered this error even though I upgraded to dev:master.

faytekin commented 1 year ago

Still encountered this error even though I upgraded to dev:master.

The issue was resolved in the barryvdh/ReflectionDocBlock package to which this package is linked. If this package updates its dependencies and releases a new version, the problem will be solved.

tayeke commented 1 year ago

+1 can we get that dependency updated?

barryvdh commented 1 year ago

Tagged

faytekin commented 1 year ago

https://github.com/barryvdh/ReflectionDocBlock/compare/v2.1.0...v2.1.1 fixed here if running composer update command