Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ruudk opened this issue Sep 17, 2024 · 3 comments
Open

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

ruudk opened this issue Sep 17, 2024 · 3 comments

Comments

@ruudk
Copy link
Contributor

ruudk commented Sep 17, 2024

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
Copy link
Contributor

stof commented Sep 18, 2024

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.

@ruudk
Copy link
Contributor Author

ruudk commented Sep 18, 2024

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
Copy link
Contributor

stof commented Sep 18, 2024

the case of validated objects is tracked at #369

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants