Type projections, part 1: call-site variance in GenericObjectType

[jiripudil]

I think I've got most of type projections figured out and prototyped, so here we go!

As you suggested, I'm starting small by introducing call-site variance to GenericObjectType (and ClassReflection, transitively), and adding support for bivariance into TemplateTypeVariance.

Looking forward to your thoughts, and to finally having type projections in PHPStan 🤞

[phpstan-bot]

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

[ondrejmirtes]

I don't trust Rector with downgrading switch to match, please rewrite the code manually 😊

I'd love to review this PR but please make the build green first - there are some failures in integration projects but everything in phpstan-src should be green. Thanks!

[jiripudil]

Ok, rewritten. From what I've skimmed through, all failures in phpstan-src were related to that match, so here's me hoping that everything turns green now 😊

[ondrejmirtes]

Soo if I remember our conversation correctly, the idea is:

• @template T allows us to reference T in method parameters, but on the other hand we can't pass Collection<Dog> into Collection<Animal>
• @template-covariant T does not allow us to reference T in method parameters, but on the other hand we can pass Collection<Dog> into Collection<Animal>

The above points describe "declaration site variance".

Type projections are basically "call site variance", meaning:

• We want T in method parameters, meaning we have to write @template T.
• But we also want to pass Collection<Dog> into Collection<Animal>
• This can be safe if we don't call the method the method that has T in parameters
• So we can allow "call site covariance" - such typehinted object will not allow us to call the method that has T in parameters

Can you tell me if this PR allows me to do the second bullet list? And do you plan to send more PRs, and what they are gonna do?

Thank you :)

[jiripudil]

Sure :)

We want T in method parameters, meaning we have to write @template T.

Correct.

But we also want to pass Collection<Dog> into Collection<Animal>

To be more precise, we want to pass Collection<Dog> into Collection<covariant Animal>, or Collection<Animal> into Collection<contravariant Dog>, or any collection into Collection<*>.

/** @param Collection<covariant Animal> $collection */
function doSomething(Collection $collection): void
{
    // now this function accepts Collection<Dog>
    // at the cost of not being able to call $collection->add()
}

This PR allows – and tests – the first part ("now this function accepts Collection<Dog>").

And do you plan to send more PRs, and what they are gonna do?

I'm planning to send two more PRs:

• one with a set of Rules that make sure you don't use call-site variance incorrectly, such as in @extends, or trying to create a contravariant projection of a covariant template type;

• and another one implementing the actual restrictions that call-site variance imposes on the object (the "at the cost of not being able to call $collection->add()" part). However:

  This can be safe if we don't call the method the method that has T in parameters

  That's a valid way of looking at it, but one I find unnecessarily restrictive. I'd like to pursue a more benevolent direction – one that preserves type safety but doesn't tie the developer's hands too much. The idea is that when template types are resolved, the call-site variance should be taken into consideration (the TemplateTypeVarianceMap introduced in this PR will come in handy there) and change the resolved type into one of the template type's bounds.

  But more on that in the subsequent PR, I don't want to sidetrack the discussion in this one :)

I have the code more or less ready, so the PRs should come in quick succession. I just didn't want to overwhelm you like last time 😅

[ondrejmirtes]

I don't get the need of having these on ClassReflection. Declare-site variance is already taken care of as part of TemplateTag and ClassReflection::getTemplateTags(). Call-site variance is taken care of in GenericObjectType. Why do we need this in ClassReflection?

[jiripudil]

Lots of other code deals with ClassReflection rather than GenericObjectType. As of now, I think the main benefit is in the getDisplayName() so that the type displayed in error messages matches the type that's written in code:

<?php

/** @template T */
interface Collection
{
    function clear(int $flags): void;
}

/** @param Collection<*> $collection */
function foo(Collection $collection): void
{
    $collection->clear();
}

12    Method Collection<*>::clear() invoked with 0 parameters, 1 required.

Without the code in ClassReflection, it would say Collection<mixed> (or, e.g., Collection<Animal> instead of Collection<covariant Animal>) which I find rather misleading.

---

I'm not so sure about the other usages. Now that I think of it, I think it's not necessary in ObjectType, because there should always be invariance anyway. I haven't yet found out if the usage in StaticType is significant or not.

Anyway, even if the methods could become private for the sake of this PR, I would need public access to them in the subsequent PRs.

[ondrejmirtes]

Alright, I get it, but I'd prefer if the method name said something like "projection" or "call site" because right now it's a bit confusing. You can access declaration variance through TemplateTag::getVariance() too and getTemplateTypeVarianceMap on ClassReflection seems like the same thing.

[jiripudil]

Ok, that's fair :)

[ondrejmirtes]

Otherwise good to merge :)

[ondrejmirtes]

Alright, I get it, but I'd prefer if the method name said something like "projection" or "call site" because right now it's a bit confusing. You can access declaration variance through TemplateTag::getVariance() too and getTemplateTypeVarianceMap on ClassReflection seems like the same thing.

[ondrejmirtes]

Decided to put this on 1.10.x :) So that it can be released sooner once finished.

Thank you!