psalm: False positive PropertyNotSetInConstructor

<?php

trait CreatedAt {
    protected DateTimeImmutable $createdAt;

    private function onCreated(): void {
        $this->createdAt = new DateTimeImmutable();
    }
}

class Foo {
    use CreatedAt;

    public function __construct() {
    	$this->onCreated();
    }
}

class Bar extends Foo {}

Property $createdAt was initialized it Foo constructor. But psalm knows nothing about it in Bar.

https://psalm.dev/r/106a0483f7

About this issue

Original URL
State: open
Created 3 years ago
Comments: 20 (2 by maintainers)

Most upvoted comments

Hey @rulatir, feel free to take a look at it. I’d be fine with not raising this error when we can show that a parent correctly initialize the property, even if a child could theoretically override the method

orklah on May 8, 2022

@rulatir You seem engaged in your own little crusade against final that is completely out of scope here, given we stated earlier that final doesn’t even solve the issue

However, you managed to both massively overestimate the time it would take you to learn how to fix this issue and also massively underestimate the time it would take me (or anyone else in the project) to fix it. And you did this to justify your demand for other to fix a problem that doesn’t even exists in a tool you don’t use.

That’s quite a performance there, pal.

This bug tracker currently has 900 open issues and many are more critical than this. So as I said, if you’re concerned enough to do anything more than complain, feel free to take a look at it.

orklah on May 14, 2022

I do feel free to “take a look at it”. I don’t feel able to “take a look at it”, not without investing weeks to learn the basics of the project, all the while someone who already knows the codebase could probably fix this bug in 10 minutes. And this is normal. People who come to a bugtracker should be assumed to be in the position I am in: harmed by a bug, but not knowing the rocket science required to fix it.

I no longer use psalm in my projects, but this bug doesn’t just harm psalm users. It harms users of all libraries developed with psalm as a linter, which end up being completely locked down with final in order to keep this overzealous rule quiet.

rulatir on May 12, 2022

Interesting. Could you point me to some further reading about why thta need to be final?

Well, that’s fairly simple. Given class C:

class C {
   private int $prop;

   protected function initProp(): void { $this->prop = 1; }

   public function __construct() { $this->initProp(); }

   public function getProp(): int { return $this->prop; }
}

it can be shown that $this->prop can be left uninitialized when descendant overrides initProp(), e.g.

class D extends C {
    protected function initProp(): void {}
}

(new D)->getProp(); // will fail

Psalm cannot assume this won’t happen, unless the method is private or final or the class itself is final.

weirdan on Apr 16, 2021

My crusade is against false positives in general. They are massively harmful.

rulatir on May 23, 2022

Seriously, this needs more urgent attention. It is absolutely unacceptable for a lint tool to force programmers to entirely throw away a huge chunk of what constitutes OO programming. This bug forces programmers to lock their libraries down with final all over the place, or engineer ultra-pedantic, bloated, resource-hungry monstrosities with 7× more classes than necessary.

rulatir on Apr 30, 2022