Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Generics #1692

Closed
wants to merge 1 commit into from

Conversation

@ondrejmirtes
Copy link
Member

commented Dec 9, 2018

Thinking out loud about generics support which I would like to have as the main feature of 0.12. These are the use cases that come to mind:

  • Repository<T>::find() - returns T
  • EntityManager::find(T, id) - returns T (both are complements for certain types of dynamic return type extensions)
  • Callbacks:
    1. Validating that the callback parameters and return types are correct
    2. Inferring types when the callback typehint is missing (both are applicable for example to array_map)

Here are some write-ups from PHP and TypeScript of how generics are used there:

How to implement them in PHPStan:

  • GenericType and TemplateType are the cornerstones of generics support. GenericType represents EntityRepository<Foo> in phpDoc and TemplateType represents type placeholder in:
    1. @template T
    2. @param T $foo
    3. array<T>
    4. etc.
  • TemplateType can become part of types consisting of other types, like ArrayType, CallableType etc. Right now it's unknown what effect this has on the codebase, there's a lot of concrete instanceofs that would break with TemplateType.
  • GenericType and TemplateType are hinted as part of this PR.
  • @template T indicates a placeholder type identifier and can be bound to a class or to a function/method. It's the only new phpDoc tag that I would add in the first round.
  • We have to think about when the TemplateType should be resolved to a concrete type - when a method is called? When an object is created? It should probably be done through a method on Type - something like Type::resolveTemplateTypes().
  • What about analysis of classes that contain TemplateType? I incline towards that TemplateType should behave mostly like MixedType.
  • There's a duality that in some places, we have types indexed from zero - like Collection<IntegerType, ObjectType(Article)> coming from TypeNodeResolver, and in some other places, they will be indexed by their TemplateType identifier. Can we unify this? TypeNodeResolver has to be able to represent invalid data so we can write rules for them. (like having Foo<A, B> when there's only one @template above Foo)
  • How are we going to declare templates for 3rd party code? I'm inclining towards config in phpstan.neon.
  • How are we going to declare templates for built-in PHP functions? Something similar to functionMap.php, or even modifying the current file?

Additionally, there will have to be some rules for wrong generics usage, like:

  • Foo<A, B> when there's only one @template above Foo (and also vice versa)
  • Will we allow usage of a templated class in phpDoc without defining the type placeholders? Should they fall back to mixed?
  • Template types with the same identifiers as existing classes/interfaces
  • Using undefined template type identifiers - these will probably result in "unknown classes" error.
  • Using the same type template identifier above a class and a class method.
  • If a class has @template T, then T must occur in constructor parameters. If it's there multiple times, the passed arguments must have the same T.
  • If a function/method has @template T, then T must occur in its parameters. If it's there multiple times, the passed arguments must have the same T.

What I consider out of scope for the first iteration of this feature:

  • Thinking about inheritance at all - we will not be able to do extends Foo<Bar>. Let's implement this proposed first version, gain benefits and feedback and then decide what should be done.
  • Generic constraints like Foo<T extends Bar>. Again, not needed to gain benefits from generics in the first version.
wip
@JanTvrdik

This comment has been minimized.

Copy link
Member

commented Dec 9, 2018

@rainbow-alex

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2018

I hope you don't mind if I just jump in out of the blue with some thoughts as well:

Another use case would be Symfony's constraint validators. Each validator implements Validator<SomeConstraint>.

Dlang templates are an incredibly powerful take on generics, definitely worth a look. Not all of it can possibly be applied to PHP, but still.

Regarding GenericType: a key thing about generics is that different instantiations of a generic class are (generally) not interchangeable. Collection<Foo> and Collection<Bar> are distinct types (even when Foo extends Bar or vice versa). This also becomes the case for phpstan's Type. A generic type is not (generally) interchangeable with an instantiated one. So I believe if you somehow made Type itself generic you could statically verify its instances are always used correctly. At any point in the code you would also definitely know whether you are handling a generic or instantiated type:

interface Type<X> { ... }

function instantiate(Type<Generic> $type): Type<Instantiated> { ... }

// code that only works with one kind of Type:
class Scope {
    public function getType(Expr $node): Type<Instantiated> { ... }
    ...
}

// code that works for both:
class TypeUtils {
    public function flattenTypes<X>(Type<X> $type): Type<X>
    ...
}
...

What is X? You could make some placeholder classes here to use only for the sake of typing. Or you could allow generic parameters to be values as well as types, as in Dlang.

Finally, I think leaving out inheritance could be very limiting. Except for callbacks the use cases that come to mind all seem to involve some inheritance or interfaces. Collection<Item> is all most people think of when they hear 'generics'...

Good luck tackling this. PHPstan is a great tool and generics will make it even better!

@arnaud-lb

This comment has been minimized.

Copy link
Member

commented Dec 10, 2018

What about analysis of classes that contain TemplateType? I incline towards that TemplateType should behave mostly like MixedType.

As proposed in #1700, a @template T annotation could also specify the base type of T. This would allow TemplateType to behave like the specified base type during the analysis of the class:

/** @template Foo T */
class Repository
{
    /** @var Foo[] */
    private $entities;

    /** @param T $object */
    public function add(string $id, Foo $object): void
    {
        $this->entities[$id] = $object;
     }
@muglug

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

Also worth thinking about @template-typeof T $foo to bind params:

https://github.com/weirdan/doctrine-psalm-plugin/blob/30a00e61f53b168eb568854d44753dcc981f9c3f/stubs/EntityManager.php#L6-L12

Or you could use @param class<T> $foo instead, as class is a reserved word so won‘t clash with anything. Psalm doesn't support the latter (yet), but thinking about it I prefer it to @template-typeof.

@muglug

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

Also if you have any questions/challenges, feel free to ask - https://github.com/vimeo/psalm/blob/master/tests/TemplateTest.php has a whole bunch of tests around this, including for functionality Phan supports.

@JanTvrdik

This comment has been minimized.

Copy link
Member

commented Dec 12, 2018

class<T> is definitely better; or perhaps sth like T::class (which looks more intuitive to me, but requires grammar change)

    /**
     * @template T
     * @param    T::class $entityName
     * @return   EntityRepository<T>
     */
    public function getRepository(string $entityName) {}
@muglug

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

When analysing function/method calls, Psalm first collects templated types from function params in https://github.com/vimeo/psalm/search?q=replaceTemplateTypesWithStandins, then replaces types for the return value: https://github.com/vimeo/psalm/search?q=replaceTemplateTypesWithArgTypes (there's an exception for by-ref values which does the replacement with arg types earlier).

The standins (in the above code) are taken from @template T as Foo - so if you have @param T Psalm will treat that as @param Foo right after the replacement, so calling with Bar would still fail.

In Psalm there's a direct mapping of class @template declarations to the order they appear in the resultant generic object - so given this:

/**
 * @template T1
 * @template T2
 */
class Foo {
  /**
   * @param T2 $a
   * @param T1 $b
   */
   public function __construct($a, $b) {...}
}

new Foo(new A(), new B()) would return Foo<B, A>

That feels intuitively ok to me, but you might feel differently.

@muglug

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

class<T> is definitely better; or perhaps sth like T::class (which looks more intuitive to me, but requires grammar change)

You might want to change the grammar anyway to support class constants in lieu of literal values e.g. https://getpsalm.org/r/079e4222d5

As an extra datapoint, Hack uses classname<T> but that obviously means that classname has to be a reserved word (which it is in Hack, but not PHP).

@muglug

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2018

@JanTvrdik T::class now implemented in Psalm: https://getpsalm.org/r/1d490435f2

@iluuu1994

This comment has been minimized.

Copy link
Member

commented Dec 13, 2018

@muglug That's awesome!

@marcospassos

This comment has been minimized.

Copy link

commented Feb 13, 2019

This is my most-wanted feature.

@ondrejmirtes ondrejmirtes force-pushed the master branch 2 times, most recently from 0994dc5 to fd5902c Mar 10, 2019

@arnaud-lb arnaud-lb referenced this pull request May 21, 2019
@ondrejmirtes

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

We're working on this with @arnaud-lb - the info here is mostly obsolete. Stay tuned!

@ondrejmirtes ondrejmirtes deleted the generics branch May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.