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

forceDelete() and restore() should not be static #917

Closed
Krunch opened this issue Apr 17, 2020 · 3 comments · Fixed by #918
Closed

forceDelete() and restore() should not be static #917

Krunch opened this issue Apr 17, 2020 · 3 comments · Fixed by #918

Comments

@Krunch
Copy link

Krunch commented Apr 17, 2020

When using the use SoftDeletes; trait ide-helper:model will add these 2 @method in the phpdoc:

 * @method static bool|null forceDelete()
 * @method static bool|null restore()

Those methods should not be static as they are not defined as static inside the trait.

@mfn
Copy link
Collaborator

mfn commented Apr 17, 2020

Hmm.

I think they should be simply removed, specifically these two lines

$this->setMethod('forceDelete', 'bool|null', []);
$this->setMethod('restore', 'bool|null', []);

In my opinion any decent editor/IDE/static code analyzer can infer methods from traits automatically anyway, no need to duplicate this (not so with the other because they're magic mixing in the phpdoc and are fine).

mfn added a commit to mfn/laravel-ide-helper that referenced this issue Apr 17, 2020
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh#917
@mfn
Copy link
Collaborator

mfn commented Apr 17, 2020

Made #918

@Krunch
Copy link
Author

Krunch commented Apr 17, 2020

You are right, I am using phpstorm and those methods are auto-completed fine without those 2 lines. Thank you.

barryvdh pushed a commit that referenced this issue Apr 19, 2020
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes #917
fatihdirikman added a commit to fatihdirikman/Laravel-IDE-Helper that referenced this issue Jan 7, 2022
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh/laravel-ide-helper#917
renaforsberg824 added a commit to renaforsberg824/ide-helper-laravel-developer that referenced this issue Oct 5, 2022
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh/laravel-ide-helper#917
lisadeloach63 added a commit to lisadeloach63/ide-helper-reso-laravel that referenced this issue Oct 7, 2022
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh/laravel-ide-helper#917
sadafrangian3 pushed a commit to sadafrangian3/ide-helper-laravel that referenced this issue Oct 18, 2022
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh/laravel-ide-helper#917
smile1130 added a commit to smile1130/laravel-IDE that referenced this issue Jun 16, 2023
These two methods are already regular methods on the trait, there's no
need to add them and additionally they're not static, so their
definition was changed for the worse.

The other methods are magic so it's fine to keep them.

Fixes barryvdh/laravel-ide-helper#917
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 a pull request may close this issue.

2 participants