PropertyNotSetInConstructor - when trait defined same property in different case

[oleg-andreyev]

Example: https://psalm.dev/r/69f9c98ac1

While it looks like a valid case, the error message could be improved and say that property is defined in trait

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/69f9c98ac1

<?php

interface ViewHandlerInterface {}

trait ControllerTrait {
	private ViewHandlerInterface $viewhandler;
}

class Controller {
    use ControllerTrait;
    
	private $viewHandler;
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->viewHandler = $viewHandler;
    }
}

Psalm output (using commit ad5a8c2):

ERROR: PropertyNotSetInConstructor - 9:7 - Property Controller::$viewhandler is not defined in constructor of Controller and in any private or final methods called in the constructor

[oleg-andreyev]

Another example https://psalm.dev/r/8c91ef717d

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8c91ef717d

<?php

interface ViewHandlerInterface {}

trait ControllerTrait {
	private ViewHandlerInterface $viewhandler;
    
    public function setViewHandler(ViewHandlerInterface $viewhandler): void {
    	$this->viewhandler = $viewhandler;
    }
}

class Controller {
    use ControllerTrait;
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->setViewHandler($viewHandler);
    }
}

Psalm output (using commit ad5a8c2):

ERROR: PropertyNotSetInConstructor - 13:7 - Property Controller::$viewhandler is not defined in constructor of Controller and in any private or final methods called in the constructor

[weirdan]

The latter is valid and can be fixed by making setViewHandler() final or private: https://psalm.dev/r/3dcfb7762c

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/3dcfb7762c

<?php

interface ViewHandlerInterface {}

trait ControllerTrait {
	private ViewHandlerInterface $viewhandler;
    
    public final function setViewHandler(ViewHandlerInterface $viewhandler): void {
    	$this->viewhandler = $viewhandler;
    }
}

class Controller {
    use ControllerTrait;
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->setViewHandler($viewHandler);
    }
}

Psalm output (using commit ad5a8c2):

No issues!

[oleg-andreyev]

The latter is valid and can be fixed by making setViewHandler() final or private: https://psalm.dev/r/3dcfb7762c

@weirdan It's a vendors code. I cannot do it. It's still weird that final is required.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/3dcfb7762c

<?php

interface ViewHandlerInterface {}

trait ControllerTrait {
	private ViewHandlerInterface $viewhandler;
    
    public final function setViewHandler(ViewHandlerInterface $viewhandler): void {
    	$this->viewhandler = $viewhandler;
    }
}

class Controller {
    use ControllerTrait;
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->setViewHandler($viewHandler);
    }
}

Psalm output (using commit ad5a8c2):

No issues!

[weirdan]

It's still weird that final is required.

Not really. If it's neither final nor private, and the class itself is not final then descendants may override the method and leave the property uninitialized.

[oleg-andreyev]

So by making my class final this issue should go away?

https://psalm.dev/r/e802436284

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/e802436284

<?php

interface ViewHandlerInterface {}

trait ControllerTrait {
	private ViewHandlerInterface $viewhandler;
    
    public function setViewHandler(ViewHandlerInterface $viewhandler): void {
    	$this->viewhandler = $viewhandler;
    }
}

final class Controller {
    use ControllerTrait;
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->setViewHandler($viewHandler);
    }
}

Psalm output (using commit ad5a8c2):

ERROR: PropertyNotSetInConstructor - 13:13 - Property Controller::$viewhandler is not defined in constructor of Controller and in any private or final methods called in the constructor

[weirdan]

It should, and it does work for inline methods: https://psalm.dev/r/c47bf2af78 , but apparently it fails to consider final when calling a method from trait.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/c47bf2af78

<?php

interface ViewHandlerInterface {}

final class Controller {
    private ViewHandlerInterface $viewhandler;
    
    public function setViewHandler(ViewHandlerInterface $viewhandler): void {
    	$this->viewhandler = $viewhandler;
    }
    
    public function __construct(ViewHandlerInterface $viewHandler) {
        $this->setViewHandler($viewHandler);
    }
}

Psalm output (using commit ad5a8c2):

No issues!

[muglug]

it's just confused about inherited properties from traits, because that property appears twice. Should be a very simple fix.

[weirdan]

because that property appears twice.

It actually doesn't. There are two different properties involved: $viewhandler and $viewHandler (note the case).

Edit: this refers to the original case, not the one discussed later.

[muglug]

ah ok, sorry

so it's the difference between

<?php

interface A {}

trait ATrait {
    private A $a;
    
    public function setA(A $a): void {
    	$this->a = $a;
    }
}

final class C {
    use ATrait;
    
    public function __construct(A $a) {
        $this->setA($a);
    }
}

and

<?php

interface A {}

trait ATrait {
    private A $a;
    
    public final function setA(A $a): void {
    	$this->a = $a;
    }
}

final class C {
    use ATrait;
    
    public function __construct(A $a) {
        $this->setA($a);
    }
}

[Jean85]

Is #6087 the same issue?

[AndrolGenhald]

It looks like this issue is actually just about improving the error message. Psalm is correct here that $viewhandler is not set in the constructor, it's just confusing because there's a very similarly named variable that is set. Maybe saying Controller::ControllerTrait::$viewhandler in the message would make it clear?