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

Improve type inference for plugin manager #56

Closed gsteel closed 2 years ago

gsteel commented 2 years ago
Q A
QA yes

Description

If https://github.com/laminas/laminas-servicemanager/pull/137 gets merged, this is what I had in mind for inputfilter, filter, validator and view … and others??

Ocramius commented 2 years ago

https://github.com/laminas/laminas-servicemanager/releases/tag/3.12.0

FabianKoestring commented 2 years ago

After this change we got a lot of PHPStan errors.

Return type (array) of method Application\Form\Fieldset\File::getInputFilterSpecification() should be compatible with return type (*NEVER*) of method
         Laminas\InputFilter\InputFilterProviderInterface::getInputFilterSpecification()

Application\Form\Fieldset\File::getInputFilterSpecification() looks like this.


public function getInputFilterSpecification(): array
{
    return [...];
}
Ocramius commented 2 years ago

@FabianKoestring can you please make a reproducer snippet on https://phpstan.org/try ?

Ocramius commented 2 years ago

Also, please do so in a new issue :)

FabianKoestring commented 2 years ago

@FabianKoestring can you please make a reproducer snippet on https://phpstan.org/try ?

I would do that but I don't know how to set dependencies (to laminas/laminas-inpufilter) on https://phpstan.org/try. The problem occurs from version 2.16.0 on.

Ocramius commented 2 years ago

@FabianKoestring copy in all symbols that affect your issue, and reduce the example to the minimum size to show the issue.

FabianKoestring commented 2 years ago

@FabianKoestring copy in all symbols that affect your issue, and reduce the example to the minimum size to show the issue.

👍 Look at error on line 62. https://phpstan.org/r/91e0e318-0ada-4777-9e50-983a0d7678a1

Return type (CollectionSpecification|InputFilterSpecification) of method HelloWorld::getInputFilterSpecification() should be compatible with return type (NEVER) of method InputFilterProviderInterface::getInputFilterSpecification()

Ocramius commented 2 years ago

Simplifying your example further, it looks like phpstan doesn't understand this syntax:

array{
     type?: class-string<InputFilterInterface>|string,
}&array<array-key, InputSpecification>

See https://phpstan.org/r/6e758234-f4ef-4ff6-8519-919f3c9297e4

I would report an issue in upstream phpstan/phpstan-src

FabianKoestring commented 2 years ago

@Ocramius https://github.com/phpstan/phpstan/issues/7475

froschdesign commented 2 years ago

@gsteel Please check https://github.com/phpstan/phpstan/issues/7475#issuecomment-1155259861

gsteel commented 2 years ago

The problem with the InputFilterSpecification type is that is seems valid to me. From what I can tell, the factory accepts [?'type' => 'FQCN or alias'] and also the array will contain a list of either Inputs or InputFilters or specification arrays. The keys of the list can be strings or integers - string keys are used as the input name (This is the assumed behaviour) and integer keys are replaced with the 'name' from the input spec (value).

At the expense of allowing integer keys, the type could be array{type?: whatever}&array<string, InputSpec|InstanceType>

It currently seems to me that the intersection type is the right thing but I'd gladly learn of a better/more precise way to represent this…