[Form] `ChoiceType` is still calling the removed `PostSubmitEvent::setData()`

[ThomasLandauer]

Symfony version(s) affected

7.0.0

Description

@HeahDude in #51043 you removed FormEvent::setData().

But ChoiceType::buildForm() still contains this line at https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php#L162

$event->setData($knownValues);

But this will now lead to a BadMethodCallException in PostSetDataEvent and PostSubmitEvent.

Did you overlook this? Or is this line never executed in fact (and thus could be deleted)?

How to reproduce

See above.

Possible Solution

No response

Additional Context

No response

[HypeMC]

The setData() method is called in a pre-submit listener, how did you get a PostSetDataEvent or PostSubmitEvent?

[HeahDude]

Hello @ThomasLandauer, I don't understand, as said @HypeMC the line you link is handling a PreSubmitEvent, so it is not concerned by a bad method call.

Did you actually encounter the error or is it a theoretical question?

[ThomasLandauer]

I have a custom type which is pretty much the same as ChoiceType. And there psalm is reporting:

ERROR: DeprecatedMethod - src/Form/Type/MyType.php:165:21 - The method Symfony\Component\Form\Event\PostSetDataEvent::setData has been marked as deprecated (see https://psalm.dev/001)
$event->setData($knownValues);

ERROR: DeprecatedMethod - src/Form/Type/MyType.php:165:21 - The method Symfony\Component\Form\Event\PostSubmitEvent::setData has been marked as deprecated (see https://psalm.dev/001)
$event->setData($knownValues);

Thanks to your answers I now changed the type-hint of $event (in the callback function) from FormEvent to PreSubmitEvent.
So now I'm wondering if this should be changed in ChoiceType too?

[HeahDude]

Changing the type hint of that callback listener in core would be a BC break even if it would be very unlikely to happen in real life.

Actually, FormEvent::setData() has not been deprecated in 6.4, only two of the subtypes have.
Although, from a static analyser perspective the current type hint is not a guarantee that we cannot pass another subtype, and such warning is legit. However, in practice, that should never happen here, so we could just ignore it.

IMHO it is not worth the trouble and would have to wait for 7.1 anyway.
@xabbuh @yceruto any thoughts?

[yceruto]

Having the typehint in core for the corresponding event class in the listener would make sense to me; it's more appropriate now that there are subtypes where calling setData is not allowed. But as you said, it's a breaking change to do that directly. Even if it would be a very weird use case, technically it's possible to dispatch a custom subtype using $form->getConfig()->getEventDispatcher() instance.

In my opinion, it isn't worth deprecating and changing the typehint in core. However, I think we should improve the documentation to typehint the correct form event in all examples, thus reducing this kind of static analyzer reporting in userland.

[ThomasLandauer]

1. See [Form] Updating the generic FormEvent to specific event (e.g. `PreS… symfony-docs#19206 for updating the docs - is this what you had in mind?
2. What about adding a phpdoc like /** @var PreSubmitEvent $event */ (or whatever the syntax is) to ChoiceType and TimeType and FileType ?