phpstan / phpstan-nette

Nette Framework class reflection extension for PHPStan & framework-specific rules
MIT License
100 stars 35 forks source link

Nette 3.2 Unreachable statement after adding types to __get method in Nette\Database\Row #144

Open Sahtii opened 3 months ago

Sahtii commented 3 months ago

Unreachable statement - code above always terminates. PHPStan wrongly assumes that this code in Nette\Database\Row always throws Exception and assumes that the further code will not execute. This was not a case in Nette 3.1, problem occured after updating to Nette 3.2

public function __get(mixed $key): never
 {
 $hint = Nette\Utils\Helpers::getSuggestion(array_map('strval', array_keys((array) $this)), $key);
 throw new Nette\MemberAccessException("Cannot read an undeclared column '$key'" . ($hint ? ", did you mean '$hint'?" : '.'));
 }

This happens for example for code

code
$row->variable 
code
ondrejmirtes commented 3 months ago

Looks like it works as designed: the __get always throws an exception (and returns never), so:

code 1
$row->variable 
code 2

"code 2" in this example will not be executed.

What here I do not understand?

ondrejmirtes commented 3 months ago

Anyway, feel free to create a small reproducing repository, so I can better understand the problem. Thanks.

MartinMystikJonas commented 3 months ago

Problem is that $row can have property variable (it is universal objevt crate) but PHPStan thinks it always terminates.

kropacekt commented 3 months ago

@MartinMystikJonas Exactly, $row->variable works without any issues and doesn't throw any errors (in our code), because the column variable is selected from the database.

ondrejmirtes commented 3 months ago

I understand now. The class doesn't have __set so properties are assigned as "dynamic" properties. The __get is only executed for properties that do not exist.

I say this class needs to be removed from universalObjectCratesClasses (https://github.com/phpstan/phpstan-nette/blob/8af94743efcc6d1e685f2ffd7ab279e39c96429c/extension.neon#L31) and needs its own PropertiesClassReflectionExtension.

kropacekt commented 3 months ago

And is this work for you or for someone else?

ondrejmirtes commented 3 months ago

Anyone who wants it fixed ;) (I'll probably not get to it anytime soon.)