Hello internals,
While working on the deprecation to/from bool type juggling in functions RFC and seeing the impact within Symfony, we found a common slightly annoying case.
The getDocComment method of various Reflection classes was always used as a string, and we thought changing the behaviour in PHP made more sense.
I submitted a PR [1] but was asked to gather feedback on the mailing list.
Please let me know what you think.
Best regards,
Gina P. Banyard
Am 25.06.2025 um 12:37 schrieb Gina P. Banyard <internals@gpb.moe>:
While working on the deprecation to/from bool type juggling in functions RFC and seeing the impact within Symfony, we found a common slightly annoying case.
The getDocComment method of various Reflection classes was always used as a string, and we thought changing the behaviour in PHP made more sense.I submitted a PR [1] but was asked to gather feedback on the mailing list.
Please let me know what you think.
In general I like out-of-band instead of in-band signalling as it seems more robust to me.
I am a bit confused why it was false in the first place and not null as the absence of something was in my book the definition of null.
But then again I realise there is a general aversion against null.
I noticed that the "empty string for absence of value" was also used in DOMElement::getAttribute() but you can argue that the existence of DOMElement::hasAttribute() fills this gap.
Anyway, I'm not a big fan of this move but won't fight it either.
Regards,
- Chris
On Wed, 25 Jun 2025 at 13:56, Christian Schneider <cschneid@cschneid.com> wrote:
Am 25.06.2025 um 12:37 schrieb Gina P. Banyard <internals@gpb.moe>:
> While working on the deprecation to/from bool type juggling in functions RFC and seeing the impact within Symfony, we found a common slightly annoying case.
> The getDocComment method of various Reflection classes was always used as a string, and we thought changing the behaviour in PHP made more sense.
>
> I submitted a PR [1] but was asked to gather feedback on the mailing list.
>
> Please let me know what you think.In general I like out-of-band instead of in-band signalling as it seems more robust to me.
I agree with this.
I was not familiar with the terms, but they make a lot of sense.
This said, I have experienced that some people feel different, and
want to return '' all the time..
In general, "return null" becomes more valuable if an empty string
would have a different meaning.
In this case, an existing doc comment can never be an empty string. So
there is no ambiguity if we would use ''.
But semantically, I think of "no doc comment" as different from "empty
doc comment", and null (or false) reflects that better than an empty
string.
I am a bit confused why it was false in the first place and not null as the absence of something was in my book the definition of null.
But then again I realise there is a general aversion against null.
I think false is just "legacy null" for a lot of php built-in
functions and methods.
Any argument for the aversion to null would equally apply to false,
when used as a "not found" value.
If starting from scratch, I would prefer "return null" in all of those
places. But at this point I don't think the BC break is worth it, so
the "return false" will follow us around for a long time.
(The BC break applies to extending and to conditonal checks with === false)
I noticed that the "empty string for absence of value" was also used in DOMElement::getAttribute() but you can argue that the existence of DOMElement::hasAttribute() fills this gap.
I think all this shows is that different parts of php were created by
different people, or that somebody changed their mind over time.
To me, "return null" would be preferable for
DOMElement::getAttribute(). Online PHP editor | output for QHXlA
Cheers
Andreas
Anyway, I'm not a big fan of this move but won't fight it either.
Regards,
- Chris
Hi,
It makes more sense to me too to change this behavior. I would rather go with null though ; but that is just personal preference ; empty string is already better than false … Did you assess how widely it is used before the users come back “angry” ?
Cheers.
On Wed, 25 Jun 2025 at 11:39, Gina P. Banyard internals@gpb.moe wrote:
Hello internals,
While working on the deprecation to/from bool type juggling in functions RFC and seeing the impact within Symfony, we found a common slightly annoying case.
The getDocComment method of various Reflection classes was always used as a string, and we thought changing the behaviour in PHP made more sense.I submitted a PR [1] but was asked to gather feedback on the mailing list.
Please let me know what you think.
Best regards,
Gina P. Banyard
On 25 June 2025 11:37:37 BST, "Gina P. Banyard" <internals@gpb.moe> wrote:
The getDocComment method of various Reflection classes was always used as a string, and we thought changing the behaviour in PHP made more sense.
Looking at the stub file, the use of false instead of null to signal "no value" seems to be used in a bunch of places in that extension. For instance, getExtensionName() apparently returns false to indicate a user-defined function, while getFileName() returns false for the opposite case.
Returning an empty string when there's actually no comment feels a bit awkward and inconsistent - for example, we don't return a "no type" object from getReturnType().
I think returning null rather than false for all of these would be preferable. From a compatibility point of view, it's not much different from changing to return '' or 0 for those cases, but feels more logical as a design.
Rowan Tommins
[IMSoP]
On 25 June 2025 13:21:22 BST, Andreas Hennings <andreas@dqxtech.net> wrote:
On Wed, 25 Jun 2025 at 13:56, Christian Schneider <cschneid@cschneid.com> wrote:
I noticed that the "empty string for absence of value" was also used in DOMElement::getAttribute() but you can argue that the existence of DOMElement::hasAttribute() fills this gap.
I think all this shows is that different parts of php were created by
different people, or that somebody changed their mind over time.
To me, "return null" would be preferable for
DOMElement::getAttribute(). Online PHP editor | output for QHXlA
In that example, PHP shouldn't be deciding its own behaviour at all, it should be following the DOM standard.
It turns out, the specification explicitly states that the return value should be null in this case, so arguably it's a bug that PHP's implementation does not: DOM Standard
Rowan Tommins
[IMSoP]
What exactly is the context in which symfony uses it?
On Wed, Jun 25, 2025, at 5:37 AM, Gina P. Banyard wrote:
Hello internals,
While working on the deprecation to/from bool type juggling in
functions RFC and seeing the impact within Symfony, we found a common
slightly annoying case.
The getDocComment method of various Reflection classes was always used
as a string, and we thought changing the behaviour in PHP made more
sense.I submitted a PR [1] but was asked to gather feedback on the mailing list.
Please let me know what you think.
Best regards,
Gina P. Banyard
The use of false as a sentinel value is always wrong[1], so I agree with the issue.
Whether the BC break is worth it is a harder question.
Whether it changes to null or empty string, I think, depends on whether in context empty string is a reasonable "zero value." Viz, will it fail gracefully automatically, the way an empty array would for "get list of things"? If so, empty string is probably better. If not, null is reasonable given the current state of the language. Null is arguably more purely correct, but empty string may be more ergonomic in practice. I defer to Nicolas who has probably had to deal with it more than I ever will. 
[1] On empty return values | GarfieldTech
--Larry Garfield
On Wed, Jun 25, 2025 at 4:22 PM Kamil Tekiela <tekiela246@gmail.com> wrote:
What exactly is the context in which symfony uses it?
Was wondering just that, and I can only imagine it’s a function call that receives the parameter as a string and it would getDocComment() result is passed directly, sometimes being false.
Example: https://github.com/symfony/symfony/blob/a3c1d1f9e9bbac9933cc3792a55e756eca5bb495/src/Symfony/Component/DependencyInjection/ContainerBuilder.php#L1177
I think that for those cases getDocComment() :? '' might be a faster fix and move on with it.
And symfony already does this in some places: https://github.com/symfony/symfony/blob/a3c1d1f9e9bbac9933cc3792a55e756eca5bb495/src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php#L53
IMHO, it’s not worth the compatibility breakage, as some others might already compare it using === false.
–
Alex