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

Should ide-helper:models be generating phpDoc for the magic Eloquent\Model static methods? #259

Closed
jessedc opened this issue Aug 25, 2015 · 9 comments

Comments

@jessedc
Copy link

jessedc commented Aug 25, 2015

I have recently setup a new Lumen project (v5.1.3) and noticed the instance methods from Eloquent\Builder provided to Eloquent\Model by using Model::__callStatic() and Model->__call() internally are not added to the phpDocs generated by ide-helper:models.

One example directly from the Larvel 5.1 documentation is the ::find() method.

screen shot 2015-08-26 at 6 50 18 am

My observation is this call to ::find() is caught by __callStatic() and __call() methods of Eloquent\Model and proxied to \Illuminate\Database\Eloquent\Builder.

screen shot 2015-08-25 at 9 51 58 pm

These methods can be added to the phpDocs of the model with @method hints like the following:

/**
* @method static \Illuminate\Database\Eloquent\Model|\App\Models\Cafe find($id)
*/

My question is, should these magic methods from Builder be added to Model subclasses by ide-helper:models? Or should the Laravel documentation be updated instead?

Worth noting that this whole situation can also be avoided by using the builder directly via the query() method.

$cafe = Cafe::query()->find($cafeId);

The find() method was defined in 5.0 but are no longer defined on the Eloquent\Model as of 5.1.

@rentalhost
Copy link

I'll works if you extends from Eloquent facade. But I think that it need to works directly from Eloquent\Model too.

@bebnev
Copy link
Contributor

bebnev commented Sep 1, 2015

I think that having such methods in Model docblocks is a real overhead, because find is only one "magic" method from EloquentBuilder. Then somebody will want methods from the QueryBuilder which are also called with __call/__callStatic.

So the easiest workaround, if you need autocompletion, is to extend Eloquent instead ofEloquent\Model, as @rentalhost said. Or, if you don't want to extend Eloquent, you can set something like this to the docblock of your model:

/**
 * App\User
 *
 * @mixin Eloquent
 */

@icedfish
Copy link
Contributor

icedfish commented Sep 7, 2015

Aha, such a tricky but useful workaround ~

@Francismori7
Copy link

@bebnev Very smart of the @mixin tag. Great work.

@shehi
Copy link

shehi commented Oct 1, 2015

I believe @mixin is a neat solution. Shouldn't we close this issue?

@Francismori7
Copy link

@shehi it should generate the @mixin itself first IMO

@shehi
Copy link

shehi commented Oct 2, 2015

@Francismori7 Agreed.

@brazenvoid
Copy link

I have 5.1 and a BaseModel in the inheritance chain which extends from Eloquent. Even with this the methods like find will always make the ide think that the class returned is Eloquent/Model rather than the calling model class.

We have over 1K models encompassing our sites to sales to warehousing to logistics. For our 50+ developers this plugin is a godsend but imagine adding the two static method definitions we use the most to the models when a scheduled reset is run through artisan(!) and then you can understand how a godsend turns into a god's wrath...

@method static [model class]|null first($columns = array())
@method static [model class]|null find($id)

matiaspub added a commit to matiaspub/laravel-ide-helper that referenced this issue Jan 12, 2016
@mfn
Copy link
Collaborator

mfn commented Jun 25, 2020

@barryvdh this is already fixed due to @mixin support long time ago, issue can be closed :}

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

No branches or pull requests

9 participants