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

Add psalm template support to several types #8289

Merged
merged 1 commit into from Oct 17, 2020

Conversation

orklah
Copy link
Contributor

@orklah orklah commented Sep 30, 2020

Hi,

This should fix #8283

I was suprised not to find all the method subbed in https://github.com/weirdan/doctrine-psalm-plugin/blob/master/stubs/EntityManagerInterface.phpstub in the EntityManagerInterface. Turns out they're defined in the parent class which is part of Doctrine\Persistence (The version installed via composer seem to already have the psalm's annotation though)

CS will probably fail. I'll fix it soon

@orklah
Copy link
Contributor Author

orklah commented Sep 30, 2020

Note that a previous commit in 2.7 made some changes to attribute names:
79cdcde#diff-cfa931dd1195fd8c5e36f3706b028dbaL391

This will have to be fixed while merging as Psalm's annotation will have the old param name. I'll gladly fix it myself if needed after the merge is performed.

@orklah
Copy link
Contributor Author

orklah commented Sep 30, 2020

I'm not sure what's up with CI...

@greg0ire
Copy link
Member

Travis is slow tonight it seems.

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PhpBench breaks in Travis are not related to the changes of this PR.

@greg0ire
Copy link
Member

greg0ire commented Oct 17, 2020

The PhpBench breaks in Travis are not related to the changes of this PR.

I think I fixed it in the past (#8252) but the merge-base of 2.7 and 2.8.x dates from August 😅

@greg0ire
Copy link
Member

This should fix #8283

If this is a fix, why are you targeting 2.8.x?

@greg0ire
Copy link
Member

#8311 should fix the build issue

@orklah
Copy link
Contributor Author

orklah commented Oct 17, 2020

This should fix #8283

If this is a fix, why are you targeting 2.8.x?

The issue #8283 mentioned the 2.8 branche, so I did as asked.

I can probably rebase if it's preferable in 2.7

@greg0ire
Copy link
Member

Ah sorry, I was confused by the wording. In that sort of case, I'd use "This should close #8283", because it is a feature request. Anyway, I fixed the build, let's restart it here.

@greg0ire greg0ire closed this Oct 17, 2020
@greg0ire greg0ire reopened this Oct 17, 2020
@greg0ire greg0ire changed the title templates psalm Add psalm template support to several types Oct 17, 2020
@greg0ire greg0ire merged commit f1219f1 into doctrine:2.8.x Oct 17, 2020
@greg0ire
Copy link
Member

Thanks @orklah !

@orklah
Copy link
Contributor Author

orklah commented Oct 17, 2020

Thanks :)

wouterj added a commit to wouterj/foundry that referenced this pull request Dec 8, 2020
Doctrine has added the required Psalm annotations since 2.8:
doctrine/orm#8289
wouterj added a commit to wouterj/foundry that referenced this pull request Dec 8, 2020
Doctrine has added the required Psalm annotations since 2.8:
doctrine/orm#8289
kbond added a commit to zenstruck/foundry that referenced this pull request Dec 9, 2020
* [minor] re-enable psalm

* Removed WeirdanDoctrinePsalmPlugin

Doctrine has added the required Psalm annotations since 2.8:
doctrine/orm#8289

* Added function-scope template for proxyResult()

Nothing actually guarantees that a TProxiedObject is provided. This local
template var somehow fixed Psalm's understanding of this method.

* Fixed CS

Co-authored-by: Kevin Bond <kevinbond@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants