phpstan/phpstan-symfony

Narrow return type of `$form->getData()` after calling `$form->isValid()`

Opened this issue · 3 comments

Every time I work with forms in Symfony and PHPStan, I'm struggling with making sure PHPStan properly understands what's going on.

After the form has been submitted and validated, I usually end up with doing a lot of assertions to get the typing right.

Let's say we have the following form:

/**
 * @extends AbstractType<array{firstName: string|null, lastName: string|null}>
 */
class UserFormType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options) : void
    {
        $builder->add('firstName', TextType::class, [
            'required' => true,
            'constraints' => [new Length(['min' => 3])],
        ]);
        $builder->add('lastName', TextType::class, [
            'required' => true,
            'constraints' => [new Length(['min' => 3])],
        ]);
    }
}

And we use it like this:

class Controller extends AbstractController
{
    public function addAction(Request $request) : array
    {
        $form = $this->createForm(UserFormType::class);
        $form->handleRequest($request);

        assertType('array{firstName: string|null, lastName: string|null}|null', $form->getData());

        if ($form->isSubmitted() && $form->isValid()) {
            $data = $form->getData();

            assertType('array{firstName: string, lastName: string}', $data);
        }

        return [
            'form' => $form->createView(),
        ];
    }

    public function editAction(Request $request) : array
    {
        $form = $this->createForm(UserFormType::class, [
            'firstName' => 'Ruud',
            'lastName' => 'Kamphuis',
        ]);
        $form->handleRequest($request);

        assertType('array{firstName: string, lastName: string}|null', $form->getData());

        if ($form->isSubmitted() && $form->isValid()) {
            $data = $form->getData();

            assertType('array{firstName: string, lastName: string}', $data);
        }

        return [
            'form' => $form->createView(),
        ];
    }
}

A few things can be improved to make it easier to work with forms.

  1. When calling $form->getData() before submitting the form, it returns TData |null. After the form has been submitted and validated, it is still TData|null. At this point I expect it to be TData only.

  2. Currently, we can only configure TData on the form. This type should always support the empty states, so this will have a lot of nulls most of the time. What if we introduce a second template TValidatedData, that can returned when calling $form->getData() after the form was submitted and validated?

Would the above be possible? And does it make sense? If so, I could give it a try.

stof commented

As discussed on Slack, adding a TValidatedData would only help for forms bound on an array shape, while the recommended practice is to bind forms on objects (running the validator on an array shape leads to weird effects when values are not scalars but object, as it cascades validation to those objects even when the Valid constraint is not applied, leading to weird issue when your array shape contains an object selected with a ChoiceType or its children).
And adding a second template type will be a BC break.

But even if I would use an object instead of an array shape, there is still the problem of nullability. The nullability is needed for initial form state.

For example, let's say you have this object:

class FormDTO {
    public ?string $firstName;
    public ?int $age;
}

And let's say you want to ensure the first name is a non empty string and the age is int<0, 150>.

You can use the Symfony validator to validate these type of things easily.

But PHPStan will never understand this. So when you call $form->getData() you get back your FormDTO instance, that is filled with the data, but is still considered null|string and null|int. Ideally, you want to read it back as non-empty-string and int<0, 150>.

How would you transform your FormDTO to be used on places that require a non-empty-string? Do you do again assertions? Even though Symfony Validator already took care of that?

stof commented

the case of validated objects is tracked at #369