Internal: CI code style and psalm

[kkmuffme]

Add 2 tasks to the CI pipeline: 1) run psalm on the plugin itself 2) run codestyle check

See psalm's CI config, as that can be reused.

[paulshryock]

I would be willing to take a stab at this if it's helpful.

[kkmuffme]

@paulshryock that would be very much appreciated

The v5 branch is merged now, so if you could add this now, we could fix any issues psalm identifies in the plugin code and then release it finally.

[paulshryock]

@kkmuffme sure thing. Want to assign this Issue to me, and I'll make one or more pull request(s) against the latest master branch?

[paulshryock]

So to start, I've forked this repo to paulshryock/psalm-plugin-wordpress. I cloned the repo locally, switched to PHP 8.3.0, ran composer install, and composer analyze.

I've got these warnings:

Warning: "findUnusedBaselineEntry" will default to "true" in Psalm 6. You should explicitly enable or disable this setting.
Warning: "findUnusedCode" will default to "true" in Psalm 6. You should explicitly enable or disable this setting.
Warning: "findUnusedBaselineEntry" will default to "true" in Psalm 6. You should explicitly enable or disable this setting.
Warning: "findUnusedCode" will default to "true" in Psalm 6. You should explicitly enable or disable this setting.

And the rest of the output from Psalm shows 29 errors and 19 other issues.

------------------------------
29 errors found
------------------------------
19 other issues found.

I will start by clearing up the errors, so once we add Psalm to the continuous integration pipeline, we have a passing pipeline out of the gate.

[kkmuffme]

Ok good, you need to create a psalm.xml and commit it to the repo so we get some consistency and also fix the errors above Check psalm's own (I think psalm.xml.dist), but should be something like:

<?xml version="1.0"?>
<psalm
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    name="psalm-plugin-wordpress"
    errorLevel="1"
    compressor="off"
    phpVersion="7.4"
    memoizeMethodCallResults="true"
    findUnusedVariablesAndParams="true"
    findUnusedPsalmSuppress="true"
    findUnusedBaselineEntry="true"
    findUnusedCode="true"
/>

Btw I used php 7.4 since that is the minimum PHP version psalm supports, so we should stick to that. Ideally you would add a 2nd pipeline where you set PHP 8.3 to check for any deprecated stuff used. (again see psalm's own pipeline matrix, they use PHP 8 - 8.3, but I think that's overkill for here)

[kkmuffme]

@paulshryock fyi all the "internal" errors you don't need to fix - in fact I suggest do suppress them via the psalm config file, as they're caused by the plugin's namespace prior from moving the plugin to psalm and are all false positives.

[paulshryock]

@paulshryock fyi all the "internal" errors you don't need to fix - in fact I suggest do suppress them via the psalm config file, as they're caused by the plugin's namespace prior from moving the plugin to psalm and are all false positives.

Good to know, thanks. I should have something in another day or two. Hope I'm not holding up the release.

[kkmuffme]

Any approx ETA for this?

[paulshryock]

Hi @kkmuffme I'm sorry about the delay. The holidays took over and I didn't get to wrap this up before end of year like I hoped. I will try to pick this up over the weekend and get back to you next week.

[kkmuffme]

@paulshryock take your time, there's no rush. I released it like it is now to gather some more feedback and so we're more easily able to distinguish between bugs in v3 vs bugs added from code style.

[paulshryock]

:heart: Amazing, glad this isn't holding up the release. And thanks for understanding.

[paulshryock]

I'm sorry, I thought I would have time to complete this but I haven't, and I don't want to hold this up. I'm going to un-assign myself, in case anyone else wants to pick this up. Sorry for the delay without any movement. If my availability opens up and this is still open and unassigned, I might take another swing at it.