Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] get_class() disallow null parameter »

[RFC] get_class() disallow null parameter

php.internals

Dan Ackroyd

10 years ago
Hi! Following an earlier discussion in January, here is a small RFC to change get_class() to disallow null being passed as parameter. https://wiki.php.net/rfc/get_class_disallow_null_parameter Allowing null to be passed to get_class() is a 'gotcha' that is almost always a mistake in someone's code. By preventing null being passed, get_class() will be far more likely to do what people think it should do, and so result in fewer bugs in code. This RFC is a small change and so is appropriate for a point release. Other ways of making this function behave as people expect it to are not as small changes, and are probably not appropriate for point releases. cheers Dan Ack

Marco Pivetta

10 years ago
Hi Dan, On Sun, Aug 14, 2016 at 1:00 PM, Dan Ackroyd <danack@basereality.com> wrote:
> Hi! > > Following an earlier discussion in January, here is a small RFC to > change get_class() to disallow null being passed as parameter. > > https://wiki.php.net/rfc/get_class_disallow_null_parameter > > Allowing null to be passed to get_class() is a 'gotcha' that is almost > always a mistake in someone's code. > > By preventing null being passed, get_class() will be far more likely > to do what people think it should do, and so result in fewer bugs in > code. > > This RFC is a small change and so is appropriate for a point release. > Other ways of making this function behave as people expect it to are > not as small changes, and are probably not appropriate for point > releases. >
Thanks for bringing up this: every scenario where `null` has been passed to `get_class()` was a bug, in my work context, so I'm really happy to see this patch going in. Still, can any of the release managers throw their opinion at the RFC? This is a SemVer violation, but it doesn't seem like PHP follows SemVer closely, so I'm a bit confused about whether this can target 7.x. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/

Dan Ackroyd

10 years ago
On 14 August 2016 at 19:01, Marco Pivetta <ocramius@gmail.com> wrote:
> Hi Dan, > > This is a SemVer violation, but it doesn't seem like PHP follows SemVer > closely, so I'm a bit confused about whether this can target 7.x.
It was broken in a minor release, it can be fixed in a minor release imo. cheers Dan

Marco Pivetta

10 years ago
Works for me 👍 On 15 Aug 2016 10:17 p.m., "Dan Ackroyd" <danack@basereality.com> wrote:

Stas Malyshev

10 years ago
Hi!
> Following an earlier discussion in January, here is a small RFC to > change get_class() to disallow null being passed as parameter. > > https://wiki.php.net/rfc/get_class_disallow_null_parameter
Could you please clarify what you mean by "disallow"? I've read the RFC but I didn't find description of what happens in that case.
-- Stas Malyshev smalyshev@gmail.com

Dan Ackroyd

10 years ago
On 14 August 2016 at 22:48, Stanislav Malyshev <smalyshev@gmail.com> wrote:
> > Could you please clarify what you mean by "disallow"?
It will be the standard behaviour for accepting params. The patch is just: - if (zend_parse_parameters(ZEND_NUM_ARGS(), "|o!", &obj) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "|o", &obj) == FAILURE) { but I have added a clear description to the RFC:
> If get_class() is called with null as the parameter, a warning will be emitted: > > Warning: get_class() expects parameter 1 to be object, null given in %s on line %d
cheers Dan

Christoph Becker

10 years ago
On 14.08.2016 at 13:00, Dan Ackroyd wrote:
> Following an earlier discussion in January, here is a small RFC to > change get_class() to disallow null being passed as parameter. > > https://wiki.php.net/rfc/get_class_disallow_null_parameter > > Allowing null to be passed to get_class() is a 'gotcha' that is almost > always a mistake in someone's code. > > By preventing null being passed, get_class() will be far more likely > to do what people think it should do, and so result in fewer bugs in > code. > > This RFC is a small change and so is appropriate for a point release. > Other ways of making this function behave as people expect it to are > not as small changes, and are probably not appropriate for point > releases.
Prohibiting `get_class(NULL)` is certainly a good idea, but I have some concerns regarding BC. While `__CLASS__` has been introduced with PHP 4.3.0, it had the glitch to return the lower-cased class name before PHP 5.0.0. So there *might* still be PHP 5 code around using `get_class(NULL)`. The deprecation might be more appropriate, and could equally well catch an inadvertent `get_class(NULL)`.
-- Christoph M. Becker

Stas Malyshev

10 years ago
Hi!
> Prohibiting `get_class(NULL)` is certainly a good idea, but I have some > concerns regarding BC. While `__CLASS__` has been introduced with PHP > 4.3.0, it had the glitch to return the lower-cased class name before PHP > 5.0.0. So there *might* still be PHP 5 code around using `get_class(NULL)`.
I note that this (i.e. behavior proposed in the RFC) was briefly the case for 5.3.0 and was changed to current behavior for 5.3.1. But pre-5.3 versions return false when NULL is passed to get_class, with no message. So, old PHP code doesn't seem to be a problem only one that was written after 5.3.1, because it doesn't look like get_class(NULL) worked before it: https://3v4l.org/4QvCs
-- Stas Malyshev smalyshev@gmail.com

Christoph Becker

10 years ago
On 15.08.2016 at 02:09, Stanislav Malyshev wrote:
>> Prohibiting `get_class(NULL)` is certainly a good idea, but I have some >> concerns regarding BC. While `__CLASS__` has been introduced with PHP >> 4.3.0, it had the glitch to return the lower-cased class name before PHP >> 5.0.0. So there *might* still be PHP 5 code around using `get_class(NULL)`. > > I note that this (i.e. behavior proposed in the RFC) was briefly the > case for 5.3.0 and was changed to current behavior for 5.3.1. But > pre-5.3 versions return false when NULL is passed to get_class, with no > message. > > So, old PHP code doesn't seem to be a problem only one that was written > after 5.3.1, because it doesn't look like get_class(NULL) worked before > it: https://3v4l.org/4QvCs
Thanks for pointing this out. Interestingly, indicated by a changelog entry in the get_class() manual, the behavior is somewhat different when no argument is passed, see <https://3v4l.org/sWX9X>. That works as least as of PHP 5.2.0; not sure why 5.0 and 5.1 fail here. As the proposed behavior had been introduced with 5.3.0 and reverted shortly after, indicates that we should be careful here. Deprecating for 7.2 and removing with 8.0 seems to be the safer way.
-- Christoph M. Becker