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

Open
#407 4 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
php
Domain
backend

Research direction

Start by reviewing the form getData() and isValid() entry points described in the issue, along with the existing form typing behavior. Determine how a validated form could expose non-null TValidatedData, then add coverage for the before-validation and after-validation types shown in the examples. Done means PHPStan infers the requested types without extra assertions.

Written by the indexing model from the issue text.

Description

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.

Dominant language
PHP
Stars
800
Forks
104
Avg merge
4m
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from phpstan/phpstan-symfony

All issues in phpstan/phpstan-symfony

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.