Add psalm assertions #282
Add psalm assertions #282
Conversation
| @@ -1834,6 +1920,8 @@ public static function e164($value, $message = null, $propertyPath = null) | |||
| * @param string|null $message | |||
| * @param string|null $propertyPath | |||
| * | |||
| * @psalm-assert non-empty-countable $value | |||
There was a problem hiding this comment.
This gives me
PHP Fatal error: Uncaught Psalm\Exception\TypeParseTreeException: no hyphens allowed in projectdir/vendor/vimeo/psalm/src/Psalm/Type/Atomic.php:187
Stack trace:
#0 projectdir/vendor/vimeo/psalm/src/Psalm/Type.php(129): Psalm\Type\Atomic::create('Assert\\non-empt...', NULL, Array)
#1 projectdir/vendor/vimeo/psalm/src/Psalm/Type.php(102): Psalm\Type::parseTokens(Array, NULL, Array)
#2 projectdir/vendor/vimeo/psalm/src/Psalm/Type/Reconciler.php(1123): Psalm\Type::parseString('Assert\\non-empt...', NULL, Array)
#3 projectdir/vendor/vimeo/psalm/src/Psalm/Type/Reconciler.php(205): Psalm\Type\Reconciler::reconcileTypes('Assert\\non-empt...', Object(Psalm\Type\Union), '$value', Object(Psalm\Internal\Analyzer\StatementsAnalyzer), false, Array, Object(Psalm\CodeLocation), Array, 0)
#4 projectdir/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer. in projectdir/vendor/vimeo/psalm/src/Psalm/Type/Atomic.php on line 187
There was a problem hiding this comment.
Sorry - fixed in 7a14e87 - will release tonight
|
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)? |
The annotations allow Psalm to understand what effect these assertions when it analyses these calls during type inference.
A mild inconvenience for people who rely on this library and Psalm.
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. |
|
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. |
|
👍 These annotations could be really useful for us. We use Assert extensively and we added psalm recently to our pipeline. |
|
Can the merge conflict be resolved (rebasing onto the latest master too please). |
|
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. |
|
Checking again today. Is there anything we can do to have this PR merged? |
|
@rquadling this branch does not seem to have conflicts with the master. Could you merge it? |
|
It now have a conflict, do you mind rebasing it? |
|
Merged in #305 |
These allow anyone using Psalm (or any other tool that understands its annotations) to understand the effect of
beberlei/assertcalls in their code.Similar to sebastianbergmann/phpunit#3708