Should complain about private typed property initialized in public/protected non-final method

[weirdan]

I think Psalm should still complain about potentially uninitialized property, because public, non-final method may be overridden in descendants: https://psalm.dev/r/4f967f6930, https://3v4l.org/juGjD

Originally posted by @weirdan in #3200 (comment)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/4f967f6930

<?php

function getEntityById(): ?string {
    return mt_rand() ? 'Possible and entity, null when there is none' : null;
}


class Presenter {
    private string $plan;
    
    public function __construct() {
        $this->nettePresenterStartup();
    }
    
	public function nettePresenterStartup(): void {
        $plan = getEntityById();
        if ($plan === null) {
            $this->redirect('Training:');
        }
        $this->plan   = $plan;
	}
    
    /** @psalm-return never-returns */
    public function redirect(string $_arg): void {
        throw new RuntimeException();
    }
    
    public function getPlan(): string {
        return $this->plan;
    }
}

class PresenterChild extends Presenter {
    public function nettePresenterStartup(): void {}
}

echo (new PresenterChild)->getPlan(); // BOOM!

Psalm output (using commit 6b42efe):

No issues!

[weirdan]

Simplified: https://3v4l.org/HmtAZ, https://psalm.dev/r/63168feb83

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/63168feb83

<?php

class ParentClass {
    private string $prop;
    
    public function __construct() {
        $this->init();
    }
    
	public function init(): void {
        $this->prop = "zxc";
	}
       
    public function getProp(): string {
        return $this->prop;
    }
}

class ChildClass extends ParentClass {
    public function init(): void {}
}

echo (new ChildClass)->getProp(); // BOOM!

Psalm output (using commit 6b42efe):

No issues!

[muglug]

So I think the thing to do here is to ignore any non-final methods when analysing initilisation of one or more private methods.

[muglug]

This imposes an additional constraint that will break some currently-working Psalm builds

[weirdan]

This imposes an additional constraint

I thought Psalm already had that constraint (see #358). Was it relaxed at some point?

[muglug]

Yeah, I didn’t have any tests with this edge-case (a private property set in an overrideable method), so blanket allowed setting properties in public methods.