Open zerkms opened 1 month ago
Good idea, it makes perfect sense. But why return nullable FormInterface<null|TData>
?
FormInterface:getData
already returns null|TData
. It has to, because create methods don't pass $data parameter, which then triggers empty_data
callback.
But why return nullable FormInterface<null|TData>
Because createForm
's second argument $data
is declared as having = null
by default in symfony, and having generic parameter nullable was the only way I could find working to make psalm happy about type-checking it.
FormInterface:getData already returns null|TData. It has to, because create methods don't pass $data parameter, which then triggers empty_data callback.
Indeed, but psalm wasn't accepting it :shrug: So it was more like - overcoming the psalm limitation (or a bug?), than a thoughtful decision :-D
Current definition is the following:
https://github.com/psalm/psalm-plugin-symfony/blob/fb801a9b3d12ace9fb619febfaa3ae0bc1dbb196/src/Stubs/5/Bundle/FrameworkBundle/Controller/AbstractController.stubphp#L18-L26
I suggest to change it to:
What has changed:
TData $data
parameter addednull|TData
generic parameter changed for the return typeWhy: this would allow check the
$this->createForm(FormType::class, ['rubbish'])
compatibility: if theFormType
does not extendAbstractType<array{0: 'rubbish'}>
then it will be marked as incompatible.Thoughts?