Use null|type instead of ?type in PHPDocs

[ntzm]

Keeps it consistent throughout the project

[GrahamCampbell]

👍

[keradus]

do the reverse. by making new fixer

ker@dus:~/github/PHP-CS-Fixer λ grep -r " ?" src tests | wc -l
790
ker@dus:~/github/PHP-CS-Fixer λ grep -r " null|" src tests | wc -l
673

[julienfalque]

Searching for " ?" gives a lot of false positive. I used "@[a-z]+ \?" (with PhpStorm) which gave 18 matches on master, including 15 test cases so 3 actual usages.

[keradus]

still, ?foo is preferred type. as it's real type in php, null|foo is not, as it cannot be typehinted

[ntzm]

I thought the project was following PSR-5? Which says it should be x|null not ?x

[keradus]

PSR-5 allows any type that is proper in PHP.
?foo is proper in PHP since 7.1

[julienfalque]

Strictly speaking, I'm not sure that ?foo is a valid syntax according to PSR-5, especially PSR-5 types' ABNF definition.

Also, using ? may bring weird cases when a value can be of several different types in addition to null. Imagine a function with a parameter that can an instance of Foo, an instance of Bar or null. Would you write it ?Foo|Bar? or maybe ?Foo|?Bar? IMO Foo|Bar|null is better in such case.

[ntzm]

Also

When the "Type" consists of multiple types then these MUST be separated with the vertical bar sign (|). Any interpreter supporting this specification MUST recognize this and split the "Type" before evaluating.
For example: @return int|null

[keradus]

Foo|Bar|null is bullshit, I beg you, don't ever use it. be strict about input/output.

Again, PHP 7.1. If you could use sth as a type, it's great, you can put it to docblock directly. if not, you need to make such monsters like Foo|Bar|null.

yet.. for nullable type, ?Foo is valid as-is from PHP point of view.

When the "Type" consists of multiple types then these MUST be separated with the vertical bar sign (|)

exactly, X|Y, eg Foo|null, is type alternation that cannot be typehinted in php function prototype.

Yet, ?X, eg ?Foo, is proper typehint for php function prototype (since 7.1). it is not multiple types, it's single type.

Psr5 happened before PHP 7.1, so it doesn't contain example with ?X.

[SpacePossum]

just my POV:
Foo|Bar|null feels like something is off, either Foo and Bar should be interfaced to the same FooBarInterface or the type hint should be dropped because it doesn't add much.
That said, I really like the intend of this PR to unify how we should type hint and check if new fixers could be in order following PHP7.1.

[keradus]

@SpacePossum , our peacemaker

[ntzm]

So what's the status on this? A huge majority of the PHPDocs in this project have null|x instead of ?x. PSR-5 obviously doesn't say anything about it due to the last update being before 7.1. I think the best course of action is to normalise all usages to null|x for now, and (if|when) PSR-5 is updated to clarify, a more informed decision can be made

[SpacePossum]

Makes sense, we never update our CS if we don't have rules/fixers for it.
So I'm 👍 to proceed with this PR and start a new one to create a fixer that takes care of null|X to ?X (and we could discuss null|X|Y fixing there)

[keradus]

and (if|when) PSR-5 is updated

It won't gonna happen. PSR-5 is abandoned.

null|X|Y shall not be touch.

[keradus]

Thank you @ntzm.

[keradus]

If anyone will end up in this PR:
Please do not make any furtner normalization of ?Foo vs null|Foo manually in our repo, go to #3562 instead ;)