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
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(). https://3v4l.org/QHXlA
Cheers
Andreas
Anyway, I'm not a big fan of this move but won't fight it either.
Regards,
- Chris
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(). https://3v4l.org/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: https://dom.spec.whatwg.org/#ref-for-dom-element-getattribute%E2%91%A0
Rowan Tommins
[IMSoP]
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]
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.
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
What exactly is the context in which symfony uses it?
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
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] https://www.garfieldtech.com/blog/empty-return-values
--Larry Garfield