Skip to content

[Form] Adding @var PHPDoc to silence psalm #52861

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

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

ThomasLandauer
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? yes (kind of)
New feature? no
Deprecations? no
Issues Fix #52829
License MIT

The goal is to add some PHPDoc to silence the psalm warnings mentioned in the linked issue, without changing the actual type hint (would be a BC break).

I found three ways to achieve this:

  • Adding @var to the variable in the next line. That's what I'm doing in this PR. The idea is taken from https://stackoverflow.com/a/46842721/1668200 . It looks weird, but it's only a small change...
  • Adding @param above the anonymous function. Would require to move it to a new line and subsequently indent the following ~50 lines:
    $builder->addEventListener(FormEvents::PRE_SUBMIT,
        /** @param PreSubmitEvent $event */
        static function (FormEvent $event) use ($choiceList, $options, &$unknownValues) {
            // ...
        }
    );
  • Refactoring by extracting the anonymous callback function to a dedicated method.

Waiting for feedback here, then I'll do the same in TimeType and FileType

@stof
Copy link
Member

stof commented Dec 1, 2023

As Symfony ensures that the FormEvents::PRE_SUBMIT event is triggered using a PreSubmitEvent object (and as this is inside the Form component, we don't have to care about older versions where this PreSubmitEvent was not there and FormEvent was instantiated), I would suggest changing the typehint instead.

@ThomasLandauer
Copy link
Contributor Author

That's what I suggested too, but others said it's not worth the BC break, see #52829 (comment)

@yceruto
Copy link
Member

yceruto commented Dec 1, 2023

@stof technically you can dispatch any FormEvent subtype (different from PreSubmitEvent) through the form event dispatcher accessible through the $form->getConfig()->getEventDispatcher() instance for a ChoiceType field. Why someone would want to do that, I don't know, but it's not only the Form class that is able to do it.

I'm not against changing it directly without deprecation, though, as it's really a special use case.

@nicolas-grekas
Copy link
Member

Thank you @ThomasLandauer.

@nicolas-grekas nicolas-grekas merged commit 4f3822e into symfony:6.4 Jan 2, 2024
ThomasLandauer added a commit to ThomasLandauer/symfony that referenced this pull request Jan 2, 2024
@ThomasLandauer ThomasLandauer deleted the patch-2 branch January 2, 2024 13:44
nicolas-grekas added a commit that referenced this pull request Jan 2, 2024
…homasLandauer)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Form] Adding more ``@var`` PHPDoc's to silence psalm

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes (kind of)
| New feature?  | no
| Deprecations? | no
| Issues        | none
| License       | MIT

... as announced in #52861 (comment)

Commits
-------

e47b557 [Form] Adding more ``@var`` PHPDoc's to silence psalm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants