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

Psalm annotations #72

Open
Jean85 opened this issue Mar 5, 2021 · 3 comments · May be fixed by #74
Open

Psalm annotations #72

Jean85 opened this issue Mar 5, 2021 · 3 comments · May be fixed by #74

Comments

@Jean85
Copy link

Jean85 commented Mar 5, 2021

I'm currently using this project and Psalm. Obviously static analysis has some issues to understand what this mapper does, but I think that with a couple of annotations it would be fairly easy to explain what's happening type-wise.

Would you be interested in a PR that adds annotations for improving static analysis?
Would you prefer replacing the current annotations or just adding @psalm-* ones beside the existing ones?

@mark-gerarts
Copy link
Owner

Hi @Jean85, I don't have any experience with psalm myself, unfortunately. I'm definitely all for a PR that makes life easier for psalm users, but could you explain the difference between replacing the annotations and adding them? Are there any (dis)advantages between the two?

From a quick glance at the docs my understanding is that the @psalm- annotations are for specific psalm functionality (e.g. type syntax or info not currently possible in regular phpbloc). If that's the case then I lean towards keeping (and/or improving) the PHPdocs and adding @psalm tags where needed.

@Jean85
Copy link
Author

Jean85 commented Mar 12, 2021

Yes, you got it right: @psalm-* annotations are more specific and can be put alongside the normal one, so to not break tools (like IDEs) that do not understand them.

I think that the best way to make it work correctly would be to introduce Psalm into your codebase and CI, so to have a tool that checks the correctness of such annotations; are you open to a PR of such kind?

@mark-gerarts
Copy link
Owner

Yes absolutely! I've added PHPStan to the 2.0 branch, but that branch is still very much WIP. Psalm and PHPStan can perfectly coexist together, so feel free to open up a PR to the master branch. It'll be my problem to merge it in the 2.0 branch then.

@Jean85 Jean85 linked a pull request Mar 12, 2021 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants