Type projections

[jiripudil]

This PR adds support for parsing type projections (phpstan/phpstan#3290) which I'd like to implement in PHPStan. Type projections are a way of declaring the variance of a type parameter at the site of use rather than at the site of its declaration (via @template-covariant). This allows you to have the type parameter in an incompatible position in the generic class, and only prevents you from using it where the projection is used.

Type projection can be created in the type argument position of a generic type by using either:

• a variance keyword ('covariant' or 'contravariant') followed by the projected type, which denotes an out- or in-projection, respectively,
• an asterisk, which denotes a star projection, or

/**
 * @param Box<covariant Animal> $covariant
 * @param Box<contravariant Cat> $contravariant
 * @param Box<*> $star
 */

The choice of the variance keywords favours consistency: while Kotlin uses in and out which might arguably be more intent-revealing, PHPStan already uses @template-covariant in a similar position, so I say let's stick with it.

One thing I'm not sure about is whether Foo<covariant> should throw a parse error (as currently implemented), or whether it should create an IdentifierTypeNode('covariant'). The former is potentially a BC break if somebody has a class named covariant in their code, and however unlikely it seems, you can always break somebody's build 🤔

[ondrejmirtes]

/cc @VincentLanglet @hrach who were interested in this in the past.

Also /cc @rvanvelzen @JanTvrdik for an opinion on the syntax :)

[JanTvrdik]

I would much more prefer the in / out syntax used by Kotlin and TypeScript. It's both shorter and easier to understand.

[ondrejmirtes]

The TL; DR is that @param Collection<covariant Animal> will allow you to pass Collection<Dog> there even in case of @template T (and not @template-covariant). But you won't be able to call methods that have T in the parameter position.

More info about current behaviour here: https://phpstan.org/blog/whats-up-with-template-covariant

Also /cc @arnaud-lb for an opinion :)

[ondrejmirtes]

I don't understand the in/out syntax, I don't know which one is which. Any mnemonic help for that?

[hrach]

My mind-dump (aka explanation) here https://hrach.dev/posts/variance/ - star projection is syntax sugar for covariant use-site type projection.

[JanTvrdik]

Any mnemonic help for that?

in is for parameters (inputs, writing "into" collection) and out is for return types (outputs, reading "out of" collection).

[VincentLanglet]

/cc @VincentLanglet @hrach who were interested in this in the past.

The TL; DR is that @param Collection<covariant Animal> will allow you to pass Collection<Dog> there even in case of @template T (and not @template-covariant). But you won't be able to call methods that have T in the parameter position.

More info about current behaviour here: https://phpstan.org/blog/whats-up-with-template-covariant

Currently:

• https://phpstan.org/r/d496e8d3-2aac-4a56-b88b-af37a9332bd1 reports an error since the template is not declared
• https://phpstan.org/r/8ce15174-c2c5-46d5-9bec-36d1b44707d8 reports an error because of the covariance

I'm still kinda interested in this feature ; but so far I found an hacky solution:

• https://phpstan.org/r/a859d377-52ba-46a1-93f0-914469d35607 It's maybe stronger than the solution proposed by this PR since
• https://phpstan.org/r/30b20739-5d6b-4deb-913a-b25b10592645 is reported as an error
• but https://phpstan.org/r/6e829e83-21bb-4bfe-a310-aba9862ed3b5 is fully working (and I'm not sure this PR will support this) And it works with psalm https://psalm.dev/r/c566a5d99a

I like the syntax

@param Box<*>

Since it basically says "I accept any Box" but I just add <*> to avoid a Phpstan level 6 error.

[jiripudil]

I am in favor of in/out too, but above all, I'd prefer the language to be consistent, so if we go that way, I think we should also support @template-out (and eventually @template-in). WDYT?

[VincentLanglet]

Might be worth to ping psalm maintainer @orklah to check if this feature can also be handled by psalm.

[hrach]

I think we should also support @template-out (and eventually @template-in). WDYT?

👍

[vhenzl]

I don't understand the in/out syntax, I don't know which one is which. Any mnemonic help for that?

TypeScript has a good explanation: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#optional-variance-annotations-for-type-parameters

in is for setters with contravariant parameters, out for getters with a covariant return type.

[ondrejmirtes]

What's most important to me is consistency. It's easy to explain "To solve this problem, you either need to write covariant at declaration or at call-site". It's much harder to grasp "you need to write covariant at declaration or in at call-site".

Sure, one day we can move on and use in/out instead everywhere but we'd have to do that in sync with Psalm maintainers and PhpStorm developers at minimum.

[VincentLanglet]

Does the following example will be valid:

	/**
	 * @param Collection<covariant mixed> $collection
	 */
	public function compute(Collection $collection): int
	{
		$item = $collection->get();
		if (null !== $item) {
		    $collection->add($item);
		}
		
		return $collection->count();
	}

? or it will only solve

	/**
	 * @param Collection<covariant mixed> $collection
	 */
	public function compute(Collection $collection): int
	{
		return $collection->count();
	}

? for the example https://phpstan.org/r/6e829e83-21bb-4bfe-a310-aba9862ed3b5

[ondrejmirtes]

You can't call $collection->add($item) on Collection<covariant mixed>.

[VincentLanglet]

You can't call $collection->add($item) on Collection<covariant mixed>.

ok, so it will be less permissive than my hacky solution

	/**
	 * @template T
	 * @param Collection<T> $collection
	 */
	public function compute(Collection $collection): int
	{
		$item = $collection->get();
		if (null !== $item) {
		    $collection->add($item);
		}
		
		return $collection->count();
	}

but at least it will not be an hacky solution and solve lot of usecase.

Updating the blog article about generics will be useful IMHO for people like me which are not a lot familiar with covariance/contravariance. I currently don't know what will be the use case for Collection<contravariant mixed>

[ondrejmirtes]

I currently don't know what will be the use case for Collection<contravariant mixed>

I don't either, there's no @template-contravariant yet...

[arnaud-lb]

I have a preference for out/in as it's easier to remember than covariant/contravariant and also shorter to type/read. But since we already use covariant/contravariant, that it's the way to go in my opinion.

One thing I'm not sure about is whether Foo<covariant> should throw a parse error (as currently implemented), or whether it should create an IdentifierTypeNode('covariant'). The former is potentially a BC break if somebody has a class named covariant in their code, and however unlikely it seems, you can always break somebody's build thinking

Are there any downsides for for the latter ?

[jiripudil]

ok, so it will be less permissive than my hacky solution

I don't think making the function itself generic is hacky, actually. See https://hrach.dev/posts/variance/ linked above, there's a section named 'Breaking the Contract' at the bottom with code very similar to yours which solves the problem exactly that way.

I don't either, there's no @template-contravariant yet...

Honestly, neither do I, all the examples you can find on the Internet look very artificial to me. Maybe we could limit type projections to covariance, for starters? Some of the logic might need to be implemented anyway because star projections somewhat act in both ways, but we could only support covariant projections on the parser level and then introduce contravariant projections together with @template-contravariant when someone with an actual use case requests them.

Are there any downsides for for the latter ?

Well, I think for 99.99 % of users, Foo<covariant> is by mistake, and parse error means failing early. But it fails anyway, and with a pretty informative error message too. So no, I don't see any downsides.

[ondrejmirtes]

@jiripudil I just watched your talk on generics on YouTube, and I’m really looking forward to this! I just checked the code again, I think there shouldn’t be class TypeProjectionNode implements TypeNode. We need to move this onto GenericTypeNode, and in some backward-compatible manner, like an extra property called typeVariances.

Thank you!

[staabm]

Is this said talk public? :-)

[mabar]

@staabm It is, but in Czech https://www.youtube.com/watch?v=-tOCEtrQPww

[jiripudil]

I just watched your talk on generics on YouTube, and I’m really looking forward to this! I just checked the code again, I think there shouldn’t be class TypeProjectionNode implements TypeNode. We need to move this onto GenericTypeNode, and in some backward-compatible manner, like an extra property called typeVariances.

Thanks! Yes, I've had that in the works for some time :) I've rebased this onto 1.9.x and changed the implementation to populate GenericTypeNode::$variances.

I think I've also managed to figure out that damned star projection – it's effectively a bivariant placeholder, so I'm treating it as such. I'm really happy about this because in phpstan-src, we'll be able to represent it the same way as here – add support for this new case of TemplateTypeVariance, and simply keep a list of use-site variances in GenericObjectType, as you've suggested :)

[ondrejmirtes]

Thank you very much!

BTW I can imagine at least 3-4 PRs in phpstan-src about this:

• @template-contravariant for completeness sake
• Basic support for this new syntax in the typesystemby reading the new node property, putting it into GenericObjectType etc.
• Doing all the stuff about limiting what method can be called based on call-site variance, this is gonna be some new logic in GenericObjectType
• Checking that the usage of type projection is sane and safe, checking declaration site variance against call-site variance etc. Checking that call-site variance isn't used in a bad place like @implements etc.