date extension / Type Hinting View as Markdown

On Thu, Jul 2, 2015 at 3:34 PM, Philip Hofstetter <
phofstetter@sensational.ch> wrote:

Hi,

as this probably requires some discussion, I'm coming here to ask.

I'm talking about bug 69977 and 61483: Both are about the fact that it's
currently not possible to properly type-hint descendant classes of DateTime
and friends.

The reason is the missing use ZEND_ARG_OBJ_INFO, so right now, users can
only create descendant classes without any type hinting on their methods.

Adding the ZEND_ARG_OBJ_INFO is quite trivial and in-fact, I have a branch
here that does it:

https://github.com/pilif/php-src/tree/bug-69977

Of course this is going to break a whole lot of tests because what was once
warnings is now a TypeError.

I'd be willing to update all the tests as needed, but of course changing
the behaviour like this is a BC issue in itself, hence I'm coming here.

The reason why I think this matters much more now than back when 61483 was
submitted is two-fold:

1. back in 61483, adding the (correct!) type-hints was "just" causing an
   E_STRICT message. But now with PHP7 it's actually an E_WARNING. So people
   running without E_STRICT enjoyed correct type hints until now which they
   now can't any more (so, there's another BC break right there :p)
2. With PHP7 type hinting becomes much more viable, so forcing users to not
   type-hint something that really should be feels kind of ugly.
3. Having a new major release is as good a time as any for chaning the
   behaviour, especially when the change is related to a major new feature
   (the addition of much stricter typing).

So I'm asking for opinions on what should be done here.

Again: If you lean /at all/ in the direction of changing this, I volunteer
to update all the tests as required.

Philip

This change is currently not possible, because it will cause a significant
BC break for everyone extending DateTime: They would now be required to
specify these typehints as well. However some libraries (including Carbon,
which is used a lot) depend on these typehint not being there, because they
overload them to e.g. allow passing a string which will then be converted
to the correct instance before passing off to DateTime.

A similar change to what you propose was reverted from PHP 7 for this
reason.

However, what we can do in PHP 7.1 (and I plan to write an RFC for this) is
allow extending classes to remove a parameter type hint, which is in
accordance with contravariance of parameter types. This is analogous to our
already existing partial covariance support for return types (which works
the other way around, by allowing to add return typehints in extending
classes).

Nikita

Hi Philip,

-----Original Message-----
From: Philip Hofstetter [mailto:phofstetter@sensational.ch]
Sent: Thursday, July 2, 2015 3:35 PM
To: PHP internals
Subject: [PHP-DEV] date extension / Type Hinting

Hi,

as this probably requires some discussion, I'm coming here to ask.

I'm talking about bug 69977 and 61483: Both are about the fact that it's
currently not possible to properly type-hint descendant classes of DateTime and
friends.

The reason is the missing use ZEND_ARG_OBJ_INFO, so right now, users can
only create descendant classes without any type hinting on their methods.

Adding the ZEND_ARG_OBJ_INFO is quite trivial and in-fact, I have a branch here
that does it:

https://github.com/pilif/php-src/tree/bug-69977

Of course this is going to break a whole lot of tests because what was once
warnings is now a TypeError.

I'd be willing to update all the tests as needed, but of course changing the
behaviour like this is a BC issue in itself, hence I'm coming here.

The reason why I think this matters much more now than back when 61483 was
submitted is two-fold:

1. back in 61483, adding the (correct!) type-hints was "just" causing an E_STRICT
   message. But now with PHP7 it's actually an E_WARNING. So people running
   without E_STRICT enjoyed correct type hints until now which they now can't any
   more (so, there's another BC break right there :p)
2. With PHP7 type hinting becomes much more viable, so forcing users to not
   type-hint something that really should be feels kind of ugly.
3. Having a new major release is as good a time as any for chaning the
   behaviour, especially when the change is related to a major new feature (the
   addition of much stricter typing).

So I'm asking for opinions on what should be done here.

Again: If you lean /at all/ in the direction of changing this, I volunteer to update
all the tests as required.
please read this thread for more info https://github.com/php/php-src/commit/8e19705a93d785cd1ff8ba3a69699b00169fea47 .

Regards

Anatol