search

laminas / laminas-inputfilter

Normalize and validate input sets from the web, APIs, the CLI, and more, including files
https://docs.laminas.dev/laminas-inputfilter/
BSD 3-Clause "New" or "Revised" License
42 stars 28 forks source link

Hotfix/return psalm types #84

Closed gromson closed 1 year ago

gromson commented 1 year ago

@return annotation specifying psalm types produces false errors for other static analysis tools, particularly for PHPStan. Therefore, returning psalm types specified with @psalm-return and native PHP types provided for @return annotation.

gromson commented 1 year ago

@Ocramius Please consider the following example (I'm only concerned about the client code issue on line 38)

https://phpstan.org/r/12d3d0f2-fe29-4ccb-bbc1-4cb4a6dfcc1f

This can be solved if InputFilterProviderInterface::getInputFilterSpecification() will have @return annotation that is not specific to a psalm type

https://phpstan.org/r/4e715832-28f6-41b4-b18a-d20d2751f1e1

Would you consider the updated pull request?

Ocramius commented 1 year ago

To me this sounds like &array<array-key, InputSpecification> is not supported in PHPStan, which is unrelated to having precise types declared here :thinking:

gromson commented 1 year ago

This is indeed not supported in PHPStan but the point is that it's not valid to expect that, since InputFilterSpecification is not a real type/class/interface this is a Psalm-specific type alias and it's not intended to be fed to PHPStan. But because there are no fallbacks for analyzers other than Psalm the client code can not satisfy PHPStan because PHPStan can't figure out what a method from the 3rd party library returns. And this seems wrong to me.

All things considered, the suggestion of the substitution of @return annotations for psalm types to @psalm-return and providing @return annotations with native types seems reasonable to me.

Ocramius commented 1 year ago

Ah yes, adding @return is acceptable 👍

Do send a new patch please