You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@@ @@-No errors+-1: Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40+Run PHPStan with --debug option and post the stack trace to:+https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md
Full report
Line
Error
-1
Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40Run PHPStan with --debug option and post the stack trace to:https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md
Template variance and bounds are different things, but they are loosely related. You indeed need to support both type of bounds (upper and lower) to allow some patterns in combination of template variances.
In my case I would like to have covariant collection, which support immutable additions (which means use template type in contravariant position). Which you cannot do off course. But what you CAN do is to introduce new invariant type lower bounded by original type, which bypasses it. (Thats how scala does it in standard library https://www.scala-lang.org/api/2.13.3/scala/collection/immutable/List.html#::[B%3E:A](elem:B):List[B] )
The #3960 indeed looks similar. It is slightly different case, but if the author could use lower bound, he would be able to specify type for his use case. The term "Contravariant bound" is I believe incorrect (I have never heard it called this way, probably author just meant lower bound).
BTW I tought that template-contravariant is already supported to some extent, not?
As of call-site variance. Projections are really nice, if implemented, I will be really happy. They are slightly different things from bounds though.
These examples are really mind-bending for me :) For example, I only understood type projections after Jiří Pudil explained it to me in person. Because before PHPStan, I never truly worked with generics and have no experience with them from other languages, I learnt them along the way, and I have no concept or need for these advanced features :)
I'm not sure how to implement it currently though. Each bound type (like @template T of int) has a custom Template*Type class with the current upper bound meaning. To invert the relationship, we'd probably have to either make these classes configurable (they use a common trait anyway) to invert the meaning of the bound.
I was considering sending PR into phpstan myself, but to be honest, phpstan is quite complex project and there is quite high barrier. Is there some kind of "internal documentation" or some kind of "concepts" described somewhere publicly?
This article is especially relevant for you https://phpstan.org/developing-extensions/type-system. The hardest part is to figure out what Type::accepts() and Type::isSuperTypeOf() should return, the rest is routine.
But as I said - I'm not sure myself how to approach this - if we should create classes like TemplateUnionLowerBoundType etc. which involves a lot of duplication, or if we should make current classes configurable.
I already read those. I have even developed some minor extensions for our need. If there is no other material I will try to dig more in source code itself.
It already has method getBound(): Type (with meaing upper bound). I need to check usage of this method in source code more (which I will do, when I find some time), but:
One way to "change" this could be changing return type of this, to return something like Bound($type: Type, $direction BoundDirection). This would be a BC for extensions and so on. BUT we could perserve getBound and add getBoundType(): Bound (or some better name) and mark getBound as deprecated.
Another way would be renaming getBound to getUpperBound and adding getLowerBound. This would be BC for extensions and so on. BUT we could perserve getBound and add getUpperBound(): Type, getLowerBound(): Type along side, where getBound would be "semantic" alias to getUpperBound and marked as deprecated.
Client code (usage of getBound) would need to change of course to support upper bound as well.
BTW: there are some other design choices.
In fact, there are 3 types of bounds (check hack https://docs.hhvm.com/hack/generics/type-constraints). There is also "exact" bound (conceptual equivalent to invariant), which can be useful as well. If upper bound will be implemented, I would take in consideration to implement exact bound as well.
Another design choice is, that some languages supports restricting type by both lower and upper bound together. This I would not do, because there is really little use for this. Also it would affect syntax design as well (you would need to design the syntax to support both extends and of along side. Also you would need to "solve" what range of classes matches the template (and how those types overlap). Supporting only one type of bound leads to 1. Supporting combinations leads to 2.
I will check the source code and revisit this issue more.
Edit:
I have noticed, that bound is also input parameter of some types. There should go information of bound type as well, which will probably be minor BC. This is probably what you meant with "make current classes configurable", which should be the way IMHO. Making extra classes for each bound type would be wrong design IMHO ("parallel inheritance" problem https://refactoring.guru/smells/parallel-inheritance-hierarchies)
I like your thinking about this. A new method should really be used for this so that the usage is fool-proof.
If we only add getBoundDirection() and leave getBound() untouched, it's basically a BC break - people currently don't know they should check the direction along with the bound.
We should force users to start using a new method so that they check bound + direction simultaneously. And the old one should be deprecated.
But I don't know if getUpperBound + getLowerBound should both be added, it implies that @template can be clamped from both sides at the same time, and you said you don't want to support that.
Maybe something like getBoundSettings(): BoundSettings would be nice.
Feel free to start with a PR to phpstan/phpdoc-parser - it'd be a good starting point to also have discussion with Psalm maintainters to see if they're interested in this, and to agree upon a syntax.
I'll throw in my two cents to revive the discussion: I wouldn't mind having both getUpperBound and getLowerBound. Conceptually, the template type is always clamped from both sides: even an unbounded type has implicit bounds of [mixed, never]. Adding an explicit upper or lower bound simply changes one side of the pair – an upper bound U turns the bounds into [U, never], and a lower bound L into [mixed, L].
This would force code to always take both bounds into account, without the need for special-casing each direction. Most importantly, it would be forward-compatible and allow PHPStan to rollout the support gradually: if we later found consensus on the syntax and chose to support specifying both bounds in the userland, it would just create explicit bounds [U, L], and an exact bound E could simply be translated into [E, E].
Activity
phpstan-bot commentedon Jun 16, 2021
@jost125 After the latest commit in dev-master, PHPStan now reports different result with your code snippet:
Full report
Parameter #2 $b of method Monoid<B>::concat() expects B, A given.phpstan-bot commentedon Sep 23, 2021
@jost125 After the latest commit in dev-master, PHPStan now reports different result with your code snippet:
Full report
Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40Run PHPStan with --debug option and post the stack trace to:https://github.com/phpstan/phpstan/issues/new?template=Bug_report.mdmarcospassos commentedon Dec 1, 2021
This would be really useful
jost125 commentedon Aug 17, 2022
@ondrejmirtes any news about this issue?
ondrejmirtes commentedon Aug 17, 2022
This sounds a lot like
@template-contravariant, is this request same as this, or is it something different? #3960There's an open WIP PR for that: phpstan/phpstan-src#1492
There's also some work underway for call-site variance which is gonna be cool too: phpstan/phpdoc-parser#138
jost125 commentedon Aug 17, 2022
Template variance and bounds are different things, but they are loosely related. You indeed need to support both type of bounds (upper and lower) to allow some patterns in combination of template variances.
In my case I would like to have covariant collection, which support immutable additions (which means use template type in contravariant position). Which you cannot do off course. But what you CAN do is to introduce new invariant type lower bounded by original type, which bypasses it. (Thats how scala does it in standard library https://www.scala-lang.org/api/2.13.3/scala/collection/immutable/List.html#::[B%3E:A](elem:B):List[B] )
The #3960 indeed looks similar. It is slightly different case, but if the author could use lower bound, he would be able to specify type for his use case. The term "Contravariant bound" is I believe incorrect (I have never heard it called this way, probably author just meant lower bound).
BTW I tought that template-contravariant is already supported to some extent, not?
As of call-site variance. Projections are really nice, if implemented, I will be really happy. They are slightly different things from bounds though.
ondrejmirtes commentedon Aug 20, 2022
These examples are really mind-bending for me :) For example, I only understood type projections after Jiří Pudil explained it to me in person. Because before PHPStan, I never truly worked with generics and have no experience with them from other languages, I learnt them along the way, and I have no concept or need for these advanced features :)
I'm not sure how to implement it currently though. Each bound type (like
@template T of int) has a customTemplate*Typeclass with the current upper bound meaning. To invert the relationship, we'd probably have to either make these classes configurable (they use a common trait anyway) to invert the meaning of the bound.jost125 commentedon Aug 22, 2022
I was considering sending PR into phpstan myself, but to be honest, phpstan is quite complex project and there is quite high barrier. Is there some kind of "internal documentation" or some kind of "concepts" described somewhere publicly?
ondrejmirtes commentedon Aug 22, 2022
You could start here https://phpstan.org/developing-extensions/core-concepts - writing extensions gives you a good idea how the internals work.
This article is especially relevant for you https://phpstan.org/developing-extensions/type-system. The hardest part is to figure out what
Type::accepts()andType::isSuperTypeOf()should return, the rest is routine.But as I said - I'm not sure myself how to approach this - if we should create classes like
TemplateUnionLowerBoundTypeetc. which involves a lot of duplication, or if we should make current classes configurable.jost125 commentedon Aug 22, 2022
I already read those. I have even developed some minor extensions for our need. If there is no other material I will try to dig more in source code itself.
As a super quick check, top of the mind, I think it should be part of iterface here: https://apiref.phpstan.org/1.8.x/PHPStan.Type.Generic.TemplateType.html
It already has method
getBound(): Type(with meaing upper bound). I need to check usage of this method in source code more (which I will do, when I find some time), but:One way to "change" this could be changing return type of this, to return something like
Bound($type: Type, $direction BoundDirection). This would be a BC for extensions and so on. BUT we could perservegetBoundand addgetBoundType(): Bound(or some better name) and markgetBoundas deprecated.Another way would be renaming
getBoundtogetUpperBoundand addinggetLowerBound. This would be BC for extensions and so on. BUT we could perservegetBoundand addgetUpperBound(): Type,getLowerBound(): Typealong side, wheregetBoundwould be "semantic" alias togetUpperBoundand marked as deprecated.Client code (usage of
getBound) would need to change of course to support upper bound as well.BTW: there are some other design choices.
In fact, there are 3 types of bounds (check hack https://docs.hhvm.com/hack/generics/type-constraints). There is also "exact" bound (conceptual equivalent to invariant), which can be useful as well. If upper bound will be implemented, I would take in consideration to implement exact bound as well.
Another design choice is, that some languages supports restricting type by both lower and upper bound together. This I would not do, because there is really little use for this. Also it would affect syntax design as well (you would need to design the syntax to support both
extendsandofalong side. Also you would need to "solve" what range of classes matches the template (and how those types overlap). Supporting only one type of bound leads to 1. Supporting combinations leads to 2.I will check the source code and revisit this issue more.
Edit:
I have noticed, that bound is also input parameter of some types. There should go information of bound type as well, which will probably be minor BC. This is probably what you meant with "make current classes configurable", which should be the way IMHO. Making extra classes for each bound type would be wrong design IMHO ("parallel inheritance" problem https://refactoring.guru/smells/parallel-inheritance-hierarchies)
ondrejmirtes commentedon Aug 23, 2022
I like your thinking about this. A new method should really be used for this so that the usage is fool-proof.
If we only add
getBoundDirection()and leavegetBound()untouched, it's basically a BC break - people currently don't know they should check the direction along with the bound.We should force users to start using a new method so that they check bound + direction simultaneously. And the old one should be deprecated.
But I don't know if
getUpperBound+getLowerBoundshould both be added, it implies that@templatecan be clamped from both sides at the same time, and you said you don't want to support that.Maybe something like
getBoundSettings(): BoundSettingswould be nice.Feel free to start with a PR to phpstan/phpdoc-parser - it'd be a good starting point to also have discussion with Psalm maintainters to see if they're interested in this, and to agree upon a syntax.
jiripudil commentedon Apr 30, 2023
I'll throw in my two cents to revive the discussion: I wouldn't mind having both
getUpperBoundandgetLowerBound. Conceptually, the template type is always clamped from both sides: even an unbounded type has implicit bounds of [mixed, never]. Adding an explicit upper or lower bound simply changes one side of the pair – an upper bound U turns the bounds into [U, never], and a lower bound L into [mixed, L].This would force code to always take both bounds into account, without the need for special-casing each direction. Most importantly, it would be forward-compatible and allow PHPStan to rollout the support gradually: if we later found consensus on the syntax and chose to support specifying both bounds in the userland, it would just create explicit bounds [U, L], and an exact bound E could simply be translated into [E, E].
Updated PHPStan to commit ea53224daa0bd61a4a5bb17175a211fd0b46ebd5
Updated PHPStan to commit 63cd4763d3203218c6af61bcf0071a66528162fc