Lower bounds support

[jost125]

Support of lower bounds for generics / templates

Hi, first of all, thanks for all the great work, that has been done on generics lately.

To have generics more complete I suggest to introduce lower bounds, since phpstan already have a way to define upper bounds.

Lower bound behaves similarly to upper bound, only difference is, that it allows types in opposite direction (super types instead of sub types).

I propose syntax @template B super A with meaning B must be same type as A or super type of A.

Here is example, that I would like to support with phpstan:
https://phpstan.org/r/3fb61a92-b113-4d99-851c-25e406ea05cc

I am not good in making simplistic examples, so if anyone has better (simpler) example to show, please add it here.

Lower bounds are supported in other languages, which supports generics, e.g.:

• hack: https://docs.hhvm.com/hack/generics/type-constraints (also uses super keyword)
• scala: https://docs.scala-lang.org/tour/lower-type-bounds.html

Here is BTW link to our library, where we would use it https://github.com/bonami/collections e.g. here bonami/collections@a0bd909#diff-5d09c8cea366687b9742aed7f42996c7d73643bc15c4cae25f21e474c7881341R712 (we cannot define ArrayList template as covariant because of this limitation, which has other impacts)

[phpstan-bot]

@jost125 After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-No errors
+58: Parameter #2 $b of method Monoid<B>::concat() expects B, A given.

Full report

Line	Error
58	Parameter #2 $b of method Monoid<B>::concat() expects B, A given.

[phpstan-bot]

@jost125 After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-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

[marcospassos]

This would be really useful

[jost125]

@ondrejmirtes any news about this issue?

[ondrejmirtes]

This sounds a lot like @template-contravariant, is this request same as this, or is it something different? #3960

There'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]

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]

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.

[jost125]

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]

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() 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.

[jost125]

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:

1. 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.

2. 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)

[ondrejmirtes]

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.

[jiripudil]

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].