fix: cant auto complete method chaining

[SocolaDaiCa]

Summary

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

• Existing tests have been adapted and/or new tests have been added
• Add a CHANGELOG.md entry
• Update the README.md
• Code style has been fixed via composer fix-style

Before:

After:

[mfn]

Hi,

cant auto complete method chaining

I find the description a bit lacking TBH. Thanks for the screenshots, at least helps a bit but…

The foreach block you added is more or less the same as the above it? The difference being the static?

I've a feeling this will make one case work and break another or generate misleading autocomplete in other cases.

Can someone else chime in here maybe?

Maybe the PR should also get a rebase against current master and I bet many test with snapshots will fail, when I'm reading the change right -> this should be adapte too.

[SocolaDaiCa]

@mfn yup, The difference being the static, because :: only hint static method, So for each method I add a corresponding static method

[mfn]

The issues I see:

• double definition issue confuses phpstan
  It does not seem like a problem per se with phpstorm, it is able to handle both variants and also jump corrects to their respective lines in phpdoc.

  The problem is that in PHP, you can't define a method with two kind of access patterns like this.

  Therefore, also static analyzers will be confused and it will break them.

  For example:

  • use phpstan on a project
  • have this code YourModel::query()
  • and now it will complain :

    Static call to instance method Models\YourModel::query().

  phpstan sees both definitions and, I'm guessing here, the latter will win.

• the magic methods newModelQuery, newQuery and query don't exist as non-static version, you will get an error like this:

  BadMethodCallException Call to undefined method Illuminate\Database\Eloquent\Builder::newModelQuery().

  I'm not sure if there may other magic methods be added due to other circumstances.

The PR description talks about the custom scopes, but the change "changes everything else too". So maybe the PR isn't well scoped code-wise and should only target your custom scope issue?

[SocolaDaiCa]

@mfn I'm sorry I was a little confused in the previous explanation, I'll explain again

I define scopeActive in the User model
Currently ide-helper is @anotation scope method as static
when i type User::ac => it suggests me User::active
but when I type User::query()->ac it won't suggest me the active method, or when I type User::query()->where(...)->ac it won't suggest the method either activate me
Therefore, I added a non-static method for better suggestion and combined with method chaining

Perhaps the best way is to remove static in the ide-helper @anotation instead of adding both static and non-static methods

[mfn]

Perhaps the best way is to remove static in the ide-helper @anotation instead of adding both static and non-static methods

But this is a double edged sword: some only work for static, other for both, some only dynamic probably.

I don't see how we can do this in the current form without causing collateral issues.

[SocolaDaiCa]

I don't see how we can do this in the current form without causing collateral issues.

Because all the @anotations that ide-helper creates for the model are scope and where, they certainly work in both static and non-static forms.
I can't think of a case where it only works in static form

and laravel also recommends using the form Model::query()-><method1>()-><method2>() instead of the form Model::<method1>()-><method2>().

So I think changing @anotation scope and where from static to non static is reasonable and safe

[mfn]

reasonable and safe

Please re-read #1446 (comment) , in the current form the PR creates issues with existing projects.

[SocolaDaiCa]

@mfn Please help me check again

I've fixed it, non-static is now only applied to function scope and where<column>

If anything is not right, please let me know

[mfn]

Can you please also run the test suite so we see the changes?

[barryvdh]

Can you try with the latest dev-version? 3.4.x@dev or dev-master

Seems to work fine for me.

[SocolaDaiCa]

@barryvdh It not work for me, whereName is still static

[barryvdh]

WhereName is supposed to be static

[SocolaDaiCa]

@barryvdh I know, but I think WhereName should support both static and non static. Example

User::whereName()->get()


User::whereEmail()->whereName()->get()

[barryvdh]

Hmm I could generate it like this;

 * @method static \Illuminate\Database\Eloquent\Builder<static>|User whereEmail($value)
 * @method \Illuminate\Database\Eloquent\Builder<static>|User whereEmail($value)

But that is a bit duplicated if I have to do that for all. Does above work for VSCode?

[GNfsys]

Laravel Idea does the following when generating model helper files, I don't know if it's helpful in this case:

class _BaseBuilder extends \Illuminate\Database\Eloquent\Builder {}

/**
* @method _IH_User_QB active()
*/
class _IH_User_QB extends _BaseBuilder {}

Which then allows to use both:
User::active() and User::query()->active();