Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108761
MIME-Version: 1.0
References: <CAF+90c-Ob7bvsSh2RF1-KZzJV8ynqyJNnuNBhKE_9GMaJrSWrA@mail.gmail.com>
 <CAEYWF=55gMsvQ8L+pnSVDBcEpao1ZBUFYBZzV=i1tY6mqsjGzA@mail.gmail.com>
In-Reply-To: <CAEYWF=55gMsvQ8L+pnSVDBcEpao1ZBUFYBZzV=i1tY6mqsjGzA@mail.gmail.com>
Date: Tue, 25 Feb 2020 21:23:22 +0100
Message-ID: <CAF+90c9KbfR5tj8uiTHSiycxHDxXSAjFEiZq=G=zA60HJX7Lnw@mail.gmail.com>
To: Bishop Bettini <bishop@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000005ae9c1059f6c4331"
Subject: Re: [PHP-DEV] Deprecate ReflectionParameter::isArray() and friends
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000005ae9c1059f6c4331
Content-Type: text/plain; charset="UTF-8"

On Tue, Feb 25, 2020 at 8:12 PM Bishop Bettini <bishop@php.net> wrote:

> On Tue, Feb 25, 2020 at 10:33 AM Nikita Popov <nikita.ppv@gmail.com>
> wrote:
>
>> I've put up https://github.com/php/php-src/pull/5209 to deprecate the
>> following reflection methods:
>>
>>  * ReflectionParameter::isArray()
>>  * ReflectionParameter::isCallable()
>>  * ReflectionParameter::getClass()
>>
>> These APIs have been superseded by ReflectionParameter::getType() since
>> PHP
>> 7.0. Types introduced since that time are not available through the old
>> APIs. The behavior of these methods becomes increasingly confusing with
>> additional type system extensions. With the addition of union types in PHP
>> 8:
>>
>>  * isArray() will return true if the type is array or ?array, but not any
>> other union type.
>>  * getClass() will return a ReflectionClass for T|int etc, as long as the
>> union only contains a single type. T1|T2 will return null.
>>
>> There is really no reasonable behavior for these methods in the presence
>> of
>> union types; we should start phasing them out.
>>
>> One point I'm not sure about is whether we want to deprecate
>> ReflectionParameter::allowsNull() as well. Logically this method belongs
>> on
>> ReflectionType (and does in fact exist there), but the way it is
>> formulated
>> is still compatible with union types, so leaving it alone is a
>> possibility.
>>
>
> Why do you say there's no "reasonable behavior ... in the presence of
> union types"? To me, the following seem like reasonable interpretations in
> a post-union-type world:
>
> ::isArray() returns true IIF is_array(type) === true for at least one type
> in the parameter
> ::isCallable() returns true IIF is_callable(type) === true for at least
> one type in the parameter
> ::getClass() returns null IIF class_exists(type) === false for all types
> in the parameter; returns ReflectionClass IIF class_exists(type) for
> exactly one type in the parameter; throws \TypeError otherwise
>

I don't think those behaviors correspond to the spirit or practical usage
of these methods. I would expect that existing code calling isArray() will
only work correctly if it really is an isolated array type (or, for
historical reasons, a nullable array type, which was not considered a
separate "type" at the time).

This is pretty useless speculation though, and I'm honestly not sure what
you are trying to tell me with your email. Sure, it's possible to come up
with a behavior for them (and in fact, they already have one, which I
described before), but that doesn't change the fact that at this time, this
is a quite incomplete API (doesn't even support scalar types), and we
should encourage people to migrate to the more complete ReflectionType
based API. No...?

Regards,
Nikita

--0000000000005ae9c1059f6c4331--