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

Development plans #84

Open
weirdan opened this issue Dec 7, 2020 · 17 comments
Open

Development plans #84

weirdan opened this issue Dec 7, 2020 · 17 comments

Comments

@weirdan
Copy link
Member

weirdan commented Dec 7, 2020

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 weirdan pinned this issue Dec 7, 2020
@muglug
Copy link
Member

muglug commented Dec 7, 2020

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

@orklah
Copy link
Collaborator

orklah commented Dec 7, 2020

FYI: doctrine/orm now has native template annotations since 2.8: doctrine/orm#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

@wouterj
Copy link

wouterj commented Dec 8, 2020

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)

@greg0ire
Copy link

greg0ire commented Dec 8, 2020

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!

@enumag
Copy link
Contributor

enumag commented Dec 15, 2020

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/dbal@3.0.0 (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.

@VincentLanglet
Copy link
Contributor

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:

@orklah
Copy link
Collaborator

orklah commented Jan 3, 2021

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

@VincentLanglet
Copy link
Contributor

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
Copy link
Contributor

@VincentLanglet
Copy link
Contributor

Any decision about the futur of this repository ? :)

@VincentLanglet
Copy link
Contributor

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 ?

@orklah
Copy link
Collaborator

orklah commented Jul 7, 2021

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...

@VincentLanglet
Copy link
Contributor

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
#77

@weirdan
Copy link
Member Author

weirdan commented Jul 9, 2021

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
Copy link
Member Author

weirdan commented Jul 9, 2021

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).

@wouterj
Copy link

wouterj commented Jul 9, 2021

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?

@VincentLanglet
Copy link
Contributor

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 #77 by introducing a MethodReturnTypeProviderInterface

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants