Psalm: switch from Phive to Composer + fix (most) issues

[jrfnl]

Psalm: switch from Phive to Composer

This switches the installation method for Psalm from Phive to Composer, while still using a Phar file for running Psalm.

Includes:

• Removing Psalm from the Phive configuration.
• Adding Psalm to the Composer configuration. Includes upgrading from version 3.11.2 to version 4.8.1.
• Adjusting the script used in the Makefile. 👉 Please verify and test this as things work differently on different OS-es and this should work for you.
• Adjusting the GH Actions script to use the Composer installed version of Psalm.

Note: due to the committed composer.lock file, Psalm will not automatically upgrade when newer versions are available.

Refs:

• https://github.com/vimeo/psalm/releases
• https://github.com/psalm/phar/releases

Utils::pregSplit: limit is not nullable

Correctly flagged by Psalm:

ERROR: PossiblyNullArgument - src\Utils.php:50:53 - Argument 3 of preg_split cannot be null, possibly null value provided (see https://psalm.dev/078)
        $parts = php_preg_split($pattern, $subject, $limit, $flags);

The $limit argument of the PHP native preg_split() function is not nullable.

Ref: https://www.php.net/manual/en/function.preg-split

Tags::__toString(): remove redundant type casts

Psalm flags these type casts as redundant:

ERROR: RedundantCastGivenDocblockType - src/DocBlock/Tags/Author.php:80:23 - Redundant cast to string given docblock-provided type (see https://psalm.dev/263)
        $authorName = (string) $this->authorName;

ERROR: RedundantCastGivenDocblockType - src/DocBlock/Tags/Example.php:150:21 - Redundant cast to string given docblock-provided type (see https://psalm.dev/263)
        $filePath = (string) $this->filePath;

ERROR: RedundantCastGivenDocblockType - src/DocBlock/Tags/Link.php:74:17 - Redundant cast to string given docblock-provided type (see https://psalm.dev/263)
        $link = (string) $this->link;

ERROR: RedundantCastGivenDocblockType - src/DocBlock/Tags/Method.php:228:23 - Redundant cast to string given docblock-provided type (see https://psalm.dev/263)
        $methodName = (string) $this->methodName;

I have verified each and can confirm that these are redundant. They are probably a left-over from the time when the __construct() method in these classes did not yet have type declarations.

Tags/Return: remove redundant condition

Psalm flags this condition as redundant:

ERROR: RedundantCondition - src/DocBlock/Tags/Return_.php:62:48 - "" can never contain non-empty-lowercase-string (see https://psalm.dev/122)
        return $type . ($description !== '' ? ($type !== '' ? ' ' : '') . $description : '');

Based on the statement in the line above - $type = $this->type ? '' . $this->type : 'mixed'; -, Psalm is correct and the $type variable can never be an empty string.

Psalm: suppress two notices

The current version of Psalm flags the following issues:

ERROR: InvalidReturnType - src\Utils.php:44:16 - The declared return type 'array<array-key, string>' for phpDocumentor\Reflection\Utils::pregSplit is incorrect, got 'list<list<int|string>|string>' (see https://psalm.dev/011)
     * @return string[] Returns an array containing substrings of subject split along boundaries matched by pattern

ERROR: InvalidReturnStatement - src\Utils.php:55:16 - The inferred type 'list<list<int|string>|string>' does not match the declared return type 'array<array-key, string>' for phpDocumentor\Reflection\Utils::pregSplit (see https://psalm.dev/128)
        return $parts;

I'm suggest ignoring this as list isn't an officially supported type.

---

After this PR, there is still one issue remaining.

Argument 4 of preg_split expects 0|1|2|3|4|5|6|7, parent type int provided (see https://psalm.dev/193)

I believe this issue is for the phpDocumentor\Reflection\Utils class and expects the pregSplit() method to apply input validation to the value received for $flags before passing it off to the PHP native preg_split() function.

IMO that's taking things a little too far as PHP will handle this internally without errors. Want me to add it to the list of issues to be ignored ? See: https://3v4l.org/NdDRK

Side-note: the weird thing is that this issue does show up in CI, but with the same PHP + Psalm Phar version, I cannot reproduce this locally.

[jrfnl]

Rebased to get past merge conflict

[jaapio]

Thanks for all your hard work!

[jrfnl]

@jaapio No problem. Let me know what you want me to do with that last remaining issue (see description at the bottom of the PR description).

[orklah]

@jrfnl you should be able to solve that with propertly in psalm with @psalm-param PREG_SPLIT_* $flags or @psalm-param int-mask<0, 1, 2, 4> $flags

[jrfnl]

@jrfnl you should be able to solve that with propertly in psalm with @psalm-param PREG_SPLIT_* $flags or @psalm-param int-mask<0, 1, 2, 4> $flags

@orklah I can't find any documentation on those notations. I can only find a generic paragraph about using @psalm-param.

Also, as I say above, IMO annotating this or adding input validation is nonsensical. The docblock contains all the relevant info about the supported values already and PHP will handle the rest.

Either way, I'll defer to whatever solution is preferred by the maintainer team.

[orklah]

@jrfnl Yeah, Psalm can sometimes be poor in documentation...

The types are mentionned there (in context of plugins): https://psalm.dev/docs/running_psalm/plugins/plugins_type_system/

TIntMask - Represents the type that is the result of a bitmask combination of its parameters. int-mask<1, 2, 4> corresponds to 1|2|3|4|5|6|7

TIntMaskOf - as above, but used with with a reference to constants in codeint-mask<MyClass::CLASSCONSTANT*> will corresponds to 1|2|3|4|5|6|7 if there are three constant 1, 2 and 4

But yeah, I admit it's pretty advanced notation. I'm surprised that PHP accepts any flag though, and this is a recipe for a disaster if PHP ever adds a new flag for this function. Any unvalidated input could change behavior without notice...

[jaapio]

ERROR: InvalidReturnStatement - src\Utils.php:55:16 - The inferred type 'list<list<int|string>|string>' does not match the declared return type 'array<array-key, string>' for phpDocumentor\Reflection\Utils::pregSplit (see https://psalm.dev/128)
        return $parts;

that is wrong here is not the list part, but palm reports that the value can be a string or a array<int|string, string> list just tells us we should ignore the key?

Regarding the bitmask, I think it is safe to just ignore this error. Anyone using our utils library should be punished :wink:, reminding me that adding a @internal tag in there would be usefull.

Other changes look good to me. :+1:

[jrfnl]

ERROR: InvalidReturnStatement - src\Utils.php:55:16 - The inferred type 'list<list<int|string>|string>' does not match the declared return type 'array<array-key, string>' for phpDocumentor\Reflection\Utils::pregSplit (see https://psalm.dev/128)
        return $parts;

that is wrong here is not the list part, but palm reports that the value can be a string or a array<int|string, string> list just tells us we should ignore the key?

Still not completely clear to me what to change this to in that case. Are you saying it should be array<int|string, string> ?

Regarding the bitmask, I think it is safe to just ignore this error.

Done ;-)

[jaapio]

array<int|string, array<int|string, string>> should be ok, I think.

[jrfnl]

array<int|string, array<int|string, string>> should be ok, I think.

Done. 🤞🏻

[jrfnl]

Argh... eh... so that type change... nope. What am I doing wrong ?

[jaapio]

@jrfnl I will pick this one :-)

[jrfnl]

@jrfnl I will pick this one :-)

Not sure what you mean by that, but shall I rebase the PR first so at least it's out of conflict ?

[jaapio]

I will rebase this and check what is needed to make psalm pass.

It looks like there is some bug in here, detected with psalm.

[jaapio]

The issue is, preg_split can return an string[] but also string[][] the code is not handling this properly as our expressions are never resulting in string[][] Which is not detected by psalm.

For now I add an extra assert.

[jaapio]

Rebased and merged manually. I don't know why this is not detected by github. Closing this pr since it has been merged.

Thanks for these contributions.