search

psalm / psalm-plugin-wordpress

WordPress stubs and plugin for Psalm
MIT License
69 stars 18 forks source link

Parameters #3

Closed fullbl closed 1 year ago

fullbl commented 4 years ago

Hi! Congrats on the plugin, it's really useful! I've noticed two false positives in functions current_user_can (which can accept also one single argument) and add_query_args (which can accept up to 3 arguments).

Hope you like it! ;)

joehoyle commented 4 years ago

Hmm yeah there is something wrong currently with generating Psalm stubs with variadic functions. I'm going to look into if we can fix that first, so then we won't need the overrides.

joehoyle commented 4 years ago

@fullbl in https://github.com/humanmade/psalm-plugin-wordpress/commit/58a7c8a1decf2334482e02d8bc25420789f43030 I committed the fixes for all variadic functions.

In https://github.com/vimeo/psalm/pull/4091 I also submitted a fix upstream for this.

This PR seems to be a bit of a generic "change stuff", which I think is going to make it difficult to merge. Are there other functions we want to override for other details here?

fullbl commented 4 years ago

Yes, sorry, I used this fork as a "solve all the problems I find" fork.

What you can find useful is: .gitignore => I added the generated output, don't seem to be useful in repo composer.json => I moved the plugin under /src, composer was giving an error because using . as root was making it find tests folder twice Plugin.php => was missing a capital P in class name (I also commented the var_dumps)

Please tell me if I can do something (like removing edits in stubs/override.php) so you can merge, or if you prefer to do it or to keep it like this!

fullbl commented 4 years ago

any news on this?

joehoyle commented 3 years ago

@fullbl thanks for the explanation. I'd rather stick to fixing an issue per PR. ATM I'd rather not move Plugin to src for example. It will mean conflicts with other in-progress PRs too. (I'm also not sure why we need a src dir, as there is no build dir etc).

I'd like to get the function overrides in a PR, as I think that's the "fix" here?

mcaskill commented 1 year ago

Closing this pull request since it is out of date. Feel free to re-open it if ever its updated.