psalm / psalm-plugin-symfony

Psalm Plugin for Symfony
MIT License
228 stars 53 forks source link

Unnecessary/incorrect Symfony Form stubs #322

Open pps1 opened 1 year ago

pps1 commented 1 year ago
Package Version
Symfony 6.3.5
Psalm 5.15.0
Plugin 5.0.3

The plugin adds generics stubs for Symfony form components, specifically: ./vendor/psalm/plugin-symfony/src/Stubs/common/Component/Form/AbstractType.stubphp ./vendor/psalm/plugin-symfony/src/Stubs/common/Component/Form/AbstractTypeExtension.stubphp ./vendor/psalm/plugin-symfony/src/Stubs/common/Component/Form/Form.stubphp

These stubs are incorrect as these artifact interfaces do not exchange generics.

Sample psalm validation errors:

ERROR: MissingTemplateParam - src/Form/Extension/FormTypeExtension.php:18:7 - App\Form\Extension\FormTypeExtension has missing template params when extending Symfony\Component\Form\AbstractTypeExtension, expecting 1 (see https://psalm.dev/182)
class FormTypeExtension extends AbstractTypeExtension

ERROR: MissingTemplateParam - src/Form/Type/DatePickerType.php:11:7 - App\Form\Type\DatePickerType has missing template params when extending Symfony\Component\Form\AbstractType, expecting 1 (see https://psalm.dev/182)
class DatePickerType extends AbstractType

Should these stubs be removed?

zmitic commented 1 year ago

I made these, and if it is a problem, I am OK with removing form stubs; another repo is fine. But FormExtension does have data just like the AbstractType. Can you put some code example, bare minimum if possible?

To explain the reasoning: primary use case for templating extensions are form events and can be void in unused. For example, this is one extending 2 types, from real code but I replaced entities:

/** @extends AbstractTypeExtension<AbstractUser> */
class PasswordExtension extends AbstractTypeExtension
{
    // I have 2 different forms, for 2 different entities, both extending AbstractUser (table inheritance in Doctrine)
    public static function getExtendedTypes(): iterable
    {
        yield DoctorType::class;
        yield PatientType::class;
    }

    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder->addEventListener(FormEvents::PRE_SET_DATA, function (PreSetDataEvent $event) {
            $user = $event->getData();  // <--- psalm knows this is null|AbstractUser
            $this->addRepeatedPasswords($event->getForm(), $user);  
        });
    }

    private function addRepeatedPasswords(FormInterface $form, ?AbstractUser $user): void
    {
        // the rest of code: add repeated passwords, validation etc...
    }
}

In above example, I could have used inherit_data and another type, but those do not yet support events; extension was the only way.

pps1 commented 1 year ago

Hey @zmitic thanks for your note! I'll try to find some time to create basic examples. In the meantime, this explanation may shed some context:

https://symfony-devs.slack.com/archives/C8SFXTD2M/p1696508243405699

zmitic commented 1 year ago

@pps1 I can't access this. Can you paste most important bits that are relevant to this?