laravel-ide-helper
laravel-ide-helper copied to clipboard
Option for generating `Collection|ElementType[]` types as `Collection<int, ElementType>` instead
Many static analysis tools now support generic notation for Iterator
or ArrayAccess
implementations. In this situation Collection|ElementType[]
is not useful as it does not convey the real type and makes workarounds necessary within static analysis tools to correct for this. Is there an option to generate the stubs in generic notation? Or would it be possible to add an option?
I’d be willing to help implement this if a PR would be welcome. I’ve been reverse engineering a lot of this logic for psalm
We need to be careful because whilst this improves the static analysis tools, this will reduce usefulness of the typehints for IDEs. (*)
Remember: the primary goal is for IDEs to help here :-)
It's only recently that tools like https://github.com/nunomaduro/larastan embrace this information too and in fact expand upon it.
(*) PhpStorm being the most popular one, I know that this syntax Collection<int, ElementType>
currently will not yield PhpStorm understanding the this is the same as ElementType[]
@bmewburn I'm interested in what issue you are running into in which you need this option first-party in laravel-ide-helper.
In larastan, I wrote a parser that treats Collection|Model[]
as Collection<Model>
here: https://github.com/nunomaduro/larastan/pull/479
And psalm can infer these types by settingallowPhpstormGenerics='true'
in your config https://psalm.dev/docs/running_psalm/configuration/#allowphpstormgenerics
@mfn I totally understand your point about this being primarily an ide-helper. I just wanted to point out that psalm has a LSP implemented, which means that (with the LSP plugin) phpstorm direcly benefits from the improvements we are making in static analysis tooling!
phpstorm direcly benefits from the improvements we are making in static analysis tooling
I don't follow this as PhpStorm doesn't support this proposed syntax change? Or am I missing something? 🤔
@mfn check this out: https://psalm.dev/docs/running_psalm/language_server/
It's pretty neat.
Don't mean to derail this thread. I think ide-helper should continue to write the phpstorm syntax, as devs shouldn't be forced to use the likes of psalm.
@mr-feek my use case is in intelephense I was hoping to avoid any special handling of this type notation.
@mfn I agree it would have to be option and understand if it is out of scope of this project.
I think until "the major IDE", which is the main benefactor of this whole library, catches up with the syntax, we can't change/introduce this syntax.
larastan has shown that's it's more feasible to simple adapt the static analyzers at the moment, as they can move faster here.
Unless I'm mistaken, for PhpStorm this would be the issue to vote on: https://youtrack.jetbrains.com/issue/WI-43843 (though it only talks about array<…>
so I might be wrong).
There's also https://youtrack.jetbrains.com/issue/WI-20193 but I would rather say this one is superseded by the above one.
it's been announced that phpstorm is going to have first party support for array key and value types, so it might be reasonable for us to switch (or at least include!) the Collection<int, ElementType>
syntax
Yep, let's wait until it lands how it works out!
PhpStorm 2020.3 does indeed support array<TKey, TValue>
syntax now. A generalized implementation of generics on iterables still isn't there yet.
That being said, I certainly prefer the technically correct annotation of Collection<T>
. How about we add a configuration option for this?
More configurations, more conditions in an already spaghetti code 😥
Now that PHPStorm 2021.2 has better support for generics, it could be interesting to look into implementing this again.
https://blog.jetbrains.com/phpstorm/2021/07/phpstorm-2021-2-release/
I would certainly agree, very well reflects what I said in https://github.com/barryvdh/laravel-ide-helper/issues/942#issuecomment-633596630 👍
And still more support is coming in the next release.
https://blog.jetbrains.com/phpstorm/2021/09/phpstorm-2021-3-early-access-program-is-open/#more_for_generics
I would certainly agree, very well reflects what I said in #942 (comment) 👍
Well, is it planned to implement the subject? Sorry, I don't get your reference to your previous comment.
I think this can be closed now that #1298 is merged
Thanks!
Note: it's not released yet, so anyone finding this issue: if there's no release after v2.12.3 yet, you need to use the dev version or wait.