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

Wrong syntax of @method tags in Flight class docblock #608

Open
anthonyaxenov opened this issue Sep 15, 2024 · 3 comments
Open

Wrong syntax of @method tags in Flight class docblock #608

anthonyaxenov opened this issue Sep 15, 2024 · 3 comments
Labels

Comments

@anthonyaxenov
Copy link

anthonyaxenov commented Sep 15, 2024

Some @methods are described incorrectly.
Types here should be in native php syntax and there is no way to describe generic types in @methods directly. Generic types are allowed in @param, @return and @template* tags. Otherwise IDE will not parse such method correctly.

Just a small example with Flight::set():

image

image

I didn't looked up closely in other files but it definitely worth to do

Reference: https://github.com/flightphp/core/blob/master/flight/Flight.php#L21-L81

Found in mikecao/flight v3.12.0

@lubiana
Copy link

lubiana commented Sep 16, 2024

Hi @anthonyaxenov
interesting find there and you are right about phpstorm not picking up on the complex array/iterable shapes for @method annotations. i would guess they are mostly used for static analysis here. So maybe that would be a nice feature requests for the jetbrains issue tracker, as i see no reason why phpstorm should not support this syntax.
update: i added an issue in the jetbrains issue tracker https://youtrack.jetbrains.com/issue/WI-79045/Missing-support-for-arrayshapes-in-method-docblock-annotation feel free to give that issue an upvote

edit: just checked and phpstan does indeed support this syntax (https://phpstan.org/r/948cd893-1920-49e6-9f19-02413890e577)
The same code in phpstorm does not correctly check the types:
grafik

@anthonyaxenov
Copy link
Author

anthonyaxenov commented Sep 17, 2024

  1. I feel I need to explain something. PhpStorm is my main IDE and I used to believe its warnings in 95% of cases because it has very powerfull static analyzer under the hood. I met this behavior while working on my own pet-project and I had no time at that moment to check if this syntax acceptable for another static analyzers used in Flight. Such behavior is pretty annoying but I never know when I return to this pet-project next time. Also I never used such syntax myself inside of @methods. So I created this issue just to let another developers know that something may be incorrect here.

  2. Unfortunately, there is not standard about phpdoc, only phpDocumentor docs and PSR-5 draft which looks pretty similar (don't they?) and very inconcrete. There are no explicit descriptions about its syntax. Instead of that, php ecosystem (mostly developed by community) and 3rd-party developers' solutions becomes standard de-facto. So that increases entropy in tooling, this case shows it well.

  3. I checked your example in psalm -- and yes, it also supports generic arguments in @method tags: https://psalm.dev/r/b837894b7e (UPD I noticed your psalm link in youtrack too late))

  4. I think this is good idea to create an issue in JB Youtrack, thanks for that. If I would create it they would definitely close it because of geopolitical reasons.

@lubiana
Copy link

lubiana commented Sep 17, 2024

  1. Yeah, I use PhpStorm myself and for coding mostly depend on its type analysis. Its an awesome tool and as you said works right almost all the time. Regarding your specific problem I think the best advice is to use the engine instance of Flight instead of the static methods on the Flight class itself. The docs currently do not advertise that way of using the framework really, but you could take a look at the skeleton project (https://github.com/flightphp/skeleton/blob/master/app/config/bootstrap.php#L17) when using the engine instance, the types picked up by phpstorm should mostly be correct.
  2. You are absolutely right, that the phpDocumentor docs are not always helpfull nowadays. Personally I stick to the phpstan documentation about declaring types (https://phpstan.org/writing-php-code/phpdoc-types) There are more and more projects using phpstan nowadays so in my experience those type declarations are often the standard way to use. But as I said, that is mostly my personal experience and preference.

Thanks again for bringing this issue up and lets hope that JetBrains will fix the type discovery on their side. If not we might need to revisit this again here and look if we can improve that in flight itselt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants