Use objects, not strings, for assertions#7410
Conversation
|
This looks really good! |
|
Hi @muglug ! Do you think you'll finish this soon? I'd really like having this in the alpha for Psalm 5! |
|
Yup, should have unit tests passing today |
284c23a to
a707303
Compare
a707303 to
52aeeb2
Compare
|
Sorry, my last merge on master broke CI. I'll fix that |
|
Could you add some details on UPDGRADE.md to explain that the old Assertion was renamed and other BC breaks introduced? |
|
every call to getAssertionString is now with $exact to true. Can we drop the param? |
|
This makes so many things looks a lot more clearer! Very cool! Thanks :) |
|
|
||
| public function __toString(): string | ||
| { | ||
| return '!non-empty'; |
There was a problem hiding this comment.
@muglug Is this a BC break regarding projects (including Assert tools) that used empty or !empty as a docblock assertion?
Is there a reason why you didn't kept it as empty?
There was a problem hiding this comment.
It can be “empty”, but that should have no impact on the parsing of assertions either way — do you have an example of failing code?
There was a problem hiding this comment.
Somehow it still works? https://psalm.dev/r/bb6d82a6a3 https://psalm.dev/r/90da8dba12
I didn't expect that. What is the system that parses assertion strings into Assertion objects?
There was a problem hiding this comment.
Thanks! I never had to tweak here before... Weird, I thought there would have been a direct link between internal assertion names and user assertions ones. But this answer my question! Thanks both!
It's a big addition, but this would make it slightly easier to track where assertions were being used in the codebase, and removes a lot of checks for
$assertion[0] === '!'etc.