Add support for Psalm v5

[mcaskill]

This pull request continues what was started in #41 and should be merged first.

---

Currently, the dependency constraints target vimeo/psalm ^5@beta and psalm/plugin-phpunit dev-master.

Fixed errors and warnings raised by Psalm.

@kkmuffme has mentioned they also have a Psalm v5 ready fork of this plugin, which I was not aware of when I started my version. I'm sharing my implementation for reference and can be replaced by theirs.

[ntwb]

A quick glance and this looks good, lets get #41 merged and this PR updated and go from them there

[mcaskill]

@kkmuffme I would like your input on this. Even see your implementation.

[ntwb]

Unsure of the feasability of this, but maybe we should try and configure GitHub Actions to test each version

Currently, vimeo/psalm (5.0.0-beta1) is being installed

https://github.com/humanmade/psalm-plugin-wordpress/actions/runs/3342040266/jobs/5533851486#step:3:79

[mcaskill]

Good idea. I will change constraints for vimeo/psalm to ^4.4 || ^.5 (since we switch from hooks to event handlers), I will switch psalm/plugin-phpunit from dev-master to ^0.17 and in the GitHub Actions workflow, I'll make Composer require Psalm 5.

[mcaskill]

The new workflow appears to work well but there's a breaking change introduced in Psalm 5 that will require either a breaking change for the plugin or some kind of conditional statement.

TypeError: Psalm\Storage\FunctionLikeParameter::__construct(): Argument #6 ($type_location) must be of type ?Psalm\CodeLocation, bool given

The FunctionLikeParameter::__construct() method has received new parameters after the release of 5.0.0-beta1:

References:

• Psalm 4.x
• Psalm 5.0.0-beta1
• Psalm 5.x

To preserve compatibility, we could use the PSALM_VERSION constant but it might not be available and will vary a lot depending on the tag or branch of Psalm that is installed.
Alternatively, we can use ReflectionMethod to lookup the constant and detect the index of the bool $is_optional = true parameter that the plugin sets to false.

[kkmuffme]

My version of the plugin is 10 evolutions further already, making backporting all changes individually really hard/time consuming. I will only PR the full changeset now, bc I don't have time to waste either after this was abandoned for ages.

Anyway regarding v5:
do we really need to preserve v4 compatibility? Even psalm itself will drop it.
I think the better way would be to release one last working (!) v4 version of this plugin.
And then the next release (with the release of psalm v5) will be v5 only. I don't think investing scarce contributor time in unnecessary compatibility stuff is worth it.

I'd still prefer to move the plugin itself (and if this isn't possible, a fork of it) into the psalm github namespace, as this ensures long term compatibility and support - as long psalm is supported, there is the easy possibility to ensure that this plugin gets maintained too.

[ntwb]

Anyway regarding v5:
do we really need to preserve v4 compatibility? Even psalm itself will drop it.
I think the better way would be to release one last working (!) v4 version of this plugin.

I agree, what I think we need to ensure is just that, a v4 that we know as works in the v2.x.x branch, and then:

And then the next release (with the release of psalm v5) will be v5 only. I don't think investing scarce contributor time in unnecessary compatibility stuff is worth it.

This new v5 Psalm release would be a major bump to a new release v3.x.x version

Users would need to explicitly opt-in to v3.x.x

[mcaskill]

@kkmuffme Hello, when you have an opportunity: could you create a branch to share your fork's changes? Alternatively, privately share me a copy or add me as a collaborator to your fork, and I can extract out what's needed for Psalm v5.

Thanks.

[kkmuffme]

Yes, sorry, I was really busy with other stuff. Atm php-parser is changing (since it's changing to v5 soon) and psalm is updating for that (as per PR vimeo/psalm@3e54feb) which should release with psalm 5.14.0 soon.
I have already updated the plugin for this too; once psalm 5.14.0 is released, please ping me then I will create the PR with my code (that includes tons of other things besides v5 compat) and we can discuss that to see what you suggest to change on that.

In the meantime I have fixed an issue psalm had with stubs, where the assumed type often lead to errors that made no sense: vimeo/psalm#9992
The dependency we have for wordpress-globals.php (which by now is basically a static file, since it's never modified anyway) should be removed here and instead maintain this as part of the plugin with the @var comments in there. e.g.

define( 'DOING_CRON', /** @var bool */ false );

Could you PR this perhaps, so we can merge that already in the next week.

[ZebulanStanphill]

@kkmuffme Psalm 5.15.0 released 2 weeks ago.

[paulshryock]

Is there any list of outstanding work needed to complete this?

[kkmuffme]

Really sorry, didn't have time bc of family the last months.

It's ready actually - atm there is a pending PR to be merged by me in core vimeo/psalm#10370 which fixes tons of false negative/false positive errors that are especially relevant for WP/this plugin.

I will PR the new version of this plugin this week and release it beginning of next week with the next psalm core version.
In the meantime there have been tons of improvements and features added to this plugin, I just didn't find time to clean it up and release it.

[paulshryock]

Really sorry, didn't have time bc of family the last months.

It's ready actually - atm there is a pending PR to be merged by me in core vimeo/psalm#10370 which fixes tons of false negative/false positive errors that are especially relevant for WP/this plugin.

I will PR the new version of this plugin this week and release it beginning of next week with the next psalm core version.
In the meantime there have been tons of improvements and features added to this plugin, I just didn't find time to clean it up and release it.

Awesome, thank you! And I certainly don't mean to rush you. I know it's the holiday season, and totally understandable to have a lot going on with family. Just seeing if I could help out in any way. Sounds like you have everything just about ready. 👍❤️

[kkmuffme]

My PR is ready for testing #49
I guess we'll release it next week after some feedback

[mcaskill]

Closing in favour of #49.