Description
Generic types for FormTypeInterface
and the various createForm
methods have been rejected in Symfony because they are lying about the actual behavior of the form component. Those generic types allow to have a nice inferred type for the basic usage of the component, but they will analyze advanced usages of the component in a broken way (reporting errors on valid code, inferring wrong types due to making wrong assumptions about how the component actually works).
See symfony/symfony#40783 (comment) for the discussion when those generic types were submitted upstream.
In version 1.x of the extension, those generic types were configured in skipCheckGenericClasses
, which allowed projects to mostly ignore the fact that the extension stubs were adding it. In 2.0x, this is not the case anymore so phpstan will enforce that any form type defines such generic type to satisfy the extension, even when being aware that those generic types are a lie.
My opinion is that those stubs should be removed to match the decision done in Symfony. However, I would also accept a case where the loading of those simplistic generic types is conditional, allowing projects to decide whether they want them (if they rely only on the basic usage pattern where the inference would produce a good result) or not (if they want the analysis to stick to the true behavior of the component).
Activity
[-]Form APIs should not be enforcing generic types that have been rejected in Symfony[/-][+]Form APIs should not be enforcing generic types that have been rejected in Symfony because of being wrong[/+]ondrejmirtes commentedon Mar 20, 2025
These should be conditionally loaded in a StubFilesExtension based on the Symfony packages versions. We already do that in phpstan-doctrine with doctrine/collections I think.
stof commentedon Mar 20, 2025
This comment seems to be an answer for #431 rather than for this one.
For the form API, my point is that generic types added in those stubs are lying about the actual behavior of the component, which will mark some valid code as invalid and some invalid code as valid. The analysis I made in Symfony made me conclude that we cannot accurately represent the form behavior with generic types, especially not inside FormTypeInterface (which has to deal with a totally mutable form builder, and in some parts with the Form instance that is impacted by all form types involved in building it)
ondrejmirtes commentedon Mar 21, 2025
Please show some code samples where the current form stubs are correct, and when they are wrong, so I can think about this more. My thinking is that they might still be useful for the "happy path", and some projects only use those happy paths, so removing the stubs would be a loss for some.
stof commentedon Mar 24, 2025
that's why I proposed to make their loading configurable (either by using a dedicated extension or by using a parameter in that extension).
The current generics types are mostly helpful for the code you write in the controller to instantiate a form ("mostly" is because they don't consider the case of an empty form submission which can return
null
as data for the form if the initial data is not provided andempty_data
is the default one, but cannot returnnull
ifempty_data
provides a non-null value).However, most of this code can also be written without relying on those generics at all (only raw phpstan):
The bad things happen when implementing form types.
The stubs for
FormTypeInterface
are telling you that methods will get aFormInterface<TData>
, but this is totally wrong. If you create another form type that declares your form type as parent and adds a model transformer (and so has a different type for its model data which isTData
), your form type will have itsbuildView
andfinishView
methods called with aFormInterface<ChildData>
instead (withChildData
being totally unrelated toTData
). And the same happen for the type of theFormBuilderInterface
passed tobuildForm
.Typing event listeners suffers from the same issue (but I see that stubs don't actually try to describe those at all)
stof commentedon Mar 24, 2025
And putting proper generic types for the usage of FormInterface in the DataMapperInterface would also be hard (which the stubs don't do today, causing #417)
zmitic commentedon Mar 24, 2025
As an end-user of these stubs for 5 years, I would like to add something before any decision you make.
Entities will almost always have some not-null dependencies. Be it a Product with price and Category, or Car having a
Maker, or even scalars with
non-empty-string
name andpositive-int
age values.Very simple yet realistic example:
Having a form for this entity cannot be done without the use of
empty_data
:Or much simpler, use factory from qossmic/rich-model-forms-bundle (a wrapper around
empty_data
):I am not saying that these stubs are perfect, I am fully aware that some transformers can create issues (although I haven't had them). But the above are just some arguments to consider.