Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108065
MIME-Version: 1.0
Date: Thu, 9 Jan 2020 16:15:03 +0100
Message-ID: <CAF+90c_RG4Gb-ZXLMoC9bUQ_gb3j37df_s0DqoNKEkohQLDYUg@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000002b1659059bb67a21"
Subject: Properly validate abstract methods in traits
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000002b1659059bb67a21
Content-Type: text/plain; charset="UTF-8"

Hi internals,

I've prepared https://github.com/php/php-src/pull/5068 to fix validation of
abstract methods inside traits for PHP 8.

To provide some context, the situation is currently pretty weird... when
you add an abstract method in a trait, you obviously do that because the
trait requires that method to exist with the given signature. However, this
is currently enforced only spottily:

 * It is not enforced in the most common case, where the method
implementation is provided by the using class: https://3v4l.org/SeVK3
 * It is enforced if the implementation comes from a parent class:
https://3v4l.org/4VCIp
 * It is enforced if the implementation comes from a child class:
https://3v4l.org/q7Bq2

I believe that an abstract methods signature coming from a trait should
always be enforced.

Conversely, we currently have some very weird code lying around that
instead does a bi-directional signature enforcement if there are two traits
specifying the same abstract method: https://3v4l.org/8Ze2C

I believe this case should be valid instead, as the implementation of the
method provided by the class satisfies both signatures.

Does anyone see an issue with making these validation changes for PHP 8? I
can also make an RFC for this, but it seems more like an implementation bug
to me (especially seeing how there's about zero test coverage for any of
this).

Regards,
Nikita

--0000000000002b1659059bb67a21--