Conversation
|
I understand that count should normally return a positive int. But I suspect there are usecases for having it be negative |
|
@BackEndTea thx for the feedback. I agree there might be code in the wild which returns magic values like -1 or similar, but IMO this code is violating the count-contract (IMO a class implementing |
|
I'd merge this but the build failures block me. |
51f7d86 to
e5094cf
Compare
didn't realized the errors. fixed. |
| $this->analyse([__DIR__ . '/data/bug-3997.php'], [ | ||
| [ | ||
| 'Return type (string) of method Bug3997\Ipsum::count() should be compatible with return type (int) of method Countable::count()', | ||
| 'Return type (int) of method Bug3997\Baz::count() should be covariant with return type (int<0, max>) of method Countable::count()', |
There was a problem hiding this comment.
Oh, this is actually an annoying BC break :( Can you please do a similar thing that was done here phpstan/phpstan-doctrine#290 and have the Countable stub be applied only for bleedingEdge? Putting Countable stub into a separate file called Countable.stub and configure it only in bleedingEdge.neon should be sufficient.
There was a problem hiding this comment.
should be fixed, I think.
7dbb9ab to
404767b
Compare
|
I did an easier version and used your tests, thanks :) 1e77aed |
|
Ohh indeed.. didn't thought it could be that easy ;-) Thank you |
this should make https://phpstan.org/r/d06ab690-e37c-4aab-ad37-e250d3d29a29 error as expected-> it does not.. seems to be a different bug.. but still the added type-inference should make sense as is