psalm-plugin-doctrine icon indicating copy to clipboard operation
psalm-plugin-doctrine copied to clipboard

Development plans

Open weirdan opened this issue 3 years ago • 17 comments

TL;DR

This project is looking for maintainers.

Current state

This package is currently outdated and supports neither newer Psalm versions (starting from v4) nor newer Doctrine versions (there should be some, but I haven't checked). I don't currently use Doctrine, so it doesn't scratch my itch anymore. Rather it has become a burden. I recognize the need for this package is still there though.

Plans

I have reasonable hopes to get tests green with the next Psalm release and release a version compatible with Psalm 4. Tests are very important to me, because it's my only way to see if things work as I don't use Doctrine myself.

After that I'd like to negotiate with Psalm's author, @muglug, to get this package under Psalm's umbrella GH org, similar to how some other plugins are managed. There I'll try to get the plugin updated to support new Doctrine versions, one last time, while simultaneously looking for people willing to maintain this. Once the package is in good shape, I will likely step down as a maintainer. I haven't fully decided yet whether I stop maintenance should I fail to find a co-maintainer, but this is a possibility. If I decide to continue, my efforts will be at the level of merging occasional PRs, but not more.

weirdan avatar Dec 07 '20 00:12 weirdan

Oh definitely happy to have it become a psalm-specific plugin. Also there are already a lot of annotations in newer doctrine code (e.g. collections) that make the plugin slightly less crucial

muglug avatar Dec 07 '20 01:12 muglug

FYI: doctrine/orm now has native template annotations since 2.8: https://github.com/doctrine/orm/pull/8289

However, there could be a flurry of new issues coming from new major versions of doctrine, starting from doctrine/dbal V3.0.0 a few weeks ago: https://github.com/doctrine/dbal/releases/tag/3.0.0. doctrine/orm should follow at some point.

I don't know how much has changed/will change in APIs but it may impact stubs.

I wonder how feasible it would be to create a new repo with only relevant stubs for newer versions of doctrine and do some Composer magic to make it choose the right repo depending on user needs

orklah avatar Dec 07 '20 06:12 orklah

Maybe it is worth asking the Doctrine team whether they want to maintain the required annotations in their own code (or help maintaining this stub package). They seem to be quite open in integrating with Psalm in their code base.

cc @greg0ire (as you're the one that merged the psalm ORM PR)

wouterj avatar Dec 08 '20 14:12 wouterj

We are super open to integrating Psalm in our codebase indeed: not only does it help our users catching bugs without installing an extra package, it helps us catch bugs in our codebase too!

greg0ire avatar Dec 08 '20 15:12 greg0ire

However, there could be a flurry of new issues coming from new major versions of doctrine, starting from doctrine/dbal V3.0.0 a few weeks ago: doctrine/[email protected] (release). doctrine/orm should follow at some point.

From what I asked the Doctrine maintainers, doctrine/orm 3.0 is not comming any time soon, however they're planning to make doctrine/orm 2.10 or 2.11 compatible with doctrine/dbal 3.x.

enumag avatar Dec 15 '20 14:12 enumag

We are super open to integrating Psalm in our codebase indeed: not only does it help our users catching bugs without installing an extra package, it helps us catch bugs in our codebase too!

Doctrine already integrate:

  • ArrayCollection
  • Collection
  • EntityManager
  • EntityRepository
  • Expr
  • ObjectManager
  • ObjectRepository
  • Paginator

Missing integration in doctrine which are here:

  • https://github.com/doctrine/DoctrineBundle/blob/2.2.x/Repository/ServiceEntityRepository.php
  • https://github.com/doctrine/dbal/blob/2.12.x/lib/Doctrine/DBAL/Connection.php
  • https://github.com/doctrine/dbal/blob/2.12.x/lib/Doctrine/DBAL/ParameterType.php
  • https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/QueryBuilder.php
  • https://github.com/doctrine/persistence/blob/2.1.x/lib/Doctrine/Persistence/Mapping/ClassMetadata.php
  • https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
  • There is maybe something to do with EntityManagerInterface since getRepository should return an EntityRepository<T> but is returning an ObjectRepository<T> from the inherited phpdoc https://github.com/doctrine/persistence/blob/2.1.x/lib/Doctrine/Persistence/ObjectManager.php.

VincentLanglet avatar Jan 03 '21 23:01 VincentLanglet

oh, thanks for the listing. I added ClassMetadataInfo.php to an open PR I had in doctrine/orm.

I'm not sure about QueryBuilder.php because the stub don't match the real signature for those methods. I don't know why it was made like that

orklah avatar Jan 03 '21 23:01 orklah

I'm not sure about QueryBuilder.php because the stub don't match the real signature for those methods. I don't know why it was made like that

The psalm type

@psalm-type _WhereExpr=Expr\Base|Expr\Comparison|Expr\Func|string
@psalm-type _SelectExpr=Expr\Func|string

can still be used in the phpdoc. (I recommend to use @psalm-param for these)

VincentLanglet avatar Jan 03 '21 23:01 VincentLanglet

@orklah There also something to do with Expr

https://github.com/weirdan/doctrine-psalm-plugin/blob/master/stubs/Expr.phpstub#L7-L12 https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/Query/Expr.php#L54

andX($a, $b) is reporting an error without this plugin.

VincentLanglet avatar Jan 04 '21 12:01 VincentLanglet

Any decision about the futur of this repository ? :)

VincentLanglet avatar Apr 08 '21 23:04 VincentLanglet

Any update ? The repository is now psalm/psalm-plugin-doctrine, does this issue should be closed if psalm team take support of this ? Is it planned to rename the library on packagist ?

VincentLanglet avatar Jul 07 '21 13:07 VincentLanglet

For any reader in the future or those who don't follow these things, here's a statement from @muglug about the future of Psalm: https://muglug.medium.com/my-incredible-journey-with-php-df45d72ba2a5

This package has been moved to psalm's organisation, but there is no lead developer at the moment. Ironically, @weirdan has taken the bulk of the maintenance of Psalm itself so I suspect he will have even less time to maintain this repo now.

I'm not using Doctrine enough myself to engage in any big change here either.

However, Doctrine has embraced static analysis for some time, so annotation coverage is pretty good. Also there was some movement on doctrine/orm v3 recently: https://twitter.com/doctrineproject/status/1411734374308007945

Maybe a refreshed version of this plugin that only support doctrine/orm and doctrine/dbal starting from v3.0 each would be a lot lighter and a lot easier to maintain...

orklah avatar Jul 07 '21 19:07 orklah

However, Doctrine has embraced static analysis for some time, so annotation coverage is pretty good. Also there was some movement on doctrine/orm v3 recently: https://twitter.com/doctrineproject/status/1411734374308007945

Maybe a refreshed version of this plugin that only support doctrine/orm and doctrine/dbal starting from v3.0 each would be a lot lighter and a lot easier to maintain...

Indeed, phpdoc of recent version of doctrine are more and more precise. So I think less and less stub will be needed.

Some stub are still needed currently, for instance the QueryBuilder::select method (and some others) are using func_get_args but the phpdoc only allow one param. https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/QueryBuilder.php#L762

But I assume this library will still be needed in order to solve some issue like
https://github.com/psalm/psalm-plugin-doctrine/issues/77

VincentLanglet avatar Jul 07 '21 20:07 VincentLanglet

Thanks, @orklah, couldn't have put it better myself. I see Psalm itself as a more important project, and with the lack of time anyone can guess where my efforts will go.

does this issue should be closed if psalm team take support of this ?

Psalm team (as in 'people who work on Psalm itself, more or less regularly') is now entirely composed of volunteers, and there are way too few to take additional work. So, no, I don't think Psalm team would have resources to support this package anytime soon. So, as stated, this package needs somebody to step up as a maintainer.

weirdan avatar Jul 09 '21 08:07 weirdan

So if there's anyone up to the task, please contact me, either here on this issue, or over email (my address is public on the Github profile).

weirdan avatar Jul 09 '21 08:07 weirdan

Correct me if I'm wrong, but this package contains only stubs, right?

Given Doctrine already added lots of psalm annotations in their packages, I think it's worth migrating the remaining annotations to Doctrine's code as well?

wouterj avatar Jul 09 '21 08:07 wouterj

Correct me if I'm wrong, but this package contains only stubs, right?

Currently yes, but it can (and I hope it will) contains more than that. For instance I'd like to solve the issue https://github.com/psalm/psalm-plugin-doctrine/issues/77 by introducing a MethodReturnTypeProviderInterface

VincentLanglet avatar Jul 09 '21 08:07 VincentLanglet