jsonmapper
jsonmapper copied to clipboard
Support shorter ?Type syntax for nullable types
I used the following syntax in my PHP class: /** @var ?int */
Which gave me the following error: Class '?int' not found
I changed it to the following, which does work: /** @var int|null */
It would be nice if JsonMapper supported both syntaxes. PhpStorm also supports this syntax. This merge request adds support for the shorter syntax.
This change obviously needs test cases.
However I am not entirely convinced this change should be merged. First of all there is a documented method how nullable types should be used, and that is type|null in PHPDoc, not ?type, as this aligns with the really old legacy PHPDocs long before any serious typing was introduced into PHP itself, as it reproduces what other languages did. PHP 7.1 added the ?type ability, and PHP 8.0 added generic union types where type|null would only be one of many combinations. So in this regard, PHP clearly moved into the type|null direction, even though I do not expect them to phase out the ?type syntax any time soon.
Having said that, this PR is about adding syntactic sugar for parsing PHPDoc annotations. Any enforced type declarations are already supported, i.e. you could use either public Class|null $foo; or public ?Class $foo instead of relying on non-enforcing comment lines.
The suspected number of users that would benefit from this change might be quite low.
This change obviously needs test cases.
I've added some test cases! Thank you for the suggestion. :+1:
However I am not entirely convinced this change should be merged. First of all there is a documented method how nullable types should be used, and that is
type|nullin PHPDoc, not?type, as this aligns with the really old legacy PHPDocs long before any serious typing was introduced into PHP itself, as it reproduces what other languages did. PHP 7.1 added the?typeability, and PHP 8.0 added generic union types wheretype|nullwould only be one of many combinations. So in this regard, PHP clearly moved into thetype|nulldirection, even though I do not expect them to phase out the?typesyntax any time soon.
I've searched for @var ? in all GitHub repositories, and I have found 30.8k results for PHP, so this syntax is clearly used and quite common.
Having said that, this PR is about adding syntactic sugar for parsing PHPDoc annotations. Any enforced type declarations are already supported, i.e. you could use either
public Class|null $foo;orpublic ?Class $fooinstead of relying on non-enforcing comment lines.
This is only possible on the newest PHP versions, not everyone is using those already.
The suspected number of users that would benefit from this change might be quite low.
I've created this pull request because I was personally impacted. I am sure there are more people impacted.
I see your argument as being valid. ?type syntax is to be used for explicit types in PHP 7.x, which this library supports, and indeed it feels a bit weird why the same syntax isn't supported as @var docblock (even though technically these two are completely different, the type declaration is parsed using Reflection based on PHP's syntax parsing, the docblock still is a string).
I wonder if the nullability would behave differently for the two more complex things: ?array and ?SomeObject, or even ?SomeObject[] (array where content can be null or SomeObject) and ?SomeObject[]|null (same as before, but instead of the array it can also be null - as absurd as that may sound). This library has some inconsistent checks for null values that are technically not allowed (see #233) that I like to wipe out, hence I am closely watching anything related to NULL getting changed.
I resolved the conflicts and renamed the test methods a bit to make them clearer.
Thank you for the patch!
Released with v4.5.0.