Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108164
MIME-Version: 1.0
References: <CAF+90c_RG4Gb-ZXLMoC9bUQ_gb3j37df_s0DqoNKEkohQLDYUg@mail.gmail.com>
In-Reply-To: <CAF+90c_RG4Gb-ZXLMoC9bUQ_gb3j37df_s0DqoNKEkohQLDYUg@mail.gmail.com>
Reply-To: bishop@php.net
Date: Wed, 15 Jan 2020 23:09:51 -0500
Message-ID: <CAEYWF=69-huwheFG2D=qksMLPd64_sf_DT7DyUmjPvqU-M4aAQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000b92eda059c3a00c7"
Subject: Re: [PHP-DEV] Properly validate abstract methods in traits
From: bishop@php.net (Bishop Bettini)

--000000000000b92eda059c3a00c7
Content-Type: text/plain; charset="UTF-8"

On Thu, Jan 9, 2020 at 10:15 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> 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).
>

+1, for improving logical coherence around traits and for simplifying the
engine code.

Since @nicolas-grekas demonstrated a legitimate BC break, I think we do
need an RFC. That would also give some time to discuss merits of private
abstract declarations in traits.

--000000000000b92eda059c3a00c7--