`Required` attribute should remove readonly properties errors
SVillette opened this issue · 8 comments
Description
Currently, PHPStan reports an error when a readonly property is not initialized and another error when not initialized in a constructor.
Class App\Foo has an uninitialized readonly property $validator. Assign it in the constructor.Readonly property App\Foo::$validator is assigned outside of the constructor.
But with the attribute #[Required] the DIC automatically call this method to initialize the property.
I think it could be a nice feature to remove these errors in that case.
Example
trait ValidationAwareCommandTrait
{
private readonly ValidatorInterface $validator;
#[Required]
public function setValidator(ValidatorInterface $validator): void
{
$this->validator = $validator;
}
}Playground
For sure this doesn't work in any class, right? What's the requirement for this to work somewhere? Does the class need to be a DI service?
The service has to be part of the DIC to use the attribute. There are multiple types of injections as the following docs explain (the doc will explain it better than me).
- https://symfony.com/doc/current/service_container/autowiring.html#autowiring-other-methods-e-g-setters-and-public-typed-properties
- https://symfony.com/doc/current/service_container/injection_types.html#setter-injection
I think the setter injection is the most popular to deal with optional dependencies.
Also, the example was using traits which is not documented in the Symfony docs, but works too.
Yeah, I don't think we need to read the DIC for registered services in the first version, we just need to get rid of the false positives.
Feel free to contribute this, here's the guide: https://phpstan.org/developing-extensions/always-read-written-properties
I think #348 may fix half of the issue (first error).
But is there any extension point to prevent the second error (Readonly property App\Foo::$validator is assigned outside of the constructor.) to be triggered ? As it was made by purpose according to this comment.
But is there any extension point to prevent the second error
There isn't. Just remove the readonly keyword from those properties, it doesn't have any benefit in that case.
I disagree on that point. The docs mention that
The setter can be called more than once, also long after initialization, so you cannot be sure the dependency is not replaced during the lifetime of the object (except by explicitly writing the setter method to check if it has already been called).
Checking that the setter has been called or not can be done by setting the property readonly.
I guess this one can be closed now @ondrejmirtes. This error is not matched anymore in my project.
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.