ext/reflection: make getDocComment() methods return empty string instead of false

[Girgias]

Returning false is just annoying for non-existent doc comment

Found this while seeing the impact of #18879 on Symfony's test suite.
And discussing with @nicolas-grekas it seems to just make more sense to fix PHP.

[DanielEScherzer]

Hmm, so I'm torn
On the one hand, indicating no comment with an empty string suggests that maybe there is a comment but it was just empty, while false makes it clear there was absolutely no comment
On the other hand, https://3v4l.org/fJBKT suggests that you cannot have an empty comment, the /** and */ are part of the comment that is returned, so an empty string can't really indicate an empty comment
But this would also break code that checks the return without just if ($ref->getDocComment()), e.g. if ($ref->getDocComment() !== false)

I only needed to go to the second page of the GitHub search results for "->getDocComment()" lang:php to find such code:
https://github.com/salient-labs/toolkit/blob/ee9034a67f1d90201c38f304638fac8ea8a198f6/src/Toolkit/PHPDoc/PHPDocUtil.php#L55

At the very least, this should probably have a discussion on the internals list

I also saw some existing code that works around the false return, using ?: or a (string) cast - is that not possible for Symfony?

[Girgias]

Hmm, so I'm torn On the one hand, indicating no comment with an empty string suggests that maybe there is a comment but it was just empty, while false makes it clear there was absolutely no comment On the other hand, https://3v4l.org/fJBKT suggests that you cannot have an empty comment, the /** and */ are part of the comment that is returned, so an empty string can't really indicate an empty comment But this would also break code that checks the return without just if ($ref->getDocComment()), e.g. if ($ref->getDocComment() !== false)

I only needed to go to the second page of the GitHub search results for "->getDocComment()" lang:php to find such code: https://github.com/salient-labs/toolkit/blob/ee9034a67f1d90201c38f304638fac8ea8a198f6/src/Toolkit/PHPDoc/PHPDocUtil.php#L55

At the very least, this should probably have a discussion on the internals list

I also saw some existing code that works around the false return, using ?: or a (string) cast - is that not possible for Symfony?

It is possible, but I'm not sure that there is a big point in it. But I will raise this on internals.

[Ocramius]

Fine with me to restrict this type, but breaks subclassing, due to the return type changing

[Girgias]

Fine with me to restrict this type, but breaks subclassing, due to the return type changing

The return type is mark as tentative, so you'd "just" get a warning. Which is also why I want this to land prior to PHP 9