Use of @psalm-types in @param

Contributors
1

Hi there,

I am finding code like:

/**
 * @psalm-type OptionsArgument = array{
 *     callback: callable(mixed, array<string, mixed>, mixed...): bool,
 *     callbackOptions?: array<array-key, mixed>,
 *     bind?: bool,
 *     throwExceptions?: bool,
 *     messages?: array<string, string>,
 *     translator?: TranslatorInterface|null,
 *     translatorTextDomain?: string|null,
 *     translatorEnabled?: bool,
 *     valueObscured?: bool,
 * }
 */
final class Callback extends AbstractValidator
{
....
    /** @param OptionsArgument|callable $options */
    public function __construct(array|callable $options)

difficult to use because tools that don’t understand @psalm-type see the OptionsArgument on the @param an unknown class/type and raise static analysis errors.

It seems non-ideal for a library to be exposing PHP doc that isn’t widely understood as it makes the library more difficult to use properly. (Obviously there is some vagueness here, as any tool could not understand some “common” PHPDoc structures, but “Tool B” not understanding PHPdoc tags specifically created by “Tool A” seems reasonable.)

I think there are probably two options that would ease my pain:

  1. Remove @psalm-type and just use more “common” array shape PHP Doc where that is viable.

and/or

  1. Make sure any @param/@var don’t include any @psalm-type defined types and pair a more generic @param with @psalm-param including the psalm-type where there is value to be obtained for that extra info

My feeling is that (1) would be better generally as it would expose the array shape to more tools more generically, but perhaps there may be places where psalm specific support is useful and (2) ,might be more useful.

I’m happy to submit some PRs, but would appreciate some guidance as to what would be preferred.

2

And this is the problem “generic”, it does not help during static analysis and development on user side.

I would say Psalm is widely used. :smiley:
Also in IDEs, e.g.:

It is indeed not ideal, because it would be better to avoid arrays as a configuration in this place. A cleanly defined constructor should be the goal. But with laminas-filter and laminas-validator we are in a dilemma because laminas-inputfilter based on these components. And a complete change would have been too big a break - in the components and on the user’s side.

Send a pull request that updates the Callback validator and the checks on GitHub will show us if it works. We are happy to review it.
Thanks in advance! :+1:

3

An example can be find in the next major version of laminas-view:

4

Sorry, I didn’t mean that Psalm isn’t itself widely used.

I just happen to be looking at a project that uses Phan (and PHPStan), and Phan doesn’t understand the psalm-type.

I have created PR #415 against the latest 2.x branch as I am working towards that at the moment (need to work through the deprecations in that before looking at switching to 3)

Assuming it’s an acceptable change I’ll create a similar one for the 3.x branch for consistency (probably only for the constructor, no point on the property as it’s a final class in v3)

5

Oh well, thanks for taking the time to look at it and loop people in.
Not the result I’d have preferred but them’s the breaks…

6

I am sorry, I would have liked to help you, but there are decisions to be made somewhere.
Yes, sometimes it is hard, but that is the way it is with open source.
But please do not be discouraged, because the two pull requests were exactly right. This is a basis for discussion and shows interest. Your objection will definitely be taken into consideration – beside the current dilemma.
We are grateful for any help, even if it is only criticism. :+1: