psalm-plugin-symfony
psalm-plugin-symfony copied to clipboard
Add default FormView `vars` in the stub class
These keys are added to FormView::$vars here:
https://github.com/symfony/symfony/blob/4a22bcbda96f347cafd943236bd3029a9fef9467/src/Symfony/Component/Form/Extension/Core/Type/FormType.php#L107
That's the basic FormType at the very top of the Form Type hierarchy, so about any FormView instance should have these keys in their vars.
@mpdude thank you for your contribution! I am not familiar with these new array attributes. In Symfony source code, I did not see anything related. Could you please share reference for it?
Thats important information :+1: , so I've updated the initial comment on top ☝🏻
@mpdude thanks for the explanation 👍 . I got it but I have some concerns:
- adding
'valid' => falseto$varsassumes it is not valid by default which by default should be true. Even if it is changed totrueby default, it means psalm will always deduct as true thus will return in false positives - the other variables (for example,
required,label_attr, etc.) should be also added to the stub as well - the most important point is normally psalm itself should evaluate this code block and add all these missing variables:
$view->vars = array_replace($view->vars, [
I think I don’t get the first and last item… could you please try to rephrase or show me with an example what needs to be done?
thanks!
adding 'valid' => false to $vars assumes it is not valid by default which by default should be true. Even if it is changed to true by default, it means psalm will always deduct as true thus will return in false positives
I think that's fine, the inferred type should be overridden by the @psalm-var tag, which has valid: bool. It should maybe be changed to true in the array if that matches Symfony's declaration, but it doesn't really matter.
I have added the remaining vars as well
@mpdude thank you. I will try to fix test failures on master and after that a rebase might be needed
@seferov ok, ping me when I shall rebase this PR
@mpdude after merging master, the remaining failing tests are related to this changes. Could you please take a look at them?