Add psalm assertions - Githubissues

beberlei / assert

Thin assertion library for use in libraries and business-model

Other

2.41k stars 188 forks source link

Add psalm assertions #282

Closed muglug closed 3 years ago

muglug commented 5 years ago

These allow anyone using Psalm (or any other tool that understands its annotations) to understand the effect of beberlei/assert calls in their code.

rquadling commented 5 years ago

What's the benefit these annotations? Who will maintain them when we add new assertions? What's the consequences of not adding them? What's the consequence/diagnostics of an incorrect one (say we edit the method signature or behaviour)?

muglug commented 5 years ago

What's the benefit these annotations?

The annotations allow Psalm to understand what effect these assertions when it analyses these calls during type inference.

What’s the consequence of not adding them?

A mild inconvenience for people who rely on this library and Psalm.

What's the consequence/diagnostics of an incorrect one (say we edit the method signature or behaviour)?

The few annotations here are relatively simple - if you were to change the assertion operation drastically (e.g. returning false vs throwing an error) the assertions would have to be changed, but that would constitute a very breaking change for this library anyway.

rquadling commented 5 years ago

Psalm vs PHPStan? Do any IDEs support Psalm?

muglug commented 5 years ago

Psalm vs PHPStan? Do any IDEs support Psalm?

Psalm runs a language server that is compatible with most IDEs: https://psalm.dev/docs/running_psalm/language_server/

There's also this ticket: https://youtrack.jetbrains.com/issue/WI-45248

PHPStan doesn't support any docblock assertion syntax, but it may in the future.

emilioastarita commented 4 years ago

:+1: These annotations could be really useful for us. We use Assert extensively and we added psalm recently to our pipeline.

rquadling commented 4 years ago

Can the merge conflict be resolved (rebasing onto the latest master too please).

orklah commented 4 years ago

It would be cool to have those assertions. I intended to use this package to validate my input to be fully typed in psalm but it doesn't help without those.

githoober commented 4 years ago

Checking again today. Is there anything we can do to have this PR merged?

githoober commented 4 years ago

@rquadling this branch does not seem to have conflicts with the master. Could you merge it?

magnetik commented 4 years ago

It now have a conflict, do you mind rebasing it?

Wirone commented 3 years ago

@muglug maybe you can close this issue since @beberlei rebased your work and merged it in #305 :slightly_smiling_face:

beberlei commented 3 years ago

Merged in #305