Phpstan: Properties must be match their type after constructor call

Created on 25 May 2017  路  4Comments  路  Source: phpstan/phpstan

I run in this situation:

class A {

    /**
     * @var AgreementTypes[]|ArrayCollection
     */
    private $prop1;

    /**
     * @var AccessCode[]|ArrayCollection
     */
    private $prop2;

    public function __construct()
    {
        $this->prop1= new ArrayCollection();
        //$this->prop2= new ArrayCollection(); // forgot to call this
        }
}

After constructor call, properties cannot ever have types that are advised in type hints (instead there will be NULL). I'm not sure how this is complicated to implement, it looks like a candidate for symbolic execution.

Uninitialized properties (collections) often happens in Doctrine entities.

feature-request

Most helpful comment

This is really difficult to implement generally right now and precisely for the same reason why the typed properties PHP RFC was rejected. There are far too many situations what can happen in constructor that it's not really easy to tell whether a property has a certain type after the object is initialized.

But if you don't have any special code and you're just assigning properties in your constructors, it should be pretty easy to implement the rule by yourself. But I don't want to include it in PHPStan core, because it won't be general and universally usable.

All 4 comments

This is really difficult to implement generally right now and precisely for the same reason why the typed properties PHP RFC was rejected. There are far too many situations what can happen in constructor that it's not really easy to tell whether a property has a certain type after the object is initialized.

But if you don't have any special code and you're just assigning properties in your constructors, it should be pretty easy to implement the rule by yourself. But I don't want to include it in PHPStan core, because it won't be general and universally usable.

Just some food for thought. PHPStan is already is aware of setting a property to the wrong type in a constructor, or anywhere else for that matter. The only thing it does not do is notice when you fail to set a property to anything at all, even if that property does not allow null.

I think this would be a good option to have, but I'm not sure if it makes sense to be on by default, as setting all properties to a valid type inside the constructor does not always result in readable code, mostly because PHP lacks support for named parameters. Even though it may not always result in readable code, it does result in cleaner code, since it helps with encapsulation to enforce a class will already be following all of its' own rules immediately after construction. But even though this results in cleaner code, it could also contribute to mistakes. It would be easy to forget to change the order of your arguments if the order of the parameters changed, but the types were the same. This is another problem named parameters would solve... Argh, I really wish we had named parameters in PHP, at least for constructors. Anyway, my point is some people just set the properties from outside the class and outside the constructor, immediately following construction, as a workaround for not having named parameters. It results in more readable code and prevents you from thinking you are setting one property when you are actually setting a different one. And for that reason, if this feature were developed, it should probably remain an option that is not on by default.

You can follow the principle that Hack does, which is reasonably airtight:

Any property must be set either in the constructor or in private methods called in the constructor.

I just did this for properties with native types: https://github.com/phpstan/phpstan/issues/2984#issuecomment-658184718

I choose to continue be more loose about properties that have type only in their PHPDoc because there's much more code that looks like that out there and I don't want to break it. Please note you can combine native + phpdoc type and that WILL be reported.

Was this page helpful?
0 / 5 - 0 ratings