phpstan / phpstan-nette

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

Weird PHPStan errors for renamed interfaces #105

Closed MartinMystikJonas closed 2 years ago

MartinMystikJonas commented 2 years ago

In one of our legacy projects we upgraded to Nette 3. After upgrade PHPStan reports many weird errors about incompatible types in places where types should be compatible.

For example:

... expects Nette\Caching\IStorage, Nette\Caching\Storage given
... expects Nette\Database\Row, Nette\Database\Table\ActiveRow given

It seems that it reports places where signature uses old name with I prefix (probably left in Nette code for backward compatibility)? And also places where implementation of renamed Nette interfaces are used agains these interfaces.

It seems that PHPStan do not know that IRow and Row are equivalents. But I do not know why and I do not know how to debug it, Any hints? I already checked and we have latest versions of Nette packages and PHPStan with phpstan-nette extension.

ondrejmirtes commented 2 years ago

This is a hard problem. You're getting the error message:

expects Nette\Caching\IStorage, Nette\Caching\Storage given

PHPStan (due to static reflection) sees this definition: https://github.com/nette/caching/blob/55c034fe4221a58ca612376aad8f2cf08024ad58/src/compatibility.php#L22-L25

Which means that IStorage is a narrower type than Storage, they're not aliases for each other.

I just recommend you to stop using deprecated symbols.

MartinMystikJonas commented 2 years ago

These symbols are used in Nette packages itself.

ondrejmirtes commented 2 years ago

Nette itself shouldn't use deprecated code...

MartinMystikJonas commented 2 years ago

Avreed. Not sure why it kept old interfaces in some places. Maybe to prevent BC breaks.

Should this be somehow solved on PHPStan level? It seems weird that I did not find any similar issues. So I wondered if it was some error on my side.

Maybe it would be best to just ignore these errors until Nette stops using old interfaces.

ondrejmirtes commented 2 years ago

Maybe it would be best to just ignore these errors until Nette stops using old interfaces.

Yes, the easiest option.

PHPStan can't solve it, the BC layer in Nette is really weird. For example with Doctrine there are no such problems usually.

MartinMystikJonas commented 2 years ago

Some of these errors cannot be ignored (contravariance of parameters) but I found solution to most of these errors can be done by:

parameters:
  bootstrapFiles:
    - vendor/nette/caching/src/compatibility.php

Is this a valid solution?

For some reason this does not work with all error but I guess it is something that should be fixed in Nette packages itself.

ondrejmirtes commented 2 years ago

Yes, it can help, but it's brittle and it might break as a side effect of some changes in PHPStan. But for now it's fine I guess.

MartinMystikJonas commented 2 years ago

Just ideas: Would it be possible to add configuration for these cases? To define that some classes/interfaces are aliases?

ondrejmirtes commented 2 years ago

You might have some luck by writing your own bootstrap file and put some class_alias calls in there.

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.