Prototype for repository findBy type checking.

[AndrolGenhald]

I've wanted to have ObjectRepository::findBy type check the criteria for a while now, and I finally got around to working out a prototype. There are still several major problems:

1. Psalm doesn't support this use case very well, RepositoryParamsProviderClassPopulator is a workaround that severely abuses the api to allow providing return types for inherited methods.
2. Psalm doesn't give any info about the object the method is being called on. I modified Psalm locally to pass the MethodCall statement and it works fine.
3. Doctrine really doesn't play nice with this either. Most of the Doctrine api requires you to have an EntityManager, which requires a database connection, which I would like to avoid. Even when creating and populating the ClassMetadata directly it uses the Reflection api, which means everything being analyzed has to be autoloadable.
4. Due to 3, I have no idea how to go about properly unit testing this.

All that being said, I tested it on a Symfony project and it worked quite well. Unfortunately I'm not really sure where to go from here.

[orklah]

Do you have a snippet that shows what it is supposed to resolve? there's a lot I didn't understand in your message!

[AndrolGenhald]

Sorry, I seem to habitually underexplain what I'm trying to do 😕 it just makes so much sense when you're the one that did it 😛

Say you've got an entity with a repository (written on the fly so expect mistakes):

/** @extends EntityRepository<Foo> */
class FooRepsitory extends EntityRepository {}

/** @ORM\Entity(repositoryClass="FooRepository") */
class Foo
{
    /** @ORM\Column(type="integer") */
    private int $bar;

    /** @ORM\Column(type="string") */
    private string $baz;

    private bool $foobar;
}

This makes it so that findBy is properly type-checked as FooRepository::findBy(array{bar?: int, baz?: string}, array{bar?: "asc"|"ASC"|"desc"|"DESC", baz?: "asc"|"ASC"|"desc"|"DESC"}, int<0,max>, int<0,max>). If you try to call findBy(["bat" => 2, "foobar" => false]) (typo and non-column) it will warn you about the incorrect type.

After thinking on it for a bit, I'm wondering if it would actually be better to just parse the doctrine annotations ourselves and just not support yaml or xml mappings, that way the classes wouldn't need to be loaded for reflection, and we really only care about Column and the [One/Many]To[One/Many] annotations.

[orklah]

Sorry, I seem to habitually underexplain what I'm trying to do

don't worry, it seems universal. I have a hard time on a lot of PRs in psalm's repo!

Ok, I get the goal now :)

Seems cool! I also think that parsing xml/yaml config would be tedious. Now the question is whether support phpdoc annotations or only native annotations I suppose?

[AndrolGenhald]

I also think that parsing xml/yaml config would be tedious.

Well, I was originally hoping to just have Doctrine do everything for us, but most of their api requires an EntityManager so that's out. I'm also not sure how we would get the configuration for it, we would probably have to add configs to the plugin, so yeah, I'm thinking instead of having Doctrine parse everything and grabbing the fields from there it would be better to just parse both docblocks and annotations ourselves.

I remembered I wanted to do this because of vimeo/psalm#7445, when that lands this will be even more useful. Given that most entities have a decent number of columns, without that improved message it's much harder to understand what's wrong (but it's still better than not knowing at least).